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.72k stars 32.23k forks source link

[Select] aria-expanded not getting set to "false" when listbox collapsed #25399

Closed slayybelle closed 3 years ago

slayybelle commented 3 years ago

listbox collapsed state should be indicated by aria-expanded="false"; instead no aria-expanded property is present. When the listbox is open, then aria-expanded is set to "true", as expected.
Seen in 4.11.3

We are getting flagged by our a11y consultants for violating WCAG2.1: ▪ 1.3.1 Info and Relationships (Level A) ▪ 4.1.2 Name, Role, Value (Level A)

The absence of aria-expanded="false" results in an inaccurate reading of state by VoiceOver on Mac.

I think the code is in: https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Select/SelectInput.js

Screen Shot 2021-03-17 at 10 05 07 PM

Below, the first screen shot is of VoiceOver output reading the state of a hand edited Select component, collapsed, but with 'aria-expanded="false"' added to <div class="MuiSelect-root ... element using chrome developer console.

The second screen shot is of VO output of a Select component, collapsed:

Select_hand-edited Select_no_aria-expanded
eps1lon commented 3 years ago

This changed in Aria 1.2: In 1.1 it defaulted to false which meant we can omit it. The default was removed in 1.2. The new example implementation in the APG works as suggested.

oliviertassinari commented 3 years ago

I didn't follow. What change should be done?

(I'm asking because the "good first issue" label suggests that there is a straightforward solution somebody that has never contributed to the project could handle.)

eps1lon commented 3 years ago

(I'm asking because the "good first issue" label suggests that there is a straightforward solution somebody that has never contributed to the project could handle.)

aria-expanded is either 'true' or 'false' not undefined. This was described in the original issue description.

oliviertassinari commented 3 years ago

@eps1lon OK thanks. I have slightly updated your initial comment as it seems to have a typo.

I believe we should also update the demos in the documentation in this case. For instance:

https://github.com/mui-org/material-ui/blob/09addbf5ca274d4e6d449a1dcdfe49e852f5f0d4/docs/src/pages/components/menus/BasicMenu.tsx#L22

cc @DanailH as we have used ARIA 1.1 for the data grid of the menu buttons.

eps1lon commented 3 years ago

@oliviertassinari Thanks for the correction

I believe we should also update the demos in the documentation in this case.

We should investigate first if the default for aria-expanded changed there as well or if AT is affected. Slightly upgrading the issue to "good to take" since some upfront investigation would be nice.

ghost commented 3 years ago

Can i work on this issue?

eps1lon commented 3 years ago

Can i work on this issue?

@Harish-Karthick Sure, thanks for taking the time :+1: I'm assigning you for visibility. If you got any questions feel free to ask. It's totally fine to open a PR even when you're blocked at some point.

slayybelle commented 3 years ago

Any way you guys can roll this fix into 4.x?