Skip to content

Commit 79b0555

Browse files
ciampotyxlat-hamanoafercia
authored andcommitted
Tooltip: add aria-describedby to anchor only if not redundant (WordPress#65989)
* Improve comment * Add unit tests * Add temporary storybook examples * CHANGELOG * Fix lint errors * Remove temporary storybook examples --- Co-authored-by: ciampo <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: afercia <[email protected]>
1 parent aea0232 commit 79b0555

File tree

3 files changed

+87
-1
lines changed

3 files changed

+87
-1
lines changed

packages/components/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
### Bug Fixes
66

7+
- `Tooltip`: add `aria-describedby` to the anchor only if not redundant ([#65989](https://github.com/WordPress/gutenberg/pull/65989)).
78
- `PaletteEdit`: dedupe palette element slugs ([#65772](https://github.com/WordPress/gutenberg/pull/65772)).
89
- `RangeControl`: do not tooltip contents to the DOM when not shown ([#65875](https://github.com/WordPress/gutenberg/pull/65875)).
910
- `Tabs`: fix skipping indication animation glitch ([#65878](https://github.com/WordPress/gutenberg/pull/65878)).

packages/components/src/tooltip/index.tsx

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,16 @@ function UnforwardedTooltip(
107107
// TODO: this is a temporary workaround to minimize the effects of the
108108
// Ariakit upgrade. Ariakit doesn't pass the `aria-describedby` prop to
109109
// the tooltip anchor anymore since 0.4.0, so we need to add it manually.
110+
// The `aria-describedby` attribute is added only if the anchor doesn't have
111+
// one already, and if the tooltip text is not the same as the anchor's
112+
// `aria-label`
110113
// See: https://github.com/WordPress/gutenberg/pull/64066
114+
// See: https://github.com/WordPress/gutenberg/pull/65989
111115
function addDescribedById( element: React.ReactElement ) {
112-
return describedById && mounted
116+
return describedById &&
117+
mounted &&
118+
element.props[ 'aria-describedby' ] === undefined &&
119+
element.props[ 'aria-label' ] !== text
113120
? cloneElement( element, { 'aria-describedby': describedById } )
114121
: element;
115122
}

packages/components/src/tooltip/test/index.tsx

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,4 +516,82 @@ describe( 'Tooltip', () => {
516516
).not.toHaveClass( 'components-tooltip' );
517517
} );
518518
} );
519+
520+
describe( 'aria-describedby', () => {
521+
it( "should not override the anchor's aria-describedby attribute if specified", async () => {
522+
render(
523+
<>
524+
<Tooltip { ...props }>
525+
<button aria-describedby="tooltip-test-description">
526+
Tooltip anchor
527+
</button>
528+
</Tooltip>
529+
{ /* eslint-disable-next-line no-restricted-syntax */ }
530+
<p id="tooltip-test-description">Tooltip description</p>
531+
<button>focus trap outside</button>
532+
</>
533+
);
534+
535+
expect(
536+
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
537+
).toHaveAccessibleDescription( 'Tooltip description' );
538+
539+
// Focus the anchor, tooltip should show
540+
await press.Tab();
541+
expect(
542+
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
543+
).toHaveFocus();
544+
await waitExpectTooltipToShow();
545+
546+
// The anchors should retain its previous accessible description,
547+
// since the tooltip shouldn't override it.
548+
expect(
549+
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
550+
).toHaveAccessibleDescription( 'Tooltip description' );
551+
552+
// Focus the other button, tooltip should hide
553+
await press.Tab();
554+
expect(
555+
screen.getByRole( 'button', { name: 'focus trap outside' } )
556+
).toHaveFocus();
557+
await waitExpectTooltipToHide();
558+
} );
559+
560+
it( "should not add the aria-describedby attribute to the anchor if the tooltip text matches the anchor's aria-label", async () => {
561+
render(
562+
<>
563+
<Tooltip { ...props }>
564+
<button aria-label={ props.text }>
565+
Tooltip anchor
566+
</button>
567+
</Tooltip>
568+
<button>focus trap outside</button>
569+
</>
570+
);
571+
572+
expect(
573+
screen.getByRole( 'button', { name: props.text } )
574+
).not.toHaveAccessibleDescription();
575+
576+
// Focus the anchor, tooltip should show
577+
await press.Tab();
578+
expect(
579+
screen.getByRole( 'button', { name: props.text } )
580+
).toHaveFocus();
581+
await waitExpectTooltipToShow();
582+
583+
// The anchor shouldn't have an aria-describedby prop
584+
// because its aria-label matches the tooltip text.
585+
expect(
586+
screen.getByRole( 'button', { name: props.text } )
587+
).not.toHaveAccessibleDescription();
588+
589+
// Focus the other button, tooltip should hide
590+
await press.Tab();
591+
expect(
592+
screen.getByRole( 'button', { name: 'focus trap outside' } )
593+
).toHaveFocus();
594+
await waitExpectTooltipToHide();
595+
} );
596+
} );
519597
} );

0 commit comments

Comments
 (0)