material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Accessibility Issue: Screen Reader not properly announcing expanded Menu

Open sforehand opened this issue 3 years ago • 7 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Current behavior 😯

If you create a menu and apply the proper aria properties, the screen reader does not properly announce when the menu is expanded. The screen reader announces the menu is collapsed however.

I have verified with NVDA (Windows) and the OS X VoiceOver.

Expected behavior 🤔

The screen reader should announce when menu is expanded by clicking the related button or pressing enter via the keyboard.

Steps to reproduce 🕹

Steps:

  1. Go to https://mui.com/material-ui/react-menu/ and observe the Basic Menu example.
  2. Use the CodeSandBox button to run the code associated with the button and menu (https://codesandbox.io/s/u2u8cx?file=/demo.js)
  3. Using a Screen Reader (i.e. NVDA with Windows), you will notice the menu is initially announced as COLLAPSED (which is correct).
  4. Click the button or press enter on the button. You will notice the screen reader does not announce the menu as EXPANDED. However, the aria-expanded property is properly set.

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

sforehand avatar Aug 25 '22 17:08 sforehand

Thanks for the report. Can you please share a benchmark you've used for how it should behave? We use https://www.w3.org/WAI/ARIA/apg/patterns/menubutton/ as a reference for the behavior & a11y.

mnajdova avatar Aug 31 '22 08:08 mnajdova

hi @mnajdova . Thanks for the response. I believe the benchmark you are using is correct. When the menu is opened, the aria-expanded is properly set but neither NVDA (windows) or VoiceOver (OS X) reads the expanded property. I am not sure if it concerns the menu taking focus or some sort of timing issue concerning a state change. Originally I believed it was my local code but the same issue occurs with the MUI example code listed above. What do you think?

sforehand avatar Aug 31 '22 13:08 sforehand

@mnajdova Hi. Here is a screenshot of NVDA after the menu is expanded. You can see it is not reported as expanded

https://ibb.co/Jx9yggk

sforehand avatar Aug 31 '22 14:08 sforehand

i highly suspect that the problem is that the aria-expanded and/or aria-controls attributes are not used according to the spec.

aria-expanded

spec: https://www.w3.org/TR/wai-aria-1.1/#aria-expanded

These are its possible values:

true = The element, or another grouping element it controls, is expanded. false = The element, or another grouping element it controls, is collapsed. undefined = The element, or another grouping element it controls, is neither expandable nor collapsible; all its child elements are shown or there are no child elements.

in both the docs and the sandbox, when the menu is collapsed, the value of aria-expanded is undefined. i know that's what the patterns site recommends (https://www.w3.org/WAI/ARIA/apg/patterns/menubutton/), but the spec must take precedence. thus, when the menu is collapsed, we should use aria-expanded="false".

aria-controls

spec: https://www.w3.org/TR/wai-aria-1.1/#aria-controls

aria-controls is an ARIA property. unlike states, properties usually stay the same, unless there's a good reason for them to change. in this case, the only reason for aria-controls to change would be if the button was to suddenly control a different menu.

in both the docs and the sandbox, when the menu is collapsed, the value of aria-controls is undefined. the correct behavior would be to use the same value as when it is expanded: aria-controls="basic-menu".

caveats

i don't have a windows machine available right now, so can you @sforehand check if my changes fix the problem on NVDA? also, can you tell us what browser (including its version) you were using? i suspect that something about either NVDA itself or NVDA + whatever browser is triggering the following parallel logics:

  • "oh hey, here's a button with aria-popup but no aria-expanded or aria-controls" --> "sure, i'll treat it as a collapsed menu button"
  • "oh hey, here's a button with no aria-expanded --> "i surely don't need to keep an eye of it in case it becomes expanded"

on the other hand, i did test this on VoiceOver (both macOS and iOS, latest Chrome version) and in both cases i found the behavior that i would have expected if i had never read this issue in the first place: the button is initially not announced as expanded or collapsed (since it doesn't have aria-expanded), and is then announced as expanded once aria-expanded="true". so the behavior you encountered might have been the fault of the browser only, and not any of the screen readers... i recommend further testing with multiple screen readers and browsers.

xurxe avatar Dec 12 '22 06:12 xurxe

in both the docs and the sandbox, when the menu is collapsed, the value of aria-controls is undefined. the correct behavior would be to use the same value as when it is expanded: aria-controls="basic-menu".

If I remember correctly, this was changed so that the aria-controls won't reference an element that doesn't exists, for example when the menu is hidden (it is not rendered, not just hidden).

mnajdova avatar Dec 13 '22 14:12 mnajdova

If I remember correctly, this was changed so that the aria-controls won't reference an element that doesn't exists, for example when the menu is hidden (it is not rendered, not just hidden).

ah, i see. while that approach is technically acceptable in this case, it's not what i would recommend. it's a better practice to have the aria-controls point by ID to an element that always exists regardless, and only toggle the rendering of its contents according to whether the element is expanded or collapsed. for two reasons:

  • it allows user agents to build the accessibility tree in the correct way (because the controls/controlled relationship is always available).
  • it works in situations where the presence of aria-controls is required (namely, with the combobox and slider roles).

xurxe avatar Dec 15 '22 11:12 xurxe

it's a better practice to have the aria-controls point by ID to an element that always exists regardless, and only toggle the rendering of its contents according to whether the element is expanded or collapsed.

Ah nice, it's a great alternative. I learned something new today :D @michaldudak it may be worth testing this approach in MUI Base and updating if it works better.

If anyone wants to take a stab and try applying this fix in Material UI, I would be happy to review it.

mnajdova avatar Dec 15 '22 13:12 mnajdova