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.96k stars 32.27k forks source link

[ButtonGroup] CardActions spacing is not working #29819

Open fkorotkov opened 2 years ago

fkorotkov commented 2 years ago

Duplicates

Latest version

Current behavior 😯

Here is a sandbox where I took SplitButton example from the documentation and added it as an action to a card.

Screen Shot 2021-11-21 at 5 08 17 PM

On v5 version spacing is not working between a Button and the ButtonGroup. With v4 everything worked fine.

Expected behavior 🤔

Expect to see a spacing between every action.

Steps to reproduce 🕹

Please check out a sandbox here: https://codesandbox.io/s/magical-robinson-cvkwt?file=/src/Demo.tsx

Context 🔦

I'm trying to upgrade from v4 to v5 and facing some issues.

Your environment 🌎

`npx @mui/envinfo` ``` ❯ npx @mui/envinfo Need to install the following packages: @mui/envinfo Ok to proceed? (y) y System: OS: macOS 11.6 Binaries: Node: 17.0.1 - /usr/local/bin/node Yarn: 1.22.10 - /usr/local/bin/yarn npm: 8.1.0 - /usr/local/bin/npm Browsers: Chrome: 96.0.4664.55 Edge: Not Found Firefox: 85.0.2 Safari: 15.0 npmPackages: @emotion/react: ^11.6.0 => 11.6.0 @emotion/styled: ^11.6.0 => 11.6.0 @mui/core: 5.0.0-alpha.54 @mui/icons-material: ^5.1.0 => 5.1.0 @mui/lab: ^5.0.0-alpha.54 => 5.0.0-alpha.54 @mui/material: ^5.1.0 => 5.1.0 @mui/private-theming: 5.1.0 @mui/styled-engine: 5.1.0 @mui/styles: ^5.1.0 => 5.1.0 @mui/system: 5.1.0 @mui/types: 7.1.0 @mui/utils: 5.1.0 @types/react: ^16.8.0 => 16.14.8 react: ^16.8.4 => 16.14.0 react-dom: ^16.8.4 => 16.14.0 typescript: ^3.9.7 => 3.9.10 ```
fkorotkov commented 2 years ago

I guess the regression might be introduced by @vicasas in #24604: https://github.com/mui-org/material-ui/commit/1842ce8d0c735c2cdcd40b00b7405aa11030ffbf#diff-8fd90cfbeeb4f4c26dee82bfb29db2339a9908640b9368edbffa8cf37e4ba745R43

Not sure why :not(:first-child) was changed to :not(:first-of-type). 🤔

hbjORbj commented 2 years ago

@fkorotkov Thanks for creating the PR. I am afraid your codesandbox is empty. Would you update it? Thanks.

fkorotkov commented 2 years ago

@hbjORbj my bad! Just recreated it and made sure it got saved. Seems I'm to used to IntelliJ's auto-save so I didn't press Cmd+S. 🤦

siriwatknp commented 2 years ago

Not sure why :not(:first-child) was changed to :not(:first-of-type)

I think it is changed due to the emotion warning to not use :first-child. The best solution I can see is to use CSS property gap but from caniuse, it is around 89%.

in v6, we will probably fix this with gap.

isti115 commented 12 months ago

For anyone still using MUI v5 and facing this issue, you can manually override the styling when creating your theme:

createTheme({
  components: {
    MuiCardActions: {
      styleOverrides: {
        root: ({ theme }) => ({
          '& > :not(:first-of-type)': {
            marginLeft: 'unset',
          },

          gap: theme.spacing(1),
        }),
      },
    },
  },
})

Since the support for gap is now above 95%, I would suggest using that, but the same technique can also be applied if you'd like to stick with margins:

createTheme({
  components: {
    MuiCardActions: {
      styleOverrides: {
        root: ({ theme }) => ({
          '& > :not(:first-of-type)': {
            marginLeft: 'unset',
          },

          '& > :not(:first-child)': {
            marginLeft: theme.spacing(1),
          },
        }),
      },
    },
  },
})