-
Notifications
You must be signed in to change notification settings - Fork 3
Allauth: tune login/connect sorting, naming, and add GitHub App/Oauth modal #609
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
base: main
Are you sure you want to change the base?
Conversation
…hen two are visible This makes use of Allauth's app settings, which already supports `hidden`, and extends this to support provider sorting and tuning of login/connect/etc process hiding of providers.
readthedocsext/theme/templates/socialaccount/snippets/provider_list.html
Show resolved
Hide resolved
In testing I also figured I'd fix #588 Can't decide if that should be labeled |
Show icon instead of missing avatar and use more helpful strings for the listing
I'm a little confused with this screenshot. It seems that we are mixing the copy for the "Login / Signup" with the "Connect to GitHub". When I'm connecting my account, it shouldn't say "For users that connected their account before June". That doesn't make sense on that screen, but it does on "Login / Signup", tho. |
Good point, we could use an extra step in this. The copy here now is for the case of:
But I think what you're looking for is some copy for early beta that covers the case of:
There's definitely room for that here. |
readthedocsext/theme/templates/socialaccount/partials/social_account_list.html
Show resolved
Hide resolved
I thought we decided to start the beta with one button only, and only add two buttons if there is need for it? |
Connection page updates, including:
Connect view during early beta, guide users to stick with OAuthConnect view during later beta, guide users to try GHANote: we might also swap the priority of these when we make the code change to trigger this copy, it feels like this copy would make sense with GHA promoted maybe. I didn't do that for this screenshot though. |
Think we landed on the opposite, working towards a single button but giving the user explicit options especially early in the beta. We're bound to notice edge cases and benefit from giving users control. This plan was laid out in #608 A single button might be safe enough, but we can wait for user testing to prove that. I don't have any worries about users being confused by this UI either way, it's quite explicit. |
That issue seems new? This is what I wrote just after the meeting https://github.com/readthedocs/meta/issues/187#issuecomment-2917068587.
Just to be clear, having two buttons doesn't allow users to "go back" to the previous state if they don't like something about the new app or if they encounter an error. But I do like having the modal to get users into the beta, but I'd be still -1 on showing two buttons when the beta ends. We are asking users to remember if they migrated or not, or the date they created their account, which users won't have idea about. I also wasn't able to run this locally, |
Not revoking access to OAuth on migration is still on plan, that's fine.
But here, we still want users trying to log in with GHA so we can test the whole workflow. We haven't tested this ourselves yet and we also want users testing the GHA buttons so we can confirm if we can reduce to a single button. Just an OAuth button won't push much farther forward on testing and I think we just want to jump all in on testing. What is the concern with two buttons at this point?
Yeah I think at least initially it's more likely the user tries to connect their GitHub to GHA, hits an issue and can't connect and needs to fall back to OAuth, or stick with an existing OAuth connection.
This is definitely the hope, it's in the plan at #608 Now I also wouldn't say it's impossible that we have two buttons after beta either though. If we find users are absolutely blocked on migrating to GHA, we could still need this. But we deal with that if we deal with that. |
I'd like to move forward with the GHA beta and deploy this feature. I'm reviewing all these PRs and it seems we are blocked on relevant things. However, I think we can tune them in the following deploys. We won't know too much about the UX of having one or two buttons until we deploy these changes and start taking to customers. I personally prefer one button because two adds noise to me. However, the UI is explicit enough to make me feel more comfortable with it for now as long as we are flexible to change it again in the following deploys. @stsewd do you have any other technical consideration here? Anything else that's blocking this PR or are you OK to move forward for now? |
And just to be clear, I also want only one button here, I just don't think we can start with one button without potentially upsetting customers. We'll know more as users test this whole workflow. |
Nope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Work left:
{"hidden": true}
and testing on actual auth.hidden
, SAML providers were filtered fromget_providers
Examples
Business with Google provider and GitHub modal button
Business when no modal is needed
GitHub modal
Social account list