fix(a11y): Fix ARIA attributes and switch from div to nav for the explorer
#1972
Conversation
| @@ -116,7 +117,7 @@ export default ((userOpts?: Partial<Options>) => { | |||
| <polyline points="6 9 12 15 18 9"></polyline> | |||
| </svg> | |||
| </button> | |||
| <div class="explorer-content" aria-expanded={false}> | |||
| <div id="explorer-content"> | |||
There was a problem hiding this comment.
A while back all components were changed to use classes over id's to allow non-atomic usage.
There was a problem hiding this comment.
I see what you mean, but the issue with this is that aria-attributes actually requires id's and don't work with classes.
One way to deal with this would be to generate an id for these components, just like the Overflow list did.
There was a problem hiding this comment.
Except they don't. They require a unique identifier field per tag. This does not have to be the id field.
There was a problem hiding this comment.
I do think that aria-controls requires the id tag of the element. Here are the specifications
aria-controls specification
Id reference list value type
This element in particular requires it because the button has aria-controls="explorer-content".
HTML element concept, where if you scroll down you see what the unique identifier is according to the dom.
There was a problem hiding this comment.
Alright, but that still doesn't address the issue I raised.
Your proposed changes remove the classes and add id's. Removing the classes will break component functionality.
In addition, id's must be unique for aria to function properly, per your own sources. Currently, components are non-atomic. If you want to add id's for the purpose of accessibility, please ensure uniqueness in case of multiple components of the same type. (Ideally consistent as well, e.g. no random generated appended text.)
There was a problem hiding this comment.
Yeah, Like I said I could generate them if you want to support multiple of the same component.
It would also require the inline script to change a bit so it targets the correct one, but I think we can manage that.
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
The ARIA attributes didn't have correct values and/or labels on the explorer & TOC components.
The
aria-controlsshould refer to an ID reference and not a class like it currently was, this is fixed for both the TOC and explorer.Some buttons didn't have a correct label because there was no direct text-node child, fixed with either
aria-labeloraria-labelledby.I've taken the liberty to change the classes that the
aria-controlsreferred to into ID's. This could be considered a breaking change, some people's styles might refer these classes instead of ID's. I'm not opposed in keeping these classes around if we want to mitigate this.I've also added a data-* attribute on the overflow list instead of the id. This is so we could accommodate for the id that
aria-controlsrequire on the TOC.Lastly I've changed the explorer root element to a
navas it resembles the main navigation, which greatly helps screen readers as well.Valid ARIA attributes sources