From 43bcede942f20760b59725852ad9e22bc4cb117d Mon Sep 17 00:00:00 2001
From: thequestion <loic.rouch@gmail.com>
Date: Sun, 26 Aug 2018 19:28:02 +0200
Subject: [PATCH] [ListItem] Move the selected prop from MenuItem to ListItem
 (#12602)

* Added selected property to ListItem

* Added selected example in ListItem documentation

* Modified tests to include testing of ListItem'selected property

* Removed selected property from MenuItem. Modified doc to specify selected property is from ListItem. Updated test

* Corrected example. Fixed missing style

* Modified components spec

* Updated docs API

* Prettified demo

* should be ready to be merged
---
 .../src/pages/demos/lists/SelectedListItem.js | 82 +++++++++++++++++++
 docs/src/pages/demos/lists/lists.md           |  4 +
 docs/src/pages/demos/menus/menus.md           |  3 +-
 .../material-ui/src/ListItem/ListItem.d.ts    |  4 +-
 packages/material-ui/src/ListItem/ListItem.js | 12 +++
 .../material-ui/src/ListItem/ListItem.test.js |  5 ++
 .../material-ui/src/MenuItem/MenuItem.d.ts    |  1 -
 packages/material-ui/src/MenuItem/MenuItem.js |  8 +-
 .../test/typescript/components.spec.tsx       |  2 +-
 pages/api/list-item.md                        |  2 +
 pages/api/menu-item.md                        |  1 -
 pages/demos/lists.js                          |  7 ++
 test/regressions/tests/List/IconListItem.js   |  6 ++
 .../List/PrimaryActionCheckboxListItem.js     |  9 ++
 .../List/SecondaryActionCheckboxListItem.js   |  6 ++
 test/regressions/tests/List/SimpleListItem.js |  3 +
 16 files changed, 145 insertions(+), 10 deletions(-)
 create mode 100644 docs/src/pages/demos/lists/SelectedListItem.js

diff --git a/docs/src/pages/demos/lists/SelectedListItem.js b/docs/src/pages/demos/lists/SelectedListItem.js
new file mode 100644
index 00000000000000..a0785492d96e89
--- /dev/null
+++ b/docs/src/pages/demos/lists/SelectedListItem.js
@@ -0,0 +1,82 @@
+import React from 'react';
+import PropTypes from 'prop-types';
+import { withStyles } from '@material-ui/core/styles';
+import List from '@material-ui/core/List';
+import ListItem from '@material-ui/core/ListItem';
+import ListItemIcon from '@material-ui/core/ListItemIcon';
+import ListItemText from '@material-ui/core/ListItemText';
+import Divider from '@material-ui/core/Divider';
+import InboxIcon from '@material-ui/icons/Inbox';
+import DraftsIcon from '@material-ui/icons/Drafts';
+
+const styles = theme => ({
+  root: {
+    width: '100%',
+    maxWidth: 360,
+    backgroundColor: theme.palette.background.paper,
+  },
+});
+
+class SelectedListItem extends React.Component {
+  state = {
+    selectedIndex: 1,
+  };
+
+  handleListItemClick = (event, index) => {
+    this.setState({ selectedIndex: index });
+  };
+
+  render() {
+    const { classes } = this.props;
+
+    return (
+      <div className={classes.root}>
+        <List component="nav">
+          <ListItem
+            button
+            selected={this.state.selectedIndex === 0}
+            onClick={event => this.handleListItemClick(event, 0)}
+          >
+            <ListItemIcon>
+              <InboxIcon />
+            </ListItemIcon>
+            <ListItemText primary="Inbox" />
+          </ListItem>
+          <ListItem
+            button
+            selected={this.state.selectedIndex === 1}
+            onClick={event => this.handleListItemClick(event, 1)}
+          >
+            <ListItemIcon>
+              <DraftsIcon />
+            </ListItemIcon>
+            <ListItemText primary="Drafts" />
+          </ListItem>
+        </List>
+        <Divider />
+        <List component="nav">
+          <ListItem
+            button
+            selected={this.state.selectedIndex === 2}
+            onClick={event => this.handleListItemClick(event, 2)}
+          >
+            <ListItemText primary="Trash" />
+          </ListItem>
+          <ListItem
+            button
+            selected={this.state.selectedIndex === 3}
+            onClick={event => this.handleListItemClick(event, 3)}
+          >
+            <ListItemText primary="Spam" />
+          </ListItem>
+        </List>
+      </div>
+    );
+  }
+}
+
+SelectedListItem.propTypes = {
+  classes: PropTypes.object.isRequired,
+};
+
+export default withStyles(styles)(SelectedListItem);
diff --git a/docs/src/pages/demos/lists/lists.md b/docs/src/pages/demos/lists/lists.md
index e853c14b93d871..7d81ff9356fec9 100644
--- a/docs/src/pages/demos/lists/lists.md
+++ b/docs/src/pages/demos/lists/lists.md
@@ -25,6 +25,10 @@ components: Collapse, Divider, List, ListItem, ListItemAvatar, ListItemIcon, Lis
 
 {{"demo": "pages/demos/lists/NestedList.js"}}
 
+## Selected ListItem
+
+{{"demo": "pages/demos/lists/SelectedListItem.js"}}
+
 ## Pinned Subheader List
 
 Upon scrolling, subheaders remain pinned to the top of the screen until pushed off screen by the next subheader.
diff --git a/docs/src/pages/demos/menus/menus.md b/docs/src/pages/demos/menus/menus.md
index a05a7a8310ff14..3fa8cb727fc52b 100644
--- a/docs/src/pages/demos/menus/menus.md
+++ b/docs/src/pages/demos/menus/menus.md
@@ -21,7 +21,8 @@ Choosing an option should immediately ideally commit the option and close the me
 
 ## Selected menus
 
-If used for item selection, when opened, simple menus attempt to vertically align the currently selected menu item with the anchor element. The currently selected menu item is set using the `selected` prop.
+If used for item selection, when opened, simple menus attempt to vertically align the currently selected menu item with the anchor element.
+The currently selected menu item is set using the `selected` property (from [ListItem](/api/list-item)).
 
 {{"demo": "pages/demos/menus/SimpleListMenu.js"}}
 
diff --git a/packages/material-ui/src/ListItem/ListItem.d.ts b/packages/material-ui/src/ListItem/ListItem.d.ts
index 2b7003c07995a1..a100878ce9c27b 100644
--- a/packages/material-ui/src/ListItem/ListItem.d.ts
+++ b/packages/material-ui/src/ListItem/ListItem.d.ts
@@ -17,6 +17,7 @@ export interface ListItemProps
   disableGutters?: boolean;
   divider?: boolean;
   focusVisibleClassName?: string;
+  selected?: boolean;
 }
 
 export type ListItemClassKey =
