[Popover] Fix position when changing state or updated (#19046)

mui · Dec 31, 2019 · f38c262 · f38c262
1 parent d332339
commit f38c262
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 51 deletions.
diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js
@@ -307,60 +307,71 @@ const Popover = React.forwardRef(function Popover(props, ref) {
     ],
   );
 
-  const setPositioningStyles = React.useCallback(
-    element => {
-      const positioning = getPositioningStyle(element);
+  const setPositioningStyles = React.useCallback(() => {
+    const element = paperRef.current;
 
-      if (positioning.top !== null) {
-        element.style.top = positioning.top;
-      }
-      if (positioning.left !== null) {
-        element.style.left = positioning.left;
-      }
-      element.style.transformOrigin = positioning.transformOrigin;
-    },
-    [getPositioningStyle],
-  );
+    if (!element) {
+      return;
+    }
+
+    const positioning = getPositioningStyle(element);
+
+    if (positioning.top !== null) {
+      element.style.top = positioning.top;
+    }
+    if (positioning.left !== null) {
+      element.style.left = positioning.left;
+    }
+    element.style.transformOrigin = positioning.transformOrigin;
+  }, [getPositioningStyle]);
 
   const handleEntering = (element, isAppearing) => {
     if (onEntering) {
       onEntering(element, isAppearing);
     }
 
-    setPositioningStyles(element);
+    setPositioningStyles();
   };
 
   const handlePaperRef = React.useCallback(instance => {
     // #StrictMode ready
     paperRef.current = ReactDOM.findDOMNode(instance);
   }, []);
 
-  const updatePosition = React.useMemo(() => {
-    if (!open) {
-      return undefined;
+  React.useEffect(() => {
+    if (open) {
+      setPositioningStyles();
     }
+  });
 
-    return debounce(() => {
-      setPositioningStyles(paperRef.current);
-    });
-  }, [open, setPositioningStyles]);
-
-  React.useImperativeHandle(action, () => (open ? { updatePosition } : null), [
-    open,
-    updatePosition,
-  ]);
+  React.useImperativeHandle(
+    action,
+    () =>
+      open
+        ? {
+            updatePosition: () => {
+              setPositioningStyles();
+            },
+          }
+        : null,
+    [open, setPositioningStyles],
+  );
 
   React.useEffect(() => {
-    if (!updatePosition) {
+    if (!open) {
       return undefined;
     }
 
-    window.addEventListener('resize', updatePosition);
+    const handleResize = debounce(() => {
+      setPositioningStyles();
+    });
+
+    window.addEventListener('resize', handleResize);
     return () => {
-      window.removeEventListener('resize', updatePosition);
-      updatePosition.clear();
+      handleResize.clear();
+      window.removeEventListener('rezise', handleResize);
     };
-  }, [updatePosition]);
+  }, [open, setPositioningStyles]);
 
   let transitionDuration = transitionDurationProp;
 

diff --git a/packages/material-ui/src/Popover/Popover.test.js b/packages/material-ui/src/Popover/Popover.test.js
@@ -616,13 +616,14 @@ describe('<Popover />', () => {
       clock = useFakeTimers();
 
       windowInnerHeight = window.innerHeight;
+      window.innerHeight = 8;
+
       const mockedAnchor = document.createElement('div');
       stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
         left: 0,
         top: 9,
       }));
       const handleEntering = spy();
-      window.innerHeight = 8;
       wrapper = mount(
         <Popover
           anchorEl={mockedAnchor}
@@ -681,14 +682,11 @@ describe('<Popover />', () => {
     it('should be able to manually recalculate position', () => {
       let popoverActions;
       wrapper.setProps({
-        open: false,
+        open: true,
         action: actions => {
           popoverActions = actions;
         },
       });
-      wrapper.setProps({
-        open: true,
-      });
       const beforeStyle = {
         top: element.style.top,
         left: element.style.left,

diff --git a/packages/material-ui/src/Slide/Slide.js b/packages/material-ui/src/Slide/Slide.js
@@ -173,22 +173,21 @@ const Slide = React.forwardRef(function Slide(props, ref) {
 
   React.useEffect(() => {
     // Skip configuration where the position is screen size invariant.
-    if (!inProp && direction !== 'down' && direction !== 'right') {
-      const handleResize = debounce(() => {
-        if (childrenRef.current) {
-          setTranslateValue(direction, childrenRef.current);
-        }
-      });
-
-      window.addEventListener('resize', handleResize);
-
-      return () => {
-        handleResize.clear();
-        window.removeEventListener('resize', handleResize);
-      };
+    if (inProp || direction === 'down' || direction === 'right') {
+      return undefined;
     }
 
-    return undefined;
+    const handleResize = debounce(() => {
+      if (childrenRef.current) {
+        setTranslateValue(direction, childrenRef.current);
+      }
+    });
+
+    window.addEventListener('resize', handleResize);
+    return () => {
+      handleResize.clear();
+      window.removeEventListener('resize', handleResize);
+    };
   }, [direction, inProp]);
 
   React.useEffect(() => {

diff --git a/packages/material-ui/src/Tabs/ScrollbarSize.js b/packages/material-ui/src/Tabs/ScrollbarSize.js
@@ -35,7 +35,6 @@ export default function ScrollbarSize(props) {
     });
 
     window.addEventListener('resize', handleResize);
-
     return () => {
       handleResize.clear();
       window.removeEventListener('resize', handleResize);