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

Re-enable the menu item hover state on small screens #16168

Merged
merged 1 commit into from
Jun 17, 2019

Conversation

kjellr
Copy link
Contributor

@kjellr kjellr commented Jun 13, 2019

Fixes #12262. Reverses a change made in #10333.

As discussed in #12262 (comment), this PR turns on the menu-item hover state for all breakpoints.

There should be no visible difference from master if a touch-enabled user presses on a menu item, but unlike master, the hover states will be activated if that user presses them and scrolls the viewport. This isn't ideal, but but as discussed in #12262, this may be an okay tradeoff. If we merge this, we may want to reopen #10320 to keep tracking this issue.

GIF:

hover-2
hover

@kjellr kjellr added [Type] Enhancement A suggestion for improvement. [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). General Interface Parts of the UI which don't fall neatly under other labels. labels Jun 13, 2019
@kjellr kjellr self-assigned this Jun 13, 2019
@kjellr kjellr requested review from mapk and afercia June 13, 2019 20:49
Copy link
Contributor

@afercia afercia left a comment

Choose a reason for hiding this comment

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

From an accessibility perspective, LGTM. Thank you!

Copy link
Contributor

@mapk mapk left a comment

Choose a reason for hiding this comment

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

I just tested. It works good from a design perspective.

@kjellr
Copy link
Contributor Author

kjellr commented Jun 17, 2019

Thanks, all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). General Interface Parts of the UI which don't fall neatly under other labels. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Menu hover state missing, or impossibly hard to see
4 participants