From 848e3fc1e9ba5083d1ad36b2f73623512c6d5162 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Oct 2024 14:45:18 +0200 Subject: [PATCH 1/6] Improve comment --- packages/components/src/tooltip/index.tsx | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/components/src/tooltip/index.tsx b/packages/components/src/tooltip/index.tsx index 7ce9311fc942e..ce94daf67bfab 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; } From b92c5de99a1f0bcb7f10f99e9dd9f1ab67c055bd Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Oct 2024 16:55:22 +0200 Subject: [PATCH 2/6] Add unit tests --- .../components/src/tooltip/test/index.tsx | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/packages/components/src/tooltip/test/index.tsx b/packages/components/src/tooltip/test/index.tsx index 67922ab1d5ac4..3679b597b2cb1 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(); + } ); + } ); } ); From 3d4d6cc34f02868a96b9111ab24094fc04ea4c22 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Oct 2024 16:55:34 +0200 Subject: [PATCH 3/6] Add temporary storybook examples --- .../src/tooltip/stories/index.story.tsx | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/packages/components/src/tooltip/stories/index.story.tsx b/packages/components/src/tooltip/stories/index.story.tsx index b006bc03aced9..0dfc96a62e8b0 100644 --- a/packages/components/src/tooltip/stories/index.story.tsx +++ b/packages/components/src/tooltip/stories/index.story.tsx @@ -36,6 +36,16 @@ const meta: Meta< typeof Tooltip > = { controls: { expanded: true }, docs: { canvas: { sourceState: 'shown' } }, }, + decorators: [ + ( Story ) => { + return ( + <> +

Button's description

+ + + ); + }, + ], }; export default meta; @@ -74,3 +84,19 @@ Nested.args = { ), text: 'Outer tooltip text', }; + +export const AlreadyWithDescription: StoryFn< typeof Tooltip > = Template.bind( + {} +); +AlreadyWithDescription.args = { + children: ( + + ), + text: 'Tooltip Text', +}; + +export const SameLabel: StoryFn< typeof Tooltip > = Template.bind( {} ); +SameLabel.args = { + children: , + text: 'Button label', +}; From d5349b8917ab948fec7580d082a611a1b83aca5f Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Oct 2024 16:56:31 +0200 Subject: [PATCH 4/6] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index a532fda22f972..315569cec9900 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)). From 7048449be6a3f9b8d3172c8f85f9c1cca9915459 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 9 Oct 2024 18:15:36 +0200 Subject: [PATCH 5/6] Fix lint errors --- packages/components/src/tooltip/stories/index.story.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/tooltip/stories/index.story.tsx b/packages/components/src/tooltip/stories/index.story.tsx index 0dfc96a62e8b0..836238bf5ccf2 100644 --- a/packages/components/src/tooltip/stories/index.story.tsx +++ b/packages/components/src/tooltip/stories/index.story.tsx @@ -40,7 +40,8 @@ const meta: Meta< typeof Tooltip > = { ( Story ) => { return ( <> -

Button's description

+ { /* eslint-disable-next-line no-restricted-syntax */ } +

Button description

); From f4948156eb5834a0da59d6c9b85e718516b7f77a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 10 Oct 2024 14:35:29 +0200 Subject: [PATCH 6/6] Remove temporary storybook examples --- .../src/tooltip/stories/index.story.tsx | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/packages/components/src/tooltip/stories/index.story.tsx b/packages/components/src/tooltip/stories/index.story.tsx index 836238bf5ccf2..b006bc03aced9 100644 --- a/packages/components/src/tooltip/stories/index.story.tsx +++ b/packages/components/src/tooltip/stories/index.story.tsx @@ -36,17 +36,6 @@ const meta: Meta< typeof Tooltip > = { controls: { expanded: true }, docs: { canvas: { sourceState: 'shown' } }, }, - decorators: [ - ( Story ) => { - return ( - <> - { /* eslint-disable-next-line no-restricted-syntax */ } -

Button description

- - - ); - }, - ], }; export default meta; @@ -85,19 +74,3 @@ Nested.args = { ), text: 'Outer tooltip text', }; - -export const AlreadyWithDescription: StoryFn< typeof Tooltip > = Template.bind( - {} -); -AlreadyWithDescription.args = { - children: ( - - ), - text: 'Tooltip Text', -}; - -export const SameLabel: StoryFn< typeof Tooltip > = Template.bind( {} ); -SameLabel.args = { - children: , - text: 'Button label', -};