@@ -29,7 +30,8 @@ export type ListItemClassKey =
   | 'divider'
   | 'gutters'
   | 'button'
-  | 'secondaryAction';
+  | 'secondaryAction'
+  | 'selected';
 
 declare const ListItem: React.ComponentType<ListItemProps>;
 
diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js
index 432fcabdbfe727..dae987977026d6 100644
--- a/packages/material-ui/src/ListItem/ListItem.js
+++ b/packages/material-ui/src/ListItem/ListItem.js
@@ -18,6 +18,9 @@ export const styles = theme => ({
     textAlign: 'left',
     paddingTop: 12,
     paddingBottom: 12,
+    '&$selected': {
+      backgroundColor: theme.palette.action.selected,
+    },
   },
   /* Styles applied to the `container` element if `children` includes `ListItemSecondaryAction`. */
   container: {
@@ -66,6 +69,8 @@ export const styles = theme => ({
     // is absolutely positionned.
     paddingRight: 32,
   },
+  /* Styles applied to the root element if `selected={true}`. */
+  selected: {},
 });
 
 class ListItem extends React.Component {
@@ -89,6 +94,7 @@ class ListItem extends React.Component {
       disableGutters,
       divider,
       focusVisibleClassName,
+      selected,
       ...other
     } = this.props;
 
@@ -108,6 +114,7 @@ class ListItem extends React.Component {
         [classes.disabled]: disabled,
         [classes.button]: button,
         [classes.secondaryAction]: hasSecondaryAction,
+        [classes.selected]: selected,
       },
       classNameProp,
     );
@@ -205,6 +212,10 @@ ListItem.propTypes = {
    * @ignore
    */
   focusVisibleClassName: PropTypes.string,
+  /**
+   * Use to apply selected styling.
+   */
+  selected: PropTypes.bool,
 };
 
 ListItem.defaultProps = {
@@ -214,6 +225,7 @@ ListItem.defaultProps = {
   disabled: false,
   disableGutters: false,
   divider: false,
+  selected: false,
 };
 
 ListItem.contextTypes = {
diff --git a/packages/material-ui/src/ListItem/ListItem.test.js b/packages/material-ui/src/ListItem/ListItem.test.js
index 4e1779e3c527b0..fad94152a1e614 100644
--- a/packages/material-ui/src/ListItem/ListItem.test.js
+++ b/packages/material-ui/src/ListItem/ListItem.test.js
@@ -34,6 +34,11 @@ describe('<ListItem />', () => {
     assert.strictEqual(wrapper.hasClass(classes.gutters), true, 'should have the gutters class');
   });
 
+  it('should render with the selected class', () => {
+    const wrapper = shallow(<ListItem selected />);
+    assert.strictEqual(wrapper.hasClass(classes.selected), true, 'should have the selected class');
+  });
+
   it('should disable the gutters', () => {
     const wrapper = shallow(<ListItem disableGutters />);
     assert.strictEqual(wrapper.hasClass(classes.root), true);
diff --git a/packages/material-ui/src/MenuItem/MenuItem.d.ts b/packages/material-ui/src/MenuItem/MenuItem.d.ts
index e97ce7c9c96201..90130fc30230f7 100644
--- a/packages/material-ui/src/MenuItem/MenuItem.d.ts
+++ b/packages/material-ui/src/MenuItem/MenuItem.d.ts
@@ -5,7 +5,6 @@ import { ListItemProps } from '../ListItem';
 export interface MenuItemProps extends StandardProps<ListItemProps, MenuItemClassKey> {
   component?: React.ReactType<MenuItemProps>;
   role?: string;
-  selected?: boolean;
 }
 
 export type MenuItemClassKey = 'root' | 'selected';
diff --git a/packages/material-ui/src/MenuItem/MenuItem.js b/packages/material-ui/src/MenuItem/MenuItem.js
index 3aeb115f518d46..18d84276a6fc29 100644
--- a/packages/material-ui/src/MenuItem/MenuItem.js
+++ b/packages/material-ui/src/MenuItem/MenuItem.js
@@ -18,9 +18,7 @@ export const styles = theme => ({
     whiteSpace: 'nowrap',
     paddingLeft: 16,
     paddingRight: 16,
-    '&$selected': {
-      backgroundColor: theme.palette.action.selected,
-    },
+    '&$selected': {},
   },
   /* Styles applied to the root element if `selected={true}`. */
   selected: {},
@@ -34,6 +32,7 @@ function MenuItem(props) {
       button
       role={role}
       tabIndex={-1}
+      selected={selected}
       className={classNames(classes.root, { [classes.selected]: selected }, className)}
       component={component}
       {...other}
@@ -65,7 +64,7 @@ MenuItem.propTypes = {
    */
   role: PropTypes.string,
   /**
-   * Use to apply selected styling.
+   * @ignore
    */
   selected: PropTypes.bool,
 };
@@ -73,7 +72,6 @@ MenuItem.propTypes = {
 MenuItem.defaultProps = {
   component: 'li',
   role: 'menuitem',
-  selected: false,
 };
 
 export default withStyles(styles, { name: 'MuiMenuItem' })(MenuItem);
diff --git a/packages/material-ui/test/typescript/components.spec.tsx b/packages/material-ui/test/typescript/components.spec.tsx
index cbde99fcbb153d..c55169eb1c598a 100644
--- a/packages/material-ui/test/typescript/components.spec.tsx
+++ b/packages/material-ui/test/typescript/components.spec.tsx
@@ -425,7 +425,7 @@ const GridListTest = () => (
 const ListTest = () => (
   <List>
     {[0, 1, 2, 3].map(value => (
-      <ListItem dense button key={value} onClick={log}>
+      <ListItem dense button selected={false} key={value} onClick={log}>
         <Checkbox checked={true} tabIndex={-1} disableRipple />
         <ListItemText primary={`Line item ${value + 1}`} />
         <ListItemSecondaryAction>
diff --git a/pages/api/list-item.md b/pages/api/list-item.md
index a5030c371aa2bd..1d298b52224c9d 100644
--- a/pages/api/list-item.md
+++ b/pages/api/list-item.md
@@ -25,6 +25,7 @@ title: ListItem API
 | <span class="prop-name">disabled</span> | <span class="prop-type">bool | <span class="prop-default">false</span> | If `true`, the list item will be disabled. |
 | <span class="prop-name">disableGutters</span> | <span class="prop-type">bool | <span class="prop-default">false</span> | If `true`, the left and right padding is removed. |
 | <span class="prop-name">divider</span> | <span class="prop-type">bool | <span class="prop-default">false</span> | If `true`, a 1px light border is added to the bottom of the list item. |
+| <span class="prop-name">selected</span> | <span class="prop-type">bool | <span class="prop-default">false</span> | Use to apply selected styling. |
 
 Any other properties supplied will be spread to the root element (native element).
 
@@ -46,6 +47,7 @@ This property accepts the following keys:
 | <span class="prop-name">gutters</span> | Styles applied to the inner `component` element if `disableGutters={false}`.
 | <span class="prop-name">button</span> | Styles applied to the inner `component` element if `button={true}`.
 | <span class="prop-name">secondaryAction</span> | Styles applied to the `component` element if `children` includes `ListItemSecondaryAction`.
+| <span class="prop-name">selected</span> | Styles applied to the root element if `selected={true}`.
 
 Have a look at [overriding with classes](/customization/overrides#overriding-with-classes) section
 and the [implementation of the component](https://github.com/mui-org/material-ui/tree/master/packages/material-ui/src/ListItem/ListItem.js)
diff --git a/pages/api/menu-item.md b/pages/api/menu-item.md
index fd511824ed3fc9..263002ca844eb0 100644
--- a/pages/api/menu-item.md
+++ b/pages/api/menu-item.md
@@ -18,7 +18,6 @@ title: MenuItem API
 | <span class="prop-name">children</span> | <span class="prop-type">node |   | Menu item contents. |
 | <span class="prop-name">classes</span> | <span class="prop-type">object |   | Override or extend the styles applied to the component. See [CSS API](#css-api) below for more details. |
 | <span class="prop-name">component</span> | <span class="prop-type">union:&nbsp;string&nbsp;&#124;<br>&nbsp;func&nbsp;&#124;<br>&nbsp;object<br> | <span class="prop-default">'li'</span> | The component used for the root node. Either a string to use a DOM element or a component. |
-| <span class="prop-name">selected</span> | <span class="prop-type">bool | <span class="prop-default">false</span> | Use to apply selected styling. |
 
 Any other properties supplied will be spread to the root element ([ListItem](/api/list-item)).
 
diff --git a/pages/demos/lists.js b/pages/demos/lists.js
index 4f7eb49883a852..9aaadf36fd4758 100644
--- a/pages/demos/lists.js
+++ b/pages/demos/lists.js
@@ -34,6 +34,13 @@ module.exports = require('fs')
           raw: preval`
 module.exports = require('fs')
   .readFileSync(require.resolve('docs/src/pages/demos/lists/NestedList'), 'utf8')
+`,
+        },
+        'pages/demos/lists/SelectedListItem.js': {
+          js: require('docs/src/pages/demos/lists/SelectedListItem').default,
+          raw: preval`
+module.exports = require('fs')
+  .readFileSync(require.resolve('docs/src/pages/demos/lists/SelectedListItem'), 'utf8')
 `,
         },
         'pages/demos/lists/PinnedSubheaderList.js': {
diff --git a/test/regressions/tests/List/IconListItem.js b/test/regressions/tests/List/IconListItem.js
index 05b9ea54ac3244..41cd0e96b1dc56 100644
--- a/test/regressions/tests/List/IconListItem.js
+++ b/test/regressions/tests/List/IconListItem.js
@@ -25,6 +25,12 @@ export default function IconListItem() {
       <ListItem dense>
         <ListItemText inset primary="Inset" secondary="Secondary" />
       </ListItem>
+      <ListItem selected>
+        <ListItemIcon>
+          <Icon>phone</Icon>
+        </ListItemIcon>
+        <ListItemText primary="Icon" />
+      </ListItem>
     </div>
   );
 }
diff --git a/test/regressions/tests/List/PrimaryActionCheckboxListItem.js b/test/regressions/tests/List/PrimaryActionCheckboxListItem.js
index 2a16bef99b1dff..134f63bd0fb94a 100644
--- a/test/regressions/tests/List/PrimaryActionCheckboxListItem.js
+++ b/test/regressions/tests/List/PrimaryActionCheckboxListItem.js
@@ -29,6 +29,15 @@ export default function PrimaryActionCheckboxListItem() {
             </IconButton>
           </ListItemSecondaryAction>
         </ListItem>
+        <ListItem button selected>
+          <Checkbox tabIndex={-1} disableRipple />
+          <ListItemText primary="Primary" />
+          <ListItemSecondaryAction>
+            <IconButton>
+              <Icon>comment</Icon>
+            </IconButton>
+          </ListItemSecondaryAction>
+        </ListItem>
       </List>
     </div>
   );
diff --git a/test/regressions/tests/List/SecondaryActionCheckboxListItem.js b/test/regressions/tests/List/SecondaryActionCheckboxListItem.js
index 7a20e5ad5acf34..b9099b29d2bdf2 100644
--- a/test/regressions/tests/List/SecondaryActionCheckboxListItem.js
+++ b/test/regressions/tests/List/SecondaryActionCheckboxListItem.js
@@ -21,6 +21,12 @@ export default function SecondaryActionCheckboxListItem() {
             <Checkbox />
           </ListItemSecondaryAction>
         </ListItem>
+        <ListItem button selected>
+          <ListItemText primary="Primary" />
+          <ListItemSecondaryAction>
+            <Checkbox />
+          </ListItemSecondaryAction>
+        </ListItem>
       </List>
     </div>
   );
diff --git a/test/regressions/tests/List/SimpleListItem.js b/test/regressions/tests/List/SimpleListItem.js
index dc966eedcaabd0..6e1e20fcfd6dfc 100644
--- a/test/regressions/tests/List/SimpleListItem.js
+++ b/test/regressions/tests/List/SimpleListItem.js
@@ -17,6 +17,9 @@ export default function SimpleListItem() {
       <ListItem dense>
         <ListItemText primary="Primary" secondary="Secondary" />
       </ListItem>
+      <ListItem selected>
+        <ListItemText primary="Primary" />
+      </ListItem>
     </div>
   );
 }