-
Notifications
You must be signed in to change notification settings - Fork 34
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
Account List in Settings #1056
Conversation
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.
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/Root/Tabs/Settings/AccountListSettingsView.swift
Outdated
Show resolved
Hide resolved
Co-authored-by: Eric Andrews <eric.b.andrews.auto@protonmail.com>
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.
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
Fixed 👍
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. |
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.
Nice! 🚀
AvatarView
,AvatarBannerView
andAvatarStackView
.NavigationLink
for shorter syntax when navigating to aNavigationPage
.Related to #941