angular-material-css-vars icon indicating copy to clipboard operation
angular-material-css-vars copied to clipboard

"@mixin mat-css-set-palette-defaults" does not work anymore

Open pcire opened this issue 2 years ago • 4 comments

Hi,

I updated your library in my project from v1.1.x to v2.1.2 today and unfortunately the mat-css-set-palette-defaults mixin doesn't work anymore (angular material was updated from v11 to v12, too). I delved into the code to figure out what goes wrong and I found out, that Pedro removed a crucial line of code in his commit: _public-util.scss#136 @include _mat-css-root($new-map)

I'm not a SCSS expert so I don't know if there is some other magic in place, but as far as I can see, this function is basically dead right now.

After "fixing" the function locally I can see now, that the contrast isn't working correctly, which was addressed in Pedros commit, too (e.g. black font on dark blue background). So the solution isn't that easy.

Any idea how to solve this problem?

Some background information: My app loads the primary and accent color from an API, after the user logs in. So on the login screen I'd like to use the default angular material palettes. After I set the colors with setPrimaryColor() everything works as expected. My theme.scss:

@use '~@angular/material' as mat;
@import '~angular-material-css-vars/public-util';
@import '~angular-material-css-vars/main';

$mat-css-dark-theme-selector: '.isDarkTheme';
$mat-css-light-theme-selector: '.isLightTheme';

@include init-material-css-vars();

@include mat-css-set-palette-defaults(mat.$indigo-palette, 'primary');
@include mat-css-set-palette-defaults(mat.$pink-palette, 'accent');
@include mat-css-set-palette-defaults(mat.$red-palette, 'warn');

pcire avatar Sep 02 '21 16:09 pcire

Hey there! Thanks for opening this up. Unfortunately, I currently don't have the capacities to work on this, as I am too busy to work on my other open source projects, but I gladly review any PRs.

The "work around" I'm using personally, is just to stick with v11 of angular material and with v1.1 of this lib.

johannesjo avatar Sep 03 '21 09:09 johannesjo

Hey, thank you for the feedback. Fortunately there are some workarounds, so it isn't that urgent. I'm just calling the setter from Javascript now. Angular 12 + Material works fine with v2.1.2 so far.

pcire avatar Sep 09 '21 14:09 pcire

@pcire Sorry it has taken me a while to respond to this issue. I did purposely remove that line as it is really not needed to setup default values. It was also creating more variables that are actually needed (kind of polluting the root). The proper way to set defaults is to set it up in your app.modules.ts file like this:

 MaterialCssVarsModule.forRoot({
  primary: '#002744',
  accent: '#ea9c12',
  warn: '#c62828',
  isDarkTheme: false,
  isAutoContrast: true,
}),

This should in turn call the proper setters in the service program.

Please let me know if this works for you.

pedrojrivera avatar Sep 23 '21 07:09 pedrojrivera

I think that the commit c21082a introduce a regression in file projects/material-css-vars/src/lib/_main.scss where @include _mat-css-root($default-theme); is removed in mixin init-css-vars.

With this commit, the palette variables are not set whereas foreground and background variables are set with @include _mat-css-root($text);.

I can't propose a PR for this because I'm not sufficient knowledge on this library to determine impacts.

Hope this info helps you to fix this issue.

clemdesign avatar Apr 07 '22 08:04 clemdesign

Initialization of $default-theme is present again in the code. Does the error still persist?

json-derulo avatar Apr 24 '23 17:04 json-derulo