[core] Add aria-describedby to Tooltip target#8100
Open
dokson wants to merge 2 commits into
Open
Conversation
Tooltip targets had no aria-describedby pointing at the tooltip content, making the tooltip text unavailable to assistive tech. Wrap the content in a div with role="tooltip" and a unique id, and pass aria-describedby on the target via Popover's targetProps. When content is empty/disabled, aria-describedby is omitted so the target doesn't reference a non-existent element. User-supplied targetProps still override. Fixes palantir#7672.
Generate changelog in
|
Author
|
@vnkommu — this addresses the missing A couple of behavioral notes worth confirming with your use case:
Feedback welcome on whether this covers what you needed. |
Add Popover.popoverContentProps (HTMLProps<HTMLDivElement>) symmetrical to targetProps, spread onto the .bp6-popover-content element. Tooltip now uses it to set id and role="tooltip" directly on the existing content div, avoiding an extra wrapper.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7672.
Tooltiptargets had noaria-describedbyreferencing the tooltip content, so screen readers had no way to announce the tooltip text to users when the target was focused.This PR implements the standard WAI-ARIA tooltip pattern by:
Popover.popoverContentPropsAPI (mirrors the existingtargetProps):HTMLProps<HTMLDivElement>spread onto the.bp6-popover-contentelement. Useful for attaching ARIA attributes (or any HTML props) to the floating content without extra DOM nodes.Tooltipto setid={uniqueId}androle="tooltip"directly on the existing content div, and passingaria-describedby={tooltipId}on the target viatargetProps.Why a new API instead of a wrapper div
An earlier iteration of this PR wrapped
contentin<div role="tooltip" id={tooltipId}>{content}</div>. Reviewer feedback pointed out that introducing a new pattern just for Tooltip would be a regression in API consistency, since Popover already exposestargetPropsfor the symmetric concern on the target side. The newpopoverContentPropsis the natural counterpart: same shape, same behavior, no extra DOM, and immediately usable by all consumers that forwardpopoverProps(Select, MultiSelect, Suggest, ContextMenu, DateInput, etc.) if they ever need to attach attributes to their popover content.API specifics
Spread on the existing
<div className={Classes.POPOVER_CONTENT}>wrapper, withclassNamemerged so consumer-supplied classes don't clobber Blueprint's. Other consumers can opt-in via<Popover popoverProps={{ popoverContentProps: { ... } }}>— no proactive change is required for them; they continue to render as before.Edge cases handled in Tooltip
POPOVER_WARN_EMPTY_CONTENTwarn path still fires;aria-describedbyis also omitted to avoid pointing at a missing element.aria-describedbyis omitted (the tooltip never opens, so the id never exists in the DOM).targetProps: their props are spread after our default, so a consumer-suppliedaria-describedbystill wins.popoverContentProps: their props are spread after our defaults, so a consumer can overrideid/roleif needed (e.g. for a custom ARIA pattern). They cannot accidentally drop thebp6-popover-contentclass — that's merged in popover.tsx.Why no other components need to change
role="menu"on its inner<Menu>element; nopopoverContentPropsneeded.role="listbox"on the QueryList; no change needed.PopoverSharedPropsfor content rendering.The new API is a general-purpose escape hatch; no consumer is forced to use it.
Test plan
pnpm --filter @blueprintjs/core run compile(esm, cjs, esnext, css)pnpm --filter @blueprintjs/core run test:typeCheckpnpm --filter @blueprintjs/core run test:vitest:run— 1250 passed (including existing Popover empty-content warn assertions)npx prettier --checkandeslintclean on changed filesFiles changed
packages/core/src/components/popover/popoverSharedProps.ts— newpopoverContentPropsprop with TSDocpackages/core/src/components/popover/popover.tsx— spreadpopoverContentPropson the.bp6-popover-contentdiv, mergingclassNamepackages/core/src/components/tooltip/tooltip.tsx— generate stabletooltipId, setid/roleviapopoverContentProps, setaria-describedbyviatargetProps, with the edge cases above