[RFC] color prop API
Nobody has claimed this yet.
Assessment
- Difficulty
- 5/5
- Estimated time
- Over a week
- Newbie friendliness
- 25/100
- Issue type
- Feature
- Clarity
- Needs clarification
- Activity status
- Stale
- Tech stack
- react, typescript
- Domain
- frontend
Research direction
Start with the TypeScript color prop declarations and the listed AppBar, Badge, Button, Chip, Icon, IconButton, SvgIcon, and Typography implementations. Review the RFC discussion first, resolve the proposed behavior for default and inherit, and treat consistent color behavior across the listed components as done.
Written by the indexing model from the issue text.
Description
Currently our type declarations contain the following definition for color props:
type Color = 'inherit' | 'primary' | 'secondary' | 'default';
indicating that there is a library wide understanding what these color represent and that every component that has a color prop should implement each variant.
However only primary and secondary are implemented for every component with a color prop. inherit and default are not implemented in every component. default doesn't even have a consistent style.
Implementation overview
| Component | primary | secondary | inherit | default |
|---|---|---|---|---|
| AppBar | x | x | x | x |
| Badge | x | x | x | |
| Button | x | x | x | x |
| Chip | x | x | x | |
| Icon | x | x | x | |
| IconButton | x | x | x | x |
| SvgIcon | x | x | x | |
| Typography | x | x | x | x |
default variant
Implementation
| Component | color | background-color |
|---|---|---|
| AppBar | theme.palette.getContrastText(backgroundColorDefault) |
theme.palette.type === 'light' ? theme.palette.grey[100 ] : theme.palette.grey[900 ] |
| Badge | theme.palette.textColor (which is undefined) |
theme.palette.color (also undefined) |
| Button | theme.typography.button |
global stylesheet |
| Chip | theme.palette.getContrastText(backgroundColor) |
theme.palette.type === 'light' ? theme.palette.grey[300 ] : theme.palette.grey[700 ] |
| IconButton | theme.palette.action.active |
fade(theme.palette.action.active, theme.palette.action.hoverOpacity) if :hover |
| Typography | global stylesheet | global stylesheet |
Proposal
Remove it because:
- not even the actual default value for the components
- not mentioned in the material spec
- broken for
Badgewith no report (I was not able to determine when this actually broke but I guess this happened a few months ago; Edit: passed undefined even in 1.0.0-alpha.2)
People can always set the color prop to undefined which will result in no applied css rules concerning color which is a proper default in my opinion.
inherit variant
Implementation
| Component | color | backgroundColor |
|---|---|---|
| AppBar | global stylesheet | global stylesheet |
| Button | inherit |
global stylesheet |
| Icon | global stylesheet | global stylesheet |
| IconButton | inherit |
global stylesheet |
| SvgIcon | global stylesheet | global stylesheet |
| Typography | inherit |
global stylesheet |
Funny enough in Icon fontSize="inherit" color="inherit" causes font-size: inherit; but no defined color in css.
Also the default for fontSize in those components is default and applies always no css rules but the default for color is inherit which applies sometimes no css rules. This might as well be removed. There is no value in a named default value in my opinion but this is should be discussed separately.
Proposal
No strong opinion about that one. Either repurpose this as a default replacement which means color and background-color are not set or actually set inherit which is the most obvious. AppBar for example does not do anything with inherit which might be confusing.
/cc @mui-org/core-contributors
- Dominant language
- JavaScript
- Stars
- 99.1k
- Forks
- 32.5k
- Avg merge
- 2d 17h
- Merged PRs (30d)
- 106
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from mui/material-ui
-
type: new feature waiting for 👍
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
mui/material-ui#48254 · 3 comments · 1 reaction ·
-
has workaround scope: avatar waiting for 👍
Difficulty 2/5 1-3 hours Newbie friendliness 64/100
mui/material-ui#48190 · 5 comments ·
-
has workaround scope: typography type: new feature waiting for 👍
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
mui/material-ui#46488 · 2 comments · 1 reaction ·
-
has workaround scope: text field type: bug
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
mui/material-ui#37216 · 6 comments ·
-
docs scope: autocomplete
Difficulty 1/5 1-3 hours Newbie friendliness 68/100
mui/material-ui#35713 · 5 comments ·
Similar issues
-
code-quality refactoring
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
github/gh-aw-firewall#8816 ·
-
integration:quickjs org:external priority:backlog topic:code-interpreter topic:middleware type:feature
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
langchain-ai/deepagents#6450 ·
-
optimization optimization:agents-md-curator
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
githubnext/gh-aw-cao#13143 ·
-
status: needs triage
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 88/100