Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

feat(theme): Add support for CSS vars with fallback #4470

Merged
merged 15 commits into from
Mar 14, 2019
Merged

feat(theme): Add support for CSS vars with fallback #4470

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
5d49c5a
feat(theme): Add support for CSS vars with fallback
patrickrodee Mar 1, 2019
7d985e2
WIP fix failing CSS lint
patrickrodee Mar 1, 2019
fd51863
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 1, 2019
195bcba
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 4, 2019
7c1cd33
WIP use "varname" instead of "name"
patrickrodee Mar 4, 2019
a43e2ff
Merge branch 'feat/theme/vars-with-fallback' of github.com:material-c…
patrickrodee Mar 4, 2019
bdebc93
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 11, 2019
45161a9
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 12, 2019
f5dfc59
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 13, 2019
ef5d84f
WIP add alternate tags
patrickrodee Mar 13, 2019
a0293a0
WIP update from review
patrickrodee Mar 13, 2019
65a61b0
WIP updated readme
patrickrodee Mar 13, 2019
e58979e
Merge branch 'master' into feat/theme/vars-with-fallback
patrickrodee Mar 13, 2019
5643f95
WIP revert list change
patrickrodee Mar 14, 2019
b92a208
WIP update readme
patrickrodee Mar 14, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/mdc-card/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@
}

@include mdc-feature-targets($feat-color) {
border-color: mdc-theme-prop-value($color);
@include mdc-theme-prop(border-color, $color);
patrickrodee marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
15 changes: 15 additions & 0 deletions packages/mdc-theme/_functions.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,18 @@
@function mdc-theme-contrast-tone($color) {
@return if(mdc-theme-tone($color) == "dark", "light", "dark");
}

@function mdc-theme-is-var-with-fallback_($style) {
@return type-of($style) == "map" and map-has-key($style, "varname") and map-has-key($style, "fallback");
}

@function mdc-theme-get-var-fallback_($style) {
@return map-get($style, "fallback");
}

@function mdc-theme-var_($style) {
$var: map-get($style, "varname");
$fallback: mdc-theme-get-var-fallback_($style);

@return var(#{$var}, $fallback);
}
13 changes: 12 additions & 1 deletion packages/mdc-theme/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
@import "@material/feature-targeting/functions";
@import "@material/feature-targeting/mixins";
@import "./variables";
@import "./functions";

@mixin mdc-theme-core-styles($query: mdc-feature-all()) {
$feat-color: mdc-feature-create-target($query, color);
Expand Down Expand Up @@ -67,7 +68,17 @@
// $edgeOptOut controls whether to feature-detect around Edge to avoid emitting CSS variables for it,
// intended for use in cases where interactions with pseudo-element styles cause problems due to Edge bugs.
@mixin mdc-theme-prop($property, $style, $important: false, $edgeOptOut: false) {
@if mdc-theme-is-valid-theme-prop-value_($style) {
@if mdc-theme-is-var-with-fallback_($style) {
patrickrodee marked this conversation as resolved.
Show resolved Hide resolved
@if $important {
#{$property}: mdc-theme-get-var-fallback_($style) !important;
/* @alternate */
#{$property}: mdc-theme-var_($style) !important;
} @else {
#{$property}: mdc-theme-get-var-fallback_($style);
/* @alternate */
#{$property}: mdc-theme-var_($style);
}
} @else if mdc-theme-is-valid-theme-prop-value_($style) {
@if $important {
#{$property}: $style !important;
} @else {
Expand Down
4 changes: 4 additions & 0 deletions packages/mdc-theme/_variables.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,10 @@ $mdc-theme-property-values: (
//
// NOTE: This function must be defined in _variables.scss instead of _functions.scss to avoid circular imports.
@function mdc-theme-prop-value($style) {
@if mdc-theme-is-var-with-fallback_($style) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Returning the new style type as is causes an issue to other mixins that uses mdc-theme-prop-value() function. For example, mdc-card-outline uses this function to set border-color. mdc-card/_mixins.scss:L303

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can we update that line to be as follows?

@include mdc-theme-prop(border-color, $color);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

List divider uses the approach described above. I'll include that update in this PR.

material-components-web/packages/mdc-list/_mixins.scss

Line 360 in b9c5fc6

@include mdc-theme-prop(border-bottom-color, $color);

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused though, what issue was this causing? Do we need to be concerned for existing valid cases of mdc-theme-prop-value being used?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is still an issue if component needs to mix colors or apply opacity using rgba() sass function.

For example, dialog uses rgba() to apply opacity to color.

dialog/_mixins.scss:

background-color: rgba(mdc-theme-prop-value($color), $opacity);

Where, mdc-theme-prop-value returns new color format which rgba function doesn't support.

This is another advanced use case where text field uses mix() function two mix two colors:

textfield/_variables.scss:

$mdc-text-field-background:
    mix(mdc-theme-prop-value(on-surface), mdc-theme-prop-value(surface), 4%);

Copy link
Collaborator

@abhiomkar abhiomkar Mar 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@kfranqueiro It shouldn't break existing valid usage. This is an issue only for users who send new color format to component's mixins.

mdc-dialog-title-ink-color('purple'); // works
mdc-dialog-title-ink-color({
  varname: '--custom-css-var-ink-color',
  fallback: purple
}); // breaks since this applies opacity on given color using rgba

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mdc-theme-prop-value has been updated to always return the fallback value.

material-components-web/packages/mdc-theme/_variables.scss

Lines 131 to 138 in e58979e

@function mdc-theme-prop-value($style) {
@if mdc-theme-is-var-with-fallback_($style) {
@return mdc-theme-get-var-fallback_($style);
}
@if mdc-theme-is-valid-theme-prop-value_($style) {
@return $style;
}

@return mdc-theme-get-var-fallback_($style);
}

@if mdc-theme-is-valid-theme-prop-value_($style) {
@return $style;
}
Expand Down