mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.95k stars 32.27k forks source link

[Grid] Grid system spacing not creating proper gutters in v5 #31244

Closed theMyth721 closed 2 years ago

theMyth721 commented 2 years ago

Duplicates

Latest version

Current behavior 😯

MUI V5 Grid doesn't create gutters. Instead when spacing is applied the entire grid overlaps other elements and the items inside the grid are squashed to the bottom. This looks terrible. I have the basic code from my app but also have managed to reproduce it on my first code sandbox which I dont know how to use properly.

Here is the basic code:

{/* Testing Grid Spacing */}
          <Box component='section'>
              <Typography variant='h5'>Testing Spacing</Typography>
              <Box>
                <Grid container spacing={5}>
                    <Grid item sx={{backgroundColor: 'primary.dark'}}>Item 1</Grid>
                    <Grid item sx={{backgroundColor: 'primary.main'}}>Item 2</Grid>
                    <Grid item sx={{backgroundColor: 'primary.light'}}>Item 3</Grid>
                </Grid>
              </Box>
          </Box>

Here are the screenshots before and after adding spacing:

Before spacing:

Screenshot 2022-02-28 at 13 50 41

After adding a spacing of 5:

Screenshot 2022-02-28 at 13 51 10

I have managed to reproduce this in code sandbox as well But I am new to this so if I haven't done anything properly please let me know:

https://codesandbox.io/s/quirky-lake-50pqf5?file=/src/Demo.tsx

Expected behavior πŸ€”

There should be clean clear gutters between the items

Steps to reproduce πŸ•Ή

Steps:

  1. Create a basic Grid with three or four items in it, provide spacing and see if the gutters are there
  2. Or See the Code sandbox provided in the link

Context πŸ”¦

If I must provide a context, I am learning MUI to use for my react projects so Im just testing the basics. I would expect this to work out of the box but it isn't

Most importantly, If nothing is wrong and I have just made a silly error, please do let me know what it is because I have been banging my head against the wall for hours over this.

Thank you

Your environment 🌎

`npx @mui/envinfo` ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```
theMyth721 commented 2 years ago

I have also posted a detailed question here: https://stackoverflow.com/questions/71294950/mui-v5-grid-system-spacing-not-producing-gutters-between-grid-items

justinrusso commented 2 years ago

From looking at the docs, it looks like it is not ideal to apply styles directly to the Grid item. In all examples, a Paper is used within the Grid item, which results in the gutter working as intended.

I do notice though that the padding from the Grid will overlap elements above it though (like the text in your example) which is not ideal. This behavior is consistent in v4 though. image

import * as React from "react";
import { Box, Grid, Typography } from "@mui/material";

export default function Demo() {
  return (
    <Box component="section">
      <Typography variant="h5">Testing Spacing</Typography>
      <Grid container spacing={5}>
        <Grid item>
          <Box sx={{ backgroundColor: "primary.dark" }}>Item 1</Box>
        </Grid>
        <Grid item>
          <Box sx={{ backgroundColor: "primary.main" }}>Item 2</Box>
        </Grid>
        <Grid item>
          <Box sx={{ backgroundColor: "primary.light" }}>Item 3</Box>
        </Grid>
      </Grid>
    </Box>
  );
}
SpookyGlitches commented 2 years ago

@justinrusso. Adding a <Box> inside <Grid item> worked. Thanks.

theMyth721 commented 2 years ago

I have tried adding a box, It didn't make a difference, but will check again. Either way, at this point I have two issues with this:

  1. Why are they calling it a grid when its clearly just glorified flexbox
  2. Why would I use this when CSS native Grid, Flex is so much more sophisticated but requires a bit more boiler plate no doubt.

The whole point of using this is to make things easier. I will try putting a Box inside

pahrizal commented 2 years ago

using spacing props in Grid container is adding a top and left padding to the grid items. if the goal is to make a gap between the items column, just replace the spacing with columnGap props.

theMyth721 commented 2 years ago

Thank you, Yes I guess I could do that, but it just doesn't seem like a clean solution. I have decided I will just use native Grid/Flex and use the gap property. When I get time I can create my own Grid system to integrate with this, It's not that hard I guess

boutahlilsoufiane commented 2 years ago

The prop spacing defines the space between the type item components, not the Grid item components. You can check the doc https://mui.com/api/grid/. I think it’s not a bug.

image

dohomi commented 2 years ago

Another major issue on v5 is nesting of grid rows, due to the padding top. In v4 it was possible to have a grid row nested and vertical aligned to the center, now due to the paddingTop a grid row will never be centered.

rowe-stamy commented 2 years ago

The behavior of the MUI grid spacing is a bit unexpected since it moves the grid items to the bottom right. To keep the grid items centered you'll need to manually add paddings to the right and bottom of the container or items. One option is simply to add the same amount of spacing as padding to the right and bottom of the grid. The drawback is that you' ll have to calculate the proper amount of padding, if your grid container itself has already a padding. The second option is to make the grid items context aware and add right padding to the last items in a grid row when the row is full. This will still leave you with the task to calculate the proper bottom padding of the grid. It would be more intuitive if the calculations of the right and bottom paddings were done internally by the MUI grid.

siriwatknp commented 2 years ago

The behavior of the MUI grid spacing is a bit unexpected since it moves the grid items to the bottom right. To keep the grid items centered you'll need to manually add paddings to the right and bottom of the container or items. One option is simply to add the same amount of spacing as padding to the right and bottom of the grid. The drawback is that you' ll have to calculate the proper amount of padding, if your grid container itself has already a padding. The second option is to make the grid items context aware and add right padding to the last items in a grid row when the row is full. This will still leave you with the task to calculate the proper bottom padding of the grid. It would be more intuitive if the calculations of the right and bottom paddings were done internally by the MUI grid.

Would you mind sharing a CodeSanbox with me? I am working on the new grid implementation and I want to make sure that it covers your case.

rowe-stamy commented 2 years ago

The behavior of the MUI grid spacing is a bit unexpected since it moves the grid items to the bottom right. To keep the grid items centered you'll need to manually add paddings to the right and bottom of the container or items. One option is simply to add the same amount of spacing as padding to the right and bottom of the grid. The drawback is that you' ll have to calculate the proper amount of padding, if your grid container itself has already a padding. The second option is to make the grid items context aware and add right padding to the last items in a grid row when the row is full. This will still leave you with the task to calculate the proper bottom padding of the grid. It would be more intuitive if the calculations of the right and bottom paddings were done internally by the MUI grid.

Would you mind sharing a CodeSanbox with me? I am working on the new grid implementation and I want to make sure that it covers your case.

Sure! So I ended up doing something similar to this: https://codesandbox.io/s/sleepy-swartz-py56i9?file=/src/GridWrapper.tsx

It's basically a simple wrapper that overrides the padding of the children.

siriwatknp commented 2 years ago

@theMyth721 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';
theMyth721 commented 2 years ago

Hi, thank you, I will try this out very soon.

ElizPN commented 1 year ago

Nice! That works, how risky is to use Unstable_ for production?

siriwatknp commented 1 year ago

Nice! That works, how risky is to use Unstable_ for production?

I don't see any risk. I even encourage you to use the Unstable_ version because it is much better. It will be stable in the next major version of Material UI.

siriwatknp commented 1 year ago

I'm still having the same issue even with Grid2

Please open a new issue with reproduced CodeSandbox. You can tag me and I will look into it right away.