From 02ba9850a1d30c936e0bedf6da1a60a158a356ca Mon Sep 17 00:00:00 2001 From: Sandra Marcela Herrera Arriaga Date: Fri, 27 Dec 2019 11:52:54 -0600 Subject: [PATCH 1/3] add useImperativeHandler and move some setPositioningStyles and addeventListener in popover.js --- packages/material-ui/src/Popover/Popover.js | 40 ++++++++++++--------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index 58e4d23d7a2e7c..fb6b1b7c2c31d7 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -335,32 +335,38 @@ 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 && paperRef.current){ + setPositioningStyles(paperRef.current); } - - return debounce(() => { + }); +React.useImperativeHandle( + action, + () => + open ? { + updatePosition: () => { setPositioningStyles(paperRef.current); - }); - }, [open, setPositioningStyles]); - - React.useImperativeHandle(action, () => (open ? { updatePosition } : null), [ - open, - updatePosition, - ]); + }, + } + : null, + [open, setPositioningStyles], +); React.useEffect(() => { - if (!updatePosition) { + if (!open) { return undefined; } - window.addEventListener('resize', updatePosition); + const handleResize = debounce(() => { + setPositioningStyles(paperRef.current); + }); + + window.addEventListener('resize',handleResize); return () => { - window.removeEventListener('resize', updatePosition); - updatePosition.clear(); + handleResize.clear(); + window.removeEventListener('rezise',handleResize); }; - }, [updatePosition]); + }, [open,setPositioningStyles]); let transitionDuration = transitionDurationProp; From f2da6029035c36a8223d01e8a25eb8aec6b9581d Mon Sep 17 00:00:00 2001 From: Sandra Marcela Herrera Arriaga Date: Mon, 30 Dec 2019 10:50:13 -0600 Subject: [PATCH 2/3] yarn prettier --- packages/material-ui/src/Popover/Popover.js | 37 +++++++++++---------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index fb6b1b7c2c31d7..f6839a1ec767f1 100644 --- a/packages/material-ui/src/Popover/Popover.js +++ b/packages/material-ui/src/Popover/Popover.js @@ -336,37 +336,38 @@ const Popover = React.forwardRef(function Popover(props, ref) { }, []); React.useEffect(() => { - if (open && paperRef.current){ + if (open && paperRef.current) { setPositioningStyles(paperRef.current); } }); -React.useImperativeHandle( - action, - () => - open ? { - updatePosition: () => { - setPositioningStyles(paperRef.current); - }, - } - : null, - [open, setPositioningStyles], -); + React.useImperativeHandle( + action, + () => + open + ? { + updatePosition: () => { + setPositioningStyles(paperRef.current); + }, + } + : null, + [open, setPositioningStyles], + ); React.useEffect(() => { if (!open) { return undefined; } - const handleResize = debounce(() => { - setPositioningStyles(paperRef.current); - }); + const handleResize = debounce(() => { + setPositioningStyles(paperRef.current); + }); - window.addEventListener('resize',handleResize); + window.addEventListener('resize', handleResize); return () => { handleResize.clear(); - window.removeEventListener('rezise',handleResize); + window.removeEventListener('rezise', handleResize); }; - }, [open,setPositioningStyles]); + }, [open, setPositioningStyles]); let transitionDuration = transitionDurationProp; From cc7e57d8d266278fb0165f853295ad6a1a944052 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Tue, 31 Dec 2019 11:06:43 +0100 Subject: [PATCH 3/3] fix tests --- packages/material-ui/src/Popover/Popover.js | 40 ++++++++++--------- .../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, 38 insertions(+), 38 deletions(-) diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js index f6839a1ec767f1..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 => { @@ -336,17 +339,18 @@ const Popover = React.forwardRef(function Popover(props, ref) { }, []); React.useEffect(() => { - if (open && paperRef.current) { - setPositioningStyles(paperRef.current); + if (open) { + setPositioningStyles(); } }); + React.useImperativeHandle( action, () => open ? { updatePosition: () => { - setPositioningStyles(paperRef.current); + setPositioningStyles(); }, } : null, @@ -359,7 +363,7 @@ const Popover = React.forwardRef(function Popover(props, ref) { } const handleResize = debounce(() => { - setPositioningStyles(paperRef.current); + setPositioningStyles(); }); window.addEventListener('resize', handleResize); 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);