[Grid] Support custom columns with nested grid

[Devesh21700Kumar]

Closes #28554

A better representation of the above issue would be this
It can be clearly seen from the below image that the container inside the item uses the spacing props of the parent

The correct case should have been this

This can be fixed by just some boolean fixes
The prop had to be put before the value obtained for Gridcontext in the OR operation for it to be used first
The breakpoint unit 12 had to be set later so that it can be used as a default provided the earlier two parameters are null

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

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against e069cd6

[siriwatknp]

I think a unit test is needed in this PR.

[Devesh21700Kumar]

I think a unit test is needed in this PR.

Cool. Will try to implement the unit test for the change.

[ru4ert]

This line is redundant.

[hbjORbj]

I suggest removing this line.

[hbjORbj]

Suggested change

	const columnSet = columnsProp || columnsContext || 12;
	const columns = columnsProp || columnsContext || 12;

[Devesh21700Kumar]

Thanks, I'll remove this line

[hbjORbj]

@Devesh21700Kumar Hi, thanks for the PR. You can add the following unit test. A codesandbox (e.g., https://codesandbox.io/s/determined-brook-j2bur?file=/src/App.js) or a test is necessary for the maintainers to confirm that the PR is indeed fixing the relevant issue.

diff --git a/packages/mui-material/src/Grid/Grid.test.js b/packages/mui-material/src/Grid/Grid.test.js
index 943f3bcaae..a6c1d6a74f 100644
--- a/packages/mui-material/src/Grid/Grid.test.js
+++ b/packages/mui-material/src/Grid/Grid.test.js
@@ -25,6 +25,24 @@ describe('<Grid />', () => {
       const { container } = render(<Grid container />);
       expect(container.firstChild).to.have.class(classes.container);
     });
+
+    it('should apply the correct number of columns for nested containers', () => {
+      const { getByTestId } = render(
+        <Grid container columns={16}>
+          <Grid item xs={8}>
+            <Grid container columns={8} data-testid="nested-container">
+              <Grid item xs={8} />
+            </Grid>
+          </Grid>
+        </Grid>,
+      );
+      const container = getByTestId('nested-container');
+      // `columns` of nested container should have a higher priority than that of root container
+      // otherwise, max-width would be 50% in this test
+      expect(container.firstChild).toHaveComputedStyle({ maxWidth: '100%' });
+    });
   });

[Devesh21700Kumar]

@Devesh21700Kumar Hi, thanks for the PR. You can add the following unit test. A codesandbox (e.g., https://codesandbox.io/s/determined-brook-j2bur?file=/src/App.js) or a test is necessary for the maintainers to confirm that the PR is indeed fixing the relevant issue.

diff --git a/packages/mui-material/src/Grid/Grid.test.js b/packages/mui-material/src/Grid/Grid.test.js
index 943f3bcaae..a6c1d6a74f 100644
--- a/packages/mui-material/src/Grid/Grid.test.js
+++ b/packages/mui-material/src/Grid/Grid.test.js
@@ -25,6 +25,24 @@ describe('<Grid />', () => {
       const { container } = render(<Grid container />);
       expect(container.firstChild).to.have.class(classes.container);
     });
+
+    it('should apply the correct number of columns for nested containers', () => {
+      const { getByTestId } = render(
+        <Grid container columns={16}>
+          <Grid item xs={8}>
+            <Grid container columns={8} data-testid="nested-container">
+              <Grid item xs={8} />
+            </Grid>
+          </Grid>
+        </Grid>,
+      );
+      const container = getByTestId('nested-container');
+      // `columns` of nested container should have a higher priority than that of root container
+      // otherwise, max-width would be 50% in this test
+      expect(container.firstChild).toHaveComputedStyle({ maxWidth: '100%' });
+    });
   });

Hey @hbjORbj , Thanks for the help. I've added the test and all the test suites and tests passed without showing any errors.

[hbjORbj]

@Devesh21700Kumar Can you run yarn prettier:all and push again? test_static is failing.

@siriwatknp Hey, argos test is failing. Where do you think this comes from?

[Devesh21700Kumar]

@Devesh21700Kumar Can you run yarn prettier:all and push again? test_static is failing.

@siriwatknp Hey, argos test is failing. Where do you think this comes from?

This one looks like there was a change in the icons of sorts

These were the instances where the Snaps did not match

[siriwatknp]

Hey, argos test is failing. Where do you think this comes from?

This PR is not updated with master branch, that's all. I have updated it, waiting for green CIs.

[hbjORbj]

Looking good to me!

[mnajdova]

Nice! It's a great first pull request on MUI 👌 Thank you for working on it!