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

feat(theming): dark theme elevation helpers don't match material spec for elevation behavior #23457

Open
michaelfaith opened this issue Aug 26, 2021 · 10 comments
Labels
area: theming feature This issue represents a new feature or feature request rather than a bug or bug fix material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions

Comments

@michaelfaith
Copy link

michaelfaith commented Aug 26, 2021

Feature Description

The material spec for elevation with dark theme (https://material.io/design/color/dark-theme.html#properties), says that elevation should be reflected by an increased lightening of the surface color (by applying more and more opaque white overlays to a base surface color), rather than using drop shadow alone to reflect elevation (since that's not reliable when you have dark surface backgrounds). This request is to have the mat-elevation-z# classes and elevation mixins implement this lightening behavior from the spec, rather than only using drop shadow like it does for light theme.

image

@michaelfaith michaelfaith added feature This issue represents a new feature or feature request rather than a bug or bug fix needs triage This issue needs to be triaged by the team labels Aug 26, 2021
@michaelfaith michaelfaith changed the title feat(theming): dark theme elevation classes/helpers don't match material spec for elevation behavior feat(theming): dark theme elevation helpers don't match material spec for elevation behavior Aug 26, 2021
@michaelfaith
Copy link
Author

Wasn't sure if this should be tracked as a feature or a bug...

@jelbourn
Copy link
Member

I'm not sure there's a way for us to realistically implement this today. Having the elevation classes "lighten" an existing background color isn't something that can be accomplished with CSS, and changing the classes to set fixed background colors based on the elevation would be too major of a breaking change to land.

I'll keep this in mind the next time we're revisiting how our theming system works, but it's unlikely this would make it in.

@jelbourn jelbourn added material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions and removed needs triage This issue needs to be triaged by the team labels Aug 26, 2021
@michaelfaith
Copy link
Author

@jelbourn thanks for the feedback. I wonder if it's something that could be introduced in some limited form. Like cards for instance. There's a card surface base color in the background palette, if the card styles used that as the base and a sass color function to derive the different elevation levels from the base color (mixing with white at the different percentages)? So it would still just use the one base color in the def, but derive the rest using a mixing function. I don't know, just a thought.

@mrmokwa
Copy link

mrmokwa commented Nov 16, 2021

I noticed that both foreground elevation color for dark and light themes are black.
Shouldn't we change the dark themes elevation to white? This way we can notice the elevation in dark themes

@michaelfaith
Copy link
Author

@mrmokwa the box shadow color?

@mrmokwa
Copy link

mrmokwa commented Nov 16, 2021

I mean this one
image

@michaelfaith
Copy link
Author

michaelfaith commented Nov 16, 2021

Ok yeah. Looking at how that value's used https://github.com/angular/components/blob/40f0674e3959d53cd6413cf3a5c30053ca0973d9/src/material/core/style/_elevation.scss that's the shadow color. You wouldn't want that to be anything other than black. In the Material spec, elevation is not communicated as much via drop shadow in dark theme like it is in light theme. The shadow is still black because that more closely mimics the real physical world. Shadows don't turn white in the real world either. But what does mimic the real world is the way surfaces react to a light source. So changing the surface color, brightening it the higher it is (i.e. the closer it is to the light source), is the primary indicator of elevation in dark theme, rather than shadow, which becomes secondary.

@michaelfaith
Copy link
Author

michaelfaith commented Jan 21, 2022

I'm not sure there's a way for us to realistically implement this today. Having the elevation classes "lighten" an existing background color isn't something that can be accomplished with CSS, and changing the classes to set fixed background colors based on the elevation would be too major of a breaking change to land.

I'll keep this in mind the next time we're revisiting how our theming system works, but it's unlikely this would make it in.

@jelbourn I was thinking a bit more about this today, and went to check out how the React material library is doing it, since they seem to have this implemented. And surprisingly they're doing it with css, and I thought it was actually a pretty creative solution. Perhaps something that might work with the way Angular Material theming operates. What they're doing is sort of exploiting the backgroundImage css property and using a linear-gradient function to create a semi-transparent white overlay that mixes with whatever the backgroundColor is to lighten it. And the alpha value on the rgba function is just progressively higher the higher the elevation is.

So, for example, elevation of 2 would have this to match the spec

backgroundImage: linear-gradient(rgba(255, 255, 255, 0.07), rgba(255, 255, 255, 0.07));

While elevation 8 would have

backgroundImage: linear-gradient(rgba(255, 255, 255, 0.12), rgba(255, 255, 255, 0.12));

With the mat-elevation-z# classes, I think it would be possible to have the define-dark-theme function include those kinds of backgroundImage adjustments, whereas the light-theme define function wouldn't. Though I'm not sure how to handle it with the sass elevation helper mixins, since those wouldn't know if the current theme was dark or light...

Just wanted to bring that here, in case it sparks some ideas.

@angular-robot
Copy link
Contributor

angular-robot bot commented Mar 13, 2022

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

@angular-robot
Copy link
Contributor

angular-robot bot commented Apr 1, 2022

Thank you for submitting your feature request! Looks like during the polling process it didn't collect a sufficient number of votes to move to the next stage.

We want to keep Angular rich and ergonomic and at the same time be mindful about its scope and learning journey. If you think your request could live outside Angular's scope, we'd encourage you to collaborate with the community on publishing it as an open source package.

You can find more details about the feature request process in our documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: theming feature This issue represents a new feature or feature request rather than a bug or bug fix material spec Issue related to the Material Design spec https://material.io/design/ P4 A relatively minor issue that is not relevant to core functions
Projects
None yet
Development

No branches or pull requests

5 participants