search

thunder-app / thunder

Thunder - An open-source cross-platform Lemmy client for iOS and Android built with Flutter
https://thunderapp.dev
GNU Affero General Public License v3.0
732 stars 62 forks source link

Fix a couple logout issues #980

Closed micahmo closed 4 months ago

micahmo commented 5 months ago

Pull Request Description

This PR fixes some inconsistencies between logging out of an account via the profile modal and via the user page.

Scenarios

  1. There are at least two accounts, and the user logs out of one.
    • Modal: Makes another account active.
    • User page: Switches to anonymous mode, even if there are no anonymous instances.
      • Fixed this to match modal behavior.
  2. There is only one logged in account and no anonymous accounts.
    • Modal: Logout not allowed.
    • User page: Logout is allowed, and a non-existent anonymous instance is activated.
      • I didn't want to disallow logout, so I fixed this by creating and activating a new anonymous instance.

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

https://github.com/thunder-app/thunder/assets/7417301/b5935676-8fe4-4b21-b05c-9d8dfb559bbb

Checklist

hjiangsu commented 5 months ago

Thanks for doing this! I do have one discussion point that maybe others can chime in. @machinaeZER0

There are at least two accounts, and the user logs out of one

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)

micahmo commented 5 months ago

It feels a bit weird from a UI perspective that I see myself being logged out, and then being "logged in" to another account.

That's fair! A couple other options are...

machinaeZER0 commented 5 months ago

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

machinaeZER0 commented 5 months ago

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?

machinaeZER0 commented 5 months ago

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!

micahmo commented 5 months ago

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!

machinaeZER0 commented 5 months ago

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!)

micahmo commented 5 months ago

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. 😊

hjiangsu commented 5 months ago

Thanks for contributing to the discussion @machinaeZER0! I hope you don't mind that I pulled you in for this 😅

I personally kinda like the ideas of asking what to do, and/or streamlining to just using the modal

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 lemmy.world account, and I log out, I would expect to be thrown back into a anonymous account on lemmy.world. Likewise, if I logged out of lemmy.ca, then the anonymous account would be on lemmy.ca

I hope this helps with the discussion so far and let me know if anything is unclear!

machinaeZER0 commented 5 months ago

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!

micahmo commented 5 months ago

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.

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)

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.

https://github.com/thunder-app/thunder/assets/7417301/9eb1b798-b4cd-42ed-99a4-825b4fb37df0

To 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!).

Another minor benefit to this approach is that it allows us to centralize all login/logout logic in the modal.

hjiangsu commented 5 months ago

the easiest fix would be to show the modal when pressing log out from the account page.

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)

micahmo commented 5 months ago

I think this works for now

Great! I just pushed the changes demonstrated in my last comment. Hopefully this is good to go for now. 😊