Skip to content
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

[material-ui][Grid2] styleOverride does not work for container, only root #44411

Open
smerling-coremedia opened this issue Nov 14, 2024 · 2 comments · May be fixed by #44420
Open

[material-ui][Grid2] styleOverride does not work for container, only root #44411

smerling-coremedia opened this issue Nov 14, 2024 · 2 comments · May be fixed by #44420
Assignees
Labels
bug 🐛 Something doesn't work component: Grid The React component. package: material-ui Specific to @mui/material

Comments

@smerling-coremedia
Copy link

smerling-coremedia commented Nov 14, 2024

Steps to reproduce

Steps:

  1. Open this link to live example: A modified Codebox.io example
  2. In the Codebox Example, go to /src and open Demo.tsx
  3. You get an Typescript error, that MuiGrid2 -> StyleOverrides does not have the property "container"
  4. In Demo.tsx have a look at the theme overrides. The override for MuiGrid2 for "root" works, but not "container"

Current behavior

The only style overrides that work for Grid2 are for "root", "container" is not working anymore.

Expected behavior

See the Grid2 documentation. It states: You can use MuiGrid2 to change the default props of this component with the theme.
But does not show the properties listed, like for the old Grid (see here)

Context

I upgraded from Grid to Grid2 and suddenly my styleOverrides for MuiGrid2-container do not work anymore, only MuiGrid2-root. But as far as I can see there has been no mention of changes in the theme api. Even the documentation states, that you can access the theme props like allways. Is this difference between Grid and Grid2 on purpose or a bug?

Your environment

npx @mui/envinfo
  System:
    OS: macOS 15.1
  Binaries:
    Node: 20.18.0 - /opt/homebrew/bin/node
    npm: 10.8.2 - /opt/homebrew/bin/npm
    pnpm: 9.13.0 - /opt/homebrew/bin/pnpm
  Browsers:
    Chrome: 130.0.6723.117
    Edge: 130.0.2849.80
    Safari: 18.1
  npmPackages:
    @emotion/react: ^11.13.3 => 11.13.3
    @emotion/styled: ^11.13.0 => 11.13.0
    @mui/icons-material: ^6.1.7 => 6.1.7
    @mui/material: ^6.1.7 => 6.1.7
    react: ^18.3.1 => 18.3.1
    react-dom: ^18.3.1 => 18.3.1

    developing the App using vite 5.4.11

Search keywords: Grid Grid2 Override StyleOverride Theme Root Container

@smerling-coremedia smerling-coremedia added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 14, 2024
@Sammieblz
Copy link

Sammieblz commented Nov 14, 2024

Hi @smerling-coremedia,

Thank you for reporting this issue. I investigated the problem, and it seems like there might be a discrepancy between the intended functionality of MuiGrid2 style overrides and the current implementation.

Analysis

  • Current Behavior: Only the "root" styleOverride seems to be working for MuiGrid2. The "container" styleOverride is not being applied, leading to a TypeScript error.
  • Expected Behavior: According to the documentation for MuiGrid2, it is expected that we can use the theme overrides for "container", similar to the original Grid component. However, the current implementation does not seem to support "container" in the theme overrides, causing confusion and a lack of consistent behavior.

Suggested Solution

  1. Use sx Prop for Container Styling:
    • As a workaround, you can use the sx prop to apply specific styles to MuiGrid2 components that use the container prop. This will allow you to apply styles directly to the container instance.
    <Grid2 container spacing={2} sx={{ border: '2px solid blue' }}>
      <Grid2 xs={6}>Item 1</Grid2>
      <Grid2 xs={6}>Item 2</Grid2>
    </Grid2>

This approach helps bypass the current limitation of the styleOverrides for "container" while maintaining the same functionality.

Update Documentation:

It would be helpful to update the documentation to either clarify that "container" overrides are not supported in MuiGrid2, or to include an example of how the component should be used to style containers effectively.

Possible Fix in the Codebase:

If "container" is meant to be supported in MuiGrid2 style overrides, then it might be a bug in the current implementation. The root cause seems to be the absence of "container" from the utility classes applied internally by Grid2.
A potential solution would involve updating the Grid2 implementation to include the "container" class in its list of overrideable styles.

Next Steps

If this difference between Grid and Grid2 was intentional, adding a note to the documentation would be extremely helpful to avoid confusion.
If it is a bug, I would recommend adding support for "container" in the styleOverrides so that it matches the behavior of Grid.

Thanks again for bringing this up, and I hope the above suggestions help. I'm happy to assist further if needed.

@sai6855 sai6855 added bug 🐛 Something doesn't work component: Grid The React component. package: material-ui Specific to @mui/material and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 15, 2024
@sai6855 sai6855 self-assigned this Nov 15, 2024
@sai6855 sai6855 changed the title Grid2 styleOverride does not work for container, only root [material-ui][Grid2] styleOverride does not work for container, only root Nov 15, 2024
@sai6855 sai6855 linked a pull request Nov 15, 2024 that will close this issue
@smerling-coremedia
Copy link
Author

Thank you so much for your extended feedback! I'm glad this is an already know issue and will be resolved soon :)

@DiegoAndai DiegoAndai moved this to In progress in Material UI Nov 21, 2024
@DiegoAndai DiegoAndai self-assigned this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: Grid The React component. package: material-ui Specific to @mui/material
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

4 participants