Select Accessibility

[austinChappell]

The Select component is no longer accessible as of v4.0 if the Select input does not have a selected option.

• This is not a v3.x issue.
• I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior 🤔

If rendering a Select component that does not have a selected value, the options should be accessible via the keyboard

Current Behavior 😯

The options are only available if there is a selection already made. (Note: This bug does not occur in MUI v3.x)

Steps to Reproduce 🕹

Link:

1. sandbox

Context 🔦

Your Environment 🌎

Tech	Version
Material-UI	v4.0.1
React	v16.8.6
Browser
TypeScript
etc.

[oliviertassinari]

@austinChappell You are not supposed to provide value outside of the options available. However, I believe we can solve the problem like this:

diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index ef02c38f1..a43bcd215 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -91,7 +91,7 @@ const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : Reac
 const ListItem = React.forwardRef(function ListItem(props, ref) {
   const {
     alignItems = 'center',
-    autoFocus,
+    autoFocus = false,
     button = false,
     children: childrenProp,
     classes,
diff --git a/packages/material-ui/src/MenuList/MenuList.js b/packages/material-ui/src/MenuList/MenuList.js
index 8226da2fe..c7008613f 100644
--- a/packages/material-ui/src/MenuList/MenuList.js
+++ b/packages/material-ui/src/MenuList/MenuList.js
@@ -44,7 +44,11 @@ function textCriteriaMatches(nextFocus, textCriteria) {

 function moveFocus(list, currentFocus, disableListWrap, traversalFunction, textCriteria) {
   let wrappedOnce = false;
-  let nextFocus = traversalFunction(list, currentFocus, currentFocus ? disableListWrap : false);
+  let nextFocus = traversalFunction(
+    list,
+    currentFocus,
+    currentFocus && currentFocus !== list ? disableListWrap : false,
+  );

   while (nextFocus) {
     // Prevent infinite loop.
@@ -76,7 +80,14 @@ function moveFocus(list, currentFocus, disableListWrap, traversalFunction, textC
 const useEnhancedEffect = typeof window === 'undefined' ? React.useEffect : React.useLayoutEffect;

 const MenuList = React.forwardRef(function MenuList(props, ref) {
-  const { actions, autoFocus, className, onKeyDown, disableListWrap = false, ...other } = props;
+  const {
+    actions,
+    autoFocus = false,
+    className,
+    disableListWrap = false,
+    onKeyDown,
+    ...other
+  } = props;
   const listRef = React.useRef(null);
   const textCriteriaRef = React.useRef({
     keys: [],
@@ -116,10 +127,7 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
     const key = event.key;
     const currentFocus = ownerDocument(list).activeElement;

-    if (
-      (key === 'ArrowUp' || key === 'ArrowDown') &&
-      (!currentFocus || (currentFocus && !list.contains(currentFocus)))
-    ) {
+    if ((key === 'ArrowUp' || key === 'ArrowDown') && !list.contains(currentFocus)) {
       moveFocus(list, null, disableListWrap, nextItem);
     } else if (key === 'ArrowDown') {
       event.preventDefault();
diff --git a/packages/material-ui/src/Select/SelectInput.js b/packages/material-ui/src/Select/SelectInput.js
index 812aa8ae0..7388707cb 100644
--- a/packages/material-ui/src/Select/SelectInput.js
+++ b/packages/material-ui/src/Select/SelectInput.js
@@ -172,7 +172,7 @@ const SelectInput = React.forwardRef(function SelectInput(props, ref) {
   delete other['aria-invalid'];

   let display;
-  let displaySingle = '';
+  let displaySingle;
   const displayMultiple = [];
   let computeDisplay = false;

@ryancogswell Could you confirm I'm on the right track?
@austinChappell Do you want to submit a pull request?

[ryancogswell]

Could you confirm I'm on the right track?

@oliviertassinari Yes -- looks good.

Here's a sandbox where I played around with it a bit: https://codesandbox.io/s/select-focus-rf8wl

[ianschmitz]

Just a heads up that this is a bigger issue than Select. Menu/MenuList have lost keyboard a11y. I've been trying to fix it in a branch but it's proving to be pretty tricky.

[eps1lon]

Just a heads up that this is a bigger issue than Select. Menu/MenuList have lost keyboard a11y. I've been trying to fix it in a branch but it's proving to be pretty tricky.

Seems like a regression from 4.0.2. At least if I understand the issue https://v4-0-2.material-ui.com/components/menus/ seems to work fine.

But that is not directly related to this issue, no?

[oliviertassinari]

@ianschmitz Definitely, hence the important flag :), only 10% of our issues have this flag.
@eps1lon v4.0.2 doesn't behave correctly from my perspective. Use the keyboard. Tab to focus the element. Enter to open it. Keydown doesn't work.

[ianschmitz]

Disabling keepMounted in the "Simple Menu" example in docs gets us as far as having the ul focused, which allows the up/down arrow keys to work, however the first item is not focused by default.

The useLayoutEffect that runs in MenuList runs just after the initial render. By the time the Menu is opened, the list.focus() doesn't run from what i can tell.

[oliviertassinari]

But that is not directly related to this issue, no?

@eps1lon It's the same fix: #15973 (comment).

[oliviertassinari]

@plumpNation I see that you are experiencing this problem too in #16242. Do you want to submit a pull request with the fix proposed in #15973 (comment)? :)

[eps1lon]

Closed with #16323

[pelikhan]

It seems that this issue back. When trying the samples in https://material-ui.com/components/selects/ in latest Edge, the keyboard navigation does not work on menu items.