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

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

Closed
2 tasks done
lindapaiste opened this issue Jul 22, 2022 · 7 comments 路 Fixed by #33820
Closed
2 tasks done
Labels
bug 馃悰 Something doesn't work component: icon button This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript

Comments

@lindapaiste
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • 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:

...(ownerState.color !== 'inherit' &&
ownerState.color !== 'default' && {
color: (theme.vars || theme).palette[ownerState.color].main,

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:

/** Styles applied to the root element if `color="inherit"`. */
colorInherit: string;
/** Styles applied to the root element if `color="primary"`. */
colorPrimary: string;
/** Styles applied to the root element if `color="secondary"`. */
colorSecondary: string;

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 lindapaiste added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Jul 22, 2022
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Jul 27, 2022

@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 ZeeshanTamboli added component: icon button This is the name of the generic UI component, not the React module! typescript ready to take Help wanted. Guidance available. There is a high chance the change will be accepted and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Jul 27, 2022
@ZeeshanTamboli ZeeshanTamboli changed the title iconButtonClasses does not contain classes for colorError, colorInfo, etc. [IconButton] iconButtonClasses does not contain classes for colorError, colorInfo, etc. Jul 27, 2022
@ZeeshanTamboli ZeeshanTamboli added the bug 馃悰 Something doesn't work label Jul 27, 2022
@PunitSoniME
Copy link
Contributor

PunitSoniME commented Jul 29, 2022

@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

@Zetta56
Copy link
Contributor

Zetta56 commented Aug 5, 2022

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

@PunitSoniME
Copy link
Contributor

@Zetta56

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

@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Aug 6, 2022

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.

@PunitSoniME
Copy link
Contributor

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

@lindapaiste
Copy link
Author

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
      }
    }
  }
}));

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: icon button This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted typescript
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants