Skip to content

Commit

Permalink
fix(material/core): M3 themes not inserting loaded marker
Browse files Browse the repository at this point in the history
Fixes that all M3 themes were causing a "no theme has been loaded" warning to be logged, because they weren't inserting the loaded marker.

Note: it's tempting to create the marker as a token, but we can't do it because tokens are output under a selector, whereas we want the marker to always be at the top level since we detect it by creating an element and inserting it into the `body`.

Fixes #29115.

(cherry picked from commit 233c8a3)
  • Loading branch information
crisbeto committed May 28, 2024
1 parent e1a9f76 commit d96b5e3
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 12 deletions.
30 changes: 22 additions & 8 deletions src/material/core/_core-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@
@use './tokens/m2/mat/full-pseudo-checkbox' as tokens-mat-full-pseudo-checkbox;
@use './tokens/m2/mat/minimal-pseudo-checkbox' as tokens-mat-minimal-pseudo-checkbox;

$_has-inserted-loaded-marker: false;

@mixin _theme-loaded-marker {
@if not $_has-inserted-loaded-marker {
$_has-inserted-loaded-marker: true !global;

// Marker that is used to determine whether the user has added a theme to their page.
// Needs to be generated at the root, because themes may be nested inside classes.
@at-root {
.mat-theme-loaded-marker {
display: none;
}
}
}
}

@mixin base($theme) {
@if inspection.get-theme-version($theme) == 1 {
@include _theme-from-tokens(inspection.get-theme-tokens($theme, base));
Expand All @@ -26,6 +42,9 @@
@include optgroup-theme.base($theme);
@include pseudo-checkbox-theme.base($theme);
}

// The marker is a concrete style no matter which Material version we're targeting.
@include _theme-loaded-marker;
}

@mixin color($theme) {
Expand Down Expand Up @@ -54,14 +73,6 @@
}
}
}

// TODO(crisbeto): move this into the base.
// Marker that is used to determine whether the user has added a theme to their page.
@at-root {
.mat-theme-loaded-marker {
display: none;
}
}
}

@mixin typography($theme) {
Expand Down Expand Up @@ -127,6 +138,9 @@
}
}
}

// The marker is a concrete style no matter which Material version we're targeting.
@include _theme-loaded-marker;
}

@mixin _theme-from-tokens($tokens, $options...) {
Expand Down
10 changes: 7 additions & 3 deletions src/material/core/theming/tests/m3-theme.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {parse} from 'postcss';
import {parse, Rule} from 'postcss';
import {compileString} from 'sass';
import {runfiles} from '@bazel/runfiles';
import * as path from 'path';
Expand Down Expand Up @@ -68,14 +68,18 @@ describe('M3 theme', () => {
root.walkRules(rule => {
selectors.push(rule.selector);
});
expect(selectors).toEqual(['html']);
expect(selectors).toEqual(['html', '.mat-theme-loaded-marker']);
});

it('should only emit CSS variables', () => {
const root = parse(transpile(`html { @include mat.all-component-themes($theme); }`));
const nonVarProps: string[] = [];
root.walkDecls(decl => {
if (!decl.prop.startsWith('--')) {
if (
!decl.prop.startsWith('--') &&
// Skip the theme loaded marker since it can't be a variable.
(decl.parent as Rule).selector !== '.mat-theme-loaded-marker'
) {
nonVarProps.push(decl.prop);
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,14 @@ describe('theming inspection api', () => {
div {
@include mat.all-component-themes($theme);
}`);
expect(css).toBe('');
expect(css).toBe(
[
// The marker is always included.
`.mat-theme-loaded-marker {`,
` display: none;`,
`}`,
].join('\n'),
);
});
});
});

0 comments on commit d96b5e3

Please sign in to comment.