Skip to content
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] Wrapping the app in TooltipProvider causes every single tooltip to re-render on hover #2375

Open
lauri865 opened this issue Sep 2, 2023 · 9 comments · May be fixed by #3209
Open

Comments

@lauri865
Copy link

lauri865 commented Sep 2, 2023

Bug report

Current Behavior

When wrapping the app in Tooltip provider, every single component that implements a tooltip gets re-rendered on hovering any of the components that implement a tooltip, even if nothing changes, causing excessive re-renders.

Expected behavior

Only components with an active tooltip should be affected or when default settings change.

Reproducible example

Wrap an app in TooltipProvider and turn on Highlight updates when components render in React devtools, and then hover any component with a tooltip to notice that every component that implements a tooltip gets re-rendered.

Suggested solution

Additional context

Your environment

Software Name(s) Version
Radix Package(s) @radix-ui/react-tooltip 1.0.6
React react 18.2.0
Browser Chrome Version 116.0.5845.140 (Official Build) (arm64)
Assistive tech
Node n/a
npm/yarn
Operating System
DHolmstrom referenced this issue in Z-Teknologsektionen/ztek Feb 28, 2024
@CarlosVergikosk
Copy link

bump?

@divmgl
Copy link

divmgl commented Sep 15, 2024

Oh wow, we also just ran into this. Does this mean that we need to be wrapping every single Tooltip in TooltipProvider? I thought the opposite was supposed to be true.

@aboveyunhai
Copy link

aboveyunhai commented Sep 20, 2024

Based on the Profiler highlight,

  1. It seems like anything under the Tooltip Provider will re-render upon the first detection (mouse-wise, keyboard-wise)
  2. after that, only the target item will be re-render when you hover them, rest wouldn't.
  3. then the last detection when you leave the Provider "gray area", will trig the re-render of all, like the first detection.

Hence, the Provider is pretty similar to how React.Context works,
Where your issue might be from the first and last detection as far as I can tell, can't tell about the detection algorithm without digging though the source code. So you were claiming might not be true.

That also means, the following case can cause excessive re-renders, where you have a local Tooltip Provider that wraps lots of small items. For example, a calendar panel. That the total size of the Tooltip Provider (panel) in page is also small enough and your mouse is constantly in and out across the Provider detection area (panel edge). So you constantly trig the first(in)/last(out) detection. If you also have some heavy computations in each date button, without some sort of useMemo or useCallback. You might experience lag.

But you can't always make the conclusion based on the dev-tools highlight, always need to measure, the render might be cheap.

@hichemfantar
Copy link

hichemfantar commented Oct 6, 2024

ran into the same thing, this should be mentioned in the docs.
would having multiple providers cause issues with render priority?

@misha-erm
Copy link

misha-erm commented Oct 14, 2024

Here is a patch that fixes the issue

The trick is to use useRef for isOpenDelayed instead of useState. Seems like a safe change since the variable is read inside event callback and not inside render

Our app has tons of elements with tooltips on screen so the issue was quite critical for us. After the change only one element re-renders on hover

diff --git a/dist/index.js b/dist/index.js
index a805b534d2f34981afe9270bfbf0ad9e9a115d21..17f07a96ec77e4b1dcefa8554ee14fab602ba95b 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -79,7 +79,7 @@ var TooltipProvider = (props) => {
     disableHoverableContent = false,
     children
   } = props;
-  const [isOpenDelayed, setIsOpenDelayed] = React.useState(true);
+  const isOpenDelayedRef = React.useRef(true);
   const isPointerInTransitRef = React.useRef(false);
   const skipDelayTimerRef = React.useRef(0);
   React.useEffect(() => {
@@ -90,16 +90,16 @@ var TooltipProvider = (props) => {
     TooltipProviderContextProvider,
     {
       scope: __scopeTooltip,
-      isOpenDelayed,
+      isOpenDelayedRef,
       delayDuration,
       onOpen: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
-        setIsOpenDelayed(false);
+        isOpenDelayedRef.current = (false);
       }, []),
       onClose: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
         skipDelayTimerRef.current = window.setTimeout(
-          () => setIsOpenDelayed(true),
+          () => isOpenDelayedRef.current = (true),
           skipDelayDuration
         );
       }, [skipDelayDuration]),
@@ -178,9 +178,9 @@ var Tooltip = (props) => {
       trigger,
       onTriggerChange: setTrigger,
       onTriggerEnter: React.useCallback(() => {
-        if (providerContext.isOpenDelayed) handleDelayedOpen();
+        if (providerContext.isOpenDelayedRef.current) handleDelayedOpen();
         else handleOpen();
-      }, [providerContext.isOpenDelayed, handleDelayedOpen, handleOpen]),
+      }, [providerContext.isOpenDelayedRef, handleDelayedOpen, handleOpen]),
       onTriggerLeave: React.useCallback(() => {
         if (disableHoverableContent) {
           handleClose();
diff --git a/dist/index.mjs b/dist/index.mjs
index a6ac5447b420e89a93415510e2a6fc4b11ab96bd..065f5845826afdee9e01aa3191d6a48eeeed936d 100644
--- a/dist/index.mjs
+++ b/dist/index.mjs
@@ -32,7 +32,7 @@ var TooltipProvider = (props) => {
     disableHoverableContent = false,
     children
   } = props;
-  const [isOpenDelayed, setIsOpenDelayed] = React.useState(true);
+  const isOpenDelayedRef = React.useRef(true);
   const isPointerInTransitRef = React.useRef(false);
   const skipDelayTimerRef = React.useRef(0);
   React.useEffect(() => {
@@ -43,16 +43,16 @@ var TooltipProvider = (props) => {
     TooltipProviderContextProvider,
     {
       scope: __scopeTooltip,
-      isOpenDelayed,
+      isOpenDelayedRef,
       delayDuration,
       onOpen: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
-        setIsOpenDelayed(false);
+        isOpenDelayedRef.current = false;
       }, []),
       onClose: React.useCallback(() => {
         window.clearTimeout(skipDelayTimerRef.current);
         skipDelayTimerRef.current = window.setTimeout(
-          () => setIsOpenDelayed(true),
+          () => isOpenDelayedRef.current = true,
           skipDelayDuration
         );
       }, [skipDelayDuration]),
@@ -131,9 +131,9 @@ var Tooltip = (props) => {
       trigger,
       onTriggerChange: setTrigger,
       onTriggerEnter: React.useCallback(() => {
-        if (providerContext.isOpenDelayed) handleDelayedOpen();
+        if (providerContext.isOpenDelayedRef.current) handleDelayedOpen();
         else handleOpen();
-      }, [providerContext.isOpenDelayed, handleDelayedOpen, handleOpen]),
+      }, [providerContext.isOpenDelayedRef, handleDelayedOpen, handleOpen]),
       onTriggerLeave: React.useCallback(() => {
         if (disableHoverableContent) {
           handleClose();

misha-erm added a commit to misha-erm/primitives that referenced this issue Nov 3, 2024
@misha-erm misha-erm linked a pull request Nov 3, 2024 that will close this issue
@suitux
Copy link

suitux commented Nov 9, 2024

Do you have plan to merge this? Still happening

@Fatihkrty
Copy link

I have same issue

@drewlowe
Copy link

drewlowe commented Dec 3, 2024

bump

@CarlosVergikosk
Copy link

bump x2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants