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

fix: tab controls not labeled properly #916

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

andrejkaPry
Copy link

@andrejkaPry andrejkaPry commented May 23, 2024

The tab controls should be announced with the correct tab role, number and position as e.g., 'Smileys and People, selected, tab, 1 of 8'. Adding tablist role for the parent and tab role for the button does the trick.

I also modified aria-selected prop. Why? Using aria-selected={selected || undefined} can be problematic because it leads to inconsistent behaviour with how assistive technologies interpret the presence and value of the aria-selected attribute.

The aria-selected attribute is meant to have a clear true or false value to indicate the selected state.

The tab controls should be announced with the correct tab role, number and position as e.g., 'Smileys and People, selected, tab, 1 of 8'. Adding tablist role for the parent and tab role for the button does the trick.

I also modified aria-selected prop. Why? Using aria-selected={selected || undefined} can be problematic because it leads to inconsistent behaviour with how assistive technologies interpret the presence and value of the aria-selected attribute.

The aria-selected attribute is meant to have a clear true or false value to indicate the selected state.
Copy link

@JanPodmajersky JanPodmajersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

Once closing skin tone after choosing an option the focus got lost which is not a11y friendly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants