[Alert] Improve dark theme coloring #19105

ahtcx · 2020-01-06T13:35:37Z

Add inverting the color utility function depending on the theme type, making the dark theme less agressive.

Closes #19104

I have followed (at least) the PR section of the contributing guide.

mui-pr-bot · 2020-01-06T13:46:02Z

@material-ui/lab: parsed: -0.20% 😍, gzip: -0.03% 😍

Details of bundle changes.

Comparing: 4f9c2fc...594ec67

bundle	Size Change	Size	Gzip Change	Gzip
Alert	▼ -363 B (-0.43% )	83.6 kB	▼ -27 B (-0.10% )	26.4 kB
@material-ui/lab	▼ -363 B (-0.20% )	183 kB	▼ -19 B (-0.03% )	55 kB
@material-ui/core	--	359 kB	--	98.6 kB
@material-ui/core[umd]	--	314 kB	--	90.8 kB
@material-ui/styles	--	51.3 kB	--	15.5 kB
@material-ui/system	--	14.5 kB	--	4.05 kB
AlertTitle	--	64.4 kB	--	20.4 kB
AppBar	--	64.2 kB	--	20.2 kB
Autocomplete	--	131 kB	--	41.2 kB
Avatar	--	65.5 kB	--	20.7 kB
AvatarGroup	--	62.5 kB	--	19.7 kB
Backdrop	--	68 kB	--	21.1 kB
Badge	--	65.6 kB	--	20.5 kB
BottomNavigation	--	62.7 kB	--	19.7 kB
BottomNavigationAction	--	75.7 kB	--	24 kB
Box	--	71 kB	--	21.7 kB
Breadcrumbs	--	68.2 kB	--	21.5 kB
Button	--	79.9 kB	--	24.6 kB
ButtonBase	--	74.2 kB	--	23.4 kB
ButtonGroup	--	83.4 kB	--	25.6 kB
Card	--	63.1 kB	--	19.8 kB
CardActionArea	--	75.3 kB	--	23.8 kB
CardActions	--	62.3 kB	--	19.6 kB
CardContent	--	62.2 kB	--	19.6 kB
CardHeader	--	65.3 kB	--	20.6 kB
CardMedia	--	62.6 kB	--	19.8 kB
Checkbox	--	83.2 kB	--	26.4 kB
Chip	--	82.8 kB	--	25.5 kB
CircularProgress	--	64.4 kB	--	20.4 kB
ClickAwayListener	--	3.85 kB	--	1.54 kB
Collapse	--	68.1 kB	--	21.2 kB
colorManipulator	--	3.85 kB	--	1.52 kB
Container	--	63.5 kB	--	19.9 kB
CssBaseline	--	57.8 kB	--	18.2 kB
Dialog	--	83 kB	--	26 kB
DialogActions	--	62.3 kB	--	19.6 kB
DialogContent	--	62.5 kB	--	19.7 kB
DialogContentText	--	64.3 kB	--	20.3 kB
DialogTitle	--	64.5 kB	--	20.3 kB
Divider	--	62.8 kB	--	19.8 kB
docs.landing	--	50.9 kB	--	13.5 kB
docs.main	--	615 kB	--	196 kB
Drawer	--	84.7 kB	--	25.9 kB
ExpansionPanel	--	72.4 kB	--	22.8 kB
ExpansionPanelActions	--	62.3 kB	--	19.6 kB
ExpansionPanelDetails	--	62.2 kB	--	19.6 kB
ExpansionPanelSummary	--	78.3 kB	--	24.8 kB
Fab	--	77 kB	--	24.1 kB
Fade	--	23.3 kB	--	7.99 kB
FilledInput	--	73.8 kB	--	23 kB
FormControl	--	64.6 kB	--	20.2 kB
FormControlLabel	--	65.8 kB	--	20.7 kB
FormGroup	--	62.3 kB	--	19.6 kB
FormHelperText	--	63.5 kB	--	20 kB
FormLabel	--	63.7 kB	--	19.8 kB
Grid	--	65.3 kB	--	20.6 kB
GridList	--	62.7 kB	--	19.8 kB
GridListTile	--	64 kB	--	20.1 kB
GridListTileBar	--	63.5 kB	--	20 kB
Grow	--	23.9 kB	--	8.21 kB
Hidden	--	66.2 kB	--	20.8 kB
Icon	--	63 kB	--	19.8 kB
IconButton	--	76.4 kB	--	23.9 kB
Input	--	72.7 kB	--	22.8 kB
InputAdornment	--	65.3 kB	--	20.6 kB
InputBase	--	70.8 kB	--	22.3 kB
InputLabel	--	65.6 kB	--	20.3 kB
LinearProgress	--	65.6 kB	--	20.5 kB
Link	--	66.8 kB	--	21.2 kB
List	--	62.6 kB	--	19.6 kB
ListItem	--	77.3 kB	--	24.3 kB
ListItemAvatar	--	62.4 kB	--	19.6 kB
ListItemIcon	--	62.4 kB	--	19.6 kB
ListItemSecondaryAction	--	62.3 kB	--	19.6 kB
ListItemText	--	65.2 kB	--	20.6 kB
ListSubheader	--	63 kB	--	19.9 kB
Menu	--	88.7 kB	--	27.4 kB
MenuItem	--	78.4 kB	--	24.6 kB
MenuList	--	66.2 kB	--	20.8 kB
MobileStepper	--	68.1 kB	--	21.4 kB
Modal	--	14.3 kB	--	5.03 kB
NativeSelect	--	77 kB	--	24.4 kB
NoSsr	--	2.19 kB	--	1.04 kB
OutlinedInput	--	74.2 kB	--	23.2 kB
Paper	--	62.6 kB	--	19.6 kB
Popover	--	83 kB	--	25.8 kB
Popper	--	28.7 kB	--	10.3 kB
Portal	--	2.9 kB	--	1.3 kB
Radio	--	84.3 kB	--	26.7 kB
RadioGroup	--	64.7 kB	--	20.1 kB
Rating	--	70.7 kB	--	22.8 kB
RootRef	--	4.21 kB	--	1.64 kB
Select	--	116 kB	--	34.5 kB
Skeleton	--	63.2 kB	--	20.1 kB
Slide	--	25.3 kB	--	8.72 kB
Slider	--	76.8 kB	--	24.4 kB
Snackbar	--	75.4 kB	--	23.7 kB
SnackbarContent	--	63.8 kB	--	20.2 kB
SpeedDial	--	86.3 kB	--	27.3 kB
SpeedDialAction	--	118 kB	--	37.5 kB
SpeedDialIcon	--	64.8 kB	--	20.4 kB
Step	--	62.9 kB	--	19.8 kB
StepButton	--	82.5 kB	--	26.2 kB
StepConnector	--	63 kB	--	19.9 kB
StepContent	--	69.2 kB	--	21.8 kB
StepIcon	--	64.9 kB	--	20.3 kB
StepLabel	--	68.8 kB	--	21.7 kB
Stepper	--	65.1 kB	--	20.6 kB
styles/createMuiTheme	--	16.5 kB	--	5.85 kB
SvgIcon	--	63.3 kB	--	19.8 kB
SwipeableDrawer	--	92.1 kB	--	29 kB
Switch	--	82.4 kB	--	26.1 kB
Tab	--	76.6 kB	--	24.3 kB
Table	--	62.8 kB	--	19.8 kB
TableBody	--	62.4 kB	--	19.6 kB
TableCell	--	64.3 kB	--	20.3 kB
TableContainer	--	62.2 kB	--	19.6 kB
TableFooter	--	62.4 kB	--	19.6 kB
TableHead	--	62.4 kB	--	19.6 kB
TablePagination	--	142 kB	--	41.8 kB
TableRow	--	62.8 kB	--	19.8 kB
TableSortLabel	--	77.6 kB	--	24.5 kB
Tabs	--	85.7 kB	--	27.3 kB
TextareaAutosize	--	5.09 kB	--	2.14 kB
TextField	--	125 kB	--	36.5 kB
ToggleButton	--	76.4 kB	--	24.3 kB
ToggleButtonGroup	--	63.5 kB	--	20 kB
Toolbar	--	62.6 kB	--	19.7 kB
Tooltip	--	102 kB	--	32.4 kB
TreeItem	--	74 kB	--	23.5 kB
TreeView	--	66.9 kB	--	21.1 kB
Typography	--	63.9 kB	--	20.1 kB
useAutocomplete	--	14.3 kB	--	5.18 kB
useMediaQuery	--	2.5 kB	--	1.05 kB
Zoom	--	23.4 kB	--	8.11 kB

