[BUG] When using default rt-transition-show-delay, tooltip stays in DOM after visibly closing

[johannkor]

[BUG] When using default rt-transition-show-delay, tooltip stays in DOM after visibly closing

[johannkor]

I've been trying to reproduce the issue with v5.25.1 but the "hidden but stays in DOM" issue appears to have been resolved.

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:

The number on the left is "milliseconds since module initialization" to help understand where things happened synchronously and where after a delay.

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:

1. Clone this repo and checkout tag v5.25.1, ensure working directory is clean and has no changes (git reset --hard or git checkout -- .)
2. Download that 0001-log-debug-patch.patch into the repo's directory, it is a text file you can inspect with an editor
3. Run git apply --verbose 0001-log-debug-patch.patch
4. Run git status and git diff to see what changed
5. Run yarn to pick up the new react version, then just yarn dev

[gabrieljablonski]

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]

Although the .patch you've provided isn't working (getting error: corrupt patch at line 7, if you could check why I'd appreciate it), I've managed to reproduce the tooltip getting "stuck" on the DOM.

The obvious reason is the component is "missing" the mouseleave event, leaving the tooltip in a weird state. I'd expect the tooltip to also stay open if that was simply the case, so there has to be something more to this.

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 isOpen={true}, which is a valid use case) updateTooltipPosition() is getting called as expected, since it has to update the tooltip position on scroll.

The reason that happens is because the tooltip is placed using position: absolute, so by default, it is placed relative to the nearest ancestor with a set position attribute. Since in this example there are none, it defaults to the root html tag. So, when scrolling, the tooltip has to update its top and left CSS attributes.

To verify that's the case, try moving the tooltip component directly inside the scrollable element, and setting position: relative to it. My guess is there won't be any logs for updateTooltipPosition() calls.

If this is not a correct assessment of what you're describing as "infinite loop", please provide more details.

[gabrieljablonski]

@johannkor please take a look at #1145. Also, the beta version [email protected] should fix this.

In summary, we were relying on onTransitionEnd always firing when the tooltip closes (i.e. opacity changes to 0). When the value for show changes too quickly, it doesn't give enough time for the different styles to be applied to the tooltip, so the transition never fires, leaving it stuck on the DOM.

---

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.

[gabrieljablonski]

Official release [email protected] fixes this and some other stuff.

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.

[johannkor]

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 git checkout v5.25.0 before applying it.

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

[gabrieljablonski]

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.

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.

[johannkor]

The rerender loop appears to be a separate issue from the issue above. I was able to reproduce it with just tooltipRef.current.open() and scrolling and thus have opened a separate issue with an easier reproducible example here: #1146

Would you like me to move the "Tooltip stays visible" issue (described above) to a separate issue?

[johannkor]

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.

[johannkor]

It seems that the crux of the issue is the following:

• Tooltip is open on anchor A
• Cursor leaves anchor A, debouncedHandleHideTooltip calls handleHideTooltip() immediately and starts a 50ms debounce
• If during this 50ms debounce the cursor leaves an anchor again (e.g. enter&leave anchor B within that 50ms), that handleHideTooltip() call never gets called because it occurred during the debounce.

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 mouseenter and mouseleave events into [data-tooltip-content="tooltip 0"] and [data-tooltip-content="tooltip 1"] with specific times between the events. The key there appears to be to wait for the tooltip to be fully open on one element, then quickly enter&leave another anchor.

(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

[gabrieljablonski]

Would you like me to move the "Tooltip stays visible" issue (described above) to a separate issue?

Leave it as is. Creating #1146 to isolate the render loop issue should be fine for now.

this is easier to reproduce if you set CPU throttling to 6x in Chrome's devtools.

This is something I'll keep in mind for all future tests. Great suggestion for performance testing.

If during this 50ms debounce, the cursor leaves an anchor again (e.g. enter&leave anchor B within that 50ms), that handleHideTooltip() call never gets called because it occurred during the debounce.

I'll test it out myself later, but taking a quick look at how debounce() is being used, I'm 95% sure your analysis is correct.

I'd appreciate any suggestions on how to fix this, but my first thought was to extend debounce() to return a "clear" function as a second return value, which can be used to clear the debounce timeout for that function. Then, clearing the show debounce on hide, and clearing the hide debounce on show should fix it.

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).

[johannkor]

That was exactly my thought - lodash provides a cancel method on functions created with its debounce utility. I've opened #1147 which provides a similar API (just named reset because I forgot to look up the name lodash used)

[gabrieljablonski]

Fix available on official version [email protected]