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 <[email protected]> Co-authored-by: tyxla <[email protected]> Co-authored-by: t-hamano <[email protected]> Co-authored-by: afercia <[email protected]>
WordPress · Oct 10, 2024 · b06549d · b06549d · github-actions · Oct 10, 2024
1 parent b52cf1f
commit b06549d
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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();
+		} );
+	} );
 } );