BrightspaceUI / core

A collection of accessible, free, open-source web components for building Brightspace applications.
Apache License 2.0
64 stars 24 forks source link

We shouldn't be putting aria-* attributes on custom elements #1954

Open svanherk opened 2 years ago

svanherk commented 2 years ago

axe-core version 4.3.4 caused a number of our tests to start breaking because we place aria-expanded on things like d2l-button, d2l-subtle-button, etc. We then pass down the value into the aria-expanded attribute on the actual native button. Even though this works from a screenreader POV, we really should be using something like just expanded here. This may be an issue for other aria-* attributes as well.

The change was reverted in 4.3.5 after discussion in this issue: https://github.com/dequelabs/axe-core/issues/3241 It's still unknown whether this will be added back in version 4.4.0, or if it will be added as a warning. But we should go through and fix these when we can to avoid lots of warnings (in both core tests and everywhere that uses d2l-button tests) or having to lock our axe-core version everywhere.

svanherk commented 2 years ago

Update - axe-core v4.4.0 (releasing at the end of this month) will add back the violation for non-global ARIA attributes on elements with a role. Custom elements will report as Needs Review rather than a failure.

So this shouldn't fail our builds (unless we have violations on native elements), and we should get a log to help us track down places we need to fix.

KaiPrince commented 2 years ago

Does this include role?

svanherk commented 2 years ago

I believe role is fine and sometimes necessary, and we've definitely leveraged that in a few places. role and any global aria properties won't fail the tests, but I still think we'd want to avoid the global aria-* properties if all we're doing is passing them down to the web component. Like if we want to put an optional aria-live on an internal element, we should have the custom element's property just be live (or whatever).