[Menu] Improve hover/select/focus UI display

[rosskevin]

Problem description

After initial open of menu, first item remains highlighted even after hovering off.

Steps to reproduce

next branch, menus demo http://localhost:3000/#/component-demos/menus

1. click to open
2. hover to a lower item

Versions

next branch a38068f

• Browser: Mac Chrome 52.0.2743.116
• Browser: Mac Firefox 48.0.2
• Browser: Mac Safari 9.1.2

Images

[ArcanisCz]

@oliviertassinari Hi, sorry to be inactive for awhile!

Moving discussion from closed PR

I am not sure i understand Nathan...

Problem is with hover and different expexcted UX for touch and mouse devices. This way, when you open menu with something selected (first item is selected by default, but lets put this aside now), this remain selected until you click/touch another option. Alternatively, you can move your selected item with keyboard arrows.

Selected item has grey background. Hovered items also have grey background. So visually, you have 2 selected items when you move your mouse inside Menu (1 selected, 1 by hover). What should mouse hovering signify?

1. my proposal in rejected PR was to tie together hover and select states, which is quite radical solution. But it solves this dichotomy.
2. Both states could have different background color, and we will delete preselecting first option (keyboard actions with menu open will activate that though). This will solve problem with user perceiving 2 selected options.

Should i implement that 2) option?

[oliviertassinari]

@ArcanisCz I have been doing some benchmark, it's even worse than that. We have 3 possibles state. The focus, selected, and hover. You can have all of them at the same time:

Also, we should also consider where is the original focus, we focus the selected item or the first one

• The native select input on chrome tie together the hover and select state but have a different style for the selected one:
  

The selected item is focused by default, otherwise, the first one.

• react-bootstrap dropdown use different styles
  
  The button is focused by default

[oliviertassinari]

I think that the best option is to keep the default focus behavior and to change the style of the three possible different states. I like the react-bootstrap solution (same as bootrap@4-alpha).

[blackdynamo]

I would also be interested in an option to not have the first value selected. You may want a menu that performs actions and you never select an item. In order to get my desired effect, I have had to do the following:

<MenuItem key="placeholder" style={{display: "none"}} />

This is always the first item in the list so it gets highlighted but you don't see it cause it's hidden. Although this works, it is not ideal.

[pjuke]

I would really like to see this feature. Currently I'm using a <Popover> with a <List> to get around it, which is not really optimal.

Above work-around would also work, but is not ideal.

[mbrookes]

I think that the best option is to keep the default focus behavior and to change the style of the three possible different states.

In Google inbox, hover removes keyboard focus, so there is only ever one focused item. (Same as for Mac OS). For the Drawer list, they're using a slightly different shade for selected vs focus.

[oliviertassinari]

@mbrookes The "three state" approach seems quite common:

• angular material2
  
• Vuetify
  
• Amazon
  
• Bootstrap v4
  
• material-web-components
  

I'm going with this approach.

[oliviertassinari]

The final approach:

1. disabled
2. focus
3. selected
4. hover

[oliviertassinari]

The logic is no longer implemented, something has caused a regression, somewhere. We should take care of it. Also, it's a good opportunity to challenge what the best tradeoff is.

[joshwooding]

@oliviertassinari Do you have a reproduction I can look at? I'm not sure what I'm looking for :)

[oliviertassinari]

@joshwooding We have explored different approaches in #16331 (comment). What do you think of the approach n°1?

[eps1lon]

@joshwooding We have explored different approaches in #16331 (comment). What do you think of the approach n°1?

I want to double down on this quote:

We can challenge the need for these two states to have a different UI. I assume that most people use the mouse or the keyboard exclusively, rarely the two at the same time.

I completely agree here. One important part of a11y is that you consider focus(-visible) like any cursor. A sighted user can use the mouse to navigate to actions while screen reader users can use tab-focus. The focus outline marks the item that will be activated on-enter just like the hover marks the item that activates on mousedown. Enter and mousedown+up will both trigger a click event in the DOM. Styling hover and focus the same way is the only sensible thing if you think about it from a functional standpoint.

Since it's already hard enough to distinguish selected and focus I'd be perfectly fine with styling focus and hover the same way.

[oliviertassinari]

We can challenge the need for these two states to have a different UI. I assume that most people use the mouse or the keyboard exclusively, rarely the two at the same time.

@eps1lon Agree.

---

I have looked at the specification, I couldn't find any resources on the problem.
A new benchmark can help (selected, focused & hover states, in this order top-down):

console.cloud.google.com

studio.youtube.com (merge focus & hover)

calendar.google.com (merge selected, focus & hover)

ads.google.com (merge focus & hover)

search.google.com (merge selected, focus & hover)

---

In this context, I would propose this fix:

diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index d663d8934..9d11b30a3 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -23,7 +23,7 @@ export const styles = theme => ({
     paddingTop: 8,
     paddingBottom: 8,
     '&$focusVisible': {
-      backgroundColor: theme.palette.action.selected,
+      backgroundColor: theme.palette.action.hover,
     },
     '&$selected, &$selected:hover': {
       backgroundColor: theme.palette.action.selected,

We use to have this logic, it seems to have been lost (by mistake?) between #14680 and #14212. I'm not sure what I had in mind 🤔. Would it work for you guys?

[eps1lon]

Also interesting that native <select /> makes no distinction at all between hover and selected. Once you hover over the first item the "highlight cursor" will follow hover. In keyboard modality it will follow focus. Both modalities have different behavior with regard to selection state (mouse: selection does not follow hover, keyboard: selection does follow focus).

[oliviertassinari]

Also interesting that native <select /> makes no distinction at all between hover and selected.

@eps1lon On which environment? On macOS, I have:

selected

hover/focused

Regarding the multi-select use case, the keyboard focus state needs to take precedence over the selected state, otherwise, you can't see where your focus is if you have, let's say 3 options selected consecutively.

I would propose a slightly different version to #5186 (comment)

I like this tradeoff:

• use the same style for hover/focus
• keep the outline for keyboard focus to be visible when the item is selected
• have the selected state to keep precedence over the hover/focus one.

What do you think?

diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index d663d8934..dd03def41 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -23,7 +23,9 @@ export const styles = theme => ({
     paddingTop: 8,
     paddingBottom: 8,
     '&$focusVisible': {
-      backgroundColor: theme.palette.action.selected,
+      backgroundColor: theme.palette.action.hover,
+      outline: '2px solid #999',
+      outlineOffset: -2,
     },
     '&$selected, &$selected:hover': {
       backgroundColor: theme.palette.action.selected,

or

• use the same style for hover/focus
• have the hover/focus state to keep precedence over the selected one.

diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index d663d8934..bf00ec1fe 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -22,11 +22,11 @@ export const styles = theme => ({
     textAlign: 'left',
     paddingTop: 8,
     paddingBottom: 8,
-    '&$focusVisible': {
+    '&$selected': {
       backgroundColor: theme.palette.action.selected,
     },
-    '&$selected, &$selected:hover': {
-      backgroundColor: theme.palette.action.selected,
+    '&$focusVisible': {
+      backgroundColor: theme.palette.action.hover,
     },
     '&$disabled': {
       opacity: 0.5,

[eps1lon]

What do you think?

I would need a render preview for that. Pure code is for UI/UX hard to review.

[oliviertassinari]

@eps1lon OK, I will open a pull request after #17037 is merged. The multi-select case adds constraints on the scope of the solutions available.

[oliviertassinari]

We might have a recommendation from the specification:

https://material.io/design/interaction/states.html#anatomy