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

Make crate menu onclick rather than onhover. #1081

Merged
merged 3 commits into from
Oct 8, 2020

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Oct 8, 2020

This makes for a more consistent experience on mobile and desktop.

It also, in my opinion, makes for better usability. It's always clear
how to close a menu - the same way as you opened it, by clicking. It
means that menus aren't opened by stray mouse movements. There are more
details on the usability of hover menus at
https://uxmovement.com/navigation/why-hover-menus-do-users-more-harm-than-good/.

Followup from #1077. I realize we hadn't concluded that changing to onclick
was the desired outcome, so consider this a speculative PR to demonstrate
the concept and see how it feels.

This makes for a more consistent experience on mobile and desktop.

It also, in my opinion, makes for better usability. It's always clear
how to close a menu - the same way as you opened it, by clicking. It
means that menus aren't opened by stray mouse movements. There are more
details on the usability of hover menus at
https://uxmovement.com/navigation/why-hover-menus-do-users-more-harm-than-good/.
@jyn514
Copy link
Member

jyn514 commented Oct 8, 2020

r? @GuillaumeGomez, but I think I like this better.

@jyn514 jyn514 added the A-frontend Area: Web frontend label Oct 8, 2020
@GuillaumeGomez
Copy link
Member

Looks good to me as well!

@jyn514
Copy link
Member

jyn514 commented Oct 8, 2020

This has a slight bug: when you hit space to close the menu (navigating with keyboard instead of mouse) it will scroll down the page, not just close the menu. Do you know how to fix that?

https://youtu.be/7qX5oCOeqBY

@jyn514
Copy link
Member

jyn514 commented Oct 8, 2020

Actually, this is a pre-existing bug, no need to fix it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Web frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants