Skip to content

Commit

Permalink
Tooltip: forward and merge inner tooltip props correctly (#57878)
Browse files Browse the repository at this point in the history
* Tooltip: forward and merge inner tooltip props correctly

* CHANGELOG

* Use internal context instead of connecting to the context system

* Delete unnecessary removeExtraPropsAddedByContext utility

* CHANGELOG

* Add suggested unit test
  • Loading branch information
ciampo authored Jan 18, 2024
1 parent 6ef2cbc commit 97e642c
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 29 deletions.
3 changes: 2 additions & 1 deletion packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
- `PaletteEdit` and `CircularOptionPicker`: improve unit tests ([#57809](https://github.com/WordPress/gutenberg/pull/57809)).
- `Tooltip`: no-op when nested inside other `Tooltip` components ([#57202](https://github.com/WordPress/gutenberg/pull/57202)).

### Bug Fixes
### Bug Fix

- `ToggleGroupControl`: Improve controlled value detection ([#57770](https://github.com/WordPress/gutenberg/pull/57770)).
- `Tooltip`: Improve props forwarding to children of nested `Tooltip` components ([#57878](https://github.com/WordPress/gutenberg/pull/57878)).

### Experimental

Expand Down
55 changes: 28 additions & 27 deletions packages/components/src/tooltip/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,39 @@ import * as Ariakit from '@ariakit/react';
* WordPress dependencies
*/
import { useInstanceId } from '@wordpress/compose';
import { Children, cloneElement } from '@wordpress/element';
import {
Children,
useContext,
createContext,
forwardRef,
} from '@wordpress/element';
import deprecated from '@wordpress/deprecated';

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

const TooltipInternalContext = createContext< TooltipInternalContextType >( {
isNestedInTooltip: false,
} );

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

const CONTEXT_VALUE = {
Tooltip: {
isNestedInTooltip: true,
},
isNestedInTooltip: true,
};

function UnconnectedTooltip(
function UnforwardedTooltip(
props: WordPressComponentProps< TooltipProps, 'div', false >,
ref: React.ForwardedRef< any >
) {
Expand All @@ -48,14 +53,10 @@ function UnconnectedTooltip(
shortcut,
text,

// From Internal Context system
isNestedInTooltip,

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

const { isNestedInTooltip } = useContext( TooltipInternalContext );

const baseId = useInstanceId( Tooltip, 'tooltip' );
const describedById = text || shortcut ? baseId : undefined;
Expand Down Expand Up @@ -96,16 +97,15 @@ function UnconnectedTooltip(
} );

if ( isNestedInTooltip ) {
return isOnlyChild
? cloneElement( children, {
...restProps,
ref,
} )
: children;
return isOnlyChild ? (
<Ariakit.Role { ...restProps } render={ children } />
) : (
children
);
}

return (
<ContextSystemProvider value={ CONTEXT_VALUE }>
<TooltipInternalContext.Provider value={ CONTEXT_VALUE }>
<Ariakit.TooltipAnchor
onClick={ hideOnClick ? tooltipStore.hide : undefined }
store={ tooltipStore }
Expand All @@ -117,6 +117,7 @@ function UnconnectedTooltip(
{ isOnlyChild && ( text || shortcut ) && (
<Ariakit.Tooltip
{ ...restProps }
className="components-tooltip"
unmountOnHide
gutter={ 4 }
id={ describedById }
Expand All @@ -134,10 +135,10 @@ function UnconnectedTooltip(
) }
</Ariakit.Tooltip>
) }
</ContextSystemProvider>
</TooltipInternalContext.Provider>
);
}

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

export default Tooltip;
11 changes: 11 additions & 0 deletions packages/components/src/tooltip/test/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,17 @@ describe( 'Tooltip', () => {
screen.queryByRole( 'button', { description: 'tooltip text' } )
).not.toBeInTheDocument();
} );

it( 'should not leak Tooltip props to the tooltip anchor', () => {
render(
<Tooltip data-foo>
<button>Anchor</button>
</Tooltip>
);
expect(
screen.getByRole( 'button', { name: 'Anchor' } )
).not.toHaveAttribute( 'data-foo' );
} );
} );

describe( 'keyboard focus', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/components/src/tooltip/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,5 +61,5 @@ export type TooltipProps = {
};

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

0 comments on commit 97e642c

Please sign in to comment.