[System] Fix nested grid v2

[siriwatknp]

close #35959

After: https://codesandbox.io/s/sleepy-frost-me5qwc?file=/src/App.tsx

Root cause

the nested grid >= level 2 does not behave properly because it uses variables from the top-most grid instead of the closest parent.

Changes

Change the type of the NestedContext to the number and increase it for each level. In each level, create specific variables for the next level.

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

[mui-bot]

	Messages
📖	Netlify deploy preview: https://deploy-preview-35994--material-ui.netlify.app/

@mui/joy: parsed: 0.00% 😍, gzip: +0.10%

Details of bundle changes

Generated by 🚫 dangerJS against 5920aaf

[hbjORbj]

Same suggestion as my comment above

[hbjORbj]

Left a few small question / suggestions, but looks ready for merge!

[mnajdova]

Wouldn't it be simpler to add the nested variables even if the grid is nested? That way we shouldn't need any leveling information.

I mean oversimplification, but something like this should work no?

[siriwatknp]

Not possible because the variables have 2 purposes:

• consumed by the element itself (margin)
• consumed by the next level grid (padding)

If the variables are the same name, they will behave incorrectly:

// Ex. style of the nested grid container level 1
padding: var(--Grid-rowSpacing); // --Grid-rowSpacing is from the root grid
--Grid-nested-rowSpacing: 16px;
margin: calc(var(--Grid-nested-rowSpacing) * -1);


// style of the nested grid container level 2
padding: var(--Grid-nested-rowSpacing); // --Grid-nested-rowSpacing is from the nested grid container level 1
--Grid-nested-rowSpacing: 24px; // ❌ this will cause the padding to behave incorrectly
margin: calc(var(--Grid-nested-rowSpacing) * -1);

This is why the level of information is important to create scope variables for each level.

I am able to simplify the logic by removing nested variables and using only the --Grid-rowSpacing and --Grid-columnSpacing, however, the leveling information is still necessary.

If you are not use nested grid container, you will get a simple --Grid-rowSpacing and --Grid-columnSpacing as usual.

[siriwatknp]

@mnajdova Here is the sandbox to demonstrate the inheritance.

What I like about the latest change is that the inheritance works with just row and spacing variables. The spacing inheritance can be stopped by providing spacing prop to that specific container level which is a correct behavior.

[mnajdova]

In order for what I had in mind to work, we would need to bump the specificity, which is not great. This is what I had in mind:

index 148057c105..98201a717b 100644
--- a/packages/mui-system/src/Unstable_Grid/gridGenerator.ts
+++ b/packages/mui-system/src/Unstable_Grid/gridGenerator.ts
@@ -208,6 +208,10 @@ export const generateGridStyles = ({ ownerState }: Props): {} => {
                 ...((ownerState.disableEqualOverflow || ownerState.parentDisableEqualOverflow) && {
                   padding: `calc(var(--Grid-nested-rowSpacing)) 0px 0px calc(var(--Grid-nested-columnSpacing))`,
                 }),
+                // for nested grids, set the nested variables to the current rowSpacing and columnSpacing if they are defined
+                '& .MuiGrid2-root': {
+                  '--Grid-nested-rowSpacing': 'var(--Grid-rowSpacing, var(--Grid-nested-rowSpacing))',
+                  '--Grid-nested-columnSpacing': 'var(--Grid-columnSpacing, var(--Grid-nested-columnSpacing))',
+                }
               }
             : {
                 '--Grid-nested-rowSpacing': 'var(--Grid-rowSpacing)',

I will let to you decide what you think it's better. With this one we gain simpler CSS variables, but we are increasing the specificity. I don't expect people to actually change the CSS variable when they have a first class API - prop, so this could be viable solution too.

[siriwatknp]

@mnajdova

1. CSS variables does not work if it refers to themselves, so they should be:

   + '& .MuiGrid2-root': {
   +     '--Grid-nested-rowSpacing': 'var(--Grid-rowSpacing)',
   +     '--Grid-nested-columnSpacing': 'var(--Grid-columnSpacing)',
   +  }

2. What you propose would not work if you specify spacing prop to the nested grid.

let's say that this is the style of a deeply nested grid (it could be any level), the padding of the nested grid should use the parent spacing variable to behave correctly:

// assume that --Grid-nested-rowSpacing is 8px;
padding: `calc(var(--Grid-nested-rowSpacing) / 2) calc(var(--Grid-nested-columnSpacing) / 2)`

Now if you add the CSS specificity together with spacing prop:

the styles become:

--Grid-nested-rowSpacing: 16px;
--Grid-nested-columnSpacing: 16px;
'& > .MuiGrid2-container': {
  '--Grid-nested-rowSpacing': 'var(--Grid-rowSpacing)', // 16px from itself
  '--Grid-nested-columnSpacing': 'var(--Grid-columnSpacing)', // 16px from itself
}

// --Grid-nested-rowSpacing resolves to 16px (itself) instead of 8px (parent) ❌ This is wrong.
padding: `calc(var(--Grid-nested-rowSpacing) / 2) calc(var(--Grid-nested-columnSpacing) / 2)`

[siriwatknp]

My latest change fixes the problem and is also better in one aspect. It does not produce nested variables if you don't have nested grids.

[mnajdova]

CSS variables does not work if it refers to themselves, so they should be:

Ah I didn't know this, I guess for this to work we need some kind of internal variable as described in the answer here: https://stackoverflow.com/questions/59015022/css-variables-defaults-set-if-not-already-set

I understand now what this wouldn't work, thanks for the explanation.

[siriwatknp]

This demo does not need to use a nested grid. Simple box is enough.

[siriwatknp]

Changed to level sounds better because it is not a boolean anymore, it is a number.

[mnajdova]

I learnt a lot by reviewing this PR :D

[siriwatknp]

I learnt a lot by reviewing this PR :D

Me too 😄!