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

Account List in Settings #1056

Merged
merged 25 commits into from
May 11, 2024
Merged

Account List in Settings #1056

merged 25 commits into from
May 11, 2024

Conversation

Sjmarf
Copy link
Contributor

@Sjmarf Sjmarf commented May 10, 2024

  • Adds the Account List to the settings tab.
  • Re-implemented AvatarView, AvatarBannerView and AvatarStackView.
  • Added extensions to NavigationLink for shorter syntax when navigating to a NavigationPage.
  • Fixed signing-out of accounts.

Related to #941

@Sjmarf Sjmarf requested a review from a team as a code owner May 10, 2024 06:42
@Sjmarf Sjmarf requested review from WestonHanners and ShadowJonathan and removed request for a team May 10, 2024 06:43
Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, a couple nits related to CONTRIBUTING updates.

I also get a couple of compilation errors due to read and saved still being isRead and isSaved (FeedsView:113, Interactable2Providing+Extensions:21).

Mlem/App/Views/Shared/Avatar/AvatarBannerView.swift Outdated Show resolved Hide resolved
Mlem/App/Views/Shared/Avatar/AvatarBannerView.swift Outdated Show resolved Hide resolved
Mlem/App/Views/Shared/Avatar/AvatarStackView.swift Outdated Show resolved Hide resolved
Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I've noticed a couple very slight rough edges while testing:

  • Signing out of an account that you are not currently logged into from the quick-switcher closes the switcher
  • Signing out of the current account puts the app into guest mode--is this intentional? Discussion raised in Slack
  • "Enter Guest Mode" is still available in the switcher even when you're already in Guest Mode--on this note, the switcher should probably have an active indication that guest mode is active, as it may appear broken for there to be no green active account indicator

@Sjmarf
Copy link
Contributor Author

Sjmarf commented May 11, 2024

Signing out of an account that you are not currently logged into from the quick-switcher closes the switcher

Fixed 👍

"Enter Guest Mode" is still available in the switcher even when you're already in Guest Mode--on this note, the switcher should probably have an active indication that guest mode is active, as it may appear broken for there to be no green active account indicator

This button is temporary for testing purposes - in future we'll let the user add multiple different guest accounts to a section below the list of accounts. I'll make it obvious when you're signed into a guest account once that's done.

@Sjmarf Sjmarf requested a review from EricBAndrews May 11, 2024 16:37
Copy link
Member

@EricBAndrews EricBAndrews left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! 🚀

@Sjmarf Sjmarf merged commit fe1f89c into dev2 May 11, 2024
1 check passed
@Sjmarf Sjmarf deleted the sjmarf/dev2-account-list branch May 11, 2024 16:38
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.

None yet

2 participants