Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Grid] Make all the props responsive #6140

Closed
jgoux opened this issue Feb 14, 2017 · 16 comments · Fixed by #26590
Closed

[Grid] Make all the props responsive #6140

jgoux opened this issue Feb 14, 2017 · 16 comments · Fixed by #26590
Labels
component: Grid The React component. good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request priority: important This change can make a difference
Milestone

Comments

@jgoux
Copy link
Contributor

jgoux commented Feb 14, 2017

Hello,

we-need-to-go-deeper

Description

Add extra flex properties on Layout

Currently, not all flex properties are supported.
I think we could benefit to have them so we don't have to touch the CSS every time when dealing with layout only components.

Missing properties : grow, shrink, basis, flow, order

Make the breakpoints properties (xs, sm, lg...) accept an object of subproperties

With the breakpoints properties, we have the control over the width of our components based on the screen resolution.
It would be totally awesome to have control over all the flex properties based on the resolution.
To do that, we could make the breakpoints properties accept an object of subproperties like this :

<Grid spacing={{ sm: 2, md: 4 }} alignItems={['center', 'flex-end']}>
{/*...*/}
</Grid>

Images & references

https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Flexible_Box_Layout

@oliviertassinari oliviertassinari added component: Grid The React component. next labels Feb 14, 2017
@esseswann
Copy link
Contributor

I made a component of similar behavior, we could use either the package itself underneath Layout or just implement something alike.

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 15, 2017

@jgoux thanks for opening that issue. I'm gonna try to answer your question regarding the Layout API choices.

  1. Why not using a fraction? For instance grid-styled:
<Grid
  xs={1/2}
  sm={1/3}
  md={1/4}
  lg={1/6}
/>

The API allows to simply know how much space is available and how much columns we are going to have. However, UI/UX designers often use a 12 columns layout model. At least the specification, bootstrap and my coworkers do. They give us column constraints, not percentages.

  1. Why not grouping the breakpoints by property? For instance styled-components-grid:
<Grid>
  <Grid.Unit width={1/6}>Awesome!</Grid.Unit>
  <Grid.Unit width={1/3}>Amazing!</Grid.Unit>
  <Grid.Unit width={{mobile: 1, tablet: 1/2, desktop: 1/4}}>Out of this world!</Grid.Unit>
</Grid>

I don't think that it's the best API choice as we most of the time ask ourselves. How is the component is going to render given a screen size? Not the other way around. What's that property is going to be?

  1. Why having a single <Layout /> component and not a <Container /> and <Item /> one?
    Simply because a layout can be a container and an item at the same time. We have a stress test in the visual regression for that. That's an advanced mode.

Add extra flex properties on Layout

  1. I 💯 agree with that. The challenge in around not making the injected CSS into the page grows too much. I'm not sure where is the limit, so we could start adding all of them. Alternatives are using dynamic inline style (but needs to be prefixed) or dynamic CSS injection.

It would be totally awesome to have control over all the flex properties based on the resolution.

  1. I agree with that point but I not sold on the nesting. What's wrong with?
<Grid
  xs={12}
  xsGrow={1}
  xsDirection={column}
  lg={6}
>
  {/*...*/}
</Grid>

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Mar 15, 2017
@oliviertassinari oliviertassinari added the new feature New feature or request label Jul 4, 2017
@oliviertassinari oliviertassinari removed v1.x good first issue Great for first contributions. Enable to learn the contribution process. labels Dec 7, 2017
@oliviertassinari oliviertassinari changed the title [Layout] More control over flex attributes and breakpoints [Grid] More control over flex attributes and breakpoints Jan 16, 2018
@schettino
Copy link

@oliviertassinari Going nested is more expressive than handling millions of props for a single purpose. For the Grid.Unit, I don't see much of this design here, why mix up the things? And instead of width, maybe basis (flex-basis), which makes more sense in this context.

