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

[SpeedDial] Tooltip not closing when quickly moving the mouse away #25215

Closed
2 tasks done
m4theushw opened this issue Mar 6, 2021 · 3 comments · Fixed by #25259
Closed
2 tasks done

[SpeedDial] Tooltip not closing when quickly moving the mouse away #25215

m4theushw opened this issue Mar 6, 2021 · 3 comments · Fixed by #25259
Labels
bug 🐛 Something doesn't work component: speed dial This is the name of the generic UI component, not the React module!

Comments

@m4theushw
Copy link
Member

m4theushw commented Mar 6, 2021

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

If I quickly move the mouse away from the speed dial while a tooltip is open, when I open it again that previous tooltip will still be visible. Interacting via keyboard also causes this behavior.

speed-dial

Expected Behavior 🤔

The tooltip should not be already visible on the second time that the speed dial is open.

Steps to Reproduce 🕹

https://codesandbox.io/s/1ssec?file=/demo.tsx

Steps:

  1. Open the speed dial.
  2. Move the mouse over an action to open the tooltip.
  3. Move VERY QUICKLY the mouse away from the speed dial.
  4. Open again the speed dial.

Context 🔦

I noticed while doing #25166. It was also happening with the non-emotion component.

PR on the way.

Your Environment 🌎

`npx @material-ui/envinfo`
  System:
    OS: Windows 10 10.0.18363
  Binaries:
    Node: 12.16.2 - C:\node-v12\node.EXE
    Yarn: 1.22.5 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.4 - C:\node-v12\npm.CMD
  Browsers:
    Chrome: 88.0.4324.190
    Edge: Spartan (44.18362.449.0)
@m4theushw m4theushw added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Mar 6, 2021
@oliviertassinari oliviertassinari added component: speed dial This is the name of the generic UI component, not the React module! and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Mar 6, 2021
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 7, 2021

This is a regression from v5. I can't reproduce on v4. Looking at this, I have found two other things that are not flying:

  • [Tooltip] Fix placement regression #25255
  • The placement of the tooltip is problematic. It overflows the other actions, preventing interactions. This one should be easy to fix, in case you want to have a look :)

@oliviertassinari oliviertassinari added the component: tooltip This is the name of the generic UI component, not the React module! label Mar 7, 2021
@m4theushw
Copy link
Member Author

@oliviertassinari In my investigation, I found that wrapping setOpenState in a setTimeout solves the problem. I'm just having some trouble to create a test case for the keyboard navigation because the events are being dispatched in a different order between jsdom and the browser.

diff --git a/packages/material-ui/src/SpeedDial/SpeedDial.js b/packages/material-ui/src/SpeedDial/SpeedDial.js
index 3daec5a653..ac2fd684d7 100644
--- a/packages/material-ui/src/SpeedDial/SpeedDial.js
+++ b/packages/material-ui/src/SpeedDial/SpeedDial.js
@@ -274,20 +274,13 @@ const SpeedDial = React.forwardRef(function SpeedDial(inProps, ref) {
     }
 
     clearTimeout(eventTimer.current);
-    if (event.type === 'blur') {
-      event.persist();
-      eventTimer.current = setTimeout(() => {
-        setOpenState(false);
-        if (onClose) {
-          onClose(event, 'blur');
-        }
-      });
-    } else {
+    event.persist();
+    eventTimer.current = setTimeout(() => {
       setOpenState(false);
       if (onClose) {
-        onClose(event, 'mouseLeave');
+        onClose(event, event.type === 'mouseleave' ? 'mouseLeave' : 'blur');
       }
-    }
+    });
   };
 
   const handleClick = (event) => {

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 7, 2021

@m4theushw Have done a new quick exploration (~10 minutes), I can up with an alternative solution:

diff --git a/packages/material-ui/src/SpeedDialAction/SpeedDialAction.js b/packages/material-ui/src/SpeedDialAction/SpeedDialAction.js
index 8557d632ce..8e9a8c191b 100644
--- a/packages/material-ui/src/SpeedDialAction/SpeedDialAction.js
+++ b/packages/material-ui/src/SpeedDialAction/SpeedDialAction.js
@@ -151,6 +151,10 @@ const SpeedDialAction = React.forwardRef(function SpeedDialAction(inProps, ref)

   const [tooltipOpen, setTooltipOpen] = React.useState(tooltipOpenProp);

+  if (!open && tooltipOpen) {
+    setTooltipOpen(false);
+  }
+
   const handleTooltipClose = () => {
     setTooltipOpen(false);
   };

It seems that a reset of the tooltip state to close when the speed dial is closed is a sane default. I have noticed this because the tooltip onClose callback tries to trigger but the open prop of the Tooltip is already set to false once it triggers.

Also, note that I could easily reproduce the error:

Mar-07-2021 21-52-36

Maybe it will help you with the test case. IMHO, we should favor a solution that isn't depending on the order of setTimeout resolution (it would be more resilient to future changes).

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Mar 7, 2021
@oliviertassinari oliviertassinari removed the component: tooltip This is the name of the generic UI component, not the React module! label Mar 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: speed dial This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants