search

carbon-design-system / carbon

A design system built by IBM
https://www.carbondesignsystem.com
Apache License 2.0
7.87k stars 1.82k forks source link

[a11y]: Tooltip hides buttons' aria labels #17828

Open wkeese opened 1 month ago

wkeese commented 1 month ago

Package

@carbon/react

Browser

Chrome

Operating System

MacOS

Package version

1.68

React version

18

Automated testing tool and ruleset

accessibility-checker

Assistive technology

No response

Description

Apparently anything with a Tooltip ends up with both an aria-label and an aria-labelledby.

You can see it for example on the Close button of a modal:

  <button
    aria-label="Close the window"
    aria-labelledby="tooltip-:r8:"
    class="cds--btn--icon-only cds--modal-close cds--btn cds--btn--primary"
    type="button"
  />

Also, Pagination's next and previous buttons:

<button aria-label="Previous page" aria-labelledby="tooltip-:r4q:" ...>...</button>

This is wrong, see https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label, which says:

Don't use both [aria-label and aria-labelledby] on the same element because aria-labelledby will take precedence over aria-label if both are applied.

But it's worse than that because the aria-labelledby points to a node (the tooltip) that has aria-hidden=true:

<span aria-hidden="true" id="tooltip-:r4q:" role="tooltip" class="cds--popover"><span class="cds--popover-content cds--tooltip-content">Previous page</span><span class="cds--popover-caret"></span></span>

I think the aria-hidden goes away when you focus the button, but it still seems weird (and wrong) that the button sort-of doesn't have a label until you focus it.

You've probably talked about this before, but why not use aria-describedby instead?

WCAG 2.1 Violation

Probably

Reproduction/example

https://react.carbondesignsystem.com/?path=/story/components-pagination--default

Steps to reproduce

Just navigate that that page and look at the DOM.

Suggested Severity

None

Code of Conduct

wkeese commented 1 month ago

I updated the title and description upon realizing this applies to anything with a Tooltip.

Luckily at least on Chrome, the accessibility tree takes the label from the tooltip node even though it's aria-hidden.

But it does break https://testing-library.com/, specifically if I have this Carbon CSS:

.cds--popover-content {
  display: none;
}
.cds--popover--open > .cds--popover > .cds--popover-content {
  display: block;
}

Then this statement will fail:

screen.getByRole("button", { name: "Close the window" })