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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[theme] Add warning, success and info colors to the palette #18820

Merged

Conversation

r3dm1ke
Copy link
Contributor

@r3dm1ke r3dm1ke commented Dec 13, 2019

Preview: https://deploy-preview-18820--material-ui.netlify.com/customization/palette/#palette.

Related to #13875 . I have set up the basics for the new colors in the palette, but I have a few questions:

  • For some reason, the test in packages/material-ui/src/styles/createPalette.test.js:681:14 is failing, though it does not look like I changed anything related to it.
  • Am I correct to assume that all the documentation for theming and color palettes is written by hand and not autogenerated?

Also, I am not a designer 😅, thus the colors way not be as suitable, feel free to suggest new ones.

@r3dm1ke r3dm1ke changed the title Added warning, success and info colors to the palette [theme] Added warning, success and info colors to the palette Dec 13, 2019
Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a good opportunity to improve our documentation for the colors :)

packages/material-ui/src/styles/createPalette.test.js Outdated Show resolved Hide resolved
@sakulstra
Copy link
Contributor

Wondering if/how this related to issues/prs like #17475 - won't the support of more colors in palette raise a lot of questions like why does component x only support primary and secondary but not...?

@oliviertassinari
Copy link
Member

@sakulstra agree, the goal would be to support them all with dynamic styles.

@mbrookes mbrookes changed the title [theme] Added warning, success and info colors to the palette [theme] Add warning, success and info colors to the palette Dec 13, 2019
@oliviertassinari oliviertassinari added the new feature New feature or request label Dec 16, 2019
@r3dm1ke
Copy link
Contributor Author

r3dm1ke commented Dec 17, 2019

I have attempted to refactor the unit tests in createPalette.js to cut down on redundancy and repetition. I am not sure this is the way to go, but probably somewhere in this direction. I have refactored the first two unit tests, to discuss the process before moving on to the next ones. From what I have observed, the line count stayed the same in these tests, even after adding 3 more colors, so there is an improvement for sure.

@r3dm1ke
Copy link
Contributor Author

r3dm1ke commented Dec 17, 2019

I also added documentation for the extra colors. I have edited sections System->Palette and Customization->Theming->Palette, I did not find any other sections that mention different color variants.

@r3dm1ke
Copy link
Contributor Author

r3dm1ke commented Dec 18, 2019

