-
Notifications
You must be signed in to change notification settings - Fork 44
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
: make toggle button names unique
#2383
Conversation
The latest updates on your projects. Learn more about Vercel for Git โ๏ธ
|
Accordion
: make toggle button names unique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool change!
What do you think about adding an integration test to make sure the property value is set correctly?
4b232e6
to
932b5d4
Compare
@zamoore for sure! just added a couple tests. |
packages/components/src/components/hds/accordion/item/button.hbs
Outdated
Show resolved
Hide resolved
1522a5b
to
f061634
Compare
f061634
to
e3095b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
๐ Summary
If merged, this PR would update the accordion toggle to make the button names unique.
๐ ๏ธ Detailed description
To make
aria-label
not required on each item but still create unique labels for the toggle buttons, I decided to usearia-labelledby
.aria-labelledby
is set to the id of the wrapper around the toggle block and I removed thearia-label
default value. This makes it so the accessible name is just the title of the accordion.If the developer passes an
aria-label
to the accordion item,aria-labelledby
isn't set.NOTE:
I used to have a different implementation where I also included a visually hidden piece of text that said "Expand" so the button name would be "Expand {title}", but after discussing with @MelSumner we decided that wasn't even necessary because the fact that it shows/hide content is implied by
aria-expanded
. When you navigate to the button with VoiceOver it says "{title}, collapsed, button" and then when you press the button it says "expanded". this makes including the word "expand" in the button name redundnant.๐ธ Screenshots
DOM after changes:
๐ External links
Jira ticket: HDS-3809
๐ Component checklist
๐ฌ Please consider using conventional comments when reviewing this PR.