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

Accordion header buttons need aria-controls=panel-id #5750

Closed
carmacleod opened this issue Mar 30, 2020 · 6 comments · Fixed by #5787
Closed

Accordion header buttons need aria-controls=panel-id #5750

carmacleod opened this issue Mar 30, 2020 · 6 comments · Fixed by #5787

Comments

@carmacleod
Copy link
Contributor

The accessibility documentation for the Carbon accordion says:

... the header buttons have a aria-control property set that points to the unique id of the panel it controls.

(nit: note that the sentence should say "an aria-controls property" and not "a aria-control property")

The issue is that there is no aria-controls property set on the accordion header buttons, and there should be. (react and vanilla)

@joshblack
Copy link
Contributor

Hi there @carmacleod! 👋

I wanted to clarify, does the accordion button require aria-controls or is it considered a nice-to-have? If I understand correctly, this property is only supported by JAWS and the behavior of the attribute itself could be considered confusing (although this could easily be outdated info by now)

Thanks so much for taking the time to make this issue, btw! 🙏

@carmacleod
Copy link
Contributor Author

Heh - everybody quotes Heydon's article when aria-controls comes up. I've quoted it myself. 😄

So, no, it's not required, but it is recommended for accordions. It gives screen readers the opportunity to take advantage of the relationship... although at present, I believe JAWS is the only one that does (they removed the very verbose keyboard help message that people objected to). However, longtime users of JAWS expect it to be there when they hear "expanded", and contrary to certain articles, it doesn't do any harm to have it there. The ARIA Working Group is going to be discussing aria-controls in detail sometime this year.

Will div.bx--accordion__content elements always immediately follow their corresponding button.bx--accordion__heading in the tab/reading/DOM order?

Another question... about headings. Was planning to open a different issue about this, but I'll piggyback on this one. :) The APG recommends using <hx><button>Accordion Heading</button></hx>. That's also how Heydon would do it. ;)

So I was wondering how a component library would handle headings, because you don't know the information architecture of the page that the component will be used in, so you can't know which heading level to use... if any at all. Only the author using the component can know for sure whether or not headings are appropriate, and which level. You would almost need a prop called headinglevel that could take values of 0-6, where 0 is the default and means "no headings, thanks, because I'm not creating collapsible sections (just using for an FAQ or something like that)". Thoughts?

@joshblack
Copy link
Contributor

Totally fair @carmacleod! Will definitely make sure this gets updated.

And agreed on the headings. I think we should add docs to show that folks can use headings with accordion instead of us needing to bake it into the component. I would be curious what you think of this example from Reach that documents this 🤔

@carmacleod
Copy link
Contributor Author

Actually, adding docs to show how to use headings sounds like a good idea, assuming Carbon users have that flexibility?

(Sorry to be asking such a simple question - I really need to carve out some time ⏲ to run through the Carbon tutorials so that I can create my own example pages instead of just React-ing ;) to pages my colleagues and others are creating...)

@joshblack
Copy link
Contributor

Great question! Definitely don't apologize, love talking about all this stuff with you and taking in all the great knowledge you have to share 😄 I think it should be able to support that, or at the very least we should make it work such that this is the case. Another option was exploring using aria-level but that seems like a bad idea versus just using headings lol.

Let me know if you ever want to pair up some time and go through carbon! I would love to give ya a tour when you have free time 🎉

@carmacleod
Copy link
Contributor Author

Let me know if you ever want to pair up some time and go through carbon! I would love to give ya a tour when you have free time 🎉

Wow, what a kind offer! That would be so awesome! Will DM for sure to figure out a good time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants