Skip to content

Commit

Permalink
ToolTip: refactor using ariakit (#48440)
Browse files Browse the repository at this point in the history
* Add ariakit dependency

* Initial creation of component

* Add shortcut from legacy tooltip

* Replace initial tests with legacy tooltip tests

* Add anchor styles for tooltip placement

* Ensure tooltip renders only if one child is passed

* Replace query with tooltip role and add user actions to ensure tests complete

* Remove forwarding refs to match legacy tooltip

* Add legacy positioning

* Remove unnecessary code and add describedby id

* Replace cloneElement with Radix Slot

* Replace emotion with sass

* Update tests and types

* Updated ariakit package and update-related props and types

* Remove radix-ui/react-slot related package changes

* Add arrow to fix floating-ui errors

* Replace zero value in temp fix to avoid NaN errors

* resolve test failures after package update

* Match legacy styles

* Hide tooltip onBlur to match legacy behavior

* Revert "resolve test failures after package update"

This reverts commit 9182f1e.

* Prevent leaking by ensuring tooltip is hidden after each test

* Remove ‘overlay’ option to match legacy

* hide on anchor interact to match legacy

* Temporarily replace placement with position for rollout

* Update story after storybook upgrade

* Update descriptions in readme and types

* Replace legacy tooltip with ariakit tooltip

* Add test with modal for new Esc feature

* Error for multiple children but continue to render anchor without tooltip

* Update snapshots

* Update assertions in failing tests

* Replace waitFor with findBy

* Replace workaround with utility function

* Clean up variables

* Update package-lock

* Add cleanup to test where toBePositioned was used previously

* Update Changelog

* Update event handling

* Fix failing test

* Add cleanup for other components testing tooltip

* Remove Arrow workaround after floating-ui/core update

* Refine test query to avoid conflict

* Update tests based on feedback

* remove unnecessary type and variable

* Simplify by removing unnecessary logic and adding default value to position prop

* Add timeout to test for custom delay

* Ensure anchor shows when Icon or multiple children

* Update types and README

* Update snapshots

* Remove type ignore after update in #54101

* Fix link control tests by querying explicitly for the spinner

---------

Co-authored-by: Marco Ciampini <marco.ciampo@gmail.com>
  • Loading branch information
brookewp and ciampo authored Sep 7, 2023
1 parent 2af5a24 commit 938e747
Show file tree
Hide file tree
Showing 23 changed files with 582 additions and 768 deletions.
3 changes: 0 additions & 3 deletions packages/base-styles/_z-index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,6 @@ $z-layers: (
// And block manager sticky disabled block count is higher still
".edit-post-block-manager__disabled-blocks-count": 2,

// Needs to be higher than any other element as this is an overlay to catch all events
".components-tooltip .event-catcher": 100002,

// Needs to appear below other color circular picker related UI elements.
".components-circular-option-picker__option-wrapper::before": -1,

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ exports[`ColorPaletteControl matches the snapshot 1`] = `
class="components-circular-option-picker__option-wrapper"
>
<button
aria-describedby="tooltip-0"
aria-label="Color: red"
aria-selected="true"
class="components-button components-circular-option-picker__option is-pressed"
Expand Down
16 changes: 12 additions & 4 deletions packages/block-editor/src/components/link-control/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,9 @@ describe( 'Basic rendering', () => {
await user.type( searchInput, 'Hello' );

// Wait for the spinner SVG icon to be rendered.
expect( await screen.findByRole( 'presentation' ) ).toBeVisible();
expect(
await screen.findByTestId( 'components-spinner' )
).toBeVisible();
// Check the suggestions list is not rendered yet.
expect( screen.queryByRole( 'listbox' ) ).not.toBeInTheDocument();

Expand All @@ -190,7 +192,9 @@ describe( 'Basic rendering', () => {
// Check the suggestions list is rendered.
expect( resultsList ).toBeVisible();
// Check the spinner SVG icon is not rendered any longer.
expect( screen.queryByRole( 'presentation' ) ).not.toBeInTheDocument();
expect(
screen.queryByTestId( 'components-spinner' )
).not.toBeInTheDocument();

const searchResultElements =
within( resultsList ).getAllByRole( 'option' );
Expand Down Expand Up @@ -455,14 +459,18 @@ describe( 'Searching for a link', () => {
// Simulate searching for a term.
await user.type( searchInput, searchTerm );

expect( await screen.findByRole( 'presentation' ) ).toBeVisible();
expect(
await screen.findByTestId( 'components-spinner' )
).toBeVisible();
expect( screen.queryByRole( 'listbox' ) ).not.toBeInTheDocument();

// make the search suggestions fetch return a response
resolver( fauxEntitySuggestions );

expect( await screen.findByRole( 'listbox' ) ).toBeVisible();
expect( screen.queryByRole( 'presentation' ) ).not.toBeInTheDocument();
expect(
screen.queryByTestId( 'components-spinner' )
).not.toBeInTheDocument();
} );

it.each( [ 'With spaces', 'Uppercase', 'lowercase' ] )(
Expand Down
2 changes: 2 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Making Circular Option Picker a `listbox`. Note that while this changes some public API, new props are optional, and currently have default values; this will change in another patch ([#52255](https://github.com/WordPress/gutenberg/pull/52255)).
- `ToggleGroupControl`: Rewrite backdrop animation using framer motion shared layout animations, add better support for controlled and uncontrolled modes ([#50278](https://github.com/WordPress/gutenberg/pull/50278)).
- `Popover`: Add the `is-positioned` CSS class only after the popover has finished animating ([#54178](https://github.com/WordPress/gutenberg/pull/54178)).
- `Tooltip`: Replace the existing tooltip to simplify the implementation and improve accessibility while maintaining the same behaviors and API ([#48440](https://github.com/WordPress/gutenberg/pull/48440)).

### Bug Fix

Expand All @@ -33,6 +34,7 @@

### Enhancements

- Make the `Popover.Slot` optional and render popovers at the bottom of the document's body by default. ([#53889](https://github.com/WordPress/gutenberg/pull/53889)).
- `ProgressBar`: Add transition to determinate indicator ([#53877](https://github.com/WordPress/gutenberg/pull/53877)).
- Prevent nested `SlotFillProvider` from rendering ([#53940](https://github.com/WordPress/gutenberg/pull/53940)).

Expand Down
15 changes: 11 additions & 4 deletions packages/components/src/button/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { plusCircle } from '@wordpress/icons';
*/
import Button from '..';
import Tooltip from '../../tooltip';
import cleanupTooltip from '../../tooltip/test/utils';

jest.mock( '../../icon', () => () => <div data-testid="test-icon" /> );

Expand Down Expand Up @@ -193,7 +194,7 @@ describe( 'Button', () => {

render( <Button icon={ plusCircle } label="WordPress" /> );

expect( screen.queryByText( 'WordPress' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'WordPress' ) ).not.toBeVisible();

// Move focus to the button
await user.tab();
Expand Down Expand Up @@ -230,12 +231,14 @@ describe( 'Button', () => {
/>
);

expect( screen.queryByText( 'Label' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'Label' ) ).not.toBeVisible();

// Move focus to the button
await user.tab();

expect( screen.getByText( 'Label' ) ).toBeVisible();

await cleanupTooltip( user );
} );

it( 'should populate tooltip with description content for buttons with visible labels (buttons with children)', async () => {
Expand Down Expand Up @@ -287,12 +290,14 @@ describe( 'Button', () => {
<Button icon={ plusCircle } label="WordPress" children={ [] } />
);

expect( screen.queryByText( 'WordPress' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'WordPress' ) ).not.toBeVisible();

// Move focus to the button
await user.tab();

expect( screen.getByText( 'WordPress' ) ).toBeVisible();

await cleanupTooltip( user );
} );

it( 'should not show the tooltip when icon and children defined', async () => {
Expand Down Expand Up @@ -321,12 +326,14 @@ describe( 'Button', () => {
</Button>
);

expect( screen.queryByText( 'WordPress' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'WordPress' ) ).not.toBeVisible();

// Move focus to the button
await user.tab();

expect( screen.getByText( 'WordPress' ) ).toBeVisible();

await cleanupTooltip( user );
} );
} );

Expand Down
6 changes: 5 additions & 1 deletion packages/components/src/color-palette/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,11 @@ describe( 'ColorPalette', () => {
// Clear the color, confirm that the relative values are cleared/updated.
await user.click( screen.getByRole( 'button', { name: 'Clear' } ) );
expect( screen.getByText( 'No color selected' ) ).toBeVisible();
expect( screen.queryByText( colorName ) ).not.toBeInTheDocument();
expect(
screen.queryByText( colorName, {
selector: '.components-color-palette__custom-color-name',
} )
).not.toBeInTheDocument();
expect( screen.queryByText( colorCode ) ).not.toBeInTheDocument();
expect(
screen.getByRole( 'button', {
Expand Down
1 change: 1 addition & 0 deletions packages/components/src/spinner/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export function UnforwardedSpinner(
focusable="false"
{ ...props }
ref={ forwardedRef }
data-testid="components-spinner"
>
{ /* Gray circular background */ }
<SpinnerTrack
Expand Down
23 changes: 14 additions & 9 deletions packages/components/src/tab-panel/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { wordpress, category, media } from '@wordpress/icons';
* Internal dependencies
*/
import TabPanel from '..';
import cleanupTooltip from '../../tooltip/test/utils';

const TABS = [
{
Expand Down Expand Up @@ -116,7 +117,7 @@ describe.each( [
for ( let i = 0; i < allTabs.length; i++ ) {
expect(
screen.queryByText( TABS_WITH_ICON[ i ].title )
).not.toBeInTheDocument();
).not.toBeVisible();

await user.hover( allTabs[ i ] );

Expand All @@ -128,6 +129,8 @@ describe.each( [

await user.unhover( allTabs[ i ] );
}

await cleanupTooltip( user );
} );

it( 'should display a tooltip when moving the selection via the keyboard on tabs provided with an icon', async () => {
Expand Down Expand Up @@ -157,38 +160,40 @@ describe.each( [

// Tab to focus the tablist. Make sure alpha is focused, and that the
// corresponding tooltip is shown.
expect( screen.queryByText( 'Alpha' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'Alpha' ) ).not.toBeVisible();
await user.keyboard( '[Tab]' );
expect( mockOnSelect ).toHaveBeenCalledTimes( 1 );
expect( screen.getByText( 'Alpha' ) ).toBeInTheDocument();
expect( screen.getByText( 'Alpha' ) ).toBeVisible();
expect( await getSelectedTab() ).toHaveFocus();

// Move selection with arrow keys. Make sure beta is focused, and that
// the corresponding tooltip is shown.
expect( screen.queryByText( 'Beta' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'Beta' ) ).not.toBeVisible();
await user.keyboard( '[ArrowRight]' );
expect( mockOnSelect ).toHaveBeenCalledTimes( 2 );
expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' );
expect( screen.getByText( 'Beta' ) ).toBeInTheDocument();
expect( screen.getByText( 'Beta' ) ).toBeVisible();
expect( await getSelectedTab() ).toHaveFocus();

// Move selection with arrow keys. Make sure gamma is focused, and that
// the corresponding tooltip is shown.
expect( screen.queryByText( 'Gamma' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'Gamma' ) ).not.toBeVisible();
await user.keyboard( '[ArrowRight]' );
expect( mockOnSelect ).toHaveBeenCalledTimes( 3 );
expect( mockOnSelect ).toHaveBeenLastCalledWith( 'gamma' );
expect( screen.getByText( 'Gamma' ) ).toBeInTheDocument();
expect( screen.getByText( 'Gamma' ) ).toBeVisible();
expect( await getSelectedTab() ).toHaveFocus();

// Move selection with arrow keys. Make sure beta is focused, and that
// the corresponding tooltip is shown.
expect( screen.queryByText( 'Beta' ) ).not.toBeInTheDocument();
expect( screen.queryByText( 'Beta' ) ).not.toBeVisible();
await user.keyboard( '[ArrowLeft]' );
expect( mockOnSelect ).toHaveBeenCalledTimes( 4 );
expect( mockOnSelect ).toHaveBeenLastCalledWith( 'beta' );
expect( screen.getByText( 'Beta' ) ).toBeInTheDocument();
expect( screen.getByText( 'Beta' ) ).toBeVisible();
expect( await getSelectedTab() ).toHaveFocus();

await cleanupTooltip( user );
} );
} );

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
>
<button
aria-checked="true"
aria-describedby="tooltip-4"
aria-label="Uppercase"
class="emotion-12 components-toggle-group-control-option-base"
data-active-item=""
Expand Down Expand Up @@ -284,6 +285,7 @@ exports[`ToggleGroupControl controlled should render correctly with icons 1`] =
>
<button
aria-checked="false"
aria-describedby="tooltip-5"
aria-label="Lowercase"
class="emotion-18 components-toggle-group-control-option-base"
data-command=""
Expand Down Expand Up @@ -787,6 +789,7 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
>
<button
aria-checked="true"
aria-describedby="tooltip-0"
aria-label="Uppercase"
class="emotion-12 components-toggle-group-control-option-base"
data-active-item=""
Expand Down Expand Up @@ -825,6 +828,7 @@ exports[`ToggleGroupControl uncontrolled should render correctly with icons 1`]
>
<button
aria-checked="false"
aria-describedby="tooltip-1"
aria-label="Lowercase"
class="emotion-18 components-toggle-group-control-option-base"
data-command=""
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/toggle-group-control/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
ToggleGroupControlOptionIcon,
} from '../index';
import type { ToggleGroupControlProps } from '../types';
import cleanupTooltip from '../../tooltip/test/utils';

const ControlledToggleGroupControl = ( {
value: valueProp,
Expand Down Expand Up @@ -137,9 +138,9 @@ describe.each( [
'Click for Delicious Gnocchi'
);

await waitFor( () => expect( tooltip ).toBePositionedPopover() );
await waitFor( () => expect( tooltip ).toBeVisible() );

expect( tooltip ).toBeVisible();
await cleanupTooltip( user );
} );

it( 'should not render tooltip', async () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/components/src/toggle-group-control/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import type { ReactNode } from 'react';
* Internal dependencies
*/
import type { BaseControlProps } from '../base-control/types';
import type { TooltipProps } from '../tooltip/types';

export type ToggleGroupControlOptionBaseProps = {
children: ReactNode;
Expand Down Expand Up @@ -56,7 +57,7 @@ export type WithToolTipProps = {
/**
* React children
*/
children: ReactNode;
children: TooltipProps[ 'children' ];
/**
* Label for the Tooltip component.
*/
Expand Down
45 changes: 16 additions & 29 deletions packages/components/src/tooltip/README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
# Tooltip
# ToolTip

Tooltip is a React component to render floating help text relative to a node when it receives focus or when the user places the mouse cursor atop it. If the tooltip exceeds the bounds of the page in the direction it opens, its position will be flipped automatically.

Accessibility note: the tooltip text is hidden from screen readers and assistive technologies that understand ARIA. To make it accessible, use an `aria-label` attribute on the same element the tooltip is applied to, preferably using the same text used for the tooltip.
Tooltip is a React component to render floating help text relative to a node when it receives focus or it is hovered upon by a mouse. If the tooltip exceeds the bounds of the page in the direction it opens, its position will be flipped automatically.

## Usage

Expand All @@ -22,49 +20,38 @@ const MyTooltip = () => (

The component accepts the following props:

### position

The direction in which the tooltip should open relative to its parent node. Specify y- and x-axis as a space-separated string. Supports `"top"`, `"middle"`, `"bottom"` y axis, and `"left"`, `"center"`, `"right"` x axis.

- Type: `String`
- Required: No
- Default: `"bottom"`

### children
#### `children`: `React.ReactElement`

The element to which the tooltip should anchor.

**NOTE:** You must pass only a single child. Tooltip renders itself as a clone of `children` with a [`Popover`](/packages/components/src/popover/README.md) added as an additional child.
**NOTE:** Accepts only one child element.

- Type: `Element`
- Required: Yes

### text
#### `delay`: `number`

The tooltip text to show on focus or hover.
The amount of time in milliseconds to wait before showing the tooltip.

- Type: `String`
- Required: No
- Default: `700`

### shortcut (web only)
#### `position`: `string`

The direction in which the tooltip should open relative to its parent node. Specify y- and x-axis as a space-separated string. Supports `"top"`, `"middle"`, `"bottom"` y axis, and `"left"`, `"center"`, `"right"` x axis.

- Type: `string` or `object`
- Required: No
- Default: `"bottom"`

If shortcut is a string, it is expecting the display text. If shortcut is an object, it will accept the properties of `display` (string) and `ariaLabel` (string).
#### `shortcut`: `string` | `object`

### delay (web only)
An option for adding accessible keyboard shortcuts.

Time in milliseconds to wait before showing tooltip after the tooltip's visibility is toggled. This prop is currently only available for the web platforms.
If shortcut is a string, it is expecting the display text. If shortcut is an object, it will accept the properties of `display`: `string` and `ariaLabel`: `string`.

- Type: `Number`
- Required: No
- Default: 700

### visible (native only)
#### `text`: `string`

Whether the tooltip should be displayed on initial render. This prop is currently only available for the native mobile app built with React Native.
The text shown in the tooltip when anchor element is focused or hovered.

- Type: `Boolean`
- Required: No
- Default: `false`
Loading

1 comment on commit 938e747

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flaky tests detected in 938e747.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6109604804
📝 Reported issues:

Please sign in to comment.