Generated by 🚫 dangerJS against 594ec67

dimitropoulos · 2020-01-06T16:35:33Z

Great find! Thank you!

While this is definitely a huge improvement, do you think:

the border color for the outlined variant is too bright in the dark theme?
the brightness (and perhaps saturation) of the ~~outlined~~ filled variant is a bit over the top for the dark theme?

ahtcx · 2020-01-06T16:52:18Z

@dimitropoulos

I think the border color for the outlined variant is fine considering it's only 1px wide
I'm assuming you meant the filled variant? I'm assuming it's harder to work out the coloring without losing consistency in regards to the light theme, but yeah, it'd probably be nicer on the eyes if toned down a little. 👀

dimitropoulos · 2020-01-06T17:51:39Z

oops, yes, I meant the filled variant.

oliviertassinari · 2020-01-07T19:38:15Z

@dimitropoulos Agree 👍. I have tried to reduce the luminance of the colors, with a max at 45% (hsla(x, x%, 45%, 1)). It fees more "relaxing". But I don't know how we could translate it into code.

dimitropoulos · 2020-01-10T00:42:12Z

But I don't know how we could translate it into code.

I've been using Polished.js for precisely this for a while now to good results. Not sure what its introduction would do to the bundle size, but maybe it'd be no problem at all and have the side effect of solving the problem. Tree shaking is well supported so perhaps, with all the individual functions being relatively quite minuscule amount of code it won't be a problem in the slightest.