@oliviertassinari oliviertassinari self-assigned this Dec 1, 2018
@oliviertassinari oliviertassinari removed their assignment Jan 17, 2019
@oliviertassinari
Copy link
Member

@schettino The system flexbox https://material-ui.com/system/flexbox/ inverses the responsive API. It's property driven:

<Box flexGrow={{ sm: 1, md: 0, lg: 'initial' }} />

@oliviertassinari oliviertassinari added the priority: important This change can make a difference label Mar 13, 2019
@oliviertassinari oliviertassinari changed the title [Grid] More control over flex attributes and breakpoints [Grid] Make all the props responsive Dec 1, 2019
@oliviertassinari oliviertassinari removed the priority: important This change can make a difference label Dec 1, 2019
@msimulcik
Copy link

To overcome this issue I wrote a hook that is internally using useMediaQuery:

import { useTheme } from '@material-ui/core/styles';
import useMediaQuery from '@material-ui/core/useMediaQuery';
import { findLast } from 'ramda';

const useResponsive = () => {
  const theme = useTheme();

  const matches = {
    xs: useMediaQuery(theme.breakpoints.up('xs')),
    sm: useMediaQuery(theme.breakpoints.up('sm')),
    md: useMediaQuery(theme.breakpoints.up('md')),
    lg: useMediaQuery(theme.breakpoints.up('lg')),
    xl: useMediaQuery(theme.breakpoints.up('xl')),
  };

  return function<P>(responsiveValues: { [breakpoint: string]: P }) {
    const match = findLast(
      (breakpoint) => matches[breakpoint] && responsiveValues[breakpoint] != null,
      theme.breakpoints.keys,
    );

    return match && responsiveValues[match];
  };
};

export default useResponsive;

Its usage is very similar to the responsive object props from @material-ui/system:

const Foo = () => {
  const r = useResponsive();
  
  return (
    <Grid container spacing={r({ xs: 2, md: 3 })} direction="column">
      <Grid item>Item 1</Grid>
      <Grid item>Item 2</Grid>
    </Grid>
  );
}

@LuudJanssen
Copy link

Thanks @msimulcik!

I updated your script to include typed breakpoint strings, might someone be interested in that:

import { useTheme } from "@material-ui/core/styles"
import useMediaQuery from "@material-ui/core/useMediaQuery"
import { findLast } from "ramda"

type Breakpoint = "xs" | "sm" | "md" | "lg" | "xl"

type ResponsiveValues<P> = {
  [key in Breakpoint]?: P
}

const useResponsive = () => {
  const theme = useTheme()

  const matches = {
    xs: useMediaQuery(theme.breakpoints.up("xs")),
    sm: useMediaQuery(theme.breakpoints.up("sm")),
    md: useMediaQuery(theme.breakpoints.up("md")),
    lg: useMediaQuery(theme.breakpoints.up("lg")),
    xl: useMediaQuery(theme.breakpoints.up("xl")),
  }

  return function <P>(responsiveValues: ResponsiveValues<P>) {
    const match = findLast(
      (breakpoint) =>
        matches[breakpoint] && responsiveValues[breakpoint] != null,
      theme.breakpoints.keys
    )

    return match && responsiveValues[match]
  }
}

export default useResponsive

@satazor
Copy link

satazor commented Sep 10, 2020

For anyone interested, here's an implementation where all props are responsive. It doesn't use useMediaQuery, so it's safe for server-side rendering.

// This is a replacement of the Material-UI Grid component (v5), that supports breakpoints for all props.
// See: https://github.com/mui-org/material-ui/issues/6140

import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { withStyles, requirePropFactory } from '@material-ui/core';
import { breakpoints } from '../../../themes';

const SPACINGS = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
const GRID_SIZES = ['auto', true, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];

