fix(core): skip delay when switching between grouped tooltips#903
fix(core): skip delay when switching between grouped tooltips#903
Conversation
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📦 Bundle Size Report🎨 @videojs/html(no changes) Presets (7)
Media (4)
Players (3)
Skins (16)
UI Components (21)
Sizes are marginal over the root entry point. ⚛️ @videojs/react(no changes) Presets (7)
Media (3)
Skins (14)
UI Components (17)
Sizes are marginal over the root entry point. 🧩 @videojs/core(no changes) Entries (5)
🏷️ @videojs/element(no changes) Entries (2)
📦 @videojs/store(no changes) Entries (3)
🔧 @videojs/utils(no changes) Entries (10)
📦 @videojs/spf(no changes) Entries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
There was a problem hiding this comment.
Pull request overview
Fixes grouped tooltip “hysteresis” so switching between tooltips in the same group can open instantly, and ensures tooltip group context is available synchronously to context consumers.
Changes:
- Update
TooltipGroupCore.shouldSkipDelay()to skip delay while another tooltip in the group is still open. - Provide an
initialValuefor the tooltip groupContextProviderso early consumers don’t receiveundefined. - Adjust unit tests to reflect the updated skip-delay behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
packages/html/src/ui/tooltip/tooltip-group-element.ts |
Sets a synchronous initial context value for the tooltip group so consumers can access the group core immediately. |
packages/core/src/core/ui/tooltip/tooltip-group-core.ts |
Changes delay-skipping logic to return true when the group is already open. |
packages/core/src/core/ui/tooltip/tests/tooltip-group-core.test.ts |
Updates expectations/naming around shouldSkipDelay() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
mihar-22
left a comment
There was a problem hiding this comment.
Looks like just a test name fix, otherwise looks good!
shouldSkipDelay() returned false when a tooltip was already open, preventing instant switching. The HTML ContextProvider also lacked an initialValue, so tooltip elements never received the group reference via @lit/context. Closes #898 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
shouldSkipDelay()returnedfalsewhen#isOpenwastrue— this prevented instant tooltip switching because the previous tooltip's async close hadn't fired yet when the next tooltip checked the delay.ContextProviderhad noinitialValue—@lit/contextconsumers withoutsubscribe: trueonly get the value at request time. SinceTooltipGroupElementset the value inupdate()(async), tooltips connected before that gotundefinedand never received the group reference.Closes #898
Test plan
pnpm -F @videojs/core test src/core/ui/tooltip— all 21 tests passpnpm typecheck— cleanpnpm lint— clean🤖 Generated with Claude Code