While investigating the failing builds, I came across these lines in `/docs/src/pages/premium-themes/onepirate/modules/theme.js:

const rawTheme = createMuiTheme({
  palette: {
    primary: {
    ...
    },
    ...
    success: {
      xLight: green[50],
      dark: green[700],
    },
}

This throws an error, as no main tint is supplied to success color. This was written without expectations of us adding success to the palette, so I can understand where this is coming from. However, this line throws an error now, as no main tint is supplied to the success color. The same behavior applies to primary, secondary and error colors. @oliviertassinari how do you think I should proceed? Either leave it as is and change these lines to make use of the new API, or change the API so it would accept colors without a main?

@mbrookes
Copy link
Member

mbrookes commented Dec 18, 2019

I think we should go with the first approach. Accepting colors without main will break components that depend on it, and augmenting the palette color to calculate main from light or dark will add further complexity.

@oliviertassinari
Copy link
Member

@r3dm1ke I have tried a different set of colors, let me know what you think about it :).

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 19, 2019

@material-ui/core: parsed: +0.12% , gzip: +0.19%
@material-ui/lab: parsed: +0.57% , gzip: +0.85%

Details of bundle changes.

Comparing: 0b2196c...727efb4

bundle Size Change Size Gzip Change Gzip
SwipeableDrawer ▲ +1.02 kB (+1.12% ) 91.6 kB ▲ +743 B (+2.66% ) 28.7 kB
SpeedDialAction ▲ +1.02 kB (+0.88% ) 117 kB ▲ +454 B (+1.24% ) 37 kB
Dialog ▲ +1.02 kB (+1.25% ) 82.4 kB ▲ +400 B (+1.57% ) 25.8 kB
Tabs ▲ +1.01 kB (+1.21% ) 85.1 kB ▲ +601 B (+2.27% ) 27.1 kB
Slider ▲ +1.01 kB (+1.37% ) 75.3 kB ▲ +576 B (+2.47% ) 23.9 kB
CardActionArea ▲ +1.01 kB (+1.38% ) 74.7 kB ▲ +532 B (+2.30% ) 23.6 kB
Link ▲ +1.01 kB (+1.55% ) 66.3 kB ▲ +524 B (+2.55% ) 21.1 kB
Switch ▲ +1.01 kB (+1.27% ) 80.8 kB ▲ +515 B (+2.06% ) 25.5 kB
Radio ▲ +1.01 kB (+1.24% ) 82.5 kB ▲ +501 B (+1.95% ) 26.1 kB
Box ▲ +1.01 kB (+1.46% ) 70.4 kB ▲ +498 B (+2.37% ) 21.5 kB
Rating ▲ +1.01 kB (+1.48% ) 69.7 kB ▲ +495 B (+2.25% ) 22.5 kB
CardHeader ▲ +1.01 kB (+1.59% ) 64.8 kB ▲ +485 B (+2.43% ) 20.4 kB
AppBar ▲ +1.01 kB (+1.62% ) 63.6 kB ▲ +484 B (+2.48% ) 20 kB
Checkbox ▲ +1.01 kB (+1.26% ) 81.6 kB ▲ +484 B (+1.91% ) 25.9 kB
Tab ▲ +1.01 kB (+1.35% ) 76 kB ▲ +476 B (+2.01% ) 24.1 kB
ButtonGroup ▲ +1.01 kB (+1.24% ) 82.8 kB ▲ +475 B (+1.90% ) 25.4 kB
TableSortLabel ▲ +1.01 kB (+1.33% ) 77 kB ▲ +470 B (+1.97% ) 24.3 kB
FormControlLabel ▲ +1.01 kB (+1.58% ) 65.2 kB ▲ +467 B (+2.33% ) 20.5 kB
ExpansionPanel ▲ +1.01 kB (+1.45% ) 71.1 kB ▲ +465 B (+2.13% ) 22.3 kB
ListItemText ▲ +1.01 kB (+1.59% ) 64.6 kB ▲ +463 B (+2.33% ) 20.4 kB
StepButton ▲ +1.01 kB (+1.25% ) 82 kB ▲ +462 B (+1.81% ) 26 kB
StepContent ▲ +1.01 kB (+1.50% ) 68.7 kB ▲ +462 B (+2.19% ) 21.6 kB
InputAdornment ▲ +1.01 kB (+1.59% ) 64.8 kB ▲ +460 B (+2.30% ) 20.4 kB
Breadcrumbs ▲ +1.01 kB (+1.52% ) 67.7 kB ▲ +458 B (+2.20% ) 21.3 kB
GridListTileBar ▲ +1.01 kB (+1.64% ) 62.9 kB ▲ +455 B (+2.35% ) 19.8 kB
GridListTile ▲ +1.01 kB (+1.63% ) 63.4 kB ▲ +453 B (+2.33% ) 19.9 kB
Step ▲ +1.01 kB (+1.65% ) 62.3 kB ▲ +452 B (+2.36% ) 19.6 kB
Icon ▲ +1.01 kB (+1.65% ) 62.5 kB ▲ +450 B (+2.34% ) 19.6 kB
MobileStepper ▲ +1.01 kB (+1.52% ) 67.5 kB ▲ +449 B (+2.16% ) 21.2 kB
SpeedDial ▲ +1.01 kB (+1.20% ) 85.7 kB ▲ +449 B (+1.68% ) 27.1 kB
SpeedDialIcon ▲ +1.01 kB (+1.60% ) 64.3 kB ▲ +444 B (+2.25% ) 20.2 kB
Avatar ▲ +1.01 kB (+1.59% ) 64.9 kB ▲ +443 B (+2.20% ) 20.6 kB
Container ▲ +1.01 kB (+1.64% ) 62.9 kB ▲ +442 B (+2.29% ) 19.7 kB
ToggleButtonGroup ▲ +1.01 kB (+1.64% ) 62.9 kB ▲ +442 B (+2.28% ) 19.8 kB
BottomNavigationAction ▲ +1.01 kB (+1.37% ) 75.2 kB ▲ +440 B (+1.88% ) 23.8 kB
Toolbar ▲ +1.01 kB (+1.66% ) 62 kB ▲ +440 B (+2.30% ) 19.6 kB
DialogContentText ▲ +1.01 kB (+1.62% ) 63.7 kB ▲ +438 B (+2.23% ) 20.1 kB
Table ▲ +1.01 kB (+1.66% ) 62.2 kB ▲ +438 B (+2.29% ) 19.6 kB
AvatarGroup ▲ +1.01 kB (+1.66% ) 62 kB ▲ +437 B (+2.29% ) 19.5 kB
Skeleton ▲ +1.01 kB (+1.66% ) 62.3 kB ▲ +436 B (+2.26% ) 19.7 kB
StepConnector ▲ +1.01 kB (+1.65% ) 62.4 kB ▲ +436 B (+2.26% ) 19.7 kB
Divider ▲ +1.01 kB (+1.66% ) 62.3 kB ▲ +435 B (+2.27% ) 19.6 kB
Grid ▲ +1.01 kB (+1.59% ) 64.8 kB ▲ +435 B (+2.18% ) 20.4 kB
GridList ▲ +1.01 kB (+1.66% ) 62.2 kB ▲ +435 B (+2.27% ) 19.6 kB
CircularProgress ▲ +1.01 kB (+1.62% ) 63.8 kB ▲ +433 B (+2.19% ) 20.2 kB
TableBody ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +433 B (+2.28% ) 19.4 kB
TableRow ▲ +1.01 kB (+1.66% ) 62.2 kB ▲ +433 B (+2.26% ) 19.6 kB
BottomNavigation ▲ +1.01 kB (+1.66% ) 62.1 kB ▲ +432 B (+2.26% ) 19.5 kB
CardMedia ▲ +1.01 kB (+1.66% ) 62 kB ▲ +432 B (+2.26% ) 19.6 kB
Hidden ▲ +1.01 kB (+1.57% ) 65.6 kB ▲ +432 B (+2.14% ) 20.7 kB
ListItemIcon ▲ +1.01 kB (+1.67% ) 61.9 kB ▲ +432 B (+2.27% ) 19.4 kB
TableFooter ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +432 B (+2.28% ) 19.4 kB
TableHead ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +432 B (+2.28% ) 19.4 kB
CssBaseline ▲ +1.01 kB (+1.80% ) 57.3 kB ▲ +431 B (+2.45% ) 18 kB
FormGroup ▲ +1.01 kB (+1.67% ) 61.7 kB ▲ +431 B (+2.27% ) 19.4 kB
ListItemAvatar ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +431 B (+2.27% ) 19.4 kB
DialogTitle ▲ +1.01 kB (+1.61% ) 64 kB ▲ +430 B (+2.18% ) 20.1 kB
TreeItem ▲ +1.01 kB (+1.40% ) 73.4 kB ▲ +430 B (+1.88% ) 23.3 kB
ListSubheader ▲ +1.01 kB (+1.65% ) 62.4 kB ▲ +428 B (+2.22% ) 19.7 kB
SnackbarContent ▲ +1.01 kB (+1.63% ) 63.2 kB ▲ +428 B (+2.19% ) 20 kB
Card ▲ +1.01 kB (+1.65% ) 62.5 kB ▲ +427 B (+2.22% ) 19.6 kB
ListItemSecondaryAction ▲ +1.01 kB (+1.67% ) 61.7 kB ▲ +427 B (+2.25% ) 19.4 kB
NativeSelect ▲ +1.01 kB (+1.34% ) 76.5 kB ▲ +426 B (+1.79% ) 24.2 kB
TableCell ▲ +1.01 kB (+1.62% ) 63.7 kB ▲ +425 B (+2.16% ) 20.1 kB
TableContainer ▲ +1.01 kB (+1.67% ) 61.7 kB ▲ +424 B (+2.24% ) 19.4 kB
Badge ▲ +1.01 kB (+1.58% ) 65 kB ▲ +423 B (+2.13% ) 20.3 kB
DialogActions ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +422 B (+2.22% ) 19.4 kB
ExpansionPanelDetails ▲ +1.01 kB (+1.67% ) 61.6 kB ▲ +421 B (+2.22% ) 19.4 kB
FormHelperText ▲ +1.01 kB (+1.64% ) 63 kB ▲ +421 B (+2.17% ) 19.8 kB
Snackbar ▲ +1.01 kB (+1.37% ) 74.8 kB ▲ +421 B (+1.83% ) 23.4 kB
CardActions ▲ +1.01 kB (+1.67% ) 61.7 kB ▲ +420 B (+2.21% ) 19.4 kB
DialogContent ▲ +1.01 kB (+1.67% ) 61.9 kB ▲ +420 B (+2.21% ) 19.5 kB
LinearProgress ▲ +1.01 kB (+1.58% ) 65 kB ▲ +420 B (+2.11% ) 20.4 kB
TreeView ▲ +1.01 kB (+1.56% ) 66.1 kB ▲ +417 B (+2.04% ) 20.8 kB
Zoom ▲ +1.01 kB (+4.55% ) 23.3 kB ▲ +417 B (+5.43% ) 8.1 kB
CardContent ▲ +1.01 kB (+1.67% ) 61.7 kB ▲ +416 B (+2.20% ) 19.4 kB
ExpansionPanelActions ▲ +1.01 kB (+1.67% ) 61.8 kB ▲ +415 B (+2.18% ) 19.4 kB
ToggleButton ▲ +1.01 kB (+1.36% ) 75.8 kB ▲ +415 B (+1.76% ) 24.1 kB
Slide ▲ +1.01 kB (+4.18% ) 25.3 kB ▲ +412 B (+4.96% ) 8.71 kB
RadioGroup ▲ +1.01 kB (+1.64% ) 62.9 kB ▲ +410 B (+2.12% ) 19.8 kB
Stepper ▲ +1.01 kB (+1.60% ) 64.6 kB ▲ +400 B (+2.00% ) 20.4 kB
ExpansionPanelSummary ▲ +1.01 kB (+1.32% ) 77.8 kB ▲ +376 B (+1.55% ) 24.6 kB
StepLabel ▲ +1.01 kB (+1.50% ) 68.3 kB ▲ +68 B (+0.32% ) 21.2 kB
InputLabel ▲ +1.01 kB (+1.58% ) 65 kB ▲ +177 B (+0.89% ) 20.1 kB
Fade ▲ +1.01 kB (+4.53% ) 23.2 kB ▲ +301 B (+3.92% ) 7.98 kB
Drawer ▲ +1.01 kB (+1.21% ) 84.2 kB ▲ +477 B (+1.89% ) 25.7 kB
Popover ▲ +1.01 kB (+1.24% ) 82.5 kB ▲ +474 B (+1.89% ) 25.6 kB
@material-ui/lab ▲ +1.01 kB (+0.57% ) 176 kB ▲ +448 B (+0.85% ) 53.1 kB
Menu ▲ +1.01 kB (+1.16% ) 88.1 kB ▲ +439 B (+1.64% ) 27.2 kB
Tooltip ▲ +1.01 kB (+1.01% ) 101 kB ▲ +439 B (+1.40% ) 31.9 kB
InputBase ▲ +1.01 kB (+1.45% ) 70.3 kB ▲ +435 B (+2.01% ) 22.1 kB
TextField ▲ +1.01 kB (+0.83% ) 123 kB ▲ +435 B (+1.22% ) 36.1 kB
Autocomplete ▲ +1.01 kB (+0.79% ) 129 kB ▲ +433 B (+1.08% ) 40.4 kB
Select ▲ +1.01 kB (+0.89% ) 114 kB ▲ +433 B (+1.29% ) 34 kB
TablePagination ▲ +1.01 kB (+0.72% ) 141 kB ▲ +431 B (+1.05% ) 41.3 kB
Fab ▲ +1 kB (+1.33% ) 76.5 kB ▲ +437 B (+1.86% ) 23.9 kB
OutlinedInput ▲ +1 kB (+1.38% ) 73.7 kB ▲ +437 B (+1.94% ) 23 kB
Backdrop ▲ +1 kB (+1.51% ) 67.4 kB ▲ +436 B (+2.13% ) 20.9 kB
IconButton ▲ +1 kB (+1.34% ) 75.8 kB ▲ +436 B (+1.87% ) 23.7 kB
MenuList ▲ +1 kB (+1.55% ) 65.7 kB ▲ +436 B (+2.16% ) 20.6 kB
Button ▲ +1 kB (+1.28% ) 79.4 kB ▲ +435 B (+1.82% ) 24.4 kB
Typography ▲ +1 kB (+1.61% ) 63.3 kB ▲ +434 B (+2.23% ) 19.9 kB
Collapse ▲ +1 kB (+1.51% ) 67.6 kB ▲ +432 B (+2.10% ) 21 kB
FormLabel ▲ +1 kB (+1.62% ) 63.1 kB ▲ +430 B (+2.24% ) 19.6 kB
Chip ▲ +1 kB (+1.24% ) 82.2 kB ▲ +429 B (+1.73% ) 25.2 kB
FilledInput ▲ +1 kB (+1.39% ) 73.2 kB ▲ +429 B (+1.92% ) 22.8 kB
Input ▲ +1 kB (+1.41% ) 72.2 kB ▲ +429 B (+1.94% ) 22.6 kB
ListItem ▲ +1 kB (+1.33% ) 76.8 kB ▲ +429 B (+1.82% ) 24.1 kB
MenuItem ▲ +1 kB (+1.31% ) 77.8 kB ▲ +429 B (+1.79% ) 24.4 kB
Paper ▲ +1 kB (+1.65% ) 62 kB ▲ +427 B (+2.25% ) 19.4 kB
ButtonBase ▲ +1 kB (+1.38% ) 73.6 kB ▲ +426 B (+1.87% ) 23.2 kB
List ▲ +1 kB (+1.65% ) 62 kB ▲ +425 B (+2.24% ) 19.4 kB
StepIcon ▲ +1 kB (+1.59% ) 64.3 kB ▲ +425 B (+2.16% ) 20.1 kB
FormControl ▲ +1 kB (+1.59% ) 64.1 kB ▲ +423 B (+2.16% ) 20 kB
SvgIcon ▲ +1 kB (+1.63% ) 62.7 kB ▲ +422 B (+2.20% ) 19.6 kB
Grow ▲ +1 kB (+4.40% ) 23.8 kB ▲ +395 B (+5.07% ) 8.19 kB
styles/createMuiTheme ▲ +1 kB (+6.51% ) 16.4 kB ▲ +388 B (+7.13% ) 5.83 kB
docs.main ▲ +792 B (+0.13% ) 612 kB ▲ +362 B (+0.19% ) 196 kB
@material-ui/core ▲ +429 B (+0.12% ) 358 kB ▲ +184 B (+0.19% ) 97.6 kB
@material-ui/core[umd] ▲ +276 B (+0.09% ) 314 kB ▲ +136 B (+0.15% ) 90.6 kB
Modal ▲ +2 B (+0.01% ) 14.3 kB ▲ +13 B (+0.26% ) 5.01 kB
Popper ▲ +2 B (+0.01% ) 28.7 kB ▲ +4 B (+0.04% ) 10.3 kB
Portal ▲ +2 B (+0.07% ) 2.9 kB ▼ -2 B (-0.15% ) 1.3 kB
@material-ui/styles -- 50.8 kB ▼ -4 B (-0.03% ) 15.3 kB
@material-ui/system -- 14.5 kB ▲ +4 B (+0.10% ) 4.04 kB
TextareaAutosize -- 5.09 kB ▲ +3 B (+0.14% ) 2.14 kB
ClickAwayListener -- 3.85 kB ▼ -2 B (-0.13% ) 1.55 kB
NoSsr -- 2.19 kB ▲ +2 B (+0.19% ) 1.04 kB
useMediaQuery -- 2.5 kB ▼ -2 B (-0.19% ) 1.05 kB
useAutocomplete -- 12.6 kB ▲ +1 B (+0.02% ) 4.7 kB
colorManipulator -- 3.85 kB -- 1.52 kB
docs.landing -- 50.3 kB -- 12.2 kB
RootRef -- 4.21 kB -- 1.64 kB

Generated by 🚫 dangerJS against 727efb4

@oliviertassinari oliviertassinari force-pushed the 13875-add-additional-colors-to-palette branch from f848247 to 3f93d60 Compare December 19, 2019 13:33
@oliviertassinari oliviertassinari force-pushed the 13875-add-additional-colors-to-palette branch from 3f93d60 to 9139a0d Compare December 19, 2019 13:38
@oliviertassinari
Copy link
Member

I have done a second batch of ambitious changes. I think that it's good enough now, but it's only my perspective, happy to keep iterating.

@r3dm1ke
Copy link
Contributor Author

r3dm1ke commented Dec 19, 2019

I love the new colors! I do not see anything else that can be improved in this particular PR, I think it is ready to be merged

@r3dm1ke r3dm1ke marked this pull request as ready for review December 19, 2019 23:16
@oliviertassinari oliviertassinari merged commit a1082ee into mui:master Dec 21, 2019
@oliviertassinari
Copy link
Member

Well done :)

@r3dm1ke r3dm1ke deleted the 13875-add-additional-colors-to-palette branch December 23, 2019 21:23
@bluefire2121
Copy link

Amazing job. Just in time for the holidays! Thank you!

@oliviertassinari
Copy link
Member

We might have introduced a breaking change for people that already have a success color in the palette as reported in hupe1980/gatsby-theme-material-ui#14. We will see how this problem evolves.

@calvinl
Copy link

calvinl commented Jan 2, 2020

It seems as though the new color intentions aren't supported yet by most of the APIs that have a color prop, like Button or IconButton.

At least looking in the documentation, these components still only accept default, primary, or secondary. Is this expected?

@calvinl
Copy link

calvinl commented Jan 2, 2020

Just noticed #18966. I think that's related, but I would expect that even if the prop types fail, the color would still be applied and a props error would just be thrown? I'm not seeing any color changes at all.

@oliviertassinari
Copy link
Member

@calvinl One step at the time :)

@calvinl
Copy link

calvinl commented Jan 3, 2020

@oliviertassinari apologies, I got a little excited :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants