-
Notifications
You must be signed in to change notification settings - Fork 517
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
Tooltip position is set incorrectly while using followCursor and appendTo #532
Comments
Just wondering what the use case is for not appending it to the body? |
Thanks for the quick look, @atomiks ! When a specific component on the page (a dashboard/chart) is set to fullscreen (requestFullscreen), if the tooltip is not appended to this specific component, it gets invisible in full-screen. I also need to use the followMode for specific tooltip cases. I have a temporary fix on my copy, it's just a few lines of code:) I'll appreciate if a proper fix would become a stable part of this awesome library! |
PR with that fix is welcome. |
The zIndex doesn't solve the full-screen problem - the tooltip dom gets outside the full-screen element (attached to body), so it's simply not visible in that case. I also don't have time to set up PR, or review if the code would work in all cases, as I'm not familiar with internals. I realized I had done a similar fix in earlier version (back in v2.) and the tooltips were slightly off, so I navigated the code to see which parts responds to followCursor. Below is the snippet I currently have:
|
It looks like we need to take into account the scroll offset as well, which is basically re-implementing parts of Popper.js here. It looks like we're going "against" it somehow so idk there should be a cleaner fix for this. |
Think I found a good fix. We need to use a placeholder element instead of a virtual reference object, so we append the real element to the same parent as the popper. Then overwrite its In this context, Popper.js takes into account everything it needs to like common offset parent, scroll, etc.
I think you need to use the highest 32-bit number possible: 2147483647 |
Thanks @atomiks . I will be following the thread to see if there are any updates. Please let me know if you'd like me to test later. And, to clarify again, zIndex is not the issue. When you "fullscreen" a specific dom, say "#chart" element using requestFullscreen method, anything outside of that DOM tree is not rendered. So, tooltip dom also needs to be within that element "#chart". If it's attached to body, it's no longer visible, it's not part of the page or layout. |
@adilyalcin looks like you're right. The issue I remember is #215 (comment) but it doesn't seem to work anymore. And it never worked in Firefox anyway. |
This will fix it I'm pretty sure floating-ui/floating-ui#801 You'll have to wait for a new Popper release then, the workaround I made would prob cause more bugs and problems and would prefer the Popper solution. |
Bug description
The followCursor uses the mouse event clientX and clientY without adjusting for the positioning of the appendTo element, which breaks the tooltip position within the viewport.
Reproduction
https://codepen.io/anon/pen/VJGWPV
Solution
The fix needs to adjust the mouse X and Y inside positionVirtualReferenceNearCursor using the position of the appendTo element, if it exists.
Thanks!
The text was updated successfully, but these errors were encountered: