[Paper] Support dark mode brightening based on elevation#21748
[Paper] Support dark mode brightening based on elevation#21748joshwooding wants to merge 3 commits intomui:nextfrom
Conversation
|
@material-ui/core: parsed: +0.10% , gzip: +0.10% |
| import withStyles from '../styles/withStyles'; | ||
| import { fade, useTheme } from '../styles'; | ||
|
|
||
| // Inspired by https://github.com/material-components/material-components-ios/blob/bca36107405594d5b7b16265a5b0ed698f85a5ee/components/Elevation/src/UIColor%2BMaterialElevation.m#L61 |
There was a problem hiding this comment.
It was so hard to find an algorithm for the brightening values...
There was a problem hiding this comment.
I wonder if 80% of the value wouldn't come from the dark theme palette of grey colors as opposed to the Paper supporting it (which help too). It could be very interesting to integrate it into the system.
c45ce66 to
843d1e0
Compare
843d1e0 to
80d12c5
Compare
| import withStyles from '../styles/withStyles'; | ||
| import { fade, useTheme } from '../styles'; | ||
|
|
||
| // Inspired by https://github.com/material-components/material-components-ios/blob/bca36107405594d5b7b16265a5b0ed698f85a5ee/components/Elevation/src/UIColor%2BMaterialElevation.m#L61 |
There was a problem hiding this comment.
I wonder if 80% of the value wouldn't come from the dark theme palette of grey colors as opposed to the Paper supporting it (which help too). It could be very interesting to integrate it into the system.
| paper: '#121212', | ||
| default: '#121212', |
There was a problem hiding this comment.
I think that we should explore how to integrate this:
https://github.com/mui-org/material-ui/blob/38b259eff17f56d92083058543c29c62ce22d6e3/docs/src/modules/components/ThemeContext.js#L235-L239
Into the library. The fact that we had to extend likely suggest that there is an opportunity.
There was a problem hiding this comment.
Looking into the usage of background level1 and 2 I see it is for hard-coding elevation into the documentation's appBar, drawer etc.
If the color by elevation logic will be implemented the reason for having background levels is not needed because the paper in those components will provide the proper color.
I think providing the elevation colors built in, as a default option at least, will be very comfortable in most use cases.
There was a problem hiding this comment.
Looking into the usage of background level1 and 2
We aim to remove the custom theme values of the documentation. It's a signal that the default palette is wrong.
We definitely want to fix #18309
|
I'm closing as the effort has been stale for some time, thanks for the exploration. |
Closes #18309
I've added the overlay as a separate class and not as part of elevation because you can't currently have elevation and overlay at the same time unlike in the material spec so you couldn't have the elevation class and an outline.
https://material.io/design/color/dark-theme.html