[BUG] When using default rt-transition-show-delay, tooltip stays in DOM after visibly closing #1144
Comments
|
Although the The obvious reason is the component is "missing" the Thanks for your effort on this, we'll be investigating further and get back to you with any findings. One side note about point 14, could you elaborate on what you mean by "infinite loop"? As far as I can tell, if we pretend the tooltip should be open (you can just set The reason that happens is because the tooltip is placed using To verify that's the case, try moving the tooltip component directly inside the scrollable element, and setting If this is not a correct assessment of what you're describing as "infinite loop", please provide more details. |
|
@johannkor please take a look at #1145. Also, the beta version In summary, we were relying on Reminder to also take a look at my comment above about your point 14 and what you're calling an "infinite loop", since I might've missed something in your explanation. |
|
Official release Same deal as #1136, if you feel there's still something to improve here, feel free to reopen. And thanks again for the effort in setting up the examples! They help a lot with debugging. |
|
I'm in awe of how quickly you've responded to these issues. Thank you very much for your help. I like the library as it saves me a lot of work so it is nice to be able to contribute back to it. Regarding the patch formatting, here is one that works: https://gist.githubusercontent.com/johannkor/9d23f2366e6646c2fc9eaed61557d07e/raw/515d8f38677c85399ca2afb3b46de7cbb4ae0b00/tooltip.patch - please Regarding the infinite loop, I mean that the Tooltip component continuously triggers a render, which triggers a useEffect, which triggers a render via a setState, which triggers a useEffect, etc. This loop continues without any user input. Here's a screen capture. out.mp4 |
I've had some free time the last few days, and the details you've provided made it a lot easier to track down the issue, so thanks for that 😅 And it seems you're right about the infinite loop. Either I wasn't able to reproduce it when testing the other stuff, or I just didn't notice it. I'll reopen this for now, and try again later with the patch you've provided, and we'll see how difficult this will be to fix. Though I should add, this might not be an issue anymore since the "stuck on the DOM" got fixed, but I'll have to make sure. |
|
I've been trying to reproduce the issue with I have unfortunately found a different race condition, which leaves the tooltip both in the DOM and visible. When the tooltip is open in this state, scrolling the scroll container downwards so that the tooltip's element moves up into the overflow's hidden area starts the infinite render loop as well. Scrolling it back down so it's visible stops the loop and the position is updated (and rerendered) only when necessary after a scroll. This was somewhat tricky to get to reproduce, but I found a somewhat reproducible cursor movement pattern. I rendered a list of items with the anchor being the red boxes, and moved my cursor somewhat like this very quickly: 
The tooltip then stayed open: 
I sprinkled in more logging this time. The log is in this gist, but I took a screenshot of it which is in the expandable (►) block below. The log is color coded like this: expand log screenshot
I did not reason through the log yet, as I wanted to share any details as quickly as possible. I have generated a .patch which adds all this logging, and changes the React version to 18 (to check this isn't just a React 16 thing) here: https://gist.githubusercontent.com/johannkor/92e6b94884cc182ca70034bff931d93f/raw/f2a2fa5115f85ce4ab63ba9e575bdd306f9d3e74/0001-log-debug-patch.patch To apply:
|
|
The rerender loop appears to be a separate issue from the issue above. I was able to reproduce it with just Would you like me to move the "Tooltip stays visible" issue (described above) to a separate issue? |
johannkor
changed the title
johannkor
changed the title
|
Adding to my larger reproduction steps with the log files: this is easier to reproduce if you set CPU throttling to 6x in Chrome's devtools. |
|
It seems that the crux of the issue is the following:
Here is a script you can paste into your console to reproduce the issue almost reliably, when using Chrome and 6x CPU throttling (DevTools' Performance tab -> cog wheel icon to the right) with the patch I offered earlier. It dispatches (async () => {
const log = (msg, ...args) =>
console.log(
`%c${msg}%c`,
"padding: 3px 150px;background:red;color:black;font-weight:bold;",
"padding:initial;background:initial;color:initial;font-weight:initial;",
...args
);
let lastDispatchMs = NaN;
const doDispatch = (event, selector) => {
const timeSinceLastEvent = window.performance.now() - lastDispatchMs;
log(
`${event} on ${selector} (${timeSinceLastEvent.toFixed(
1
)}ms since last event)`
);
lastDispatchMs = window.performance.now();
document.querySelector(selector).dispatchEvent(
new MouseEvent(event, {
view: window,
bubbles: true,
cancelable: true,
clientX: 0,
clientY: 0,
})
);
};
const selectors = {
firstAnchor: '[data-tooltip-content="tooltip 0"]',
secondAnchor: '[data-tooltip-content="tooltip 1"]',
};
let currentWait = 0;
const wait = (ms) => {
currentWait += ms;
};
const dispatch = (event, selector) =>
setTimeout(() => doDispatch(event, selector), currentWait);
// give 1 second of breathing room before starting
wait(1000);
dispatch("mouseenter", selectors.firstAnchor);
// wait long enough to be sure there are no events being processed after showing the tooltip
wait(1000);
dispatch("mouseleave", selectors.firstAnchor);
// now quickly dispatch enter+leave to the other anchor
wait(1);
dispatch("mouseenter", selectors.secondAnchor);
wait(1);
dispatch("mouseleave", selectors.secondAnchor);
})();
...later
|
Leave it as is. Creating #1146 to isolate the render loop issue should be fine for now.
This is something I'll keep in mind for all future tests. Great suggestion for performance testing.
I'll test it out myself later, but taking a quick look at how I'd appreciate any suggestions on how to fix this, but my first thought was to extend It'll probably be a while until I have some more time to get back to these new issues (I'm guessing only after new year's), but since you're so eager in helping out (my genuine thanks again for that 😅), this specific issue with the tooltip staying open feels pretty much done after figuring out the best fix for the debounce function (and a proper way to use it), so you can probably leave it be for now (unless you want to work on the solution, which you're totally welcome to by opening a PR). |
|
That was exactly my thought - lodash provides a |
|
Good to know it's an already existing design, which means it's probably not the worst idea. We'll be reviewing it soon. Thanks for the contribution, and happy holidays! |
gabrieljablonski
added
the
Awaiting merge
gabrieljablonski
removed
the
Awaiting merge
|
Fix available on official version |
Bug description
The default
rt-transition-show-delayis 150ms. When flicking the cursor over a tooltip quickly enough, themouseoverevent fires, but the CSS transitions never fire, leaving the tooltip in the DOM in a__closingstate.When the tooltip is in this state, its position is still being recalculated when scrolling or resizing. If the element is scrolled out of view (e.g. an
overflow: scrollparent), an infinite rerender loop occurs in the Tooltip component.Version of Package
v5.25.0
To Reproduce
Clean install this repository (picking up the latest
@floating-ui/dom)Replace
index-dev.tsxwith this:Apply this patch to litter
console.log()everywhere inTooltip.tsxtooltip.patchin the root of the repo, then rungit apply tooltip.patch:Start dev server with
yarn devOpen page
Open devtools' Inspector/Elements tab, expand
#appPress ESC to open console drawer below
Resize your window so that your cursor can move up over the browser's window
Move your mouse ~100px below the red bordered anchor element
Quickly flick your cursor up beyond the browser's window, hitting the anchor element on the way
Notice that the tooltip is not visible, but is found in the DOM
Now carefully move your cursor into the anchor's scroll container (gray border) and scroll down
Notice that your console is now full of
console.log()from rerendersIf you grab a profile with React Dev Tools, you'll see that the Tooltip component rerenders due to hooks 6 and 7 changing, which are
inlineStylesandinlineArrowStylesrespectfullyhandleTooltipPositioncalled byupdateTooltipPositionwhere the floating-ui computePosition function gets repeatedly called by theuseEffect(() => updateTooltipPosition()), ...)hook, where floating-ui creates a new IntersectionObserver that gets immediately invokedconsole.log('updateTooltipPosition ...)lines reveals where it happens, and a debugger breakpoint reveals what happens insideChange
--rt-transition-show-delayto0I am unsure if it is this library, React, or possibly browser which skips the transition if the state changes happen too quickly. Perhaps React batches the quick state updates and the DOM opacity transition never begins?
Expected behavior
I'd expect the tooltip to get removed from the DOM.
Screenshots
Desktop:
@floating-ui/dom:1.5.1Additional context
I don't know if it's related, but
@floating-ui/domv1.4.3 performed some changes to their state handling when fixing aResizeObserverbug. I have not yet tested if this reproduces with an earlier version of that library.The console log output for me when I perform this on Firefox is the following:
The text was updated successfully, but these errors were encountered: