diff --git a/.eslintrc.js b/.eslintrc.js index 211ddd9bc35860..9264786f210c50 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -147,6 +147,60 @@ const restrictedSyntaxComponents = [ message: '`disabled` used without the `accessibleWhenDisabled` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)', }, + // Rules to find tooltip usage that needs reviewing. + { + // All tooltips + selector: `JSXOpeningElement[name.name="Tooltip"]`, + message: + 'Tooltip needs to have its anchor checked to make sure it has an accessible description if needed', + }, + ...[ + 'Tooltip', + 'ToolbarButton', + 'ToolbarDropdownMenu', + 'DropdownMenu', + ].map( ( componentName ) => ( { + selector: `JSXOpeningElement[name.name="${ componentName }"]:not(:has(JSXAttribute[name.name="__nextHasNoMarginBottom"]))`, + message: + componentName + + ' needs to have its anchor checked to make sure it has an accessible description if needed.', + } ) ), + { + // All Button s with a `label` and no explicit `showTooltip={false}` (includes the ones with no children) + selector: `JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="showTooltip"][value.expression.value!=true])):has(JSXAttribute[name.name="label"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, + { + // All Button s with a `shortcut` + selector: `JSXOpeningElement[name.name="Button"]:has(JSXAttribute[name.name="shortcut"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, + { + // All Buttons with a description prop + selector: `JSXOpeningElement[name.name="Button"]:has(JSXAttribute[name.name="description"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, + { + // All Buttons with a describedBy prop + selector: `JSXOpeningElement[name.name="Button"]:has(JSXAttribute[name.name="describedBy"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, + { + // All Buttons with a aria-describedby prop + selector: `JSXOpeningElement[name.name="Button"]:has(JSXAttribute[name.name="aria-describedby"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, + { + // All Buttons with a aria-label prop + selector: `JSXOpeningElement[name.name="Button"]:has(JSXAttribute[name.name="aria-label"])`, + message: + 'Button needs to be checked to make sure it has an accessible description if needed', + }, ]; module.exports = { diff --git a/packages/components/src/tooltip/index.tsx b/packages/components/src/tooltip/index.tsx index ce94daf67bfaba..071b5e4bc92b38 100644 --- a/packages/components/src/tooltip/index.tsx +++ b/packages/components/src/tooltip/index.tsx @@ -2,7 +2,6 @@ * External dependencies */ import * as Ariakit from '@ariakit/react'; -import { useStoreState } from '@ariakit/react'; import clsx from 'clsx'; /** @@ -14,7 +13,6 @@ import { useContext, createContext, forwardRef, - cloneElement, } from '@wordpress/element'; import deprecated from '@wordpress/deprecated'; @@ -94,7 +92,6 @@ function UnforwardedTooltip( placement: computedPlacement, showTimeout: delay, } ); - const mounted = useStoreState( tooltipStore, 'mounted' ); if ( isNestedInTooltip ) { return isOnlyChild ? ( @@ -104,31 +101,12 @@ 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 && - element.props[ 'aria-describedby' ] === undefined && - element.props[ 'aria-label' ] !== text - ? cloneElement( element, { 'aria-describedby': describedById } ) - : element; - } - return ( { isOnlyChild ? undefined : children } diff --git a/packages/components/src/tooltip/test/index.tsx b/packages/components/src/tooltip/test/index.tsx index 3679b597b2cb14..753f97be0ac3f0 100644 --- a/packages/components/src/tooltip/test/index.tsx +++ b/packages/components/src/tooltip/test/index.tsx @@ -64,36 +64,6 @@ describe( 'Tooltip', () => { expectTooltipToBeHidden(); } ); - it( 'should associate the tooltip text with its anchor via the accessible description when visible', async () => { - render( ); - - // The anchor can not be found by querying for its description, - // since that is present only when the tooltip is visible - expect( - screen.queryByRole( 'button', { description: 'tooltip text' } ) - ).not.toBeInTheDocument(); - - // Hover the anchor. The tooltip shows and its text is used to describe - // the tooltip anchor - await hover( - screen.getByRole( 'button', { - name: 'Tooltip anchor', - } ) - ); - expect( - await screen.findByRole( 'button', { - description: 'tooltip text', - } ) - ).toBeInTheDocument(); - - // Hover outside of the anchor, tooltip should hide - await hoverOutside(); - await waitExpectTooltipToHide(); - expect( - screen.queryByRole( 'button', { description: 'tooltip text' } ) - ).not.toBeInTheDocument(); - } ); - it( 'should not leak Tooltip props to the tooltip anchor', () => { render( @@ -489,9 +459,7 @@ describe( 'Tooltip', () => { screen.queryByRole( 'tooltip', { name: 'Inner tooltip' } ) ).not.toBeInTheDocument(); expect( - screen.getByRole( 'button', { - description: 'Outer tooltip', - } ) + screen.getByRole( 'tooltip', { name: 'Outer tooltip' } ) ).toBeVisible(); // Hover outside of the anchor, tooltip should hide @@ -516,82 +484,4 @@ 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(); - } ); - } ); } );