search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.33k stars 32.12k forks source link

[Select] Keyboard navigation doesn't work on Firefox with NVDA enabled #19218

Open danmartinelluc opened 4 years ago

danmartinelluc commented 4 years ago

Current Behavior 😯

On Firefox, with NVDA enabled, if a menu item has been selected the up and down arrow keys will no longer navigate through the menu items. (In some circumstances navigation prior to a selection isn't possible, and only the first option can be chosen)

Expected Behavior 🤔

After selecting a menu item, keyboard navigation should be able to switch between menu items.

Steps to Reproduce 🕹

Issue can be reproduced on the select component page: https://material-ui.com/components/selects/

NVDA download here: https://www.nvaccess.org/download/

Steps:

  1. On a windows machine install and enable NVDA, open Firefox
  2. Using Tab to navigate, hover the select component
  3. Use Enter to open the menu
  4. With the Up and Down arrow keys navigate between the menu options (If a selection has already been made and is not just a placeholder, the issue can be seen here)
  5. Select an option with Enter
  6. Use Enter to open the menu again, the arrow keys are no longer able to navigate between menu items

Context 🔦

This impacts the accessibility of anything using a select component.

Your Environment 🌎

Tech Version
Material-UI v4.3.1
React v16.8.0
Browser Firefox
eps1lon commented 4 years ago

Are you using the latest NVDA version (2019.2.1)?

ahayes91 commented 4 years ago

I've just updated to NVDA 2019.2.1 and can confirm that I'm seeing the same issue.

For this component I would also expect to hear "collapsed" or "expanded" when on the button: https://a11y-guidelines.orange.com/web_EN/exemples/simple-menu/simple-menu.html# might be a helpful example, and hopefully not too far away from the current implementation.

A simple accessible select menu with a correctly associated label would probably be recommended though, for example, if using the select for pagination:

<label for="pagenum">Page</label>
<div class="pageselect">
    <select id="pagenum">
        <option value="1">1</option>
        <option value="2">2</option>
    </select>
</div>
eps1lon commented 4 years ago

I've just updated to NVDA 2019.2.1 and can confirm that I'm seeing the same issue.

This sounds like it was working in prior versions of NVDA. Could you clarify whether this changed during a Material-UI or NVDA upgrade?

For this component I would also expect to hear "collapsed" or "expanded" when on the button: a11y-guidelines.orange.com/web_EN/exemples/simple-menu/simple-menu.html# might be a helpful example, and hopefully not too far away from the current implementation.

That example illustrates a menu which is different from a HTML select which is a combobox. I'm not convinced by the linked example anyway. It doesn't use any aria attributes to indicate a dropdown/popup/expansion character (e.g. aria-expanded or aria-haspopup) and the menu is only a list not a menu. The expected navigation is very different between lists and menus.

The issue is that the whole select/combobox story is in a very bad spot currently considering the ARIA spec. We would need to conduct our own research with assistive technology users for which we do not have the capacity.

I would recommend using the NativeSelect instead. You loose customizability but have the best a11y story until spec folks figure out how an alternative markup for a combobox should look like.

I'll check out why NVDA is breaking the navigation (which is working without NVDA running).

ahayes91 commented 4 years ago

This sounds like it was working in prior versions of NVDA. Could you clarify whether this changed during a Material-UI or NVDA upgrade?

We actually came across this ourselves back in August 2019 with NVDA 2019.2.0 and Material UI v3, it took us a while to update to Material UI 4.7.0 😅 So this issue has been present since MUI v3 at least, and in NVDA 2019.2.0 at least.

I would recommend using the NativeSelect instead. You loose customizability but have the best a11y story until spec folks figure out how an alternative markup for a combobox should look like.

In fairness the native select looks pretty much exactly like that example markup above, so that will probably work for our purposes in the meantime. 👍

danmartinelluc commented 4 years ago

I can also confirm that this has been an issue for some time. While the current testing was done with the most recent version of NVDA/all other factors, the issue was first reported in May 2019.

eps1lon commented 4 years ago

TL;DR: This seems to be fixed as of 4.8.3. We did work on the a11y story of the Select component in the past months so this might have been fixed during that work.

Summary

It is working on

  1. chrome without NVDA
  2. chrome with NVDA
  3. firefox without NVDA
  4. firefox with NVDA

Reproduction

  1. Open https://material-ui.com/components/selects/#select (4.8.3)
  2. Move focus right before the first select (e.g. by clicking the fragment link in the heading)
  3. Press Tab (focuses Select)
  4. Press enter (opens Select)
  5. Press enter (selects first value, closes Select and moves focus to Select)
  6. Press enter (opens Select)
  7. Press ArrowDown (activates the 2nd value)
  8. Press ArrowDown (activates the 3rd value)
  9. Press enter (selects 3rd value, closes Select and moves focus to Select)

Current behavior

Printing NVDA speech viewer output after the page is loaded and the test is so that the focus is right before the first Select component and NVDA speech output is silent.

using firefox

NVDA 2019.2 + Firefox 72.0.1 (64-bit) (Windows 10) Age ​ menu button subMenu Age list Ten 1 of 3 Age Ten menu button subMenu Age list Ten 1 of 3 Twenty 2 of 3 Thirty 3 of 3 Age Thirty menu button subMenu

using chrome

NVDA 2019.2 + Version 79.0.3945.117 (Official Build) (64-bit) (Win 10) Age ​ menu button subMenu Age list Ten 1 of 3 Age Ten menu button subMenu Age list Ten 1 of 3 Twenty 2 of 3 Thirty 3 of 3 Age Thirty menu button subMenu

Aside: We should add a notice to check if the issue is present on the latest Material-UI version.

ahayes91 commented 4 years ago
  • Press enter (opens Select)
  • Press enter (selects first value, closes Select and moves focus to Select)

This is the issue that we were seeing, just did a quick test with NVDA and Firefox - I wouldn't expect a screenreader user to have to hit "enter" twice in order to be able to use the arrow keys to navigate the menu. I'd expect something more like this:

  1. Open https://material-ui.com/components/selects/#select (4.8.3)
  2. Move focus right before the first select (e.g. by clicking the fragment link in the heading)
  3. Press Tab (focuses Select)
  4. Press enter (opens Select with focus on first value)
  5. ArrowDown can be used to navigate the options
eps1lon commented 4 years ago

This is the issue that we were seeing, just did a quick test with NVDA and Firefox - I wouldn't expect a screenreader user to have to hit "enter" twice in order to be able to use the arrow keys to navigate the menu. I'd expect something more like this:

I did this because the issue stated that it was only observable when a value was already selected:

With the Up and Down arrow keys navigate between the menu options (If a selection has already been made and is not just a placeholder, the issue can be seen here)

You do not need it to press enter twice to use arrow keys. This is pretty obvious if you read the content of the parantheses (second enter closes the select again). But as per issue description it was required. Please open a separate issue if you have different steps to reproduce.

eps1lon commented 4 years ago

The missing piece was the mode you were in. In focus mode everything is working as expected. In browse mode it is not though (which is where nvda is intercepting key strokes). We probably don't communicate quite right what kind of element you're navigating. I guess it's a combination of using a Popover for the listbox (and hiding the element referenced in aria-labelledby) and using roving tabindex instead of aria-activedescendant. https://www.w3.org/TR/wai-aria-practices/examples/listbox/listbox-collapsible.html is working fine in NVDA in both modes.

eps1lon commented 4 years ago

I could reduce this problem using "just" react: https://codesandbox.io/s/nvda-browse-mode-listbox-nzukw

~It seems like NVDA is not supporting keyboard navigation using roving tabindex. It expects the listbox to have actual focus and the currently focused option to be marked with aria-activedescendant.~

~The good news for us is that a native <select size="3"> is also not navigateable with arrow keys in NVDA's browse mode. I'm opening an issue in the NVDA repository. I'm leaving this open to be aware of the issue. In the short term we can't do much because rewriting the component using aria-activedescendant is expensive and potentially breaking (considering :focus and :focus-within).~ Ok now I understood how you should navigate a