-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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.
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
fix(tooltip): tooltip stays open on fast movement #4482
base: canary
Are you sure you want to change the base?
fix(tooltip): tooltip stays open on fast movement #4482
Conversation
🦋 Changeset detectedLatest commit: fc1b18a The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@Connorelsea is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Hey @Connorelsea thanks, please add a changeset |
Hello @jrgarciadev, I added a changeset, tried to follow the linked changeset documentation as I have never used that tool before. If it's incorrect I can fix it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/two-insects-provide.md (1)
5-5
: Consider enhancing the changeset description.While the description captures the essence of the change, it would be more helpful for future reference if you could include:
- The specific motion keys that were added
- Details about the AnimatePresence rendering changes
- Any performance implications of these changes
-fix tooltip staying open on fast movement by adding proper motion keys and ensuring AnimatePresence stays rendered, no breaking changes or usage changes required +fix tooltip staying open on fast movement by: +- adding unique motion keys to tooltip content for proper animation tracking +- ensuring AnimatePresence component remains mounted until animations complete +- improving render performance during rapid tooltip transitions + +No breaking changes or usage changes required.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/two-insects-provide.md
(1 hunks)
🔇 Additional comments (1)
.changeset/two-insects-provide.md (1)
1-3
: LGTM! Version bump is appropriate.The minor version bump is correctly chosen as this change adds functionality in a backward-compatible manner.
Closes #4301
📝 Description
This PR fixes an issue where tooltips when hovered in quick succession would sometimes remain open in NextUI. It updates AnimatePresence and motion key handling to comply with motion's typical usage patterns.
⛳️ Current behavior (updates)
When moving a cursor quickly across a screen with many tooltips, some tooltips would stay open after mouse has left. This can be easily replicated by making a list of tooltips and moving the cursor up and down them at medium-quick speed, especially when
closeDelay
andopenDelay
are set to zero to increase the chance that interruptions of the close animation will occur.🚀 New behavior
Tooltip no longer stays open on fast mouse movements across a list of tooltips. This ensures
AnimatePresence
is not prematurely unrendered on tooltip close by ensuringAnimatePresence
is always rendered whendisableAnimation
is false, and not unrendered whenisOpen
is false. Previously animate presence was being unrendered on close due to the conditional combiningisOpen
anddisableAnimation
checks. This also adds the proper keys that allow framer-motion to properly track these animations. Removing the keys causes the regression to return.💣 Is this a breaking change (Yes/No):
No breaking changes, small internal fix to tooltip with no effect on current usage patterns.
📝 Additional Information
I tried to think of a way to test this programatically, but I could not think of anything good. I added a storybook where I was able to test manually and replicate this issue locally on the canary branch. You can replicate by removing the keys and moving your cursor quickly up and down the list. Adding the keys back, alongside the change in AnimatePresence rendering, seemingly eliminates this behavior.
If someone can think of a better way to write a test for this, or wants to remove the storybook (or present it differently) as it was mostly used for testing this issue, let me know! I am available to make changes to this PR.
Replicated on public docs:
Screen.Recording.2024-12-29.at.2.59.07.PM.mov
Replicated on the storybook I added (pre-fix)
Summary by CodeRabbit
New Features
Improvements