material-ui: [design-system] Support error|success|warning|info by default in color prop

The issue is present in the latest release.
I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

It already supported out of box in V5.

https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Button/Button.js#L137 https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Button/Button.js#L116 https://github.com/mui-org/material-ui/blob/next/packages/material-ui/src/Button/Button.js#L116

User could add it to theme via interface merge, but it would be nice to add it in core

declare module '@material-ui/core/Button' {
  interface ButtonPropsColorOverrides {
    error: true;
    info: true;
    success: true;
    warning: true;
  }
}

About this issue

Original URL
State: open
Created 3 years ago
Reactions: 4
Comments: 30 (27 by maintainers)

Most upvoted comments

It seems that we have a working POC in https://github.com/mui-org/material-ui/issues/24778#issuecomment-774627820 and we have a product direction agreement. I’m changing the label 😃

oliviertassinari on Apr 2, 2021

@oliviertassinari @mnajdova What do you think about,

fix all the type issue for the components already support all color palette (eg. Chip, Badge, …a lot more).
improve some component to support all color palette (eg. Fab)
work on color extension suggested by this comment

For 1 and 2, we can start separately now since (1) is bug fix and (2) is enhancement. For 3, wait for someone to pick up or we can work on this in v5 beta? (is it a breaking change?)

siriwatknp on Jan 30, 2023

I like the idea of inverting the logic, as in my opinion using all values of the palette may be the most common use-case (if I am wrong in this assumption then it doesn’t make sense that we do the changes). If we do this change, that means that only the more advanced scenarios will require module augmentation (wanting to restrict some colors). No objection from my side, we just have to make sure to nicely document this breaking change.

mnajdova on Apr 2, 2021

@oliviertassinari yes it could work but I can pass my own created colors from in palette keys using PaletteColor type, like “tertiary”, “silver” and more, Button component actually even now can do it no changes needed, the problem only with typings how to provide it. The biggest problem I see is class generation:

Create a type

import { Palette, PaletteColor } from "@material-ui/core/styles";

type PaletteColorKeys = keyof Pick<
  Palette,
  {
    [P in keyof Palette]: Palette[P] extends PaletteColor ? P : never;
  }[keyof Palette]
>;

DefaultPaletteColorKeys = ‘primary’ | ‘secondary’ | ‘success’ | ‘error’ | ‘info’ | ‘warning’; // for class generation and slots Generate classes only by default keys if someone implements there own do not generate them.
Apply to button props, color property as PaletteColorKeys, and classes for default palette classes.

Playground

No breaking changes and you introduced a really good API.

And ButtonPropsColorOverrides not needed anymore for augmentation (only if you want to disable colors).

povilass on Jun 6, 2021

@manorinfinity @oliviertassinari From my comment, what’s left is (3) but we could split it into another issue since it is related to color extension.

I think we can close this issue.

siriwatknp on Jan 30, 2023

What we might miss is the ability to easily add new colors, but I’m not the best person to ask.

oliviertassinari on Jan 29, 2023

The plan sounds great. The outcome will likely make many developers happy. I have added it to the v5.1 milestone to communicate that we want to get it in sooner rather than later.

oliviertassinari on Jun 10, 2021

I really like @povilass solution to this! Will it make the solution in #13875 obsolete ?

@mngu I don’t think it makes the solution obsolete. Even if we inverse the logic, there is always a need to restrict the customization options available.

oliviertassinari on Mar 5, 2021

@povilass How about we move forward with https://github.com/mui-org/material-ui/issues/24778#issuecomment-774552626 but delay the idea to automatically support in TypeScript any theme.palette properties added in the theme?

Supporting the existing colors in the theme is aligned with the idea to have added success/error/info/warning in the palette in the first place.

At this point, my main concern is that we can’t easily know which keys are compatible. In our rebranding many keys aren’t: https://github.com/mui-org/material-ui/blob/b8c6dee91abd4919bb87ae87a1929dd8f8fcb830/docs/src/modules/branding/BrandingRoot.tsx#L54

I also believe that we should encourage design consistency, designer shouldn’t need many different new colors.

oliviertassinari on Feb 9, 2021