const generateGrid = (theme, breakpoint) => {
    const styles = {};

    GRID_SIZES.forEach((size) => {
        const key = `grid-${breakpoint}-${size}`;

        if (size === true) {
            // For the auto layouting.
            styles[key] = {
                flexBasis: 0,
                flexGrow: 1,
                maxWidth: '100%',
            };

            return;
        }

        if (size === 'auto') {
            styles[key] = {
                flexBasis: 'auto',
                flexGrow: 0,
                maxWidth: 'none',
            };

            return;
        }

        // Keep 7 significant numbers.
        const width = `${Math.round((size / 12) * 10e7) / 10e5}%`;

        // Close to the bootstrap implementation:
        // https://github.com/twbs/bootstrap/blob/8fccaa2439e97ec72a4b7dc42ccc1f649790adb0/scss/mixins/_grid.scss#L41
        styles[key] = {
            flexBasis: width,
            flexGrow: 0,
            maxWidth: width,
        };
    });

    return styles;
};

const generateGutter = (theme, breakpoint) => {
    const styles = {};

    SPACINGS.forEach((spacing) => {
        const themeSpacing = theme.spacing(spacing);

        styles[`spacing-${breakpoint}-${spacing}`] = {
            margin: `-${getOffset(themeSpacing, 2)}`,
            width: `calc(100% + ${getOffset(themeSpacing)})`,
            '& > $item': {
                padding: getOffset(themeSpacing, 2),
            },
        };
    });

    return styles;
};

const getOffset = (val, div = 1) => {
    const parse = parseFloat(val);

    return `${parse / div}${String(val).replace(String(parse), '') || 'px'}`;
};

