feat(tooltip): add delay and closeDelay props for timing control#353
feat(tooltip): add delay and closeDelay props for timing control#353invisal wants to merge 1 commit intocloudflare:mainfrom
Conversation
commit: |
Docs PreviewCommit: |
|
/bonk review |
There was a problem hiding this comment.
Thanks for this PR! The implementation looks good overall. I have one concern about the demo that should be addressed before merging.
Overall Assessment:
- ✅ Clean API following existing patterns
- ✅ Props correctly forwarded to Base UI Trigger
- ✅ Documentation and changeset included
⚠️ Demo may not work as expected due to TooltipProvider delay grouping
The issue is that TooltipProvider implements delay grouping - after the first tooltip opens, subsequent tooltips skip their open delay. This means users may not actually experience the 1-second delay in the demo if they hover over multiple buttons in sequence.
| * Control the delay before opening and closing the tooltip. | ||
| * `delay` controls open delay (default: 600ms), `closeDelay` controls close delay (default: 0ms). | ||
| */ | ||
| export function TooltipDelayDemo() { |
There was a problem hiding this comment.
This demo has a subtle issue: TooltipProvider groups tooltips so that after the first one opens, subsequent tooltips skip their open delay. This means users may not actually experience the 1-second delay when hovering between buttons.
Consider wrapping each Tooltip in its own provider, or adding a comment explaining this behavior:
| export function TooltipDelayDemo() { | |
| /** | |
| * Control the delay before opening and closing the tooltip. | |
| * `delay` controls open delay (default: 600ms), `closeDelay` controls close delay (default: 0ms). | |
| * | |
| * Note: Each Tooltip has its own TooltipProvider so the delay grouping | |
| * behavior doesn't interfere with demonstrating the delay props. | |
| */ | |
| export function TooltipDelayDemo() { | |
| return ( | |
| <div className="flex gap-4"> | |
| <TooltipProvider> | |
| <Tooltip content="Opens after 1 second" delay={1000} asChild> | |
| <Button variant="secondary">1s open delay</Button> | |
| </Tooltip> | |
| </TooltipProvider> | |
| <TooltipProvider> | |
| <Tooltip | |
| content="Stays open 500ms after leaving" | |
| closeDelay={500} | |
| asChild | |
| > | |
| <Button variant="secondary">500ms close delay</Button> | |
| </Tooltip> | |
| </TooltipProvider> | |
| <TooltipProvider> | |
| <Tooltip | |
| content="Instant open, stays 1s" | |
| delay={0} | |
| closeDelay={1000} | |
| asChild | |
| > | |
| <Button variant="secondary">Instant + 1s close</Button> | |
| </Tooltip> | |
| </TooltipProvider> | |
| </div> | |
| ); | |
| } |
|
I've posted my review on PR #353. Here's a summary of my feedback: Review Summary The implementation is clean and follows the established patterns in the codebase:
Key Suggestion: Each tooltip in the demo should be wrapped in its own |



















































Summary
Add
delayandcloseDelayprops to the Tooltip component to allow developers to control the timing of tooltip open and close actions.Changes
delayprop to control how long to wait before opening the tooltip (default: 600ms)closeDelayprop to control how long to wait before closing the tooltip (default: 0ms)API