[Grid] grid item doesn't respond to grid container's responsive columns correctly

[kkorach]

The Grid component is documented as supporting responsive columns but it only uses the first breakpoint's value.

<Grid container spacing={{ xs: 2, md: 3 }} columns={{ xs: 4, sm: 8, md: 12 }}>

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

The Grid renders with 4 columns regardless of browser width.

Expected Behavior 🤔

The number of columns in the Grid should change as the browser gets wider.

Steps to Reproduce 🕹

Sandbox from the Grid API documents
https://codesandbox.io/s/uzb8q?file=/demo.tsx

Steps:

1. It's easier to see if you modify the items <Grid item xs={1} key={index}> so they only take up one column.
2. Regardless of browser width, you will only get 4 columns

The debugger shows that Grid's generateGrid method gets called with the same breakpoint xs each time, even though GridRoot should be calling it with each of the breakpoints.

  // Bottom of GridRoot
  ({ theme, ownerState }) =>
    theme.breakpoints.keys.reduce((globalStyles, breakpoint) => {
      // Use side effect over immutability for better performance.
      generateGrid(globalStyles, theme, breakpoint, ownerState);
      return globalStyles;
    }, {}),

Context 🔦

Trying to implement the Material Design Responsive Layout Grid
https://material.io/design/layout/responsive-layout-grid.html#columns-gutters-and-margins

[siriwatknp]

I can reproduce it with https://codesandbox.io/s/responsivegrid-material-demo-forked-q9cdi.

@hbjORbj Do you want to take this?

[hbjORbj]

@kkorach Thanks for creating the issue. Just to clarify the problem here, it seems that grid item does not respond to grid container's responsive columns correctly. For example, in this demo, both grid container and grid item have the same three breakpoints (xs, sm, md) and hence there is no issue. Yet, in demo provided by @siriwatknp, the problem arises because grid item only has one breakpoint xs, whereas grid container has xs, sm, md. This is indeed a bug. I will create a fix for this.

[hbjORbj]

@kkorach Hey, this was discussed in a team meeting, and there was a suggestion that

instead of having:
<Grid container columns={{ xs: 4, sm: 8, md: 12 }}> and
<Grid item xs={1}> and expecting 4 columns, 8 columns and 12 columns of items, respectively, in xs, sm, md,

you can do the following:
<Grid container columns={24}> and <Grid item xs={6} sm={3} md={2}>.

Demo: https://codesandbox.io/s/responsivegrid-material-demo-forked-vmnx8?file=/demo.tsx:474-690

I think this achieves what you were trying to do in the first place. Am I right?

[kkorach]

We're trying to create a constrained version of Grid that matches the responsive design layout and a couple other things (spacing). Internally our developers will use that instead of the mui Grid. I don't think the workaround will work because I'm not trying to fix a specific usage.

[hbjORbj]

@kkorach Sure, I will have a look and work on fixing the problem I clarified earlier.

Just to clarify the problem here, it seems that grid item does not respond to grid container's responsive columns correctly. For example, in this demo, both grid container and grid item have the same three breakpoints (xs, sm, md) and hence there is no issue. Yet, in demo provided by @siriwatknp, the problem arises because grid item only has one breakpoint xs, whereas grid container has xs, sm, md. This is indeed a bug. I will create a fix for this.

[kkorach]

I dug into it a little bit. When you specify a Grid item size, <Grid item xs={1} />, the 1 column prop would carry over to the other breakpoints. If I specify <Grid item xs={1} sm={1} md={1} lg={1} xl={1} />, it works as expected.

In generateGrid the first few lines are

  const size = ownerState[breakpoint];

  if (!size) {
    return;
  }

In my case where I only specify xs={1} there won't be any breakpoints in ownerState, so it will exit out of the method.

[hbjORbj]

When you specify a Grid item size, <Grid item xs={1} />, the 1 column prop would carry over to the other breakpoints.

You mean it wouldn't carry over, right?

If I specify <Grid item xs={1} sm={1} md={1} lg={1} xl={1} />, it works as expected.

yep, that's what I meant earlier by

both grid container and grid item have the same three breakpoints (xs, sm, md) and hence there is no issue. Yet, the problem arises because grid item only has one breakpoint xs, whereas grid container has xs, sm, md.

Would you like to work on this by any chance?

[kkorach]

I can. I'm new to contributing (would be second PR) and have read the contributing guide. Is there more information on how to use the docs site as a development environment?

It would also be helpful if you could help point to where <Grid item xs={1} /> gets propagated across all breakpoints. Is that part of handleBreakpoints/resolveBreakpointValues?

[hbjORbj]

I can. I'm new to contributing (would be second PR) and have read the contributing guide. Is there more information on how to use the docs site as a development environment?

That sounds good enough! After you create a PR, you can request a review from me and I will review!

It would also be helpful if you could help point to where <Grid item xs={1} /> gets propagated across all breakpoints. Is that part of handleBreakpoints/resolveBreakpointValues?

There are utils you can use in packages/mui-system/src/breakpoints.js. For example, in order to handle columns prop in Grid, computeBreakpointsBase, resolveBreakpointValues and handleBreakpoints are used. handleBreakpoints outputs css object including media query. This PR ( #29300 ) will be helpful.

In generateGrid the first few lines are

  const size = ownerState[breakpoint];

  if (!size) {
    return;
  }

However, size props like xs, sm, ..., etc are not handled using these braekpoints utils. As you pointed out earlier, I also think that this snippet of code is the source of the discussed issue.

[hbjORbj]

@robertofabrizi I tried your example and the spacing property doesn't seem to be getting applied, is it?

Which example are you referring to? FYI, spacing in both demos in the message you quoted work. You can inspect :)

[robertofabrizi]

@robertofabrizi I tried your example and the spacing property doesn't seem to be getting applied, is it?

Which example are you referring to? FYI, spacing in both demos in the message you quoted work. You can inspect :)

My mistake sorry it does work, sorry again!