-
-
Notifications
You must be signed in to change notification settings - Fork 32.6k
[RFC] color prop API #13028
Copy link
Copy link
Open
Labels
RFCRequest For Comments.Request For Comments.breaking changeIntroduces changes that are not backward compatible.Introduces changes that are not backward compatible.design: materialThis is about Material Design, please involve a visual or UX designer in the processThis is about Material Design, please involve a visual or UX designer in the processdiscussionscope: all componentsWidespread work has an impact on almost all components.Widespread work has an impact on almost all components.type: new featureExpand the scope of the product to solve a new problem.Expand the scope of the product to solve a new problem.
Description
Metadata
Metadata
Assignees
Labels
RFCRequest For Comments.Request For Comments.breaking changeIntroduces changes that are not backward compatible.Introduces changes that are not backward compatible.design: materialThis is about Material Design, please involve a visual or UX designer in the processThis is about Material Design, please involve a visual or UX designer in the processdiscussionscope: all componentsWidespread work has an impact on almost all components.Widespread work has an impact on almost all components.type: new featureExpand the scope of the product to solve a new problem.Expand the scope of the product to solve a new problem.
Currently our type declarations contain the following definition for color props:
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
primaryandsecondaryare implemented for every component with a color prop.inheritanddefaultare not implemented in every component.defaultdoesn't even have a consistent style.Implementation overview
defaultvariantImplementation
theme.palette.getContrastText(backgroundColorDefault)theme.palette.type === 'light' ? theme.palette.grey[100 ] : theme.palette.grey[900 ]theme.palette.textColor(which isundefined)theme.palette.color(alsoundefined)theme.typography.buttontheme.palette.getContrastText(backgroundColor)theme.palette.type === 'light' ? theme.palette.grey[300 ] : theme.palette.grey[700 ]theme.palette.action.activefade(theme.palette.action.active, theme.palette.action.hoverOpacity)if:hoverProposal
Remove it because:
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
colorprop toundefinedwhich will result in no applied css rules concerning color which is a proper default in my opinion.inheritvariantImplementation
inheritinheritinheritFunny enough in
IconfontSize="inherit" color="inherit"causesfont-size: inherit;but no definedcolorin css.Also the default for
fontSizein those components isdefaultand applies always no css rules but the default forcolorisinheritwhich 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
defaultreplacement which means color and background-color are not set or actually setinheritwhich is the most obvious. AppBar for example does not do anything withinheritwhich might be confusing./cc @mui-org/core-contributors