mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.87k stars 32.26k forks source link

Implement DrawerMenuItem #15107

Open raymondsze opened 5 years ago

raymondsze commented 5 years ago

Drawer MenuItem: https://material-components.github.io/material-components-web-catalog/#/component/drawer Demo: https://material-components.github.io/material-components-web-catalog/#/component/drawer

  1. The Drawer MenuItem could capture the keyboard event.
  2. The Drawer MenuItem show the primary color when selected.
  3. The Drawer MenuItem have the rounded shape.
  4. The Drawer MenuItem Text is highlighted with darker primary color.

Shape: https://material.io/design/shape/about-shape.html#shape-customization-tool

  1. Shape could apply to small, medium and large components.
  2. This could be easily done by providing HOC or something else to change the root classes radius, but this is not easy for TextField, especially Outlined TextField and Filled TextFIeld.

Benchmark

oliviertassinari commented 5 years ago

I agree, I think that it's something we should implement and use in our documentation website.

Google has customized the style, it would make a great customization example:

Capture d’écran 2019-04-11 à 15 20 14


Capture d’écran 2019-04-11 à 15 20 01

raymondsze commented 5 years ago

Do you think it should be an enhancenent of MenuItem/ListItem or a seperate implementation?

eps1lon commented 5 years ago

I guess this is asking for the implementation of https://material.io/design/shape/about-shape.html?

oliviertassinari commented 5 years ago

We have a couple of different efforts and issue on the List component, I think that it would be great to align them, cc @mbrookes:

I have made a quick POC to get an idea on the options we have:

Capture d’écran 2019-04-14 à 19 45 59

Git diff ```diff --- a/docs/src/pages/demos/lists/SelectedListItem.js +++ b/docs/src/pages/demos/lists/SelectedListItem.js @@ -26,7 +26,7 @@ function SelectedListItem() { return (
- + ({ dense }), [dense]); + const context = React.useMemo(() => ({ dense, variant }), [dense, variant]); return ( @@ -94,10 +95,12 @@ List.propTypes = { * The content of the subheader, normally `ListSubheader`. */ subheader: PropTypes.node, + variant: PropTypes.oneOf(['nav', 'ul']), }; List.defaultProps = { component: 'ul', + variant: 'ul', dense: false, disablePadding: false, }; diff --git a/packages/material-ui/src/ListItem/ListItem.js b/packages/material-ui/src/ListItem/ListItem.js index daa06ec91..2be1632d3 100644 --- a/packages/material-ui/src/ListItem/ListItem.js +++ b/packages/material-ui/src/ListItem/ListItem.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import clsx from 'clsx'; import { chainPropTypes } from '@material-ui/utils'; import withStyles from '../styles/withStyles'; +import { fade } from '../styles/colorManipulator'; import ButtonBase from '../ButtonBase'; import { isMuiElement } from '../utils/reactHelpers'; import ListContext from '../List/ListContext'; @@ -15,7 +16,7 @@ export const styles = theme => ({ alignItems: 'center', position: 'relative', textDecoration: 'none', - width: '100%', + //width: '100%', boxSizing: 'border-box', textAlign: 'left', paddingTop: 8, @@ -24,6 +25,15 @@ export const styles = theme => ({ backgroundColor: theme.palette.action.selected, }, }, + variantNav: { + margin: 8, + overflow: 'hidden', + '&$selected, &$selected:hover': { + borderRadius: 4, + color: theme.palette.primary.dark, + backgroundColor: fade(theme.palette.primary.main, theme.palette.action.hoverOpacity), + }, + }, /* Styles applied to the `container` element if `children` includes `ListItemSecondaryAction`. */ container: { position: 'relative', @@ -106,12 +116,15 @@ const ListItem = React.forwardRef(function ListItem(props, ref) { const childContext = { dense: dense || context.dense || false, alignItems, + selected, }; const children = React.Children.toArray(childrenProp); const hasSecondaryAction = children.length && isMuiElement(children[children.length - 1], ['ListItemSecondaryAction']); + console.log('context', context) + const componentProps = { className: clsx( classes.root, @@ -120,6 +133,7 @@ const ListItem = React.forwardRef(function ListItem(props, ref) { [classes.gutters]: !disableGutters, [classes.divider]: divider, [classes.disabled]: disabled, + [classes.variantNav]: context.variant === 'nav', [classes.button]: button, [classes.alignItemsFlexStart]: alignItems === 'flex-start', [classes.secondaryAction]: hasSecondaryAction, diff --git a/packages/material-ui/src/ListItemIcon/ListItemIcon.js b/packages/material-ui/src/ListItemIcon/ListItemIcon.js index 89e61d727..41eed6824 100644 --- a/packages/material-ui/src/ListItemIcon/ListItemIcon.js +++ b/packages/material-ui/src/ListItemIcon/ListItemIcon.js @@ -2,6 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import clsx from 'clsx'; import withStyles from '../styles/withStyles'; +import ListContext from '../List/ListContext'; export const styles = theme => ({ /* Styles applied to the root element. */ @@ -11,6 +12,9 @@ export const styles = theme => ({ flexShrink: 0, display: 'inline-flex', }, + selected: { + color: 'inherit', + }, }); /** @@ -18,8 +22,15 @@ export const styles = theme => ({ */ const ListItemIcon = React.forwardRef(function ListItemIcon(props, ref) { const { classes, className, ...other } = props; + const context = React.useContext(ListContext); - return
; + return ( +
+ ); }); ListItemIcon.propTypes = { ```

What about:

We follow the MenuItem pattern, we introduce a new DrawerListItem component, it's a drop in replacement for the ListItem component. It has a custom handling of the selected property as shown in the previous git diff. If somebody wants its drawer list to be keyboard navigable, it replaces the List component with the MenuList component?

mbrookes commented 5 years ago

Alternative strategy:

Standardise on a single List and ListItem component. Add variants for Drawer and Menu. Add a disableAccessibility (😜) prop to disable Menu style keyboard navigation.

We would need to wait for 4.1 for deprecation of MenuList and MenuListItem, however it could be implemented now, with MenuList and MenuListItem becoming a thin wrapper that sets the List variant, with deprecation added later. (Or, the wrappers could become permanent for API stability...)

raymondsze commented 5 years ago

variant: "nav" may not be suitable? The MWC has this kind of style for Menu of SelectField as well. For that case, the parent component is ul but with primary style. Although Material-UI does not necessary to follow MWC.

Screenshot 2019-06-27 at 12 24 37 PM
oliviertassinari commented 4 years ago

We explore this problem further in #22780 for the requirements of the documentation. This will be a great starting point to work on this issue. There is a decision to take between bundling the style inside ListItem (1), creating a new component that extends ListItem (2), or creating a new component without ListItem (3).

With hindsight, I think that we should have used option 3 for the Menu: it helps customization (fewer layers of abstraction, style often different) and performance (fewer components, especially relevant with lists).