material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

[IconButton] `iconButtonClasses` does not contain classes for `colorError`, `colorInfo`, etc.

Open lindapaiste opened this issue 2 years ago • 7 comments

Duplicates

  • [X] I have searched the existing issues

Latest version

  • [X] I have tested the latest version

Current behavior 😯

The color prop on the IconButton component allows any of the theme.palatte colors, but the iconButtonClasses object only contains properties for colorInherit, colorPrimary, and colorSecondary.

If I want to target IconButtons with color="error" using styled or sx then I have to use a hardcoded value of '.MuiIconButton-colorError' (or look at the props directly).


The color prop will work correctly for any color in the theme:

https://github.com/mui/material-ui/blob/dd4308dc76b05bdb648c300c37054aa74c2b0cfe/packages/mui-material/src/IconButton/IconButton.js#L76-L78

It adds a targetable className MuiIconButton-colorError to the HTML:

<button
  class="MuiButtonBase-root MuiIconButton-root MuiIconButton-colorError MuiIconButton-sizeMedium css-l0zd1g-MuiButtonBase-root-MuiIconButton-root"
  tabindex="0"
  type="button"
>
  <svg /* ... */ >
</button>

The documentation lists many accepted color values:

'inherit'
| 'default'
| 'primary'
| 'secondary'
| 'error'
| 'info'
| 'success'
| 'warning'
| string

Only 3 of these colors are present in iconButtonClasses:

https://github.com/mui/material-ui/blob/dd4308dc76b05bdb648c300c37054aa74c2b0cfe/packages/mui-material/src/IconButton/iconButtonClasses.ts#L10-L15

Expected behavior 🤔

I would expect that I could use .${iconButtonClasses.colorError}.

I'm not sure if it's intentional that the "status" colors are missing or if it's an oversight. Other components such as Icon do have a colorError class (but not colorWarning, etc.).

Steps to reproduce 🕹

<IconButton color="error"></IconButton>

Context 🔦

I am trying to style a row of IconButtons and apply different hover colors depending on the component's color prop.

Your environment 🌎

npx @mui/envinfo
  System:
    OS: Windows 10 10.0.19041
    CPU: (4) x64 Intel(R) Core(TM) i5-6600K CPU @ 3.50GHz
    Memory: 1.04 GB / 15.89 GB
  Binaries:
    Node: 16.13.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.9.4 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 8.5.5 - C:\Program Files\nodejs\npm.CMD
  Managers:
    Composer: 1.8.5 - C:\ProgramData\ComposerSetup\bin\composer.BAT
    pip3: 21.2.4 - C:\Python310\Scripts\pip3.EXE
  Utilities:
    Git: 2.33.1. - C:\Program Files\Git\cmd\git.EXE
    FFmpeg: 4.2 - C:\Program Files\ImageMagick-7.0.10-Q16\ffmpeg.EXE
  IDEs:
    Android Studio: Version  3.5.0.0 AI-191.8026.42.35.5791312
    VSCode: 1.68.1 - C:\Users\Linda Paiste\AppData\Local\Programs\Microsoft VS Code\bin\code.CMD
  Languages:
    Bash: 5.0.17 - C:\WINDOWS\system32\bash.EXE
    Go: 1.18.3 - C:\Program Files\Go\bin\go.EXE
    PHP: 7.3.4 - C:\PHP7\php.EXE
    Python: 3.10.2 - C:\Python310\python.EXE
  Databases:
    SQLite: 3.28.0 - C:\Users\Linda Paiste\AppData\Local\Android\Sdk\platform-tools\sqlite3.EXE
  Browsers:
    Chrome: 103.0.5060.114
    Edge: Spartan (44.19041.1266.0), Chromium (103.0.1264.62)
    Internet Explorer: 11.0.19041.1202

lindapaiste avatar Jul 22 '22 17:07 lindapaiste

@lindapaiste Thanks for the detailed issue. Yes, some color properties are missing in IconButtonClasses type. Would you like to create a PR for it?


When you said looking at the props directly, I think you mean something along these lines with the styled API https://codesandbox.io/s/upbeat-heyrovsky-he993o. Note that this approach is more scalable as the classes like colorError etc won't scale in long term.

ZeeshanTamboli avatar Jul 27 '22 06:07 ZeeshanTamboli

@lindapaiste

Can you provide a codesandbox with example how you are going to use the exisisting colorInherit or colorPrimary or colorSecondary please ?

Use this codesandbox to prepare sample code

PunitSoniME avatar Jul 29 '22 11:07 PunitSoniME

Is anyone still working on this issue? If not, can I take it?

Zetta56 avatar Aug 05 '22 20:08 Zetta56

@Zetta56

I guess someone has already create a duplicate issue #33246 and created a PR 33801

PunitSoniME avatar Aug 06 '22 06:08 PunitSoniME

I guess someone has already create a duplicate issue #33246 and created a PR 33801

@PunitSoniME That issue is for Chip component, this one is for IconButton.

ZeeshanTamboli avatar Aug 06 '22 07:08 ZeeshanTamboli

I guess someone has already create a duplicate issue #33246 and created a PR 33801

@PunitSoniME That issue is for Chip component, this one is for IconButton.

oh my bad, sorry 😃

@Zetta56 you can take it, if you are not taking please let me know, I will pick this up

PunitSoniME avatar Aug 06 '22 08:08 PunitSoniME

When you said looking at the props directly, I think you mean something along these lines with the styled API https://codesandbox.io/s/upbeat-heyrovsky-he993o. Note that this approach is more scalable as the classes like colorError etc won't scale in long term.

Yes, that's what I ended up doing.

I have a row of IconButton components which are all the same color. I want to have a different :hover color for "destructive" actions, and I am using color="error" to denote the destructive buttons. image image

Here is a CodeSandbox with a working code

const MyStyledButtonWithStyledApi = styled(IconButton)(({ theme, color }) => ({
  flex: 1,
  margin: 1,
  borderRadius: 0,
  [`& svg`]: {
    color: blueGrey["400"]
  },
  [`&:hover, &.Mui-focusVisible`]: {
    backgroundColor: alpha(theme.palette.primary.main, 0.1),
    [`& svg`]: {
      color: color === "error" ? theme.palette.error.main : blueGrey["700"]
    }
  }
}));

The iconButtonClasses approach would open up the possibility of using styled on the parent container to target children which have color="error". I don't think that's objectively better, but it's not possible right now.

Here is a CodeSandbox using iconButtonClasses.colorError from a parent

const MyStyledContainerWithStyledApi = styled("div")(({ theme }) => ({
  display: "flex",
  [`& .${iconButtonClasses.root}`]: {
    flex: 1,
    margin: 1,
    borderRadius: 0,
    [`& svg`]: {
      color: blueGrey["400"]
    },
    [`&:hover, &.Mui-focusVisible`]: {
      backgroundColor: alpha(theme.palette.primary.main, 0.1),
      // set the standard hover color for all
      [`& svg`]: {
        color: blueGrey["700"]
      },
      // override the hover color just for error buttons
      [`&.${iconButtonClasses.colorError} svg`]: {
        color: theme.palette.error.main
      }
    }
  }
}));

lindapaiste avatar Aug 06 '22 18:08 lindapaiste