Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move authentication method configuration to sidebar settings. #132

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rahulsingh321
Copy link
Contributor

@rahulsingh321 rahulsingh321 commented Feb 1, 2025

Issue: #131

Listing view:
Screenshot from 2025-02-04 02-19-52

Edit View
Screenshot from 2025-02-04 02-45-38

@rahulsingh321 rahulsingh321 force-pushed the issue/131/authentication_method_settings branch from 96c7c03 to 8053931 Compare February 1, 2025 16:47
@rahulsingh321 rahulsingh321 changed the title Move authentication method into sidebar setting menu Move authentication method configuration to sidebar settings. Feb 1, 2025
@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

@kennyadsl please Review PR with some Last minor changes to social. Tests fail only on sqlite.

@kennyadsl
Copy link
Member

@rahulsingh321 can you please post some screenshots of the visual part of the change?

@rahulsingh321
Copy link
Contributor Author

@rahulsingh321 can you please post some screenshots of the visual part of the change?

@kennyadsl I added the screenshots of the view now, please have a look when you get chance.

@rahulsingh321 rahulsingh321 marked this pull request as ready for review February 3, 2025 08:29
@kennyadsl
Copy link
Member

Am I right that we are using the legacy/classic admin style in the new admin namespace? Any reason why not using the new style (the one with tailwind)?

@rahulsingh321
Copy link
Contributor Author

rahulsingh321 commented Feb 3, 2025

Am I right that we are using the legacy/classic admin style in the new admin namespace? Any reason why not using the new style (the one with tailwind)?

@kennyadsl Just to provide support for legacy front-end, I didn't use the tailwind UI. If we don't need that, let me know, I can update that.

@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

@kennyadsl I would avoid the line break and call it oauth login if that's ok.

@kennyadsl
Copy link
Member

I'd go with Social Auth or Social Login

@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

I'd go with Social Auth or Social Login

@rahulsingh321 please rename the settings menu label to Social Auth so that we avoid line breaks

@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

Just to provide support for legacy front-end, I didn't use the tailwind UI. If we don't need that, let me know, I can update that.

@kennyadsl can we fix new admin in a later release given that it's not a setting that you set every day?

@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

@rahulsingh321 tests still failing

@rahulsingh321 rahulsingh321 force-pushed the issue/131/authentication_method_settings branch 2 times, most recently from e4ed0d4 to 3247c36 Compare February 3, 2025 21:14
@fthobe
Copy link
Contributor

fthobe commented Feb 3, 2025

@kennyadsl tests are fixed and changes implemented.

Ruby 3.0 sqlite is tested, I already raised the question to Jared. Do we still want to test for that?

@rahulsingh321 rahulsingh321 force-pushed the issue/131/authentication_method_settings branch from 3247c36 to 0d25953 Compare February 4, 2025 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants