From f38c262c009d47dfbb33361a94043bd6975a0ad3 Mon Sep 17 00:00:00 2001 From: Sandra Marcela Herrera Arriaga Date: Tue, 31 Dec 2019 05:10:41 -0600 Subject: [PATCH] [Popover] Fix position when changing state or updated (#19046) --- packages/material-ui/src/Popover/Popover.js | 73 +++++++++++-------- .../material-ui/src/Popover/Popover.test.js | 8 +- packages/material-ui/src/Slide/Slide.js | 27 ++++--- .../material-ui/src/Tabs/ScrollbarSize.js | 1 - 4 files changed, 58 insertions(+), 51 deletions(-) diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index 58e4d23d7a2e7c..7651a5cedb5ac6 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -307,27 +307,30 @@ 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 => { @@ -335,32 +338,40 @@ const Popover = React.forwardRef(function Popover(props, ref) { 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 index b43630e04dd673..6a576f7c1bcf2d 100644 --- a/packages/material-ui/src/Popover/Popover.test.js +++ b/packages/material-ui/src/Popover/Popover.test.js @@ -616,13 +616,14 @@ describe('', () => { 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( ', () => { 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 index e8f9873bbd3297..49a6b00efb7874 100644 --- 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 index f0953dfbcb1ab5..432aff9bbb9f06 100644 --- 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);