inovex / elements

Lovingly crafted ui components based on web components. Works well with all Frameworks - including Angular, React and Vue.
https://elements.inovex.de
MIT License
69 stars 9 forks source link

No mdc customization possible #153

Closed pfecht closed 3 years ago

pfecht commented 3 years ago

The https://github.com/inovex/elements/blob/master/packages/elements/src/components/styles/mdc.customize.scss contains variables to overwrite MDC variables like f.e. the base typography.

As a matter of fact, however, the mdc.customize is not used at all. If I think about its behavior correctly, it should be imported into every component before importing any material scss components.

Expected behavior

The font for every mdc component can be changed in the mdc.customize.scss file

Possible Solution

  1. Add the import to the beginning of every component.scss file.
  2. Rename mdc.customize.scss to mdc-customize.scss
  3. Change the content to sass module, f.e.
// Simple module to override sass variables of mdc
// Manly used for theming and typography.

@use 'colors';
@use 'fonts';

// @material/theme
@use "@material/theme" with (
  $primary: colors.$primary,
  $secondary: colors.$primary-contrast,
  $on-primary: colors.$secondary,
  $on-secondary: colors.$secondary-contrast,
);

// @material/typography
@use "@material/typography" with (
  $font-family: fonts.$font-family-sans-serif
);

see https://github.com/material-components/material-components-web/tree/master/packages/mdc-typography#typography-styles

janivo commented 3 years ago

The variables.scss in src/components/styles is a global stylesheet that is loaded before all others. Would it not be sufficient to import the mdc-customize.scss there? This way we would not have to import it into every file.

If I understand correctly, we could use the injectGlobalPaths of the stencil-sass plugin.

We will have to wait until https://github.com/ionic-team/stencil-sass/issues/38 is resolved or downgrade our stencil version to 1.x.x.

pfecht commented 3 years ago

I'm not quite sure if this is the proper solution.

  1. The global inject script looks like it uses @import, but we have to use @use statements to provide custom variables (see above).

  2. In my opinion it is more transparent to import our "mdc-customization" module before each "mdc import". It's different from importing variables since it resolves the mdc modules with our custom properties instead of defining variables. Furthermore, components that do not use mdc at all should not import the mdc customization file just for the sake of simplicity.

janivo commented 3 years ago
  1. Yes, it currently uses @import because it's outdated. So we would have to wait for stencil to upgrade it. I just kinda liked the solution because it's out of the box.
  2. You are right, that it's more transparent. We should do it this way.