// Default CSS values
// flex: '0 1 auto',
// flexDirection: 'row',
// alignItems: 'flex-start',
// flexWrap: 'nowrap',
// justifyContent: 'flex-start',
export const styles = (theme) => ({
    // Styles applied to the root element.
    root: {},
    // Styles applied to the root element if `container={true}`.
    container: {
        boxSizing: 'border-box',
        display: 'flex',
        width: '100%',
    },
    // Styles applied to the root element if `item={true}`.
    item: {
        boxSizing: 'border-box',
        margin: '0', // For instance, it's useful when used with a `figure` element.
    },

    ...theme.breakpoints.keys.reduce((globalStyles, breakpoint) => {
        const styles = {
            // Styles applied to the root element if `zeroMinWidth={true}`.
            [`zero-min-width-${breakpoint}`]: {
                minWidth: 0,
            },
            // Styles applied to the root element if `direction="row"`.
            [`direction-${breakpoint}-row`]: {
                flexDirection: 'row',
            },
            // Styles applied to the root element if `direction="column"`.
            [`direction-${breakpoint}-column`]: {
                flexDirection: 'column',
            },
            // Styles applied to the root element if `direction="column-reverse"`.
            [`direction-${breakpoint}-column-reverse`]: {
                flexDirection: 'column-reverse',
            },
            // Styles applied to the root element if `direction="row-reverse"`.
            [`direction-${breakpoint}-row-reverse`]: {
                flexDirection: 'row-reverse',
            },
            // Styles applied to the root element if `wrap="wrap"`.
            [`wrap-${breakpoint}-wrap`]: {
                flexWrap: 'wrap',
            },
            // Styles applied to the root element if `wrap="nowrap"`.
            [`wrap-${breakpoint}-nowrap`]: {
                flexWrap: 'nowrap',
            },
            // Styles applied to the root element if `wrap="reverse"`.
            [`wrap-${breakpoint}-wrap-reverse`]: {
                flexWrap: 'wrap-reverse',
            },
            // Styles applied to the root element if `alignItems="stretch"`.
            [`align-items-${breakpoint}-stretch`]: {
                alignItems: 'stretch',
            },
            // Styles applied to the root element if `alignItems="center"`.
            [`align-items-${breakpoint}-center`]: {
                alignItems: 'center',
            },
            // Styles applied to the root element if `alignItems="flex-start"`.
            [`align-items-${breakpoint}-flex-start`]: {
                alignItems: 'flex-start',
            },
            // Styles applied to the root element if `alignItems="flex-end"`.
            [`align-items-${breakpoint}-flex-end`]: {
                alignItems: 'flex-end',
            },
            // Styles applied to the root element if `alignItems="baseline"`.
            [`align-items-${breakpoint}-baseline`]: {
                alignItems: 'baseline',
            },
            // Styles applied to the root element if `alignContent="stretch"`.
            [`align-content-${breakpoint}-stretch`]: {
                alignContent: 'stretch',
            },
            // Styles applied to the root element if `alignContent="center"`.
            [`align-content-${breakpoint}-center`]: {
                alignContent: 'center',
            },
            // Styles applied to the root element if `alignContent="flex-start"`.
            [`align-content-${breakpoint}-flex-start`]: {
                alignContent: 'flex-start',
            },
            // Styles applied to the root element if `alignContent="flex-end"`.
            [`align-content-${breakpoint}-flex-end`]: {
                alignContent: 'flex-end',
            },
            // Styles applied to the root element if `alignContent="space-between"`.
            [`align-content-${breakpoint}-space-between`]: {
                alignContent: 'space-between',
            },
            // Styles applied to the root element if `alignContent="space-around"`.
            [`align-content-${breakpoint}-space-around`]: {
                alignContent: 'space-around',
            },
            // Styles applied to the root element if `justifyContent="center"`.
            [`justify-content-${breakpoint}-center`]: {
                justifyContent: 'center',
            },
            // Styles applied to the root element if `justifyContent="flex-start"`.
            [`justify-content-${breakpoint}-flex-start`]: {
                justifyContent: 'flex-start',
            },
            // Styles applied to the root element if `justifyContent="flex-end"`.
            [`justify-content-${breakpoint}-flex-end`]: {
                justifyContent: 'flex-end',
            },
            // Styles applied to the root element if `justifyContent="space-between"`.
            [`justify-content-${breakpoint}-space-between`]: {
                justifyContent: 'space-between',
            },
            // Styles applied to the root element if `justifyContent="space-around"`.
            [`justify-content-${breakpoint}-space-around`]: {
                justifyContent: 'space-around',
            },
            // Styles applied to the root element if `justifyContent="space-evenly"`.
            [`justify-content-${breakpoint}-space-evenly`]: {
                justifyContent: 'space-evenly',
            },
            ...generateGrid(theme, breakpoint),
            ...generateGutter(theme, breakpoint),
        };

        // No need for a media query for the first size.
        if (breakpoint === 'xs') {
            Object.assign(globalStyles, styles);
        } else {
            globalStyles[theme.breakpoints.up(breakpoint)] = styles;
        }

        return globalStyles;
    }, {}),
});

const generateResponsiveClasses = (name, value, classes) => {
    if (typeof value === 'object') {
        return Object.entries(value).reduce((accumulator, [breakpoint, value]) => {
            accumulator[classes[`${name}-${breakpoint}-${String(value)}`]] = true;

            return accumulator;
        }, {});
    }

    return {
        [classes[`${name}-xs-${String(value)}`]]: true,
    };
};

export const Grid = React.forwardRef((props, ref) => {
    const {
        alignContent,
        alignItems,
        classes,
        className,
        component: Component,
        container,
        direction,
        item,
        justifyContent,
        spacing,
        wrap,
        xs,
        sm,
        md,
        lg,
        xl,
        zeroMinWidth,
        ...rest
    } = props;

    const finalClassName = clsx(
        classes.root,
        {
            [classes.container]: container,
            [classes.item]: item,
            ...generateResponsiveClasses('zeroMinWidth', zeroMinWidth, classes),
            ...generateResponsiveClasses('spacing', spacing, classes),
            ...generateResponsiveClasses('direction', direction, classes),
            ...generateResponsiveClasses('wrap', wrap, classes),
            ...generateResponsiveClasses('align-items', alignItems, classes),
            ...generateResponsiveClasses('align-content', alignContent, classes),
            ...generateResponsiveClasses('justify-content', justifyContent, classes),
            [classes[`grid-xs-${String(xs)}`]]: xs !== false,
            [classes[`grid-sm-${String(sm)}`]]: sm !== false,
            [classes[`grid-md-${String(md)}`]]: md !== false,
            [classes[`grid-lg-${String(lg)}`]]: lg !== false,
            [classes[`grid-xl-${String(xl)}`]]: xl !== false,
        },
        className,
    );

    return <Component className={ finalClassName } ref={ ref } { ...rest } />;
});

