Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Chip] Disabled a11y regression in MUIv5 #30012

Open
2 tasks done
davakos opened this issue Dec 2, 2021 · 3 comments
Open
2 tasks done

[Chip] Disabled a11y regression in MUIv5 #30012

davakos opened this issue Dec 2, 2021 · 3 comments
Labels
component: chip This is the name of the generic UI component, not the React module! support: question Community support but can be turned into an improvement

Comments

@davakos
Copy link

davakos commented Dec 2, 2021

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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
@davakos davakos added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Dec 2, 2021
@mnajdova
Copy link
Member

mnajdova commented Dec 3, 2021

There is a disabled prop set. Is this not working for your scenario? As far as I could find, the logic was changed in #22430

@mnajdova mnajdova added component: chip This is the name of the generic UI component, not the React module! support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Dec 3, 2021
@davakos
Copy link
Author

davakos commented Dec 3, 2021

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-
Copy link

Codex- commented Jan 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: chip This is the name of the generic UI component, not the React module! support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

3 participants