@ahtcx are you able to give it a try with polished just for, if nothing else, us to see how it goes?

oliviertassinari · 2020-01-10T10:58:04Z

You can find a 9 months old pull request where we evaluate polished over our own color manipulators. It what a 3 KB gzipped increase (with tree shaking), not great. I don't think that we should use it: #15540. I think that we have also opened an issue on polished side to report the findings. Related issue: #13039

What about we accept the changes for the standard variant, and leave the filled variant untouched, for later? Would we already get most of the benefits? 🤔

ahtcx · 2020-01-10T11:01:45Z

Something like this? It's definitely a slight improvement, I wasn't sure if it should be added to the outlined variants too. I've added polished.js but realized there are many color utility functions in colorManipulator.js, so the required functions should be reimplemented?

Before	After

ahtcx · 2020-01-10T11:04:18Z

@oliviertassinari maybe I can reimplement rgbToHsl in the MUI color manipulators?

oliviertassinari · 2020-01-10T11:07:04Z

I think that it would be great to first agree on the best color transformation we can apply, and then on the implementation.

oliviertassinari · 2020-01-10T11:16:52Z

So, I did some checking in the Material Design site, I could find https://material.io/design/color/dark-theme.html#ui-application. They mentioning using desaturated color, which I interpret as reducing luminosity on the hsl geometry. So, are we good on this end? Now, considering that it's a global concern. I wonder if we should approach the problem from a systemic approach (beyond or alert use case). #18776

This reverts commit e6aea69.

oliviertassinari · 2020-01-11T00:38:58Z

I can't figure out a simple solution. The dark/light adaptation of the palette seems to pull in too many dependencies. I would propose that we scope the changes to the standard and outlined variants, which seem to be about revesting lighten with darken, depending on the theme. Something we do frequently

oliviertassinari · 2020-02-02T09:41:00Z