Grid.defaultProps = {
    alignContent: 'stretch',
    alignItems: 'stretch',
    component: 'div',
    container: false,
    direction: 'row',
    item: false,
    justifyContent: 'flex-start',
    spacing: 0,
    wrap: 'wrap',
    xs: false,
    sm: false,
    md: false,
    lg: false,
    xl: false,
    zeroMinWidth: false,
};

Grid.propTypes = {
    /**
     * Defines the `align-content` style property.
     * It can only be used on a type `container` component.
     */
    alignContent: PropTypes.oneOf([
        'center',
        'flex-end',
        'flex-start',
        'space-around',
        'space-between',
        'stretch',
    ]),
    /**
     * Defines the `align-items` style property.
     * It can only be used on a type `container` component.
     */
    alignItems: PropTypes.oneOf([
        'baseline',
        'center',
        'flex-end',
        'flex-start',
        'stretch',
    ]),
    /**
     * The content of the component.
     */
    children: PropTypes.node,
    /**
     * Override or extend the styles applied to the component.
     */
    classes: PropTypes.object,
    /**
     * @ignore
     */
    className: PropTypes.string,
    /**
     * The component used for the root node.
     * Either a string to use a HTML element or a component.
     */
    component: PropTypes.elementType,
    /**
     * If `true`, the component will have the flex *container* behavior.
     * You should be wrapping *items* with a *container*.
     */
    container: PropTypes.bool,
    /**
     * Defines the `flex-direction` style property.
     * It can only be used on a type `container` component.
     */
    direction: PropTypes.oneOf([
        'column-reverse',
        'column',
        'row-reverse',
        'row',
    ]),
    /**
     * If `true`, the component will have the flex *item* behavior.
     * You should be wrapping *items* with a *container*.
     */
    item: PropTypes.bool,
    /**
     * Defines the `justify-content` style property.
     * It can only be used on a type `container` component.
     */
    justifyContent: PropTypes.oneOf([
        'center',
        'flex-end',
        'flex-start',
        'space-around',
        'space-between',
        'space-evenly',
    ]),
    /**
     * Defines the space between the type `item` component.
     * It can only be used on a type `container` component.
     */
    spacing: PropTypes.oneOf([0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]),
    /**
     * Defines the `flex-wrap` style property.
     * It can only be used on a type `container` component.
     */
    wrap: PropTypes.oneOf(['nowrap', 'wrap-reverse', 'wrap']),
    /**
     * Defines the number of columns the component is going to use for the `xs` breakpoint or wider,
     * if not overridden.
     * It can only be used on a type `item` component.
     */
    xs: PropTypes.oneOfType([
        PropTypes.oneOf(['auto', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]),
        PropTypes.bool,
    ]),
    /**
     * Defines the number of columns the component is going to use for the `sm` breakpoint or wider,
     * if not overridden.
     * It can only be used on a type `item` component.
     */
    sm: PropTypes.oneOfType([
        PropTypes.oneOf(['auto', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]),
        PropTypes.bool,
    ]),
    /**
     * Defines the number of columns the component is going to use for the `md` breakpoint or wider,
     * if not overridden.
     * It can only be used on a type `item` component.
     */
    md: PropTypes.oneOfType([
        PropTypes.oneOf(['auto', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]),
        PropTypes.bool,
    ]),
    /**
     * Defines the number of columns the component is going to use for the `lg` breakpoint or wider,
     * if not overridden.
     * It can only be used on a type `item` component.
     */
    lg: PropTypes.oneOfType([
        PropTypes.oneOf(['auto', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]),
        PropTypes.bool,
    ]),
    /**
     * Defines the number of columns the component is going to use for the `xl` breakpoint or wider,
     * if not overridden.
     * It can only be used on a type `item` component.
     */
    xl: PropTypes.oneOfType([
        PropTypes.oneOf(['auto', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]),
        PropTypes.bool,
    ]),
    /**
     * If `true`, it sets `min-width: 0` on the item.
     * Refer to the limitations section of the documentation to better understand the use case.
     * It can only be used on a type `item` component.
     */
    zeroMinWidth: PropTypes.bool,
};

