Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Tooltip] Add custom duration #551

Merged
merged 2 commits into from
Mar 19, 2021
Merged

[Tooltip] Add custom duration #551

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fe4abf
[Tooltip] Add custom duration
benoitgrelard Mar 11, 2021
80cbded
Create 317d93f0.yml
benoitgrelard Mar 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .yarn/versions/317d93f0.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
releases:
"@radix-ui/react-tooltip": patch

declined:
- primitives
180 changes: 180 additions & 0 deletions packages/react/tooltip/src/Tooltip.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,186 @@ export const Controlled = () => {
);
};

export const CustomDurations = () => (
<>
<h1>Rest duration</h1>
<h2>Default (300ms)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>

<h2>Custom (0ms = instant open)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>

<h2>Custom (1s)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={1000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={1000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} restDuration={1000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>

<h1>Bypass rest duration</h1>
<h2>Default (300ms to move from one to another tooltip)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass}>Hover me</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>

<h2>Custom (0ms to move from one to another tooltip = never bypass)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={0}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>

<h2>Custom (5s to move from one to another tooltip)</h2>
<div style={{ display: 'flex', gap: 50 }}>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={5000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={5000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
<Tooltip>
<TooltipTrigger className={triggerClass} bypassRestDuration={5000}>
Hover me
</TooltipTrigger>
<TooltipContent className={contentClass} sideOffset={5}>
Nicely done!
<TooltipArrow className={arrowClass} offset={10} />
</TooltipContent>
</Tooltip>
</div>
</>
);

export const CustomContent = () => (
<div style={{ display: 'flex', gap: 20, padding: 100 }}>
<Tooltip>
Expand Down
34 changes: 28 additions & 6 deletions packages/react/tooltip/src/Tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ const Tooltip: React.FC<TooltipOwnProps> = (props) => {
// put the state machine in the appropriate state
useLayoutEffect(() => {
if (openProp === true) {
stateMachine.send({ type: 'MOUSE_ENTER', id: contentId });
stateMachine.send({ type: 'FOCUS', id: contentId });
}
}, [contentId, openProp]);

Expand All @@ -126,14 +126,36 @@ Tooltip.displayName = TOOLTIP_NAME;
const TRIGGER_NAME = 'TooltipTrigger';
const TRIGGER_DEFAULT_TAG = 'button';

type TooltipTriggerOwnProps = Polymorphic.OwnProps<typeof Primitive>;
type TooltipTriggerOwnProps = Polymorphic.Merge<
Polymorphic.OwnProps<typeof Primitive>,
{
/**
* The duration a user has to "rest" (not move) his mouse over the trigger
* before the tooltip gets opened.
* (default: 300)
*/
restDuration?: number;

/**
* How much time a user has to move his mouse to another trigger without incurring
* another rest duration.
* (default: 300)
*/
bypassRestDuration?: number;
}
>;
type TooltipTriggerPrimitive = Polymorphic.ForwardRefComponent<
typeof TRIGGER_DEFAULT_TAG,
TooltipTriggerOwnProps
>;

const TooltipTrigger = React.forwardRef((props, forwardedRef) => {
const { as = TRIGGER_DEFAULT_TAG, ...triggerProps } = props;
const {
as = TRIGGER_DEFAULT_TAG,
restDuration = 300,
bypassRestDuration = 300,
...triggerProps
} = props;
const context = useTooltipContext(TRIGGER_NAME);
const composedTriggerRef = useComposedRefs(forwardedRef, context.triggerRef);

Expand All @@ -146,15 +168,15 @@ const TooltipTrigger = React.forwardRef((props, forwardedRef) => {
as={as}
ref={composedTriggerRef}
onMouseEnter={composeEventHandlers(props.onMouseEnter, () =>
stateMachine.send({ type: 'MOUSE_ENTER', id: context.contentId })
stateMachine.send({ type: 'MOUSE_ENTER', id: context.contentId, restDuration })
)}
onMouseMove={composeEventHandlers(props.onMouseMove, () =>
stateMachine.send({ type: 'MOUSE_MOVE', id: context.contentId })
stateMachine.send({ type: 'MOUSE_MOVE', id: context.contentId, restDuration })
)}
onMouseLeave={composeEventHandlers(props.onMouseLeave, () => {
const stateMachineContext = stateMachine.getContext();
if (stateMachineContext.id === context.contentId) {
stateMachine.send({ type: 'MOUSE_LEAVE', id: context.contentId });
stateMachine.send({ type: 'MOUSE_LEAVE', id: context.contentId, bypassRestDuration });
}
})}
onFocus={composeEventHandlers(props.onFocus, () =>
Expand Down
18 changes: 7 additions & 11 deletions packages/react/tooltip/src/tooltipStateChart.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import { assign } from './createStateMachine';

import type { StateChart } from './createStateMachine';

// How long the mouse needs to stop moving for the tooltip to open
const REST_THRESHOLD_DURATION = 300;

// How much time does the user has to move from one tooltip to another without incurring the rest wait
const SKIP_REST_THRESHOLD_DURATION = 300;

type TooltipState =
// tooltip is closed
| 'closed'
Expand All @@ -27,9 +21,9 @@ type TooltipState =
| 'dismissed';

type TooltipEvent =
| { type: 'MOUSE_ENTER'; id: string }
| { type: 'MOUSE_MOVE'; id: string }
| { type: 'MOUSE_LEAVE'; id: string }
| { type: 'MOUSE_ENTER'; id: string; restDuration: number }
| { type: 'MOUSE_MOVE'; id: string; restDuration: number }
| { type: 'MOUSE_LEAVE'; id: string; bypassRestDuration: number }
| { type: 'REST_TIMER_ELAPSE'; id: string }
| { type: 'SKIP_REST_TIMER_ELAPSE'; id: string }
| { type: 'FOCUS'; id: string }
Expand Down Expand Up @@ -71,7 +65,8 @@ const tooltipStateChart: TooltipStateChart = {
(event, context, send) => {
restTimerId = window.setTimeout(
() => send({ type: 'REST_TIMER_ELAPSE', id: event.id }),
REST_THRESHOLD_DURATION
// @ts-ignore
event.restDuration
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know how to restrict the type further here so it knows which event it is…

Copy link
Contributor

Choose a reason for hiding this comment

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

I've been thinking about this and realising I wasn't fully understanding things when we spoke on the call. I'm now wondering why we need to pass the durations through every MOUSE_ENTER or MOUSE_MOVE event.

I would usually expect something like:

const { restDuration, bypassRestDuration, ...restProps } = props;

React.useEffect(() => {
  send({ type: 'UPDATE_REST_DURATION', restDuration });
}, [send, restDuration]);

React.useEffect(() => {
  send({ type: 'UPDATE_BYPASS_REST_DURATION', bypassRestDuration });
}, [send, bypassRestDuration]);

Then the machine would have the following (these are a good example of events that don't transition):

on: {
  UPDATE_REST_DURATION: { actions: [assignRestDuration] },
  UPDATE_BYPASS_REST_DURATION: { actions: [assignBypassRestDuration] },
}

And your waitingForRest would do:

entry: [(_, context, send) => {
  restTimerId = window.setTimeout(() => send(), context.restDuration);
}]

This would alleviate type issues I believe.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It seems strange to have events just to update the context though I guess.

But also I don't think that would work, because if we did that, every single instance of tooltip would override each other because there's one shared state machine for all tooltips.

We do want these timings to change on the fly based on the tooltip currently being used.

Copy link
Contributor

@jjenzz jjenzz Mar 18, 2021 •
edited

Choose a reason for hiding this comment

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

It seems strange to have events just to update the context though I guess.

Think my naming confused the sitch. The event could be PROP_CHANGE / REST_DURATION_CHANGE, or whatever you wanna call it. State machine events don't have to map to DOM events, they're just "things that happened".

We do want these timings to change on the fly based on the tooltip currently being used.

Ah of course! That was the missing piece of the puzzle, cheers. Agree we should leave as is in that case 🙂

);
},
],
Expand Down Expand Up @@ -104,7 +99,8 @@ const tooltipStateChart: TooltipStateChart = {
(event, context, send) => {
skipRestTimerId = window.setTimeout(
() => send({ type: 'SKIP_REST_TIMER_ELAPSE', id: event.id }),
SKIP_REST_THRESHOLD_DURATION
// @ts-ignore
event.bypassRestDuration
);
},
],
Expand Down