[Grid] spacing with full width Items / Layout shift

[zirkelc]

The Grid in v5 with spacing=1 adds padding left and top and counters it with a negative margin left and top. However, if the Grid contains only one Item with xs=12 or multiple stacked Items with xs=12, then the left padding and margin should be unnecessary.

• v5 example: https://codesandbox.io/s/mui-grid-spacing-v5-qs72y?file=/src/App.js
• v4 example: https://codesandbox.io/s/mui-grid-spacing-v4-5emrz?file=/src/App.js

• 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 😯

In v5 the Grid adds padding/margin left and causes the layout to shift left even though there is only one Item in the row and no left adjustment necessary:

Expected Behavior 🤔

In v4 the padding/margin was evenly distributed to all sides and therefore no layout shift in case of single full width Items:

[siriwatknp]

I am not sure I have the same understanding of the word "Layout shift" as you mentioned. Is it the same as Cumulative Layout Shift or do you mean "padding, margin"?

cc @mnajdova @oliviertassinari I am also wondering why the implementation in Grid changed?

[zirkelc]

Yes, the wording is probably not quite right - I meant the shifting of certain elements to the left (and top) due to the negative margin.

[rustlestuff]

I noticed the same thing while migrating to v5

I applied spacing={10} on my Grid container, which resulted in:
.MuiGrid-spacing-xs-10 > .MuiGrid-item { padding: 40px; } on production, but:

.css-11reh94-MuiGrid-root>.MuiGrid-item { padding-left: 80px; } and
.css-11reh94-MuiGrid-root>.MuiGrid-item { padding-top: 80px; } on my feature branch

V4 on production

v5 on feature branch

[mnajdova]

@hbjORbj would you like to take a look at this?

[hbjORbj]

@mnajdova Will do :)

[kutnickclose]

+1 - This issue is currently breaking my grid spacing when trying to migrate from v4 to v5.

[kutnickclose]

Confirmed this issue was introduced in the migration from emotion. It was not an issue in @material-ui/core v5.0.0-alpha.23, and is an issue in v5.0.0-alpha.24 (when the migration to emotion for the Grid was introduced). I used the sandbox above and made some modifications.

PR that introduced the change: #24395

[hbjORbj]

@kutnickclose Thanks for the update! I left a comment in your PR.

[hbjORbj]

@zirkelc @rustlestuff

Please check out the following comment by @kutnickclose ; I think it will be helpful

Just an update on my end that my issue was related to a v4 MuiGrid class interacting with a v5 Grid component. This would add the old v4 style (padding/margin all the way around) + the the extra padding from the v5 style (padding/margin only on the top and left). This would end up leaving extra padding on the right and bottom. For example, my div related to the grid component might have padding: 12px (from v4), and then paddingTop: 24px, paddingLeft: 24px (from v5). I ended up namespacing the material ui v4 code and no longer have grid issues with v5.

[zirkelc]

@hbjORbj in my example above (https://codesandbox.io/s/mui-grid-spacing-v5-qs72y?file=/src/App.js) I only used mui v5 components and the issue appears. IMO this is not related to mixing v4 and v5 components.

[kutnickclose]

I think visually (and without backgrounds), the new padding/margin strategy is not really an issue for my case. I would imagine all the visual specs for the Grid component would pass even with the changed padding/margin strategy because the textfields are correctly placed. That said, the padding/margin change is a breaking change that's not captured in the docs that off-centers the background color of grid items.

[hbjORbj]

This seems to be a regression of this change: #24332

This is briefly described in grid docs: "Limitation - Negative margin - The spacing between items is implemented with a negative margin. This might lead to unexpected behaviors. For instance, to apply a background color, you need to apply display: flex; to the parent."

However, this isn't sufficient and not very informative. I think the quick solution to this issue seems to be manually tweaking padding/margin when using background-color property. Check out https://codesandbox.io/s/mui-grid-spacing-v5-forked-7x7nw?file=/src/App.js.

[Hummel37]

I think the Masonry has the same bug as you can see here.
I have only added the border and there is space on the right but not on the left.
https://codesandbox.io/s/basicmasonry-material-demo-forked-duvqs

The code example is from the mui.com page.

[hbjORbj]

@Hummel37

No, it is not. You've added a border to Box component, not Masonry component. The reason you think there is margin on the right but not on the left is because Box has a fixed width of 500px.

If you add a border to Masonry, there is no such issue. Please check out https://codesandbox.io/s/basicmasonry-material-demo-forked-ncmp7

[dohomi]

I just started the migration from v4 => v5 and bumped into this issue as well. I use quite frequently borders on MuiGrid--container and this seems a breaking change which I haven't found in the migration guide:
v5:
https://codesandbox.io/s/basicgrid-material-demo-forked-7pyh6w?file=/demo.tsx
v4:
https://codesandbox.io/s/material-demo-forked-ltihvn?file=/demo.js

In v4 setting borders would still align the div container centered, but in v5 the container with borders are moving to the left without being centered.

[aschillio]

I have exactly the same issue as @dohomi. Any update on this matter ? This is also breaking all my app grids.

[hbjORbj]

Hi.

As suggested in this comment, we are likely to be on our way pretty soon to changing the implementation of Grid component to fix the issue. In the meantime, I think the quickest way to go about it is to simply tweak margins and paddings as suggested here.

[ZachArens]

I'm having the same issue here with a Meteor-React-MUI app that we recently upgraded to MUI5. :)

[mnajdova]

@hbjORbj how about we update the migration guide with this one? Looks like a frequent confusion when doing the migration. cc @samuelsycamore as he is currently tweaking the migration guides.

[wilsoft-gt]

Exact same issue here.

[siriwatknp]

@zirkelc Hey, we have released Grid v2 in v5.9.1 which fixes this issue. Can you give it a try?

The doc is not live but you can take a look at https://deploy-preview-33554--material-ui.netlify.app/material-ui/react-grid2/. (Here is the PR for the doc)

The usage is like this:

import Grid from '@mui/material/Unstable_Grid2';

[zirkelc]

@siriwatknp it looks good:
https://codesandbox.io/s/mui-grid-spacing-v5-gridv2-05ho28?file=/src/App.js

[arcataroger]

Is there any plan to fix this on v5 stable (Grid v1), or is the recommendation to migrate over to Unstable_Grid2 even in production?

[siriwatknp]

Is there any plan to fix this on v5 stable (Grid v1), or is the recommendation to migrate over to Unstable_Grid2 even in production?

Please migrate to Grid2, it will replace the version 1 in the next major release.

[Cristy94]

I just encountered the same problem (only top and left padding is added).

So, the issue won't be fixed for the old Grid component? The only solution is to use Unstable_Grid2?