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

mdc-theme CSS variable customization for calculated colors #4431

Closed
monstasat opened this issue Feb 19, 2019 · 4 comments
Closed

mdc-theme CSS variable customization for calculated colors #4431

monstasat opened this issue Feb 19, 2019 · 4 comments

Comments

@monstasat
Copy link

monstasat commented Feb 19, 2019

Hi!
I am trying to implement switching between light and dark themes in my app.
As far as I understand, to switch between two themes at runtime, without sass recompilation, I should override some css variables. I've declared a CSS class applied to the :root element where the variables like --mdc-theme-primary, --mdc-theme-background and others are redefined.
Despite the seeming simplicity of the described approach, I am facing some issues. Here is a couple of them:

1. MDCDrawer SASS mixins for styling inner lists are defined in the following manner:

@mixin mdc-drawer-item-text-ink-color($color) {
  $value: rgba(mdc-theme-prop-value($color), $mdc-drawer-item-inactive-text-ink-opacity);

  .mdc-list-item {
    @include mdc-list-item-primary-text-ink-color($value);
  }
}

Inside this mixin, another MDCTheme mixin, mdc-theme-prop-value, is used. It means that the color of list item text is not controlled by CSS variables, hence not customizable.

2. MDCTopAppBar SASS mixins for customizing fill color are defined as follows:

@mixin mdc-top-app-bar-fill-color-accessible($color) {
  @include mdc-top-app-bar-fill-color($color);
  @include mdc-top-app-bar-ink-color(mdc-theme-accessible-ink-color($color));
}

Inside this mixin, another MDCTheme mixin, mdc-theme-accessible-ink-color, is used.
It is defined as follows:

@function mdc-theme-accessible-ink-color($fill-color, $text-style: primary) {
  $fill-color-value: mdc-theme-prop-value($fill-color);
  $color-map-for-tone: map-get($mdc-theme-text-colors, mdc-theme-contrast-tone($fill-color-value));

  @if not map-has-key($color-map-for-tone, $text-style) {
    @error "Invalid $text-style: '#{$text-style}'. Choose one of: #{map-keys($color-map-for-tone)}";
  }

  @return map-get($color-map-for-tone, $text-style);
}

Once again, the mdc-them-prop-value mixin is used inside, and this breaks ink color customization by CSS variables.

How can I fix the mentioned issues? Maybe my approach for theme switching is wrong?
Or the color system of material components web is not yet finished/debugged?

@monstasat
Copy link
Author

I've already partly referenced this issue in #4256, but when I've tried to change theme colors globally via css vars, it seems that the problem affects many components.
In fact, the question comes down to the following: do you consider to make it possible in the future to change color theme via css variables redefinition? If not, what other approach should be used?

@acdvorak
Copy link
Contributor

Thanks for filing this issue! 😀

In order to meet accessibility requirements for contrast ratios, we frequently need to calculate the lightness/darkness level of a color. It's not possible to do this in pure CSS, so we do it in Sass mixins and functions instead.

The mdc-theme-prop() mixin automatically emits an alternate CSS variable declaration for theme color values:

#{$property}: $value;
@if $edgeOptOut {
// stylelint-disable max-nesting-depth
@at-root {
@supports not (-ms-ime-align: auto) {
// stylelint-disable scss/selector-no-redundant-nesting-selector
& {
/* @alternate */
#{$property}: var(--mdc-theme-#{$style}, $value);

Unfortunately, we don't do the same thing for calculated colors, which leaves CSS-only users hanging.

If you're looking for runtime theme customization, your best option is to compile two separate stylesheets with Sass, and switch between them. Our old Demo pages do this (clone the repo and run npm i && npm run dev to try it out):

swapTheme_(safeNewTheme) {
const oldLink = this.root_.getElementById(ids.THEME_STYLESHEET);
const newUri = `/assets/theme/theme-${unwrapSafeDemoTheme(safeNewTheme)}.css`;
this.hotSwapper_.hotSwapStylesheet(oldLink, newUri);
this.selectThemeInMenu_(safeNewTheme);
}

swapItLikeItsHot_(oldLink, oldUri, newUri) {
this.logHotSwap_('swapping', oldUri, newUri, '...');
// Ensure that oldLink has a unique ID so we can remove all stale stylesheets from the DOM after newLink loads.
// This is a more robust approach than holding a reference to oldLink and removing it directly, because a user might
// quickly switch themes several times before the first stylesheet finishes loading (especially over a slow network)
// and each new stylesheet would try to remove the first one, leaving multiple conflicting stylesheets in the DOM.
if (!oldLink.id) {
oldLink.id = `stylesheet-${Math.floor(Math.random() * Date.now())}`;
}
const newLink = /** @type {!Element} */ (oldLink.cloneNode(false));
newLink.setAttribute('href', newUri);
newLink.setAttribute(attrs.IS_LOADING, 'true');
// IE 11 and Edge fire the `load` event twice for `<link>` elements.
newLink.addEventListener('load', util.debounce(() => {
this.handleStylesheetLoad_(newLink, newUri, oldUri);
}, 50));
oldLink.parentNode.insertBefore(newLink, oldLink);
this.pendingRequests_.push(newUri);
this.toolbarProvider_.setIsLoading(true);
}

We'd love to fix this at some point, but it's unlikely we'll have the resources to address it in the foreseeable future 😢

@acdvorak acdvorak changed the title MDCTheme customization question mdc-theme CSS variable customization for calculated colors Feb 21, 2019
@kfranqueiro
Copy link
Contributor

Notable suggestions for solving this:

#3066 (comment)

#3145

There is also likely to be more work on the color system this year which may also impact this in some way.

@abhiomkar
Copy link
Collaborator

Closing this as dup of #4709

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants