From 1fdd5d9b0338efba362a5f5f4acf210de7ae6b28 Mon Sep 17 00:00:00 2001
From: Marco Ciampini <marco.ciampo@gmail.com>
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 <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>
---
 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 eb5d15a335887d..f42d14ebfc59f6 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)).
 
 ## 28.9.0 (2024-10-03)
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(
+				<>
+					<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();
+		} );
+	} );
 } );