Thank you guys. We will likely need to keep iterating on the dark mode colors for v5 :)

dimitropoulos self-assigned this Jan 6, 2020

oliviertassinari added the component: alert This is the name of the generic UI component, not the React module! label Jan 6, 2020

oliviertassinari changed the title ~~[Alert] improve dark theme coloring~~ [Alert] Improve dark theme coloring Jan 6, 2020

mbrookes added the new feature New feature or request label Jan 7, 2020

[Alert] improve dark theme coloring

95841fb

ahtcx requested a review from dimitropoulos as a code owner January 10, 2020 10:56

[Alert] improve filled variant coloring

e6aea69

oliviertassinari mentioned this pull request Jan 10, 2020

[theme] Improve default theme dark colors #18776

Closed

2 tasks

Revert "[Alert] improve filled variant coloring"

594ec67

This reverts commit e6aea69.

oliviertassinari approved these changes Jan 11, 2020

View reviewed changes

oliviertassinari added PR: ready to ship and removed PR: ready to ship labels Jan 11, 2020

oliviertassinari added the PR: ready to ship label Feb 1, 2020

oliviertassinari merged commit 861f82b into mui:master Feb 2, 2020

EsoterikStare pushed a commit to EsoterikStare/material-ui that referenced this pull request Feb 13, 2020

[Alert] Improve dark theme coloring (mui#19105)

8052040

This was referenced Feb 25, 2020

[Snyk] Upgrade @material-ui/core from 4.1.1 to 4.9.1 baaahs/sparklemotion#199

Closed

[Snyk] Upgrade @material-ui/core from 4.9.0 to 4.9.1 cybersecmoo/brownie-tub#19

Merged

snyk-bot mentioned this pull request Feb 25, 2020

[Snyk] Upgrade @material-ui/icons from 4.5.1 to 4.9.1 cybersecmoo/brownie-tub#20

Merged

snyk-bot mentioned this pull request Mar 22, 2020

[Snyk] Upgrade @material-ui/core from 4.9.1 to 4.9.4 mdfaizan7/react-bitConnect-client#1

Open

This was referenced Apr 16, 2020

[Snyk] Upgrade @material-ui/core from 4.3.3 to 4.9.7 reactioncommerce/reaction#6208

Merged

[Snyk] Upgrade: @material-ui/core, @material-ui/icons BrainBackup/bb8#100

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Alert] Improve dark theme coloring #19105

[Alert] Improve dark theme coloring #19105

ahtcx commented Jan 6, 2020

mui-pr-bot commented Jan 6, 2020 •

edited

Loading

dimitropoulos commented Jan 6, 2020 •

edited

Loading

ahtcx commented Jan 6, 2020

dimitropoulos commented Jan 6, 2020

oliviertassinari commented Jan 7, 2020

dimitropoulos commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020 •

edited

Loading

ahtcx commented Jan 10, 2020

ahtcx commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020 •

edited

Loading

oliviertassinari commented Jan 11, 2020

oliviertassinari commented Feb 2, 2020

[Alert] Improve dark theme coloring #19105

[Alert] Improve dark theme coloring #19105

Conversation

ahtcx commented Jan 6, 2020

mui-pr-bot commented Jan 6, 2020 • edited Loading

dimitropoulos commented Jan 6, 2020 • edited Loading

ahtcx commented Jan 6, 2020

dimitropoulos commented Jan 6, 2020

oliviertassinari commented Jan 7, 2020

dimitropoulos commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020 • edited Loading

ahtcx commented Jan 10, 2020

ahtcx commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020

oliviertassinari commented Jan 10, 2020 • edited Loading

oliviertassinari commented Jan 11, 2020

oliviertassinari commented Feb 2, 2020

mui-pr-bot commented Jan 6, 2020 •

edited

Loading

dimitropoulos commented Jan 6, 2020 •

edited

Loading

oliviertassinari commented Jan 10, 2020 •

edited

Loading

oliviertassinari commented Jan 10, 2020 •

edited

Loading