From b67fdfee17571e72c79ee5f5c7a7aff454387817 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 10 Oct 2024 15:13:02 +0200 Subject: [PATCH] Tooltip: add aria-describedby to anchor only if not redundant (#65989) * Improve comment * Add unit tests * Add temporary storybook examples * CHANGELOG * Fix lint errors * Remove temporary storybook examples --- Co-authored-by: ciampo Co-authored-by: tyxla Co-authored-by: t-hamano Co-authored-by: afercia --- packages/components/CHANGELOG.md | 1 + packages/components/src/tooltip/index.tsx | 9 ++- .../components/src/tooltip/test/index.tsx | 78 +++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 87fd00dd8c826d..870f96308aa462 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -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)). diff --git a/packages/components/src/tooltip/index.tsx b/packages/components/src/tooltip/index.tsx index 7ce9311fc942ea..ce94daf67bfaba 100644 --- a/packages/components/src/tooltip/index.tsx +++ b/packages/components/src/tooltip/index.tsx @@ -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; } diff --git a/packages/components/src/tooltip/test/index.tsx b/packages/components/src/tooltip/test/index.tsx index 67922ab1d5ac41..3679b597b2cb14 100644 --- a/packages/components/src/tooltip/test/index.tsx +++ b/packages/components/src/tooltip/test/index.tsx @@ -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( + <> + + + + { /* eslint-disable-next-line no-restricted-syntax */ } +

Tooltip description

+ + + ); + + 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( + <> + + + + + + ); + + 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(); + } ); + } ); } );