[ListItem] Move the selected prop from MenuItem to ListItem (mui#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
marcelpanse · Oct 2, 2018 · 43bcede · 43bcede
1 parent c61d059
commit 43bcede
Show file tree

Hide file tree

Showing 16 changed files with 145 additions and 10 deletions.
diff --git a/docs/src/pages/demos/lists/SelectedListItem.js 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,15 +64,14 @@ MenuItem.propTypes = {
    */
   role: PropTypes.string,
   /**
-   * Use to apply selected styling.
+   * @ignore
    */
   selected: PropTypes.bool,
 };
 
 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -17,6 +17,9 @@ export default function SimpleListItem() {
       <ListItem dense>
         <ListItemText primary="Primary" secondary="Secondary" />
       </ListItem>
+      <ListItem selected>
+        <ListItemText primary="Primary" />
+      </ListItem>
     </div>
   );
 }