Merge pull request #2353 from oliviertassinari/popover-on-request-close

[Popover] Fix onRequestClose called 3 times

docs/src/app/components/pages/components/popover.jsx

@@ -69,9 +69,9 @@ let PopoverPage = React.createClass({
           },
           {
             name: 'onRequestClose',
-            type: 'func',
+            type: 'function(reason)',
             header: 'default: no-op',
-            desc: 'This is a callback that fires when the popover thinks it should close. (e.g. click-away or scroll off-screen)',
+            desc: 'This is a callback that fires when the popover thinks it should close. (e.g. clickAway or offScreen)',
           },
           {
             name: 'zDepth',

src/menu/menu-item.jsx

@@ -78,12 +78,6 @@ const MenuItem = React.createClass({
     };
   },
 
-  componentWillUnmount() {
-    if (this.state.open) {
-      this.setState({open:false});
-    }
-  },
-
   getTheme() {
     return this.state.muiTheme.menuItem;
   },

src/menus/menu-item.jsx

@@ -10,10 +10,16 @@ import DefaultRawTheme from '../styles/raw-themes/light-raw-theme';
 import ThemeManager from '../styles/theme-manager';
 import Menu from './menu';
 
-const nestedMenuStyle = {position:'relative'};
+const nestedMenuStyle = {
+  position: 'relative',
+};
+
 const MenuItem = React.createClass({
 
-  mixins: [PureRenderMixin, StylePropable],
+  mixins: [
+    PureRenderMixin,
+    StylePropable,
+  ],
 
   contextTypes: {
     muiTheme: React.PropTypes.object,
@@ -52,7 +58,7 @@ const MenuItem = React.createClass({
   getInitialState() {
     return {
       muiTheme: this.context.muiTheme ? this.context.muiTheme : ThemeManager.getMuiTheme(DefaultRawTheme),
-      open:false,
+      open: false,
     };
   },
 
@@ -83,7 +89,9 @@ const MenuItem = React.createClass({
 
   componentWillUnmount() {
     if (this.state.open) {
-      this.setState({open:false});
+      this.setState({
+        open: false,
+      });
     }
   },
 
@@ -176,7 +184,7 @@ const MenuItem = React.createClass({
     if (menuItems) {
       childMenuPopover = (
         <Popover
-          anchorOrigin={{horizontal:'right', vertical:'top'}}
+          anchorOrigin={{horizontal: 'right', vertical: 'top'}}
           anchorEl={this.state.anchorEl}
           open={this.state.open}
           onRequestClose={this._onRequestClose}>
@@ -210,40 +218,38 @@ const MenuItem = React.createClass({
   },
 
   _cloneMenuItem(item) {
-    let props = {
-      onTouchTap: (e) =>
+    return React.cloneElement(item, {
+      onTouchTap: (event) =>
       {
-        this._onRequestClose();
-        if (item.props.onTouchTap) {
-          item.props.onTouchTap(e);
+        if (!item.props.menuItems) {
+          this._onRequestClose();
         }
-        if (this.props.onTouchTap) {
-          this.props.onTouchTap(e);
+
+        if (item.props.onTouchTap) {
+          item.props.onTouchTap(event);
         }
       },
       onRequestClose: this._onRequestClose,
-    };
-    return React.cloneElement(item, props);
+    });
   },
 
-  _onTouchTap(e) {
-    e.preventDefault();
+  _onTouchTap(event) {
+    event.preventDefault();
+
     this.setState({
-      open:true,
-      anchorEl:ReactDOM.findDOMNode(this),
+      open: true,
+      anchorEl: ReactDOM.findDOMNode(this),
     });
+
     if (this.props.onTouchTap) {
-      this.props.onTouchTap(e);
+      this.props.onTouchTap(event);
     }
   },
 
   _onRequestClose() {
-    if (!this.isMounted()) {
-      return;
-    }
     this.setState({
-      open:false,
-      anchorEl:null,
+      open: false,
+      anchorEl: null,
     });
   },
 });

src/popover/popover.jsx

@@ -3,17 +3,14 @@ import ReactDOM from 'react-dom';
 import WindowListenable from '../mixins/window-listenable';
 import RenderToLayer from '../render-to-layer';
 import StylePropable from '../mixins/style-propable';
-import CssEvent from '../utils/css-event';
 import PropTypes from '../utils/prop-types';
 import Transitions from '../styles/transitions';
 import Paper from '../paper';
 import throttle from 'lodash.throttle';
 import AutoPrefix from '../styles/auto-prefix';
-import ContextPure from '../mixins/context-pure';
 
 const Popover = React.createClass({
   mixins: [
-    ContextPure,
     StylePropable,
     WindowListenable,
   ],
@@ -36,27 +33,28 @@ const Popover = React.createClass({
   getDefaultProps() {
     return {
       anchorOrigin: {
-        vertical:'bottom',
-        horizontal:'left',
+        vertical: 'bottom',
+        horizontal: 'left',
       },
-      animated:true,
-      autoCloseWhenOffScreen:true,
-      canAutoPosition:true,
+      animated: true,
+      autoCloseWhenOffScreen: true,
+      canAutoPosition: true,
       onRequestClose: () => {},
-      open:false,
+      open: false,
       style: {},
       targetOrigin: {
-        vertical:'top',
-        horizontal:'left',
+        vertical: 'top',
+        horizontal: 'left',
       },
       zDepth: 1,
     };
   },
 
   getInitialState() {
     this.setPlacementThrottled = throttle(this.setPlacement, 100);
+
     return {
-      open: false,
+      open: this.props.open,
     };
   },
 
@@ -71,10 +69,18 @@ const Popover = React.createClass({
 
   componentWillReceiveProps(nextProps) {
     if (nextProps.open !== this.state.open) {
-      if (nextProps.open)
-        this._showInternal(nextProps.anchorEl);
-      else
-        this._hideInternal();
+      if (nextProps.open) {
+        this.anchorEl = nextProps.anchorEl || this.props.anchorEl;
+        this.setState({
+          open: true,
+        });
+      } else {
+        this.setState({
+          open: false,
+        }, () => {
+          this._animateClose();
+        });
+      }
     }
   },
 
@@ -84,7 +90,7 @@ const Popover = React.createClass({
 
   componentWillUnmount() {
     if (this.state.open) {
-      this.props.onRequestClose();
+      this._animateClose();
     }
   },
 
@@ -143,56 +149,39 @@ const Popover = React.createClass({
         <div>
           <div style={horizontalAnimation}>
             <div style={verticalAnimation}>
-                {this.props.children}
+              {this.props.children}
            </div>
           </div>
         </div>
       </Paper>
     );
   },
 
-  requestClose() {
-    if (this.props.onRequestClose)
-      this.props.onRequestClose();
+  requestClose(reason) {
+    if (this.props.onRequestClose) {
+      this.props.onRequestClose(reason);
+    }
   },
 
-  componentClickAway(e) {
-    if (e.defaultPrevented) {
+  componentClickAway(event) {
+    if (event.defaultPrevented) {
       return;
     }
-    this._hideInternal();
+
+    this.requestClose('clickAway');
   },
 
   _resizeAutoPosition() {
     this.setPlacement();
   },
 
-  _showInternal(anchorEl) {
-    this.anchorEl = anchorEl || this.props.anchorEl;
-    this.setState({open: true});
-  },
-
-  _hideInternal() {
-    if (!this.state.open) {
-      return;
-    }
-    this.setState({
-      open: false,
-    }, () => {
-      this._animateClose();
-    });
-  },
-
   _animateClose() {
     if (!this.refs.layer || !this.refs.layer.getLayer()) {
       return;
     }
-    let el = this.refs.layer.getLayer().children[0];
-    this._animate(el, false);
-  },
 
-  _animateOpen(el) {
-    this._animate(el, true);
+    const el = this.refs.layer.getLayer().children[0];
+    this._animate(el, false);
   },
 
   _animate(el) {
@@ -206,12 +195,6 @@ const Popover = React.createClass({
     if (this.state.open) {
       value = '1';
     }
-    else {
-      CssEvent.onTransitionEnd(inner, () => {
-        if (!this.state.open)
-          this.requestClose();
-      });
-    }
 
     AutoPrefix.set(el.style, 'transform', `scale(${value},${value})`);
     AutoPrefix.set(innerInner.style, 'transform', `scaleX(${value})`);
@@ -223,8 +206,9 @@ const Popover = React.createClass({
   },
 
   getAnchorPosition(el) {
-    if (!el)
+    if (!el) {
       el = ReactDOM.findDOMNode(this);
+    }
 
     const rect = el.getBoundingClientRect();
     const a = {
@@ -238,28 +222,31 @@ const Popover = React.createClass({
     a.bottom = a.top + a.height;
     a.middle = a.left + a.width / 2;
     a.center = a.top + a.height / 2;
+
     return a;
   },
 
   getTargetPosition(targetEl) {
     return {
-      top:0,
+      top: 0,
       center: targetEl.offsetHeight / 2,
       bottom: targetEl.offsetHeight,
-      left:0,
-      middle:targetEl.offsetWidth / 2,
-      right:targetEl.offsetWidth,
+      left: 0,
+      middle: targetEl.offsetWidth / 2,
+      right: targetEl.offsetWidth,
     };
   },
 
   setPlacement() {
-    if (!this.state.open)
+    if (!this.state.open) {
       return;
+    }
 
     const anchorEl = this.props.anchorEl || this.anchorEl;
 
-    if (!this.refs.layer.getLayer())
+    if (!this.refs.layer.getLayer()) {
       return;
+    }
 
     const targetEl = this.refs.layer.getLayer().children[0];
     if (!targetEl) {
@@ -276,8 +263,9 @@ const Popover = React.createClass({
       left: anchor[anchorOrigin.horizontal] - target[targetOrigin.horizontal],
     };
 
-    if (this.props.autoCloseWhenOffScreen)
+    if (this.props.autoCloseWhenOffScreen) {
       this.autoCloseWhenOffScreen(anchor);
+    }
 
     if (this.props.canAutoPosition) {
       target = this.getTargetPosition(targetEl); // update as height may have changed
@@ -287,18 +275,18 @@ const Popover = React.createClass({
 
     targetEl.style.top = targetPosition.top + 'px';
     targetEl.style.left = targetPosition.left + 'px';
-    this._animateOpen(targetEl);
+
+    this._animate(targetEl, true);
   },
 
   autoCloseWhenOffScreen(anchorPosition) {
-    if (!this.props.autoCloseWhenOffScreen)
-      return;
     if (anchorPosition.top < 0
         || anchorPosition.top > window.innerHeight
         || anchorPosition.left < 0
         || anchorPosition.left > window.innerWith
-        )
-      this._hideInternal();
+        ) {
+      this.requestClose('offScreen');
+    }
   },
 
   applyAutoPositionIfNeeded(anchor, target, targetOrigin, anchorOrigin, targetPosition) {