Skip to content

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 afercia and mapk 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!

@kjellr kjellr merged commit e13b639 into master Jun 17, 2019
@kjellr kjellr deleted the update/menu-item-hover-state-mobile branch June 17, 2019 11:40
@youknowriad youknowriad added this to the Gutenberg 6.0 milestone Jun 24, 2019
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

5 participants