[Popper] Fix update logic

mui · Jul 20, 2018 · 7994cf5 · 7994cf5
1 parent ba2ba25
commit 7994cf5
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 9 deletions.
diff --git a/docs/src/pages/utils/popper/PositionedPopper.js b/docs/src/pages/utils/popper/PositionedPopper.js
@@ -28,7 +28,7 @@ class PositionedPopper extends React.Component {
     const { currentTarget } = event;
     this.setState(state => ({
       anchorEl: currentTarget,
-      open: !state.placement === placement || !state.open,
+      open: state.placement !== placement || !state.open,
       placement,
     }));
   };

diff --git a/packages/material-ui/src/Popper/Popper.js b/packages/material-ui/src/Popper/Popper.js
@@ -43,20 +43,21 @@ class Popper extends React.Component {
   }
 
   componentDidUpdate(prevProps) {
-    if (prevProps.open && !this.props.open && !this.props.transition) {
+    if (prevProps.open !== this.props.open && !this.props.open && !this.props.transition) {
       // Otherwise handleExited will call this.
       this.handleClose();
     }
 
     // Let's update the popper position.
     if (
+      prevProps.open !== this.props.open ||
       prevProps.anchorEl !== this.props.anchorEl ||
       prevProps.popperOptions !== this.props.popperOptions ||
       prevProps.modifiers !== this.props.modifiers ||
       prevProps.disablePortal !== this.props.disablePortal ||
       prevProps.placement !== this.props.placement
     ) {
-      this.handleRendered();
+      this.handleOpen();
     }
   }
 
@@ -81,7 +82,7 @@ class Popper extends React.Component {
     return null;
   }
 
-  handleRendered = () => {
+  handleOpen = () => {
     const {
       anchorEl,
       modifiers,
@@ -93,15 +94,15 @@ class Popper extends React.Component {
     } = this.props;
     const popperNode = ReactDOM.findDOMNode(this);
 
+    if (!popperNode || !anchorEl || !open) {
+      return;
+    }
+
     if (this.popper) {
       this.popper.destroy();
       this.popper = null;
     }
 
-    if (!popperNode || !anchorEl || !open) {
-      return;
-    }
-
     this.popper = new PopperJS(getAnchorEl(anchorEl), popperNode, {
       placement: flipPlacement(theme, placement),
       ...popperOptions,
@@ -178,7 +179,7 @@ class Popper extends React.Component {
     }
 
     return (
-      <Portal onRendered={this.handleRendered} disablePortal={disablePortal} container={container}>
+      <Portal onRendered={this.handleOpen} disablePortal={disablePortal} container={container}>
         <div
           role="tooltip"
           style={{

diff --git a/packages/material-ui/src/Popper/Popper.test.js b/packages/material-ui/src/Popper/Popper.test.js
@@ -106,6 +106,22 @@ describe('<Popper />', () => {
       wrapper.setProps({ open: false });
       assert.strictEqual(wrapper.find('span').length, 0);
     });
+
+    it('should position the popper when opening', () => {
+      const wrapper = mount(<PopperNaked {...defaultProps} open={false} anchorEl={anchorEl} />);
+      const instance = wrapper.instance();
+      assert.strictEqual(instance.popper, null);
+      wrapper.setProps({ open: true });
+      assert.strictEqual(instance.popper !== null, true);
+    });
+
+    it('should not position the popper when closing', () => {
+      const wrapper = mount(<PopperNaked {...defaultProps} open anchorEl={anchorEl} />);
+      const instance = wrapper.instance();
+      assert.strictEqual(instance.popper !== null, true);
+      wrapper.setProps({ open: false });
+      assert.strictEqual(instance.popper, null);
+    });
   });
 
   describe('prop: transition', () => {
@@ -124,6 +140,12 @@ describe('<Popper />', () => {
       assert.strictEqual(wrapper.find('span').text(), 'Hello World');
       assert.strictEqual(instance.popper !== null, true);
       wrapper.setProps({ anchorEl: null });
+      assert.strictEqual(instance.popper !== null, true);
+      wrapper.setProps({ open: false });
+      wrapper
+        .find(Grow)
+        .props()
+        .onExited();
       assert.strictEqual(instance.popper, null);
     });
   });