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

Scrollbar width padding misaligning pages caused by Menu + Tooltip #19203

Closed
2 tasks done
archfz opened this issue Jan 12, 2020 · 6 comments · Fixed by #19304
Closed
2 tasks done

Scrollbar width padding misaligning pages caused by Menu + Tooltip #19203

archfz opened this issue Jan 12, 2020 · 6 comments · Fixed by #19304
Labels
bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@archfz
Copy link

archfz commented Jan 12, 2020

  • 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 😯

When using menu + menu items + tooltip: Opening the menu adds scrollbar width padding to the right side of the page and de-aligns all other elements.

Expected Behavior 🤔

No disalignement. No additional padding.

Steps to Reproduce 🕹

https://codesandbox.io/s/compassionate-keldysh-t4ebw?fontsize=14&hidenavigation=1&theme=dark
Note that the menu is duplicated only so that it can be seen better that items get shifted on open menu.

@mbrookes
Copy link
Member

With MacOS you have to set "Show scroll bars" to "Always" in System Preferences / General in order to see this issue.

@oliviertassinari oliviertassinari added low priority bug 🐛 Something doesn't work labels Jan 12, 2020
@LorenzHenk
Copy link

LorenzHenk commented Jan 14, 2020

This problem seems to start appearing with version 4.7.0.

@LorenzHenk
Copy link

Forked the codesandbox: https://codesandbox.io/s/material-ui-issue-padding-menu-mwnze

Changes:

  • use @material-ui/core version 4.7.0, the oldest one where this problem appears
  • remove toolbar from the second button to better show the difference

@xhanix
Copy link

xhanix commented Jan 16, 2020

This solved the issue for me:

html {
overflow: hidden;
height: 100%;
}

body {
height: 100%;
overflow: auto;
}

@oliviertassinari
Copy link
Member

You are right, the behavior has changed between v4.6.1 (OK) and v4.7.0 (KO). It's related to the changes in packages/material-ui/src/Tooltip/Tooltip.js. v4.6.1...v4.7.0#diff-db872c13568b6f9d9dd4ea1f5e876cdb

What about this fix?

diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js
index 70829de9c..67f55a5f7 100644
--- a/packages/material-ui/src/Tooltip/Tooltip.js
+++ b/packages/material-ui/src/Tooltip/Tooltip.js
@@ -482,6 +482,19 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
     }
   }

+  // Avoid the creation of a new Popper.js instance at each render.
+  const popperOptions = React.useMemo(
+    () => ({
+      modifiers: {
+        arrow: {
+          enabled: Boolean(arrowRef),
+          element: arrowRef,
+        },
+      },
+    }),
+    [arrowRef],
+  );
+
   return (
     <React.Fragment>
       {React.cloneElement(children, { ref: handleRef, ...childrenProps })}
@@ -495,14 +508,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) {
         open={childNode ? open : false}
         id={childrenProps['aria-describedby']}
         transition
-        popperOptions={{
-          modifiers: {
-            arrow: {
-              enabled: Boolean(arrowRef),
-              element: arrowRef,
-            },
-          },
-        }}
+        popperOptions={popperOptions}
         {...interactiveWrapperListeners}
         {...PopperProps}
       >

@oliviertassinari oliviertassinari added component: tooltip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. labels Jan 17, 2020
@netochaves
Copy link
Contributor

Working on this.

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: tooltip This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants