fix(OverflowTooltip): Add missing showDelay and hideDelay prop support for OverflowTooltip - #4133
fix(OverflowTooltip): Add missing showDelay and hideDelay prop support for OverflowTooltip#4133BillGeoghegan wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesOverflowTooltip delays
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
modules/react/tooltip/spec/OverflowTooltip.spec.tsx (1)
27-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStart this component spec with
verifyComponent.Call
verifyComponent(OverflowTooltip, {})fromtest-utilsbefore the delay-specific tests. This component spec does not use the required baseline verification.As per coding guidelines, element-component specs must start with
verifyComponent(Component, {})fromtest-utils.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@modules/react/tooltip/spec/OverflowTooltip.spec.tsx` around lines 27 - 34, Start the OverflowTooltip spec with a baseline verifyComponent(OverflowTooltip, {}) call from test-utils before the delay-specific tests, preserving the existing markAsOverflowed helper and test cases.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@modules/react/tooltip/stories/examples/Ellipsis.tsx`:
- Around line 55-58: Update the Ellipsis example around OverflowTooltip so its
button text accurately reflects the configured delays: either describe only the
two-second show delay or set hideDelay to 2000 as well, keeping the example
configuration and displayed text consistent.
---
Nitpick comments:
In `@modules/react/tooltip/spec/OverflowTooltip.spec.tsx`:
- Around line 27-34: Start the OverflowTooltip spec with a baseline
verifyComponent(OverflowTooltip, {}) call from test-utils before the
delay-specific tests, preserving the existing markAsOverflowed helper and test
cases.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6570f60c-b777-4a96-a945-18865d91c033
📒 Files selected for processing (3)
modules/react/tooltip/lib/OverflowTooltip.tsxmodules/react/tooltip/spec/OverflowTooltip.spec.tsxmodules/react/tooltip/stories/examples/Ellipsis.tsx
There was a problem hiding this comment.
Pull request overview
This PR fixes an API gap in OverflowTooltip by adding support for showDelay and hideDelay, aligning it with the underlying tooltip timing behavior used elsewhere in the tooltip package.
Changes:
- Added
showDelayandhideDelayprops toOverflowTooltipand wired them through touseTooltip. - Added unit tests verifying delayed show and delayed hide behavior for
OverflowTooltip. - Updated the Ellipsis Storybook example to demonstrate delayed tooltip behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| modules/react/tooltip/lib/OverflowTooltip.tsx | Adds showDelay/hideDelay props and passes them to useTooltip. |
| modules/react/tooltip/spec/OverflowTooltip.spec.tsx | Adds fake-timer tests asserting show/hide delay behavior for OverflowTooltip. |
| modules/react/tooltip/stories/examples/Ellipsis.tsx | Adds a Storybook example demonstrating delayed tooltip display. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <OverflowTooltip showDelay={2000}> | ||
| <SecondaryButton cs={{maxWidth: px2rem(200)}}> | ||
| 2 second delay on show and hide content | ||
| </SecondaryButton> | ||
| </OverflowTooltip> |
|
Can you open this back up against our support branch since it existed in v15, please! |
Workday/canvas-kit
|
||||||||||||||||||||||||||||||||||||||||
| Project |
Workday/canvas-kit
|
| Branch Review |
adding-overflow-delay-props
|
| Run status |
|
| Run duration | 02m 25s |
| Commit |
|
| Committer | Bill Geoghegan |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
17
|
|
|
0
|
|
|
812
|
| View all changes introduced in this branch ↗︎ | |
UI Coverage
19.65%
|
|
|---|---|
|
|
1523
|
|
|
370
|
Accessibility
99.47%
|
|
|---|---|
|
|
5 critical
5 serious
0 moderate
2 minor
|
|
|
72
|
|
Closing and reopening against support branch, my bad. |
Summary
Fixes: #3614
Release Category
Components
Release Note
Checklist
ready for reviewhas been added to PRFor the Reviewer
Where Should the Reviewer Start?
Areas for Feedback? (optional)
Testing Manually
hideDelayandshowDelayvalues, so you can see it working there.Screenshots or GIFs (if applicable)
Thank You Gif (optional)
Summary by CodeRabbit
New Features
Tests