// Make propTypes responsive.
['alignContent', 'alignItems', 'direction', 'justifyContent', 'spacing', 'wrap', 'zeroMinWidth'].forEach((prop) => {
    /* eslint-disable react/forbid-foreign-prop-types */
    Grid.propTypes[prop] = PropTypes.oneOfType([
        Grid.propTypes[prop],
        PropTypes.shape(breakpoints.keys.reduce((shape, breakpoint) => {
            shape[breakpoint] = Grid.propTypes[prop];

            return shape;
        }, {})),
    ]);
    /* eslint-enable react/forbid-foreign-prop-types */
});

const StyledGrid = withStyles(styles, { name: 'MuiGrid' })(Grid);

const requireProp = requirePropFactory('Grid');

StyledGrid.propTypes = {
    alignContent: requireProp('container'),
    alignItems: requireProp('container'),
    direction: requireProp('container'),
    justifyContent: requireProp('container'),
    spacing: requireProp('container'),
    wrap: requireProp('container'),
    xs: requireProp('item'),
    sm: requireProp('item'),
    md: requireProp('item'),
    lg: requireProp('item'),
    xl: requireProp('item'),
    zeroMinWidth: requireProp('item'),
};

export default StyledGrid;

I haven't tested it throughly yet, but seems to be working. Hope this helps someone.

@cyrilchapon
Copy link

cyrilchapon commented Nov 10, 2020

@msimulcik @LuudJanssen
Splitted version, completely dynamic breakpoints according to theme, configurable defaultValue (initially undefined as many props ignore undefined)

import { useMediaQuery, useTheme } from '@material-ui/core'
import { Breakpoint } from '@material-ui/core/styles/createBreakpoints'
import { findLast } from 'ramda'

type MediaQueryMatches = Record<Breakpoint, boolean>
const useMediaQueryMatches = () => {
  const theme = useTheme()

  const matches = theme.breakpoints.keys
    .reduce((acc, bp) => {
      return {
        ...acc,
        [bp]: useMediaQuery(theme.breakpoints.up(bp))
      }
    }, {} as MediaQueryMatches)

  return matches
}

type ResponsiveValues<ValueT> = Partial<Record<Breakpoint, ValueT>>
const useResponsive = () => {
  const theme = useTheme()

  const matches = useMediaQueryMatches()

    return <P, DefaultT = undefined>(responsiveValues: ResponsiveValues<P>, defaultValue?: DefaultT ) => {
      const match = findLast(
        bp => matches[bp] && responsiveValues[bp] != null,
        theme.breakpoints.keys
      )

      return match ? responsiveValues[match] : defaultValue
    }
}

export {
  useMediaQueryMatches,
  useResponsive
}

export type {
  MediaQueryMatches,
  ResponsiveValues
}

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 18, 2021

An update, since v5.0.0-alpha.24, alignContent, alignItems, justifyContent accept responsive values. This support inherits from https://next.material-ui.com/system/flexbox/.

