fix(material/core): elevation classes emit invalid shadows with system theme - #33661
Open
ManicardiFrancesco wants to merge 1 commit into
Open
fix(material/core): elevation classes emit invalid shadows with system theme#33661ManicardiFrancesco wants to merge 1 commit into
ManicardiFrancesco wants to merge 1 commit into
Conversation
…m theme `mat.elevation-classes()` resolves each `mat-elevation-z*` class to `var(--mat-app-elevation-shadow-level-<n>, <fallback>)`, where the fallback is built from `--mat-sys-shadow`. Since that shadow color is a CSS variable name rather than a color, it was interpolated into the shadow verbatim, producing `0px 3px 5px -1px --mat-sys-shadow, ...`. That value is invalid CSS, so the browser drops the entire declaration. Apps that theme with `mat.theme` never define the `--mat-app-elevation-shadow-level-*` tokens (only the older `mat.core-theme`/prebuilt theming path does), which means they always hit the broken fallback and get no shadow at all from any `mat-elevation-z*` class. Wrap the variable in `var()` and apply the shadow opacities through `color-mix`, mirroring what `m3-utils.color-with-opacity` already does elsewhere. Themes that define the app elevation tokens are unaffected, since the fallback is unused there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the current behavior?
mat.elevation-classes()emits no shadow at all for apps themed withmat.theme.Each
mat-elevation-z*class resolves tovar(--mat-app-elevation-shadow-level-<n>, <fallback>), and the fallback is built from theshadowentry of the system theme. In the$fallbacksmap that entry is the CSS variable name--mat-sys-shadowrather than a color, and_compute-color-opacity()incore/style/_elevation.scssonly handled Sasscolorvalues — anything else was returned untouched and interpolated into the shadow verbatim:A bare
--mat-sys-shadowis not a valid<color>, so the declaration is invalid at computed-value time and the browser drops it.This only bites the
mat.themeAPI. The--mat-app-elevation-shadow-level-*tokens are emitted exclusively bymat.core-theme(i.e. theall-component-themes/ prebuilt theme path), so those apps never reach the fallback.mat.themenever defines them, which means the broken fallback is the only value they ever get. The dev-app themes withmat.m2-theme+all-component-themes, which is why this isn't visible there.Reproduction (against
main):What is the new behavior?
_compute-color-opacity()now recognizes a CSS variable name, wraps it invar()and applies the umbra/penumbra/ambient opacities withcolor-mix, reusing them3-utils.color-with-opacityhelper that already handles this exact case for other tokens:I compiled a stylesheet exercising all three theming paths (
mat.m2-theme,mat.define-theme+all-component-themes, andmat.theme) plus all four prebuilt themes before and after the change. The only difference is the 25mat-elevation-z*rules; every other declaration in the bundle is byte-identical. Prebuilt and M2 themes are unaffected because they define the app elevation tokens, so the fallback is never used there.The added test in
m3-theme.spec.tsasserts that nobox-shadowemitted bymat.elevation-classes()contains a bare token name. It reports 25 failures before this change and none after.Does this PR introduce a breaking change?