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

feat(SegmentedControl): make listbox role to fix a11y #6691

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

bvandercar-vt
Copy link
Contributor

Improves a11y of new SegmentedControl item

@adidahiya
Copy link
Contributor

Various open source UI toolkits have different approaches for Segmented Control a11y:

Can you explain the motivations for your change and make an argument for setting the default as you've done in this PR?

I think a safer, better approach might be to add good documentation to the component docs suggesting that users should set their own ARIA role appropriately by specifying the role attribute.

@bvandercar-vt
Copy link
Contributor Author

bvandercar-vt commented Mar 14, 2024

@adidahiya that is tricky. After reading through those, I definitely think my original choice of listbox was incorrect.

It sounds like radiogroup is the best option, I changed the code to that. Despite what the Primer link says, I see no evidence why it shouldn't be radiogroup when that's exactly how it behaves.

If you would like me to add a prop so that users can set the role manually (role?="group" | "list" | "radiogroup") and then we set the children roles and aria props based on that as well, I'm down. Otherwise, I don't think anyone would complain about it being radiogroup

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

Successfully merging this pull request may close these issues.

None yet

2 participants