<Grid justifyContent={{ xs: 'start', md: 'center' }} alignItems={['center', 'flex-end']}>

We have yet to add support for the following props

  • spacing
  • direction

@igor-pavlichenko

This comment has been minimized.

@abhagsain
Copy link

abhagsain commented Mar 4, 2021

An update, since v5.0.0-alpha.24, alignContent, alignItems, justifyContent accept responsive values. This support inherits from https://next.material-ui.com/system/flexbox/.

<Grid justifyContent={{ xs: 'start', md: 'center' }} alignItems={['center', 'flex-end']}>

We have yet to add support for the following props

  • spacing
  • direction

@oliviertassinari
There's no prop as justifyContent for Grid and for even justify it's not responsive.

<Grid
 container
 alignItems="center"
 justify={{ sm: 'flex-end', md: 'flex-start' }}
 spacing={3}
/>

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 4, 2021

@abhagsain The justify prop was renamed in v5. Make sure you are on the lastest version.

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 19, 2021

In #25149, we have introduced custom responsive props. There are only two props left to make responsive in the Grid: direction and spacing. We can apply the same approach here:

diff --git a/packages/material-ui-system/src/breakpoints.js b/packages/material-ui-system/src/breakpoints.js
index 4a95f92020..d06464f35e 100644
--- a/packages/material-ui-system/src/breakpoints.js
+++ b/packages/material-ui-system/src/breakpoints.js
@@ -26,6 +26,10 @@ export function handleBreakpoints(props, propValue, styleFromPropValue) {
     }
   }

