Skip to content

Commit

Permalink
Tooltip: add aria-describedby to anchor only if not redundant (#65989)
Browse files Browse the repository at this point in the history
* Improve comment

* Add unit tests

* Add temporary storybook examples

* CHANGELOG

* Fix lint errors

* Remove temporary storybook examples

---

Co-authored-by: ciampo <mciampini@git.wordpress.org>
Co-authored-by: tyxla <tyxla@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: afercia <afercia@git.wordpress.org>
  • Loading branch information
5 people committed Oct 14, 2024
1 parent 8675a06 commit b67fdfe
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 1 deletion.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Bug Fixes

- `Tooltip`: add `aria-describedby` to the anchor only if not redundant ([#65989](https://github.com/WordPress/gutenberg/pull/65989)).
- `PaletteEdit`: dedupe palette element slugs ([#65772](https://github.com/WordPress/gutenberg/pull/65772)).
- `RangeControl`: do not tooltip contents to the DOM when not shown ([#65875](https://github.com/WordPress/gutenberg/pull/65875)).
- `Tabs`: fix skipping indication animation glitch ([#65878](https://github.com/WordPress/gutenberg/pull/65878)).
Expand Down
9 changes: 8 additions & 1 deletion packages/components/src/tooltip/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,16 @@ function UnforwardedTooltip(
// TODO: this is a temporary workaround to minimize the effects of the
// Ariakit upgrade. Ariakit doesn't pass the `aria-describedby` prop to
// the tooltip anchor anymore since 0.4.0, so we need to add it manually.
// The `aria-describedby` attribute is added only if the anchor doesn't have
// one already, and if the tooltip text is not the same as the anchor's
// `aria-label`
// See: https://github.com/WordPress/gutenberg/pull/64066
// See: https://github.com/WordPress/gutenberg/pull/65989
function addDescribedById( element: React.ReactElement ) {
return describedById && mounted
return describedById &&
mounted &&
element.props[ 'aria-describedby' ] === undefined &&
element.props[ 'aria-label' ] !== text
? cloneElement( element, { 'aria-describedby': describedById } )
: element;
}
Expand Down
78 changes: 78 additions & 0 deletions packages/components/src/tooltip/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -516,4 +516,82 @@ describe( 'Tooltip', () => {
).not.toHaveClass( 'components-tooltip' );
} );
} );

describe( 'aria-describedby', () => {
it( "should not override the anchor's aria-describedby attribute if specified", async () => {
render(
<>
<Tooltip { ...props }>
<button aria-describedby="tooltip-test-description">
Tooltip anchor
</button>
</Tooltip>
{ /* eslint-disable-next-line no-restricted-syntax */ }
<p id="tooltip-test-description">Tooltip description</p>
<button>focus trap outside</button>
</>
);

expect(
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
).toHaveAccessibleDescription( 'Tooltip description' );

// Focus the anchor, tooltip should show
await press.Tab();
expect(
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
).toHaveFocus();
await waitExpectTooltipToShow();

// The anchors should retain its previous accessible description,
// since the tooltip shouldn't override it.
expect(
screen.getByRole( 'button', { name: 'Tooltip anchor' } )
).toHaveAccessibleDescription( 'Tooltip description' );

// Focus the other button, tooltip should hide
await press.Tab();
expect(
screen.getByRole( 'button', { name: 'focus trap outside' } )
).toHaveFocus();
await waitExpectTooltipToHide();
} );

it( "should not add the aria-describedby attribute to the anchor if the tooltip text matches the anchor's aria-label", async () => {
render(
<>
<Tooltip { ...props }>
<button aria-label={ props.text }>
Tooltip anchor
</button>
</Tooltip>
<button>focus trap outside</button>
</>
);

expect(
screen.getByRole( 'button', { name: props.text } )
).not.toHaveAccessibleDescription();

// Focus the anchor, tooltip should show
await press.Tab();
expect(
screen.getByRole( 'button', { name: props.text } )
).toHaveFocus();
await waitExpectTooltipToShow();

// The anchor shouldn't have an aria-describedby prop
// because its aria-label matches the tooltip text.
expect(
screen.getByRole( 'button', { name: props.text } )
).not.toHaveAccessibleDescription();

// Focus the other button, tooltip should hide
await press.Tab();
expect(
screen.getByRole( 'button', { name: 'focus trap outside' } )
).toHaveFocus();
await waitExpectTooltipToHide();
} );
} );
} );

0 comments on commit b67fdfe

Please sign in to comment.