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 quick switch on web #7190

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Account quick switch on web #7190

wants to merge 16 commits into from

Conversation

mozzius
Copy link
Member

@mozzius mozzius commented Dec 19, 2024

Clicking the profile picture in the top left now shows an account quick switch menu

Animation is disabled for prefers-reduced-motion

Screen.Recording.2025-01-06.at.19.39.46.mov

Test plan

  • Click all the buttons
  • Check breakpoints

Copy link

github-actions bot commented Dec 19, 2024

Old size New size Diff
6.86 MB 6.86 MB -16 B (-0.00%)

Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

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

Yesssss, I love it. I think we should add a quick hover state and probably some icons, what do you think?

src/view/shell/desktop/LeftNav.tsx Outdated Show resolved Hide resolved
src/view/shell/desktop/LeftNav.tsx Outdated Show resolved Hide resolved
src/view/shell/desktop/LeftNav.tsx Outdated Show resolved Hide resolved
@echarles
Copy link

Usability question: Lets say you are viewing a specific post, eg. https://bsky.app/profile/echarles.net/post/3lehhnon3ns2t and you switch account via the profile picture menu, will you stay on the same post, or will you be driven to the new account home page?

I have not looked at the code of this PR ... as a user, I would rather stay on the post I was looking at. Pretty sure others will have other opinions :)

@mozzius
Copy link
Member Author

mozzius commented Jan 6, 2025

Usability question: Lets say you are viewing a specific post, eg. https://bsky.app/profile/echarles.net/post/3lehhnon3ns2t and you switch account via the profile picture menu, will you stay on the same post, or will you be driven to the new account home page?

No, out of scope for this PR. would be nice though :)

@@ -119,6 +119,11 @@ export function Trigger({children, label, role = 'button'}: TriggerProps) {
} = useInteractionState()
const {state: focused, onIn: onFocus, onOut: onBlur} = useInteractionState()

const prevControlIsOpen = React.useRef(control.isOpen)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use state for this instead, it's not legit to modify refs during render.

https://react.dev/reference/react/useState#storing-information-from-previous-renders

We have a bunch of code like this, look for prev

Copy link
Member

@estrattonbailey estrattonbailey left a comment

Choose a reason for hiding this comment

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

This LGTM (and I love it) but wanna hear back from Dan on if we can handle prev state more correctly

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.

5 participants