angular-material-css-vars
angular-material-css-vars copied to clipboard
johannesjo
Bug: css variables mapping doesn't work for additional properties (default, lighter, darker)
Summary
I've noticed the mapping of css variables is incorrect for the following properties: default, lighter, darker, text, default-contrast, lighter-contrast, darker-contrast. The reason is that the mat-palette() calls used in init-mat-theme produces a hue specific variable (--palette-primary-500) instead of the original named one (--palette-primary-default for default).
In detail
I've got a product which uses material extensively, but we're trying to shift towards css variables. This is how our material theme was configured:
$app-primary: mat-palette($page-primary, 700, 200, 900);
$app-accent: mat-palette($page-accent, 500, 200, 900);
$app-warn: mat-palette($page-error);
$app-material-theme: mat-light-theme($app-primary, $app-accent, $app-warn);
Shifting to css variables using your awesome library was actually really easy.
@include init-material-css-vars() {}
$app-primary: mat-palette($page-primary, 700, 200, 900);
$app-accent: mat-palette($page-accent, 500, 200, 900);
$app-warn: mat-palette($page-error);
@include mat-css-set-palette-defaults($app-primary, 'primary');
@include mat-css-set-palette-defaults($app-accent, 'accent');
@include mat-css-set-palette-defaults($app-warn, 'warn');
Done, except for one issue.
For $app-primary, the material components now take 500 as default color, instead of the 700 specified in the mat-pallete(). This is because the actual style in material color: map-get($primary) is rendered as color: rgb(var(--palette-primary-500)). This should be color: rgb(var(--palette-primary-default)).
The reason this happens is how init-mat-theme makes its themes. It calls mat-palette(), which executes the following code:
@function mat-palette($base-palette, $default: 500, $lighter: 100, $darker: 700, $text: $default) {
$result: map_merge($base-palette, (
default: map-get($base-palette, $default),
lighter: map-get($base-palette, $lighter),
darker: map-get($base-palette, $darker),
text: map-get($base-palette, $text),
default-contrast: mat-contrast($base-palette, $default),
lighter-contrast: mat-contrast($base-palette, $lighter),
darker-contrast: mat-contrast($base-palette, $darker)
));
...
}
Here, default is assigned map-get($base-palette, $default), which fetches the --palette-primary-500 css variable for primary. This should be --palette-primary-default.
Because of this bug, any palette with non-default hues will unfortunately not work with your library.
To end on a good note, thanks for making this awesome library. It made my work a hell of a lot easier. You rock! 👍
It seems like a managed to fix (read: hack) it by adding default, lighter and darker to the palettes:
$mat-css-palette-primary: (
default: var(--palette-primary-default),
lighter: var(--palette-primary-lighter),
darker: var(--palette-primary-darker),
50: var(--palette-primary-50),
100: var(--palette-primary-100),
200: var(--palette-primary-200),
...
and changing the following in init-mat-theme:
@mixin init-mat-theme($dark-theme-selector, $typography-config: null) {
@include mat-core($typography-config);
$primary: mat-palette($mat-css-palette-primary, 'default', 'lighter', 'darker');
$accent: mat-palette($mat-css-palette-accent, 'default', 'lighter', 'darker');
$warn: mat-palette($mat-css-palette-warn, 'default', 'lighter', 'darker');
@Jrubzjeknf thank you very much for digging into this. You are probably much more familiar with the subject, than I am. What do you think would be the best/cleanest & correct way to fix this? Should we add:
default: var(--palette-primary-default),
lighter: var(--palette-primary-lighter),
darker: var(--palette-primary-darker),
to $mat-css-default-light-theme and $mat-css-default-dark-theme and/or $mat-css-palette-primary, $mat-css-palette-accent and $mat-css-palette-warn? Or maybe even something like this:
$mat-css-default-light-theme: (
default: var(--palette-primary-500),
lighter: var(--palette-primary-300),
darker: var(--palette-primary-600),
?
I've looked a bit deeper into it, but at the moment I don't see a clear cut solution. The point is that the library currently consumes a user defined palette (without default, lighter, or darker properties), while I feed it a palette from mat-palette (including those properties). Simply adding those properties will likely be a breaking change.
I've forked the repo and I'll see if I can produce a solution. When I can, I'll make a pull request.
Others looking for an intermediate solution until then, here's a very ugly override:
@import '~@angular/material/theming';
@import '~angular-material-css-vars/main';
// START OVERRIDE FOR THEME HUE SUPPORT
// TODO remove when this is fixed: https://github.com/johannesjo/angular-material-css-vars/issues/52
$mat-css-palette-primary-override: (
default: var(--palette-primary-default),
lighter: var(--palette-primary-lighter),
darker: var(--palette-primary-darker),
50: var(--palette-primary-50),
100: var(--palette-primary-100),
200: var(--palette-primary-200),
300: var(--palette-primary-300),
400: var(--palette-primary-400),
500: var(--palette-primary-500),
600: var(--palette-primary-600),
700: var(--palette-primary-700),
800: var(--palette-primary-800),
900: var(--palette-primary-900),
A100: var(--palette-primary-A100),
A200: var(--palette-primary-A200),
A400: var(--palette-primary-A400),
A700: var(--palette-primary-A700),
contrast: (
default: var(--palette-primary-contrast-default),
lighter: var(--palette-primary-contrast-lighter),
darker: var(--palette-primary-contrast-darker),
50: var(--palette-primary-contrast-50),
100: var(--palette-primary-contrast-100),
200: var(--palette-primary-contrast-200),
300: var(--palette-primary-contrast-300),
400: var(--palette-primary-contrast-400),
500: var(--palette-primary-contrast-500),
600: var(--palette-primary-contrast-600),
700: var(--palette-primary-contrast-700),
800: var(--palette-primary-contrast-800),
900: var(--palette-primary-contrast-900),
A100: var(--palette-primary-contrast-A100),
A200: var(--palette-primary-contrast-A200),
A400: var(--palette-primary-contrast-A400),
A700: var(--palette-primary-contrast-A700),
),
) !default;
$mat-css-contrast-palette: map_get($mat-css-palette-primary-override, 'contrast') !default;
$mat-css-palette-accent-override: (
default: var(--palette-accent-default),
lighter: var(--palette-accent-lighter),
darker: var(--palette-accent-darker),
50: var(--palette-accent-50),
100: var(--palette-accent-100),
200: var(--palette-accent-200),
300: var(--palette-accent-300),
400: var(--palette-accent-400),
500: var(--palette-accent-500),
600: var(--palette-accent-600),
700: var(--palette-accent-700),
800: var(--palette-accent-800),
900: var(--palette-accent-900),
A100: var(--palette-accent-A100),
A200: var(--palette-accent-A200),
A400: var(--palette-accent-A400),
A700: var(--palette-accent-A700),
contrast: (
default: var(--palette-accent-contrast-default),
lighter: var(--palette-accent-contrast-lighter),
darker: var(--palette-accent-contrast-darker),
50: var(--palette-accent-contrast-50),
100: var(--palette-accent-contrast-100),
200: var(--palette-accent-contrast-200),
300: var(--palette-accent-contrast-300),
400: var(--palette-accent-contrast-400),
500: var(--palette-accent-contrast-500),
600: var(--palette-accent-contrast-600),
700: var(--palette-accent-contrast-700),
800: var(--palette-accent-contrast-800),
900: var(--palette-accent-contrast-900),
A100: var(--palette-accent-contrast-A100),
A200: var(--palette-accent-contrast-A200),
A400: var(--palette-accent-contrast-A400),
A700: var(--palette-accent-contrast-A700),
),
) !default;
$mat-css-contrast-palette-accent: map_get($mat-css-palette-accent-override, 'contrast') !default;
$mat-css-palette-warn-override: (
default: var(--palette-warn-default),
lighter: var(--palette-warn-lighter),
darker: var(--palette-warn-darker),
50: var(--palette-warn-50),
100: var(--palette-warn-100),
200: var(--palette-warn-200),
300: var(--palette-warn-300),
400: var(--palette-warn-400),
500: var(--palette-warn-500),
600: var(--palette-warn-600),
700: var(--palette-warn-700),
800: var(--palette-warn-800),
900: var(--palette-warn-900),
A100: var(--palette-warn-A100),
A200: var(--palette-warn-A200),
A400: var(--palette-warn-A400),
A700: var(--palette-warn-A700),
contrast: (
default: var(--palette-warn-contrast-default),
lighter: var(--palette-warn-contrast-lighter),
darker: var(--palette-warn-contrast-darker),
50: var(--palette-warn-contrast-50),
100: var(--palette-warn-contrast-100),
200: var(--palette-warn-contrast-200),
300: var(--palette-warn-contrast-300),
400: var(--palette-warn-contrast-400),
500: var(--palette-warn-contrast-500),
600: var(--palette-warn-contrast-600),
700: var(--palette-warn-contrast-700),
800: var(--palette-warn-contrast-800),
900: var(--palette-warn-contrast-900),
A100: var(--palette-warn-contrast-A100),
A200: var(--palette-warn-contrast-A200),
A400: var(--palette-warn-contrast-A400),
A700: var(--palette-warn-contrast-A700),
),
) !default;
$mat-css-contrast-palette-warn: map_get($mat-css-palette-warn-override, 'contrast') !default;
/** Overrides init-mat-theme from ~angular-material-css-vars/main */
@mixin init-mat-theme($dark-theme-selector, $typography-config: null) {
@include mat-core($typography-config);
$primary: mat-palette($mat-css-palette-primary-override, 'default', 'lighter', 'darker');
$accent: mat-palette($mat-css-palette-accent-override, 'default', 'lighter', 'darker');
$warn: mat-palette($mat-css-palette-warn-override, 'default', 'lighter', 'darker');
$theme: mat-light-theme($primary, $accent, $warn);
$dark-theme: mat-dark-theme($primary, $accent, $warn);
$theme: map_merge(
$theme,
(
background: $mat-css-palette-background,
foreground: $mat-css-palette-foreground,
)
);
// set global variable so passed-in content can use the theme
$mat-css-theme: $theme !global;
@at-root {
// NOTE: light theme is the default theme
@include angular-material-theme($theme);
@content;
@if $dark-theme-selector {
$mat-css-theme: $dark-theme !global;
#{$dark-theme-selector} {
@include angular-material-theme($dark-theme);
// add content passed in, which can use the $theme variable to apply the dark theme to
// other theme mixins needed by the app
@content;
}
}
}
@include mat-css-other-overwrites;
$mat-css-theme: null !global;
}
// END OVERRIDE FOR THEME HUE SUPPORT
// optional
// $mat-css-dark-theme-selector: '.isDarkTheme';
// $mat-css-light-theme-selector: '.isLightTheme';
// init theme
@include init-material-css-vars() {
// If your app has any theme mixins, call them here.
// $mat-css-theme gets set to an appropriate value before this content is called.
// @include your-custom-component-theme($mat-css-theme);
}
// Use your own palette definitions here.
$app-primary: mat-palette($page-primary, 700, 200, 900);
$app-accent: mat-palette($page-accent, 500, 200, 900);
$app-warn: mat-palette($page-error);
@include mat-css-set-palette-defaults($app-primary, 'primary');
@include mat-css-set-palette-defaults($app-accent, 'accent');
@include mat-css-set-palette-defaults($app-warn, 'warn');
That's great. Thank you very much! I am alright with making breaking changes if it is a solid solution and if I understand the issue correctly it might should have been designed differently in the first place.