Skip to content

Remove 'Remove Account' option from login page#2178

Closed
differsthecat wants to merge 2 commits intomasterfrom
SG-808
Closed

Remove 'Remove Account' option from login page#2178
differsthecat wants to merge 2 commits intomasterfrom
SG-808

Conversation

@differsthecat
Copy link
Copy Markdown
Member

@differsthecat differsthecat commented Nov 9, 2022

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Remove options from the login page altogether as the items in it are redundant with the updates.

Code changes

  • src/App/Pages/Accounts/LoginPage.xaml: Remove options
  • src/App/Pages/Accounts/LoginPage.xaml.cs: Remove Android specific check on the removeAccount option
  • src/App/Pages/Accounts/LoginPageViewModel.cs: Remove options

Screenshots

Screen Shot 2022-11-09 at 10 34 36 AM

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@differsthecat differsthecat requested a review from a team November 9, 2022 15:15
Comment thread src/App/Pages/Accounts/LoginPageViewModel.cs Outdated
Comment thread src/App/Pages/Accounts/LoginPage.xaml
@mpbw2
Copy link
Copy Markdown
Contributor

mpbw2 commented Nov 9, 2022

Remove Account is for accounts that are already logged in (it's part of account switching). I'm actually a bit confused as to how we're able to get to this screen for a user that's already logged in though, which is why the original "remember me" functionality was removed when account switching was added.

@differsthecat
Copy link
Copy Markdown
Member Author

Remove Account is for accounts that are already logged in (it's part of account switching). I'm actually a bit confused as to how we're able to get to this screen for a user that's already logged in though, which is why the original "remember me" functionality was removed when account switching was added.

I could be wrong here, but users should have still seen this screen before when adding an account. Does that sound right @andrebispo5 ?

@mpbw2
Copy link
Copy Markdown
Contributor

mpbw2 commented Nov 9, 2022

Remove Account is for accounts that are already logged in (it's part of account switching). I'm actually a bit confused as to how we're able to get to this screen for a user that's already logged in though, which is why the original "remember me" functionality was removed when account switching was added.

I could be wrong here, but users should have still seen this screen before when adding an account. Does that sound right @andrebispo5 ?

On other clients yes, but mobile handled it differently to support quick account switching back to an account that was logged out via timeout vs manually logging out (which is how those accounts remained in the account switching list when they timed out).

@differsthecat
Copy link
Copy Markdown
Member Author

Remove Account is for accounts that are already logged in (it's part of account switching). I'm actually a bit confused as to how we're able to get to this screen for a user that's already logged in though, which is why the original "remember me" functionality was removed when account switching was added.

I could be wrong here, but users should have still seen this screen before when adding an account. Does that sound right @andrebispo5 ?

On other clients yes, but mobile handled it differently to support quick account switching back to an account that was logged out via timeout vs manually logging out (which is how those accounts remained in the account switching list when they timed out).

I see what you are saying now, I just did some testing with it. If we remove the 'Remove Account' option here, a user can't remove the user unless they log in and then log back out. @andrebispo5 This will require more thought.

@differsthecat differsthecat deleted the SG-808 branch February 23, 2023 14:39
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