[Grid] add equal spacing on sides of grid

[kutnickclose]

• I have followed (at least) the PR section of the contributing guide.

Fixes #29266

I confirmed this fixes the issue I was seeing in my application. I would love feedback on how to get this across the finish line.

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against d1056dc

[hbjORbj]

@kutnickclose Hi. Thanks for opening the PR. While I confirmed in here that this change fixes the issue, I am afraid this introduces a breaking change. This means that this solution can only be included in our next alpha release, which is quite far away.

[oliviertassinari]

This change seems to effectively be the opposite of #24332 by @greguintow. Won't we introduce the same regressions?

[greguintow]

This change seems to effectively be the opposite of #24332 by @greguintow. Won't we introduce the same regressions?

Hey @oliviertassinari , I'll test the use cases and see if cause the issues I fixed. Thaaanks for tagging me

[greguintow]

@oliviertassinari @kutnickclose Please check the demo

As you can see in there, the width is still right in the nested grid but it adds the x scrollbar because is bigger than viewport width. That's an issue I also faced when doing this which was why I had to use a negative margin to make it work as expected

[kutnickclose]

I'm still using negative margin as well, just more equally spread out rather than all on the top and left. Could the demo be fixed with instructions from v4 about negative margin? - https://v4.mui.com/components/grid/#negative-margin.

[kutnickclose]

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.

I think this PR can be closed.

[hbjORbj]

@kutnickclose Thank you for the update. I will close this PR :)

[hbjORbj]

I reopened this pull request, because it is indeed a regression from #24332

In grid docs, there is a section that briefly describes the issue when using background-color property:

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, applying display: flex isn't doing the job. User should apply specific margin to grid container and padding to grid item depending on the value of spacing in order to not have this issue (component looking asymmetric as padding/margin is only applied to top and left). Check out https://codesandbox.io/s/mui-grid-spacing-v5-forked-7x7nw?file=/src/App.js

We can either improve the docs or try to come up with another solution. What do you think? @oliviertassinari

[siriwatknp]

👋 The migration PR has been merged.

Please follow these steps to make sure that the contents are all updated. (Sorry for the inconvenience)

1. pull latest master from upstream to your branch
2. if your PR has changes on the *.md or demo files, you are likely to encounter conflict because all of them have been moved to the new folder.
   2.1 revert the change on those markdown files you did
   2.2 pull latest master from upstream (you should not see conflict)
   2.3 make the changes again at docs/data/material/*
3. run yarn docs:api
   • you might see the changes in docs/pages/material/api/* if your PR touches some of the components
   • it is okay if there is no changes

If you are struggle with the steps above, feel free to tag @siriwatknp

[Vakme]

Hi, I'd like to ask about the status of this PR.
I'm asking because grid padding problem the only thing blocking me and my team from migrating to v5.

Can you give me an estimated time when it will be merged? If you need any help - either with review or with docs, let me know.

[siriwatknp]

@mui/core This might be a very good use case for adding a feature flag for enabling the fix.

[mnajdova]

https://github.com/orgs/mui/teams/core This might be a very good use case for adding a feature flag for enabling the fix.

@siriwatknp I agree, I've seen this popped up in multiple issues, and for some is blocking the upgrade to v5. I propose we try adding a feature flag for it. cc @michaldudak for thoughts

[michaldudak]

Yeah, we could go this way. Or we can explore what @siriwatknp proposed in #32057 (comment) and have a new version of the Grid in System, then deprecate the one in Material UI. The imports may be problematic, though (we will have to change the name of the new Grid when reexporting in from Material UI).

[siriwatknp]

Yeah, we could go this way. Or we can explore what @siriwatknp proposed in #32057 (comment) and have a new version of the Grid in System, then deprecate the one in Material UI. The imports may be problematic, though (we will have to change the name of the new Grid when reexporting in from Material UI).

Sounds like a plan, we don't have to reexport in v5 though.

v5
improve the grid in the system and then let the community import the Grid from the system (for those who are blocked by the Material UI Grid v5)

v6
Reexport the system grid from Material UI and remove the old grid. Since the API is the same, the people who don't have any issue with the current grid in Material UI v5 should not see any changes.

[mnajdova]

@siriwatknp If this is being captures in the new version of the Grid inside the system, should we close this?

[siriwatknp]

@siriwatknp If this is being captures in the new version of the Grid inside the system, should we close this?

Yes, the new grid (system) uses the same behavior as v4 (equal spacing on all sides). I think we can close this in favor of #32746