Skip to content

Commit

Permalink
Tooltip: no-op when nested inside another Tooltip component (#57202)
Browse files Browse the repository at this point in the history
* Tooltip: no-op when nested inside another Tooltip component

* Fix Storybook control type

* Use internal components context system

* Add Storybook example

* Snapshots

* Add nested unit test

* CHANGELOG

* Avoid ESLint disable

* Keep nested tooltip storybook example

* isNestedInParentTooltip => isNestedInTooltip

* toBeInTheDocument => toBeVisible

* "inner" => "nested"

* Forward rest props to the tooltip instead of the ancor, update snapshots

* Format warning message

* Add paragraph in docs

* Complete comment
  • Loading branch information
ciampo authored Jan 10, 2024
1 parent 4b1beb4 commit 714e6b1
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
- `InputControl`, `NumberControl`, `UnitControl`, `SelectControl`, `TreeSelect`: Add `compact` size variant ([#57398](https://github.com/WordPress/gutenberg/pull/57398)).
- `ToggleGroupControl`: Update button size in large variant to be 32px ([#57338](https://github.com/WordPress/gutenberg/pull/57338)).
- `Tooltip`: improve unit tests ([#57345](https://github.com/WordPress/gutenberg/pull/57345)).
- `Tooltip`: no-op when nested inside other `Tooltip` components ([#57202](https://github.com/WordPress/gutenberg/pull/57202)).

### Experimental

Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/tooltip/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const MyTooltip = () => (
);
```

### Nested tooltips

In case one or more `Tooltip` components are rendered inside another `Tooltip` component, only the tooltip associated to the outermost `Tooltip` component will be rendered in the browser and shown to the user appropriately. The rest of the nested `Tooltip` components will simply no-op and pass-through their anchor.

## Props

The component accepts the following props:
Expand Down
56 changes: 47 additions & 9 deletions packages/components/src/tooltip/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,37 @@ import * as Ariakit from '@ariakit/react';
* WordPress dependencies
*/
import { useInstanceId } from '@wordpress/compose';
import { Children } from '@wordpress/element';
import { Children, cloneElement } from '@wordpress/element';
import deprecated from '@wordpress/deprecated';

/**
* Internal dependencies
*/
import type { TooltipProps } from './types';
import type { TooltipProps, TooltipInternalContext } from './types';
import Shortcut from '../shortcut';
import { positionToPlacement } from '../popover/utils';
import {
contextConnect,
useContextSystem,
ContextSystemProvider,
} from '../context';
import type { WordPressComponentProps } from '../context';

/**
* Time over anchor to wait before showing tooltip
*/
export const TOOLTIP_DELAY = 700;

function Tooltip( props: TooltipProps ) {
const CONTEXT_VALUE = {
Tooltip: {
isNestedInTooltip: true,
},
};

function UnconnectedTooltip(
props: WordPressComponentProps< TooltipProps, 'div', false >,
ref: React.ForwardedRef< any >
) {
const {
children,
delay = TOOLTIP_DELAY,
Expand All @@ -32,7 +47,15 @@ function Tooltip( props: TooltipProps ) {
position,
shortcut,
text,
} = props;

// From Internal Context system
isNestedInTooltip,

...restProps
} = useContextSystem< typeof props & TooltipInternalContext >(
props,
'Tooltip'
);

const baseId = useInstanceId( Tooltip, 'tooltip' );
const describedById = text || shortcut ? baseId : undefined;
Expand All @@ -43,7 +66,7 @@ function Tooltip( props: TooltipProps ) {
if ( 'development' === process.env.NODE_ENV ) {
// eslint-disable-next-line no-console
console.error(
'Tooltip should be called with only a single child element.'
'wp-components.Tooltip should be called with only a single child element.'
);
}
}
Expand All @@ -64,24 +87,37 @@ function Tooltip( props: TooltipProps ) {
}
computedPlacement = computedPlacement || 'bottom';

const tooltipStore = Ariakit.useTooltipStore( {
// Removing the `Ariakit` namespace from the hook name allows ESLint to
// properly identify the hook, and apply the correct linting rules.
const useAriakitTooltipStore = Ariakit.useTooltipStore;
const tooltipStore = useAriakitTooltipStore( {
placement: computedPlacement,
showTimeout: delay,
} );

if ( isNestedInTooltip ) {
return isOnlyChild
? cloneElement( children, {
...restProps,
ref,
} )
: children;
}

return (
<>
<ContextSystemProvider value={ CONTEXT_VALUE }>
<Ariakit.TooltipAnchor
onClick={ hideOnClick ? tooltipStore.hide : undefined }
store={ tooltipStore }
render={ isOnlyChild ? children : undefined }
ref={ ref }
>
{ isOnlyChild ? undefined : children }
</Ariakit.TooltipAnchor>
{ isOnlyChild && ( text || shortcut ) && (
<Ariakit.Tooltip
{ ...restProps }
unmountOnHide
className="components-tooltip"
gutter={ 4 }
id={ describedById }
overflowPadding={ 0.5 }
Expand All @@ -98,8 +134,10 @@ function Tooltip( props: TooltipProps ) {
) }
</Ariakit.Tooltip>
) }
</>
</ContextSystemProvider>
);
}

export const Tooltip = contextConnect( UnconnectedTooltip, 'Tooltip' );

export default Tooltip;
19 changes: 18 additions & 1 deletion packages/components/src/tooltip/stories/index.story.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const meta: Meta< typeof Tooltip > = {
'bottom right',
],
},
shortcut: { control: { type: 'text' } },
shortcut: { control: { type: 'object' } },
},
parameters: {
controls: { expanded: true },
Expand All @@ -57,3 +57,20 @@ KeyboardShortcut.args = {
ariaLabel: shortcutAriaLabel.primaryShift( ',' ),
},
};

/**
* In case one or more `Tooltip` components are rendered inside another
* `Tooltip` component, only the tooltip associated to the outermost `Tooltip`
* component will be rendered in the browser and shown to the user
* appropriately. The rest of the nested `Tooltip` components will simply no-op
* and pass-through their anchor.
*/
export const Nested: StoryFn< typeof Tooltip > = Template.bind( {} );
Nested.args = {
children: (
<Tooltip text="Nested tooltip text (that will never show)">
<Button variant="primary">Tooltip Anchor</Button>
</Tooltip>
),
text: 'Outer tooltip text',
};
46 changes: 46 additions & 0 deletions packages/components/src/tooltip/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -436,4 +436,50 @@ describe( 'Tooltip', () => {
await waitExpectTooltipToHide();
} );
} );

describe( 'nested', () => {
it( 'should render the outer tooltip and ignore nested tooltips', async () => {
render(
<Tooltip text="Outer tooltip">
<Tooltip text="Middle tooltip">
<Tooltip text="Inner tooltip">
<Button>Tooltip anchor</Button>
</Tooltip>
</Tooltip>
</Tooltip>
);

// Hover the anchor. Only the outer tooltip should show.
await hover(
screen.getByRole( 'button', {
name: 'Tooltip anchor',
} )
);

await waitFor( () =>
expect(
screen.getByRole( 'tooltip', { name: 'Outer tooltip' } )
).toBeVisible()
);
expect(
screen.queryByRole( 'tooltip', { name: 'Middle tooltip' } )
).not.toBeInTheDocument();
expect(
screen.queryByRole( 'tooltip', { name: 'Inner tooltip' } )
).not.toBeInTheDocument();
expect(
screen.getByRole( 'button', {
description: 'Outer tooltip',
} )
).toBeVisible();

// Hover outside of the anchor, tooltip should hide
await hoverOutside();
await waitFor( () =>
expect(
screen.queryByRole( 'tooltip', { name: 'Outer tooltip' } )
).not.toBeInTheDocument()
);
} );
} );
} );
4 changes: 4 additions & 0 deletions packages/components/src/tooltip/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,7 @@ export type TooltipProps = {
*/
text?: string;
};

export type TooltipInternalContext = {
isNestedInTooltip?: boolean;
};

0 comments on commit 714e6b1

Please sign in to comment.