-
Notifications
You must be signed in to change notification settings - Fork 1k
feat(tooltip): add isLabelledBy prop for screen reader label support #3595
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
Conversation
🦋 Changeset detectedLatest commit: 1b3b526 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Hey would be really awesome to get a review on this small accessibility improvement as its the only thing preventing us from using this primitive 😁 It replaces the awkward workaround currently featured in storybook to use the tooltip as a label. |
Thanks for this PR, I'm looking forward to this, it will make our testing library tests easier! |
Not entirely clear why this is needed. Can't you pass |
@chaance Unfortunately, that results in buggy behaviour where the screen reader announces the label three times. Since this is a common use case for tooltips, it would make sense for Radix to support it directly in the API to avoid this bug or the need for fiddly workarounds. Untitled.mov |
I do see the larger point, which is that perhaps associating the tooltip is more appropriate in some scenarios as the desired behavior. This is certainly common, but I'm not sure if it should be encouraged. Tooltips are already clunky from an accessibility/usability perspective, and they are least problematic when used to provide context rather than serve as a label. There are cases IMO where API friction is a good thing. If your product needs demand it (I know developers don't always get the final say here), we don't stop you from implementing the behavior you want. We just don't want to make doing the usually bad thing too easy. It's a tightrope to be sure. You can pass <Tooltip.Root>
<Tooltip.Trigger aria-labelledby="foo" aria-describedby={null!}>
<button />
</Tooltip.Trigger>
<Tooltip.Content id="foo" />
</Tooltip.Root> I'll leave this open for now. I could be convinced that it's worthwhile, but I'm ambivalent. I'll seek some outside opinions and come back to it if I decide one way or another. |
Unfortunately that only reduced the number of times the screenreader would repeat the label from 3 to 2 times. Totally agree tooltips are clunky from an accessibility / usability perspective. That said, I do see a common use for it as a label for example
In the cases above, the semantically correct thing would be to use |
Hey, I'm affiliated with @alizehkhan and we're currently looking into implementing fully accessible tooltips. We did some research and identified two valid use cases:
While it's good to provide developers a stable basis for accessible components without the need for additional configuration, the devil is in the detail – especially in accessibility. We didn't find any established component library that reflects the necessary differences between these two use cases, although it's just about offering developers the option to change one attribute. I believe that making a considered decision with the help of good documentation should be simple enough and will eliminate a lot of confusion about the accessible name and description of an element. |
Description
Accessibility improvement: Adds an optional
isLabelledBy
prop to theTooltipTrigger
component, allowing the tooltip content to be announced by screen readers as a label (viaaria-labelledby
) instead of the default description (aria-describedby
). This is useful when the tooltip should serve as the accessible name of the trigger element.AriaLabel story was adjusted to use and explain the new prop.
Screen.Recording.2025-06-13.at.12.24.17.PM.mov