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.54k stars 32.19k forks source link

[Chip] Disabled a11y regression in MUIv5 #30012

Open davakos opened 2 years ago

davakos commented 2 years ago

Duplicates

Latest version

Current behavior 😯

In updating to Material-UI v5.2.2 (from v5.0.6) I have noticed what appears to be a regression related to accessibility in the <Chip> component. It looks like the underlying <div><span> created by the component only designates the component as disabled via CSS class .Mui-disabled. This does not pass an automated a11y test using Jest's expect(chipComponent).toHaveAttribute("aria-disabled");

Expected behavior πŸ€”

Previously (last tested MUIv5.0.6 & v4.12.2), a disabled <Chip> component was marked disabled using the accessible aria-disabled attribute

Steps to reproduce πŸ•Ή

Steps: (Tested in Jest tests and in latest Chrome browser on MacOS)

  1. Install MUI v5.2.2
  2. Render a disabled chip, <Chip disabled .../>
  3. Inspect DOM element created by Chip. There is no aria-disabled="true" attribute
  4. To compare, install MUI <= 5.0.6 or v4, perform steps 2-3 and there should be an aria-disabled="true" attribute

Context πŸ”¦

I am using Chip components to display some status information and need to differentiate some information as disabled. I need this to be accessible so it's clear to all users what is applicable or not.

Note: These are Basic informational/non-interactive Chips, I am not using actions, deletion, etc.

Your environment 🌎

`npx @mui/envinfo` ``` @emotion/react: ^11.7.0 => 11.7.0 @emotion/styled: ^11.6.0 => 11.6.0 @mui/base: 5.0.0-alpha.58 @mui/icons-material: ^5.2.0 => 5.2.0 @mui/lab: ^5.0.0-alpha.58 => 5.0.0-alpha.58 @mui/material: ^5.2.2 => 5.2.2 @mui/private-theming: 5.2.2 @mui/styled-engine: 5.2.0 @mui/styles: ^5.2.2 => 5.2.2 @mui/system: 5.2.2 @mui/types: 7.1.0 @mui/utils: 5.2.2 @types/react: ^17.0.37 => 17.0.37 react: ^17.0.2 => 17.0.2 react-dom: ^17.0.2 => 17.0.2 typescript: ~4.5.2 => 4.5.2 ```
mnajdova commented 2 years ago

There is a disabled prop set. Is this not working for your scenario? As far as I could find, the logic was changed in https://github.com/mui-org/material-ui/pull/22430

davakos commented 2 years ago

Thank you for the quick response!

I see the change here: https://github.com/mui-org/material-ui/pull/22430/files#diff-3ee3a1b94d868a490d64bec24f5d4b3ee75a4a8ff4bbda5f9380be1b23b1850aL424

It looks like the updated code is relying on clickable being set to determine if the element should be disabled, but in my use-case I want to disable these Chips when they are not clickable actions, just display items.

I tried switching my automated tests from expect(chipComponent).toHaveAttribute("aria-disabled") to expect(chipComponent).toBeDisabled() (also tried expect(chipComponent).toHaveAttribute("disabled")) but am still getting the same failure that the element is not disabled.

Looking at the DOM produced by the Chip, it looks like:

<div class="MuiChip-root MuiChip-outlined Mui-disabled MuiChip-sizeSmall MuiChip-colorDefault MuiChip-outlinedDefault css-3l3a7h-MuiChip-root"><span class="MuiChip-label MuiChip-labelSmall css-wjsjww-MuiChip-label">Feature 1</span></div>

Besides the CSS class, I don't see any props there that would indicate the component is disabled for a11y purposes. I can always add an extra aria-disabled={true} to my usage of <Chip> but it was nice when the component handled this when disabled={true} was set.

Codex- commented 9 months ago

Encountered this too while migrating to Mui5 (large codebase)

This comment:

I don't think so, the aria-disabled is meant for when we don't use a ButtonBase.

is interesting because I found even when disabled the button does not have the aria-disabled attribute