+  if (propValue == null) {
+    return {};
+  }
+
   if (Array.isArray(propValue)) {
     const themeBreakpoints = props.theme.breakpoints || defaultBreakpoints;
     return propValue.reduce((acc, item, index) => {
diff --git a/packages/material-ui/src/Grid/Grid.d.ts b/packages/material-ui/src/Grid/Grid.d.ts
index b52203882b..5596733f09 100644
--- a/packages/material-ui/src/Grid/Grid.d.ts
+++ b/packages/material-ui/src/Grid/Grid.d.ts
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { SxProps, SystemProps } from '@material-ui/system';
+import { ResponsiveStyleValue, SxProps, SystemProps } from '@material-ui/system';
 import { Theme } from '../styles';
 import { OverridableComponent, OverrideProps } from '../OverridableComponent';

@@ -78,7 +78,7 @@ export interface GridTypeMap<P = {}, D extends React.ElementType = 'div'> {
        * It is applied for all screen sizes.
        * @default 'row'
        */
-      direction?: GridDirection;
+      direction?: ResponsiveStyleValue<GridDirection>;
       /**
        * If `true`, the component will have the flex *item* behavior.
        * You should be wrapping *items* with a *container*.
@@ -108,7 +108,7 @@ export interface GridTypeMap<P = {}, D extends React.ElementType = 'div'> {
        * It can only be used on a type `container` component.
        * @default 0
        */
-      spacing?: GridSpacing;
+      spacing?: ResponsiveStyleValue<GridSpacing>;
       /**
        * The system prop that allows defining system overrides as well as additional CSS styles.
        */
@@ -118,7 +118,7 @@ export interface GridTypeMap<P = {}, D extends React.ElementType = 'div'> {
        * It's applied for all screen sizes.
        * @default 'wrap'
        */
      wrap?: GridWrap;
       /**
        * Defines the number of grids the component is going to use.
        * It's applied for the `xl` breakpoint and wider screens.
diff --git a/packages/material-ui/src/Grid/Grid.js b/packages/material-ui/src/Grid/Grid.js
index e87d15e940..bf5abab760 100644
--- a/packages/material-ui/src/Grid/Grid.js
+++ b/packages/material-ui/src/Grid/Grid.js
@@ -13,7 +13,7 @@ import * as React from 'react';
 import PropTypes from 'prop-types';
 import clsx from 'clsx';
 import { deepmerge } from '@material-ui/utils';
-import { unstable_extendSxProp as extendSxProp } from '@material-ui/system';
+import { unstable_extendSxProp as extendSxProp, handleBreakpoints } from '@material-ui/system';
 import { unstable_composeClasses as composeClasses } from '@material-ui/unstyled';
 import requirePropFactory from '../utils/requirePropFactory';
 import experimentalStyled from '../styles/experimentalStyled';
@@ -84,19 +84,23 @@ function generateGap({ theme, styleProps }) {
   let styles = {};

   if (container && spacing !== 0) {
-    const themeSpacing = theme.spacing(spacing);
+    styles = handleBreakpoints({ theme }, styleProps.spacing, (propValue) => {
+      const themeSpacing = theme.spacing(propValue);

-    if (themeSpacing !== '0px') {
-      styles = {
-        width: `calc(100% + ${getOffset(themeSpacing)})`,
-        marginTop: `-${getOffset(themeSpacing)}`,
-        marginLeft: `-${getOffset(themeSpacing)}`,
-        [`& > .${gridClasses.item}`]: {
-          paddingTop: getOffset(themeSpacing),
-          paddingLeft: getOffset(themeSpacing),
-        },
-      };
-    }
+      if (themeSpacing !== '0px') {
+        return {
+          width: `calc(100% + ${getOffset(themeSpacing)})`,
+          marginTop: `-${getOffset(themeSpacing)}`,
+          marginLeft: `-${getOffset(themeSpacing)}`,
+          [`& > .${gridClasses.item}`]: {
+            paddingTop: getOffset(themeSpacing),
+            paddingLeft: getOffset(themeSpacing),
+          },
+        };
+      }
+
+      return {};
+    });
   }

   return styles;
@@ -159,28 +163,25 @@ const GridRoot = experimentalStyled(
     ...(styleProps.zeroMinWidth && {
       minWidth: 0,
     }),
-    ...(styleProps.direction === 'column' && {
-      flexDirection: 'column',
-      [`& > .${gridClasses.item}`]: {
-        maxWidth: 'none',
-      },
-    }),
-    ...(styleProps.direction === 'column-reverse' && {
-      flexDirection: 'column-reverse',
-      [`& > .${gridClasses.item}`]: {
-        maxWidth: 'none',
-      },
-    }),
-    ...(styleProps.direction === 'row-reverse' && {
-      flexDirection: 'row-reverse',
-    }),
    ...(styleProps.wrap === 'nowrap' && {
      flexWrap: 'nowrap',
    }),
    ...(styleProps.wrap === 'reverse' && {
      flexWrap: 'wrap-reverse',
    }),
   }),
+  ({ theme, styleProps }) =>
+    handleBreakpoints({ theme }, styleProps.direction, (propValue) => {
+      const output = {
+        flexDirection: propValue,
+      };
+
+      if (propValue.indexOf('column') === 0) {
+        output[`& > .${gridClasses.item}`] = {
+          maxWidth: 'none',
+        };
+      }
+
+      return output;
+    }),
   generateGap,
   ({ theme, styleProps }) =>
     theme.breakpoints.keys.reduce((globalStyles, breakpoint) => {

And, then, it works:

<Grid container spacing={{ xs: 1, md: 5 }}>

If somebody wants to work on it, it's good to grab.

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Mar 19, 2021
@oliviertassinari oliviertassinari modified the milestones: v5-beta, v5.1 May 9, 2021
@likitarai1
Copy link
Contributor

I would like to work on this issue.

@oliviertassinari
Copy link
Member

@likitarai1 Feel free to 👍

@oliviertassinari
Copy link
Member

This is done! https://next--material-ui.netlify.app/components/grid/#responsive-values

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: Grid The React component. good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request priority: important This change can make a difference
Projects
None yet
Development

Successfully merging a pull request may close this issue.