[Menu] Better focus, hover, select logic

mui · Dec 31, 2017 · 8b20c1e · 8b20c1e
1 parent 60a498d
commit 8b20c1e
Show file tree

Hide file tree

Showing 16 changed files with 123 additions and 83 deletions.
diff --git a/docs/src/pages/demos/menus/FadeMenu.js b/docs/src/pages/demos/menus/FadeMenu.js
@@ -0,0 +1,47 @@
+import React from 'react';
+import Button from 'material-ui/Button';
+import Menu, { MenuItem } from 'material-ui/Menu';
+import Fade from 'material-ui/transitions/Fade';
+
+class FadeMenu extends React.Component {
+  state = {
+    anchorEl: null,
+  };
+
+  handleClick = event => {
+    this.setState({ anchorEl: event.currentTarget });
+  };
+
+  handleClose = () => {
+    this.setState({ anchorEl: null });
+  };
+
+  render() {
+    const { anchorEl } = this.state;
+
+    return (
+      <div>
+        <Button
+          aria-owns={anchorEl ? 'fade-menu' : null}
+          aria-haspopup="true"
+          onClick={this.handleClick}
+        >
+          Open with fade transition
+        </Button>
+        <Menu
+          id="fade-menu"
+          anchorEl={anchorEl}
+          open={Boolean(anchorEl)}
+          onClose={this.handleClose}
+          transition={Fade}
+        >
+          <MenuItem onClick={this.handleClose}>Profile</MenuItem>
+          <MenuItem onClick={this.handleClose}>My account</MenuItem>
+          <MenuItem onClick={this.handleClose}>Logout</MenuItem>
+        </Menu>
+      </div>
+    );
+  }
+}
+
+export default FadeMenu;
diff --git a/docs/src/pages/demos/menus/LongMenu.js b/docs/src/pages/demos/menus/LongMenu.js
@@ -36,13 +36,13 @@ class LongMenu extends React.Component {
   };
 
   render() {
-    const open = Boolean(this.state.anchorEl);
+    const { anchorEl } = this.state;
 
     return (
       <div>
         <IconButton
           aria-label="More"
-          aria-owns={open ? 'long-menu' : null}
+          aria-owns={anchorEl ? 'long-menu' : null}
           aria-haspopup="true"
           onClick={this.handleClick}
         >
@@ -51,7 +51,7 @@ class LongMenu extends React.Component {
         <Menu
           id="long-menu"
           anchorEl={this.state.anchorEl}
-          open={open}
+          open={Boolean(anchorEl)}
           onClose={this.handleClose}
           PaperProps={{
             style: {

diff --git a/docs/src/pages/demos/menus/MenuListComposition.js b/docs/src/pages/demos/menus/MenuListComposition.js
@@ -47,7 +47,7 @@ class MenuListComposition extends React.Component {
         <Manager>
           <Target>
             <Button
-              aria-owns={this.state.open ? 'menu-list' : null}
+              aria-owns={open ? 'menu-list' : null}
               aria-haspopup="true"
               onClick={this.handleClick}
             >

diff --git a/docs/src/pages/demos/menus/SimpleListMenu.js b/docs/src/pages/demos/menus/SimpleListMenu.js
@@ -22,26 +22,27 @@ const options = [
 class SimpleListMenu extends React.Component {
   state = {
     anchorEl: null,
-    open: false,
     selectedIndex: 1,
   };
 
   button = undefined;
 
   handleClickListItem = event => {
-    this.setState({ open: true, anchorEl: event.currentTarget });
+    this.setState({ anchorEl: event.currentTarget });
   };
 
   handleMenuItemClick = (event, index) => {
-    this.setState({ selectedIndex: index, open: false });
+    this.setState({ selectedIndex: index, anchorEl: null });
   };
 
   handleClose = () => {
-    this.setState({ open: false });
+    this.setState({ anchorEl: null });
   };
 
   render() {
     const { classes } = this.props;
+    const { anchorEl } = this.state;
+
     return (
       <div className={classes.root}>
         <List>
@@ -60,8 +61,8 @@ class SimpleListMenu extends React.Component {
         </List>
         <Menu
           id="lock-menu"
-          anchorEl={this.state.anchorEl}
-          open={this.state.open}
+          anchorEl={anchorEl}
+          open={Boolean(anchorEl)}
           onClose={this.handleClose}
         >
           {options.map((option, index) => (

diff --git a/docs/src/pages/demos/menus/SimpleMenu.js b/docs/src/pages/demos/menus/SimpleMenu.js
@@ -5,31 +5,32 @@ import Menu, { MenuItem } from 'material-ui/Menu';
 class SimpleMenu extends React.Component {
   state = {
     anchorEl: null,
-    open: false,
   };
 
   handleClick = event => {
-    this.setState({ open: true, anchorEl: event.currentTarget });
+    this.setState({ anchorEl: event.currentTarget });
   };
 
   handleClose = () => {
-    this.setState({ open: false });
+    this.setState({ anchorEl: null });
   };
 
   render() {
+    const { anchorEl } = this.state;
+
     return (
       <div>
         <Button
-          aria-owns={this.state.open ? 'simple-menu' : null}
+          aria-owns={anchorEl ? 'simple-menu' : null}
           aria-haspopup="true"
           onClick={this.handleClick}
         >
           Open Menu
         </Button>
         <Menu
           id="simple-menu"
-          anchorEl={this.state.anchorEl}
-          open={this.state.open}
+          anchorEl={anchorEl}
+          open={Boolean(anchorEl)}
           onClose={this.handleClose}
         >
           <MenuItem onClick={this.handleClose}>Profile</MenuItem>

diff --git a/docs/src/pages/demos/menus/menus.md b/docs/src/pages/demos/menus/menus.md
@@ -52,3 +52,9 @@ The `MenuItem` is a wrapper around `ListItem` with some additional styles.
 You can use the same list composition features with the `MenuItem` component:
 
 {{"demo": "pages/demos/menus/ListItemComposition.js"}}
+
+## Change Transition
+
+Use a different transition all together.
+
+{{"demo": "pages/demos/menus/FadeMenu.js"}}
diff --git a/pages/api/popover.md b/pages/api/popover.md
@@ -32,7 +32,7 @@ filename: /src/Popover/Popover.js
 | <span style="color: #31a148">open *</span> | bool |  | If `true`, the popover is visible. |
 | PaperProps | object |  | Properties applied to the `Paper` element. |
 | transformOrigin | shape | {  vertical: 'top',  horizontal: 'left',} | This is the point on the popover which will attach to the anchor's origin.<br>Options: vertical: [top, center, bottom, x(px)]; horizontal: [left, center, right, x(px)]. |
-| transitionClasses | shape |  | The animation classNames applied to the component as it enters or exits. This property is a direct binding to [`CSSTransition.classNames`](https://reactcommunity.org/react-transition-group/#CSSTransition-prop-classNames). |
+| transition | union:&nbsp;string&nbsp;&#124;<br>&nbsp;func<br> | Grow | Transition component. |
 | transitionDuration | union:&nbsp;number&nbsp;&#124;<br>&nbsp;{enter?: number, exit?: number}&nbsp;&#124;<br>&nbsp;{0?: undefined}<br> | 'auto' | Set to 'auto' to automatically calculate transition time based on height. |
 
 Any other properties supplied will be [spread to the root element](/guides/api#spread).

diff --git a/pages/api/snackbar.md b/pages/api/snackbar.md
@@ -29,7 +29,7 @@ filename: /src/Snackbar/Snackbar.js
 | open | bool |  | If true, `Snackbar` is open. |
 | resumeHideDuration | number |  | The number of milliseconds to wait before dismissing after user interaction. If `autoHideDuration` property isn't specified, it does nothing. If `autoHideDuration` property is specified but `resumeHideDuration` isn't, we default to `autoHideDuration / 2` ms. |
 | SnackbarContentProps | object |  | Properties applied to the `SnackbarContent` element. |
-| transition | union:&nbsp;string&nbsp;&#124;<br>&nbsp;func<br> |  | Transition component. |
+| transition | union:&nbsp;string&nbsp;&#124;<br>&nbsp;func<br> | Slide | Transition component. |
 | transitionDuration | union:&nbsp;number&nbsp;&#124;<br>&nbsp;{enter?: number, exit?: number}<br> | {  enter: duration.enteringScreen,  exit: duration.leavingScreen,} | The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object. |
 
 Any other properties supplied will be [spread to the root element](/guides/api#spread).

diff --git a/pages/demos/menus.js b/pages/demos/menus.js
@@ -41,6 +41,13 @@ module.exports = require('fs')
           raw: preval`
 module.exports = require('fs')
   .readFileSync(require.resolve('docs/src/pages/demos/menus/ListItemComposition'), 'utf8')
+`,
+        },
+        'pages/demos/menus/FadeMenu.js': {
+          js: require('docs/src/pages/demos/menus/FadeMenu').default,
+          raw: preval`
+module.exports = require('fs')
+  .readFileSync(require.resolve('docs/src/pages/demos/menus/FadeMenu'), 'utf8')
 `,
         },
       }}

diff --git a/src/Drawer/Drawer.js b/src/Drawer/Drawer.js
@@ -248,4 +248,4 @@ Drawer.defaultProps = {
   type: 'temporary', // Mobile first.
 };
 
-export default withStyles(styles, { flip: false, withTheme: true, name: 'MuiDrawer' })(Drawer);
+export default withStyles(styles, { name: 'MuiDrawer', flip: false, withTheme: true })(Drawer);
diff --git a/src/Menu/Menu.js b/src/Menu/Menu.js
@@ -36,14 +36,6 @@ class Menu extends React.Component {
     }
   }
 
-  componentDidUpdate(prevProps) {
-    if (!prevProps.open && this.props.open) {
-      // Needs to refocus as when a menu is rendered into another Modal,
-      // the first modal might change the focus to prevent any leak.
-      this.focus();
-    }
-  }
-
   getContentAnchorEl = () => {
     if (!this.menuList || !this.menuList.selectedItem) {
       return findDOMNode(this.menuList).firstChild;
@@ -66,7 +58,7 @@ class Menu extends React.Component {
     }
   };
 
-  handleEnter = (element: HTMLElement) => {
+  handleEnter = element => {
     const { theme } = this.props;
 
     const menuList = findDOMNode(this.menuList);
@@ -90,7 +82,7 @@ class Menu extends React.Component {
     }
   };
 
-  handleListKeyDown = (event: SyntheticUIEvent<>, key: string) => {
+  handleListKeyDown = (event, key) => {
     if (key === 'tab') {
       event.preventDefault();
 
@@ -106,7 +98,6 @@ class Menu extends React.Component {
       classes,
       MenuListProps,
       onEnter,
-      open,
       PaperProps = {},
       PopoverClasses,
       theme,
@@ -119,7 +110,6 @@ class Menu extends React.Component {
         getContentAnchorEl={this.getContentAnchorEl}
         classes={PopoverClasses}
         onEnter={this.handleEnter}
-        open={open}
         anchorOrigin={themeDirection === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN}
         transformOrigin={themeDirection === 'rtl' ? RTL_ORIGIN : LTR_ORIGIN}
         PaperProps={{
@@ -224,4 +214,4 @@ Menu.defaultProps = {
   transitionDuration: 'auto',
 };
 
-export default withStyles(styles, { withTheme: true, name: 'MuiMenu' })(Menu);
+export default withStyles(styles, { name: 'MuiMenu', withTheme: true })(Menu);
diff --git a/src/Popover/Popover.d.ts b/src/Popover/Popover.d.ts
@@ -24,17 +24,14 @@ export interface PopoverProps
   anchorReference?: PopoverReference;
   children?: React.ReactNode;
   elevation?: number;
-  enteredClassName?: string;
-  enteringClassName?: string;
-  exitedClassName?: string;
-  exitingClassName?: string;
   getContentAnchorEl?: Function;
   marginThreshold?: number;
   modal?: boolean;
   PaperProps?: Partial<PaperProps>;
   role?: string;
   theme?: Object;
   transformOrigin?: PopoverOrigin;
+  transition?: React.ReactType;
   transitionDuration?: TransitionDuration;
 }
 

diff --git a/src/Popover/Popover.js b/src/Popover/Popover.js
@@ -260,15 +260,22 @@ class Popover extends React.Component {
       PaperProps,
       role,
       transformOrigin,
-      transitionClasses,
+      transition: TransitionProp,
       transitionDuration,
       action,
       ...other
     } = this.props;
 
+    const transitionProps = {};
+
+    // The provided transition might not support the auto timeout value.
+    if (TransitionProp === Grow) {
+      transitionProps.timeout = transitionDuration;
+    }
+
     return (
       <Modal open={open} BackdropProps={{ invisible: true }} {...other}>
-        <Grow
+        <TransitionProp
           appear
           in={open}
           onEnter={this.handleEnter}
@@ -278,11 +285,10 @@ class Popover extends React.Component {
           onExited={onExited}
           onExiting={onExiting}
           role={role}
-          rootRef={node => {
+          ref={node => {
             this.transitionEl = node;
           }}
-          timeout={transitionDuration}
-          transitionClasses={transitionClasses}
+          {...transitionProps}
         >
           <Paper
             className={classes.paper}
@@ -293,7 +299,7 @@ class Popover extends React.Component {
             <EventListener target="window" onResize={this.handleResize} />
             {children}
           </Paper>
-        </Grow>
+        </TransitionProp>
       </Modal>
     );
   }
@@ -428,17 +434,9 @@ Popover.propTypes = {
     vertical: PropTypes.oneOfType([PropTypes.number, PropTypes.oneOf(['top', 'center', 'bottom'])]),
   }),
   /**
-   * The animation classNames applied to the component as it enters or exits.
-   * This property is a direct binding to [`CSSTransition.classNames`](https://reactcommunity.org/react-transition-group/#CSSTransition-prop-classNames).
+   * Transition component.
    */
-  transitionClasses: PropTypes.shape({
-    appear: PropTypes.string,
-    appearActive: PropTypes.string,
-    enter: PropTypes.string,
-    enterActive: PropTypes.string,
-    exit: PropTypes.string,
-    exitActive: PropTypes.string,
-  }),
+  transition: PropTypes.oneOfType([PropTypes.string, PropTypes.func]),
   /**
    * Set to 'auto' to automatically calculate transition time based on height.
    */
@@ -461,6 +459,7 @@ Popover.defaultProps = {
     vertical: 'top',
     horizontal: 'left',
   },
+  transition: Grow,
   transitionDuration: 'auto',
 };