fix(tooltip): tooltip stays open on fast movement #4482
Conversation
🦋 Changeset detectedLatest commit: 3e1d2b8 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
✨ Finishing Touches
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 |
wingkwong
changed the title
|
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hello, i dont know if its done or not, but in this preview (The build from vercel above) i was still able to replicate the issue. OS: Ubuntu 24.04.1 LTS x86_64 clideo_editor_c9abb29b475f42639202803dd419dd15.mp4 |
|
@wingkwong Will look into it and see if I can reproduce and fix. And I'll update the Multiple storybook Edit: apologies for the delay, gonna try to get to this soon |
|
Just a post to thank @Connorelsea, I use tooltips a lot and this bug was really annoying my users and I, I look forward to have the fix in official release! |
|
@Connorelsea seems I couldn't reproduce on the current production. Are you able to reproduce? |
|
@wingkwong Able to re-produce what exactly? The main scenario I saw with the most recent round was: move lazymotion above animate presence, can't get any tooltips to stick. VS, move lazymotion back to its original location, tooltips start sticking Edit: Oh you mean tooltips sticking on current prod, I will test |
|
I'm not talking about the PR. I mean the original issue. After the recent release, I could no longer reproduce the issue in the production. |
|
@wingkwong I am able to reproduce on the currently deployed doc pages. If that is not running latest I can update my personal project to latest as well and check there. It is easiest to reproduce on a high refresh rate screen with no other monitors slowing down rendering. Seems like a weird set of constraints but for arbitrary reproductions it happens easiest when I'm just on my laptop, although it does happen but less often in a normal multi-monitor scenario. 
|
|
Will this actually get merged soon? it's been approved and in the milestone 3 times now but is just getting pushed back every time. I have quite a powerful machine with high refresh monitors and i get this problem so bad to the point that half my screen gets filled with tooltips after a few seconds of usage. The image shows how bad it currently is. And that is from regularly moving the mouse. I am not moving it fast on purpose.
|
This is infact effecting our production application as well. If its a bandwidth issue from a contributor stand point, I'll raise my hand to help out. |
|
No issues on my end, as the opener of this PR. I think it's just waiting on final review and merge. I am still around to make any updates if necessary, but I've found that these changes seem to fix the issue |
|
I'll bring this up in tomorrow's meeting. Previously none of members could reproduce the issue on our side so it's hard to verify this PR. @aaronhawkey @Arch-Storm are you guys able to check on your dev environment with this PR build version |
Have just tested using the pr preview and the issue is indeed fully fixed on my end! |
|
@wingkwong Tested and confirmed to fix the bug. |
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
closeDelayandopenDelayare 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
AnimatePresenceis not prematurely unrendered on tooltip close by ensuringAnimatePresenceis always rendered whendisableAnimationis false, and not unrendered whenisOpenis false. Previously animate presence was being unrendered on close due to the conditional combiningisOpenanddisableAnimationchecks. 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