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

Menu hover state missing, or impossibly hard to see #12262

Closed
JJJ opened this issue Nov 24, 2018 · 10 comments · Fixed by #16168
Closed

Menu hover state missing, or impossibly hard to see #12262

JJJ opened this issue Nov 24, 2018 · 10 comments · Fixed by #16168
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. Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.

Comments

@JJJ
Copy link
Contributor

JJJ commented Nov 24, 2018

Describe the bug
There is a severe lack of contrast on the hover state of the vertical ellipsis menu.

To Reproduce
Steps to reproduce the behavior:

  1. Compose a new post
  2. Click on the vertical ellipsis button to expose the drop-down menu
  3. Hover over a menu item
  4. Try to tell which item is being hovered over

Expected behavior
I expect some visual indication that a menu item is being hovered over with a mouse.

Screenshots
screen shot 2018-11-23 at 11 24 10 pm

screen shot 2018-11-23 at 11 33 57 pm

Desktop (please complete the following information):

  • OS: any
  • Browser: any
  • Version: latest

Additional context

  • WordPress 5.0 RC 1
@Mariyan96
Copy link

From designer point of view, this is probably not an issue, though I would like to hear other designer's opinions on this as it might change the overall appearance for hover effects :)

@designsimply designsimply added the Needs Testing Needs further testing to be confirmed. label Nov 26, 2018
@designsimply
Copy link
Member

Tested and confirmed with 4.5.1 master @ e4fd51cb5 that hovered menu items only receive a very slight color change.

12262
Seen at http://localhost:8888/wp-admin/post.php?post=501&action=edit running WordPress 4.9.8 and Gutenberg 4.5.1 master @ e4fd51cb5 using Firefox 63.0.3 on macOS 10.13.6.

@designsimply designsimply added [Type] Enhancement A suggestion for improvement. General Interface Parts of the UI which don't fall neatly under other labels. Needs Design Feedback Needs general design feedback. and removed Needs Testing Needs further testing to be confirmed. labels Nov 28, 2018
@mapk
Copy link
Contributor

mapk commented Apr 6, 2019

I'd also like to know if this is an a11y problem. @LukePettway can you provide some insight here?

@mapk mapk added the Needs Accessibility Feedback Need input from accessibility label Apr 6, 2019
@afercia
Copy link
Contributor

afercia commented Apr 26, 2019

Discussed during today's accessibility bug scrub. Worth considering a clear indication of the hover state is important for some users and the cursor shape is not sufficient.

One of the most obvious options would be to make the focus and hover styles the same. Worth considering the Customizer does that, as it introduced some releases ago a new pattern with a "left border":

Screenshot 2019-04-26 at 16 40 57

This is also what we (as accessibility team) are recommending for the admin menu in the (long pending) Trac ticket https://core.trac.wordpress.org/ticket/28599

Some consistent pattern across WordPress would be nice :)

@afercia afercia added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). and removed Needs Accessibility Feedback Need input from accessibility labels Apr 26, 2019
@gziolo gziolo added the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Apr 26, 2019
@m-e-h
Copy link
Member

m-e-h commented May 9, 2019

From what I can tell, there currently is a hover effect on medium+ size screens. For some reason this is wrapped in a media query...

@media (min-width:782px) {
	.components-menu-item__button.components-icon-button:hover:not(:disabled):not([aria-disabled=true]),
	.components-menu-item__button:hover:not(:disabled):not([aria-disabled=true]) {
		color:#191e23;
		border:none;
		box-shadow:none;
		background:#f3f4f5
	}
}

I guess this is the reason #10333
I don't really understand it though. Surely there's a better solution.

@mapk
Copy link
Contributor

mapk commented May 15, 2019

Testing this, I see a hover state for desktop but when I switch my browser to responsive view, I get the very subtle text fade on mobile screen sizes. I assume this is due to mobile devices not needing a hover state.

screencast 2019-05-14 18-12-35

@m-e-h
Copy link
Member

m-e-h commented May 16, 2019

So I'm thinking, at some point after this gh issue, the hover effect was implemented without knowing about this gh issue?

I assume this is due to mobile devices not needing a hover state.

That probably is why the hover was removed for screens smaller than 782px.
But I think that might remove the hover for some maybe not so edge cases.

There are a couple media queries we can use to cover some of those "smaller screen + mouse" users.

The safest I think would be something like this:

@include break-medium() {
	@include menu-style__hover;
}

@media (hover: hover) {
	@include menu-style__hover;
}

That would cover screens larger than the medium breakpoint and then also screens that might be smaller but whose primary input method is capable of hovering.
https://developer.mozilla.org/en-US/docs/Web/CSS/@media/hover

The browser support is surprisingly solid for how little known these MQs are.

Some others that might also work:
@media (pointer: fine)
@media (any-hover: hover)
@media (any-pointer: fine)

What are your thoughts on this @jasmussen ?

@afercia
Copy link
Contributor

afercia commented May 16, 2019

I assume this is due to mobile devices not needing a hover state.

Worth noting that assuming a CSS media breakpoint triggers exclusively on mobile devices is misleading. For example, users with vision impairments may use high zoom levels in the browser. Depending also on the screen, at a 200% zoom level (or slightly more) the "responsive view" may kick in. In this scenario, users still need hover states.

@mapk
Copy link
Contributor

mapk commented May 17, 2019

Based on Andrea's comment, can we just include the hover state... always? Are there any arguments against going this route?

@m-e-h
Copy link
Member

m-e-h commented May 17, 2019

#10333 is why it was originally removed for mobile.

Just reading that issue, I don't think it justifies removing the hover state. But I've never actually seen the issue on iOS so I don't know how bad it is.

I'll try to play around with it tomorrow.

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. Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants