Sitelet https://web.archive.org/web/20210924074103/https://github.com/mui-org/material-ui/issues/28554
Skip to content

/ material-ui Public

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Nested Grid containers ignore columns prop #28554

Open
2 tasks done
camdarley opened this issue Sep 23, 2021 · 1 comment
Open
2 tasks done

Nested Grid containers ignore columns prop #28554

camdarley opened this issue Sep 23, 2021 · 1 comment
Labels
bug 🐛 component: Grid good first issue

Comments

@camdarley
Copy link

@camdarley camdarley commented Sep 23, 2021

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

When nesting a Grid container inside a Grid item, only the columns prop of the root container is used.

Expected Behavior 🤔

It should be allowed to place a Grid container with X columns inside a Grid container with Z columns.

Steps to Reproduce 🕹

https://codesandbox.io/embed/columnsgrid-material-demo-forked-s2vsr?fontsize=14&hidenavigation=1&theme=dark

Steps:

  1. Create a Grid container with columns=16
  2. Create a Grid item inside with xs=8: Grid item is half the width of the container
  3. Create a Grid container inside the Grid item with columns=8
  4. Create a Grid item inside with xs=8: Grid item is half the width of the parent container, but should be full width

Context 🔦

I have a responsive media grid, with custom columns prop depending on breakpoints.
Inside each media pods, I want to arrange text and button element, and keep the layout regardless the amount of columns the media grid have.

Your Environment 🌎

`npx @mui/envinfo` 
  System:
    OS: macOS 11.5.2
  Binaries:
    Node: 16.8.0 - /usr/local/bin/node
    Yarn: 1.22.11 - /usr/local/bin/yarn
    npm: 7.21.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 93.0.4577.82
    Edge: Not Found
    Firefox: 91.0.2
    Safari: 14.1.2
  npmPackages:
    @emotion/react: 11.4.1 => 11.4.1 
    @emotion/styled: 11.3.0 => 11.3.0 
    @mui/core: 5.0.0-alpha.47 => 5.0.0-alpha.47 
    @mui/icons-material: 5.0.0 => 5.0.0 
    @mui/lab: 5.0.0-alpha.47 => 5.0.0-alpha.47 
    @mui/material: 5.0.0 => 5.0.0 
    @mui/private-theming:  5.0.0 
    @mui/styled-engine:  5.0.0 
    @mui/styles: 5.0.0 => 5.0.0 
    @mui/system: 5.0.0 => 5.0.0 
    @mui/types:  7.0.0 
    @mui/utils: 5.0.0 => 5.0.0 
    @mui/x-data-grid: 5.0.0-beta.1 => 5.0.0-beta.1 
    @types/react:  17.0.3 
    react: 17.0.2 => 17.0.2 
    react-dom: 17.0.2 => 17.0.2 
    styled-components: 5.3.1 => 5.3.1
@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Sep 24, 2021

Root cause: context is used before prop, so nested does not work if the top Grid has specified columns !== 12

// Grid.js

const {
    className,
    columns: columnsProp = 12,
 }

const columns = React.useContext(GridContext) || columnsProp

const wrapChild = (element) =>
    columns !== 12 ? (
      <GridContext.Provider value={columns}>{element}</GridContext.Provider>
    ) : (
      element
    );

Suggested Fix

diff --git a/packages/mui-material/src/Grid/Grid.js b/packages/mui-material/src/Grid/Grid.js
index 61c964e1fb..d725639502 100644
--- a/packages/mui-material/src/Grid/Grid.js
+++ b/packages/mui-material/src/Grid/Grid.js
@@ -246,7 +246,7 @@ const Grid = React.forwardRef(function Grid(inProps, ref) {
   const props = extendSxProp(themeProps);
   const {
     className,
-    columns: columnsProp = 12,
+    columns: columnsProp,
     columnSpacing: columnSpacingProp,
     component = 'div',
     container = false,
@@ -267,7 +267,8 @@ const Grid = React.forwardRef(function Grid(inProps, ref) {
   const rowSpacing = rowSpacingProp || spacing;
   const columnSpacing = columnSpacingProp || spacing;
 
-  const columns = React.useContext(GridContext) || columnsProp;
+  const columnsContext = React.useContext(GridContext);
+  const columns = columnsProp || columnsContext || 12;
 
   const ownerState = {
     ...props,

cc @mnajdova

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 component: Grid good first issue
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
@camdarley @siriwatknp