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

Allow tooltips to label non-interactive triggers #246

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

robintown
Copy link
Member

The combination of tooltip labels and non-interactive triggers was problematic: The tooltip component would place the label on the span element that it creates, rather than your provided trigger element. If your trigger was an icon or image, then accessibility tooling would flag it as missing a label. You might try putting aria-hidden on the element, but then you would still end up with the label being on an element that lacks a role (which is illegal).

The solution I've implemented here is to move the aria-labelledby and aria-describedby attributes to the trigger.

The combination of tooltip labels and non-interactive triggers was problematic: The tooltip component would place the label on the span element that it creates, rather than your provided trigger element. If your trigger was an icon or image, then accessibility tooling would flag it as missing a label. You might try putting aria-hidden on the element, but then you would still end up with the label being on an element that lacks a role (which is illegal).

The solution I've implemented here is to move the aria-labelledby and aria-describedby attributes to the trigger.
@robintown robintown requested a review from a team as a code owner September 5, 2024 19:48
@robintown robintown requested review from dbkr and removed request for a team September 5, 2024 19:48
Copy link

Deploying compound-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: ecb25ce
Status: ✅  Deploy successful!
Preview URL: https://8ea1bdb7.compound-web.pages.dev
Branch Preview URL: https://tooltip-label-child.compound-web.pages.dev

View logs

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Nice catch

@robintown robintown merged commit 1248b2c into main Sep 6, 2024
6 checks passed
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