fix(tooltip): fix wrong initial and focus tooltip positions in followMouse mode#1278
Open
bmichalowski wants to merge 4 commits intocanonical:mainfrom
Open
fix(tooltip): fix wrong initial and focus tooltip positions in followMouse mode#1278bmichalowski wants to merge 4 commits intocanonical:mainfrom
bmichalowski wants to merge 4 commits intocanonical:mainfrom
Conversation
|
bmichalowski is not a collaborator of the repo |
Author
|
@canonical/react-library-maintainers please take a look at the proposed changes |
edlerd
reviewed
Dec 10, 2025
Contributor
edlerd
left a comment
edlerd
left a comment
There was a problem hiding this comment.
Thank you for the contribution. This is an improved handling of the follow mouse mode indeed!
Some small nitpicks on formatting below, then this LGTM to merge.
Comment on lines
+311
to
+318
| if (followMouse) { | ||
| if (wrapperRef.current) { | ||
| // set initial position for the tooltip | ||
| setPositionStyle( | ||
| getPositionStyle(adjustedPosition, wrapperRef.current) | ||
| ); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Suggested change
| if (followMouse) { | |
| if (wrapperRef.current) { | |
| // set initial position for the tooltip | |
| setPositionStyle( | |
| getPositionStyle(adjustedPosition, wrapperRef.current) | |
| ); | |
| } | |
| } | |
| if (followMouse && wrapperRef.current) { | |
| // set initial position for the tooltip | |
| setPositionStyle(getPositionStyle(adjustedPosition, wrapperRef.current)); | |
| } | |
| openPortal(); |
A bit simpler and please use correct spacing and formatting.
| "mousemove", | ||
| true, | ||
| followMouse && isOpen, | ||
| followMouse, |
Contributor
There was a problem hiding this comment.
Suggested change
| followMouse, | |
| useListener(wrapperRef.current, mouseHandler, "mousemove", true, followMouse); |
This whole block should be in a single line now.
Contributor
|
Please stash all your commits with the formatting changes into one for a clean history. |
Contributor
|
Also, please rebase on the main branch to pull in recent updates. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Done
The tooltip did not handle the initial position correctly in followMouse mode.
There was also an issue with listening to mouse move events only when the tooltip was open.
QA
Pinging @canonical/react-library-maintainers for a review.
QA steps
Bug 1: Tooltip is not visible on keyboard focus in followMouse mode
Bug 2: Tooltip appears at previous cursor location instead of current
Percy steps
No visual changes expected