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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Container] - property disableGutters doesn't work if theme styleOverrides have been applied #30825

Open
2 tasks done
RemyMachado opened this issue Jan 28, 2022 · 4 comments
Open
2 tasks done
Labels
support: question Community support but can be turned into an improvement

Comments

@RemyMachado
Copy link

RemyMachado commented Jan 28, 2022

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 馃槸

When the Container padding style is overridden, the disableGutters property stops working.

Expected behavior 馃

The disableGutters property should work even with the style override.

Steps to reproduce 馃暪

If the container style have been overridden, the disableGutters property won't work.

// theme.tsx
    ...
    components: {
      MuiContainer: {
        styleOverrides: {
          root: {
            [customBreakpoints.between('xs', 'sm')]: {
              padding: '0 20px',
            },
            [customBreakpoints.up('md')]: {
              padding: '0 90px',
            },
          },
        },
      },
    },
    ...
      <Container disableGutters> // won't remove gutters from 'xs' to 'sm' and above 'md'
@RemyMachado RemyMachado added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jan 28, 2022
@siriwatknp
Copy link
Member

siriwatknp commented Feb 3, 2022

As a workaround (must have @mui/material v5.3.0+), you can use callback and do this:

components: {
  MuiContainer: {
    styleOverrides: {
      root: ({ ownerState, theme }) => ({
        ...!ownerState.disableGutters && {
          [theme.breakpoints.between('xs', 'sm')]: {
            padding: '0 20px',
          },
          [theme.breakpoints.up('md')]: {
            padding: '0 90px',
          },
        },
      }),
    },
  },
},

https://codesandbox.io/s/simplecontainer-material-demo-forked-g9q93?file=/demo.tsx

I mention this as a workaround because this makes customization very confusing. The real fix in my opinion is disableGutters should have higher specificity similar to Mui-disable, ...states. This applies to other components as well (not only disableGutters but any props that overrides existing CSS properties).

cc @oliviertassinari @mnajdova for more opinions.

@siriwatknp siriwatknp added support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Feb 3, 2022
@mnajdova
Copy link
Member

mnajdova commented Mar 3, 2022

I am not sure that this is unexpected behavior to be honest. The override should win, that is why it is an override. It is your responsibility to make sure that if you add a override the other props combination still work as expected. I wouldn't be bumping t he specificity to be honest.

It is similar to as if you change the color of a button in a style override. It would have that color even if the color prop is used, because that is the override you've added. You can use the props combination to specify when the style override should be applied.

@RemyMachado
Copy link
Author

Interesting. Let's assume that anything specified in styleOverrides can break the props, then what is the proper way to specify a different gutter size for the Container in the theme while still being able to use disableGutter?

@mnajdova
Copy link
Member

mnajdova commented Mar 3, 2022

@RemyMachado is what's proposed in #30825 (comment) not working for you?

Note that we are working on adding CSS variables for customization like this, that would allow us to specify these per component in the long term.

Till then, I think what's proposed in the above comment is the best option. You could also add a CSS selector that would apply the styles if the class .MuiContainer-disableGutters is not applied, if you are on older version that does not support props in the styleOverrides - https://stackblitz.com/edit/react-xajkta?file=demo.tsx

Does this makes sense?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

3 participants