shlomiassaf / ngrid

A angular grid for the enterprise
https://shlomiassaf.github.io/ngrid
MIT License
241 stars 40 forks source link

[Bug] material `define-...-theme` edits theme too excessive #213

Open spali opened 3 years ago

spali commented 3 years ago

What is the expected behavior?

ngrid's define-dark-theme and define-dark-theme should only merge to the theme as it does with the palettes.

What is the current behavior?

having a "custom" key in the theme:

  $my-light-theme: mat.define-light-theme(
    (
      color: (
        primary: $my-primary-palette,
        accent: $my-accent-palette,
        warn: $my-warn-palette,
      ),
      typography: $my-typography,
    )
  );

drops typography. This happens to all key's which are not specific handled within ngrid.

What are the steps to reproduce?

create a global styles.scss with the above theme and call ngrid.define-light-theme on it. The return value would drop any key that is not handled by it.

Which versions of Angular, CDK, Material, NGrid, OS, TypeScript, browsers are affected?

12

Is there anything else we should know?

Workaround:

$theme: map.merge($theme, ngrid.define-light-theme($theme));

I would suggest to either document the workaround in the examples or use merge on the provided theme.

BTW: Is there a reason why ngrid.ngrid-typography is not called from withing ngrid.theme and get typography with mat.get-typography-config($theme)? It seems to be overall standard to have separate mixin's for color and typography and call both in theme. So the user has only to call ngrid.theme and in ngrid's case beforehand ngrid.define-...-theme.