-
Notifications
You must be signed in to change notification settings - Fork 68
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
Fix a couple logout issues #980
Conversation
Thanks for doing this! I do have one discussion point that maybe others can chime in. @machinaeZER0
It feels a bit weird from a UI perspective that I see myself being logged out, and then being "logged in" to another account. I guess the main thing here is that from the User Page, I don't really know that theres another account listed (whereas in the modal, it explicitly states that theres multiple accounts so it makes it easier to understand that I'm switching to the next available account) |
That's fair! A couple other options are...
|
I personally kinda like the ideas of asking what to do, and/or streamlining to just using the modal - in part because I think the modal is really lovely and I don't mind the extra interaction required with logout removed. But that said, I don't juggle accounts too much these days! I would agree that to me, logging out of one account doesn't feel like it should automatically log me into another account, even if 99/100 times they're both your accounts. That's what the switch behavior is for, after all. Apologies if I'm misinterpreting, but there is a default account-less state the app can be in, right? Where you can't vote and stuff? That feels like the state I'd expect the app to return to if I logged out of an account, personally, even if there are a few accounts available. But again, I do like the idea of giving folks a choice |
Per the video, one thing that feels odd is that logging out is deleting the current account from the switcher modal? Especially since there's an explicit Delete button (the trash icon). Feels like the expected behavior would be that logging out logs the account out, but that account is preserved in the account switcher list for quickly logging back in - and if one wanted that account off the list, they'd instead use the Delete function. That would line up with how the Google account switcher works, so I think that's why it feels natural to me? |
If an anonymous account is the same as the fresh install state (I haven't used one) then I might suggest showing it as an option in the account switcher by default, somehow? They may be tied to a specific instance, but if that's the case it could prompt you to select one on first press or something, maybe. Just spitballing! |
Hey @machinaeZER0 thanks for the comments! First let me address how anonymous instances work. Out of the box there is an explicit anonymous instance in the switcher for the default instance that we've picked for Thunder. Then the user can add new accounts. If they leave the default anonymous instance or add their own, then the choice of what to do when logging out is very easy. The tricky situation is when they've deleted all of the anonymous instances, and only have accounts left. That's why one of my suggestions was to create a new explicit anonymous instance for them. But that might be undesirable because they've intentionally deleted them all. Hence the tricky situation! To your point about icons, I agree that it's confusing. The logout and trash icon do the same thing. The only difference is that the logout icon is shown for the currently selected account, to imply that logging out will change your current state (whereas deleting an inactive account will not really change anything). However, we could use the trash icon on all accounts of that would help convey the action better! Finally, another option would be for the log out button on the account page to bring up the modal and then ask for confirmation. Then you'd more clearly see that you're being switched to another existing account, which would address the original concern. Thanks for the dialog as always! |
Thank you, that's all helpful context :) Would it be bad to just always have one persistent anonymous instance that can't be deleted? I feel like that combined with logging out not deleting an account would (in my opinion) be a more expected default behavior. I know that would require making logging out and deleting accounts two separate things though, which is easier said than done (and possibly out of the scope of this issue). Do you two have thoughts on that? Apologies if I'm being reductive (or off topic!) |
You are never reductive or off topic! 😊 From my perspective (and I am just one person and happy to be overruled), to log out of an account means that my credentials/token/cookies are no longer stored, and if I want to use that account again, I'd have to log back in. That's very different from switching accounts, where I'm just changing which one is active, but all the information is still available for me to switch back. That's why I would see log out and delete as similar/identical actions. I'm not sure if you use multiple GitHub accounts, but they recently added an account switcher. If you sign out of an account, it gets deleted and is no longer available for switching. As for what to do when there are no anonymous instances (should we have one that can't be deleted, like you said), I don't really have a strong opinion on that. 😊 |
Thanks for contributing to the discussion @machinaeZER0! I hope you don't mind that I pulled you in for this 😅
I agree with these points! I honestly wouldn't mind if we removed the button altogether and just use the modal to handle account activity (login/logout/switch) however I'm not too sure about how other users would feel. About the whole discussion around logging out/switching, I could go either way for this and don't have a strong opinion on the matter. I do think though, that I tend to associate logging out as an action that requires me to re-authenticate again (whether that be to provide both username/password, or just password). If this is the case, then this might be closer to "deleting" an account in our situation rather than "switching" accounts. I also don't mind the idea of having one persistent anonymous account. If we have this persistent account, then I would expect that logging out of an account would redirect me to using that anonymous account. I would also assume then that the anonymous account instance would be the same one that I just logged out of. For example, if I am currently logged into my I hope this helps with the discussion so far and let me know if anything is unclear! |
Thanks to both of you! It's fun to brainstorm/problem solve with you :) It sounds like the main discourse is (I think) whether or not an account should persist in some fashion once you've logged out of it/switched accounts. To that point, I think switching accounts will naturally preserve the previous account (which I believe is the current Thunder behavior). When it comes to logging out, I guess the question (in my mind) is whether or not the user intends to log back in to said account at some point in the future. If so, I could see keeping the account in the switcher list with a "logged out" status, that when clicked would just require re-entering the password for said account. This is how the Google account switcher works, and I believe the purpose is twofold - ease of logging back in/managing accounts, but also (I think) security. One could make the case that it is helpful to have all of a user's accounts easily available, but logged out temporarily for (for example) privacy purposes. I think this is where I'm coming from when I suggest the idea of persistent accounts even after logging out (with the option to delete unwanted/unused accounts from the switcher as needed, which is also how the Google account switcher works). And if a user wants to keep their accounts logged in at all times, I figure they're able to switch between them without re-authenticating as they please (which again, I believe is the default behavior). Hopefully that explanation helps explain where I'm coming from a bit more clearly :) I don't necessarily think there's a problem if we want to go the route of "logging out removes that account from the switcher", and happy with whatever the final consensus ends up being! It sounds like some of the other aspects of the discussion we're already syncing up on, so that's great! |
Hey all, just circling back to this... We've tossed around a bunch of ideas here, and we could definitely take it as an action item to rethink our login/logout flows in general. I wonder, for the sake of this PR (which was only intended to make the account page behavior match the existing modal behavior) if we could go back to the original feedback and try to find a simple fix.
To address this specifically, maybe the easiest fix would be to show the modal when pressing log out from the account page. Then the behavior of switching to another account would be much more obvious. qemu-system-x86_64_MIUVTW1qDS.mp4To me, this avoids some of the messy compromises like having an anonymous instance that can't be deleted (personally, I like only having my main account!).
|
I think this works for now, it makes it a bit more obvious what is happening behind the scenes! We can definitely re-think our login/logout flow in the future, and other enhancements related to this (e.g., detecting when a login token is no longer valid to prompt entering password again) |
Great! I just pushed the changes demonstrated in my last comment. Hopefully this is good to go for now. 😊 |
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.
Just one comment!
Pull Request Description
This PR fixes some inconsistencies between logging out of an account via the profile modal and via the user page.
Scenarios
Issue Being Fixed
Issue Number: N/A
Screenshots / Recordings
qemu-system-x86_64_AgkYW0bqNC.mp4
Checklist
semanticLabel
s where applicable for accessibility?