Skip to content

Commit 305af4f

Browse files
committed
build: Report duplicate style warnings for M3 themes
1 parent 3a4e0c7 commit 305af4f

File tree

6 files changed

+127
-26
lines changed

6 files changed

+127
-26
lines changed

.github/CODEOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
/src/material-experimental/popover-edit/** @andrewseguin
137137
/src/material-experimental/selection/** @andrewseguin
138138
/src/material-experimental/theming/** @mmalerba
139+
/src/material-experimental/theming-next/** @mmalerba
139140

140141
# CDK experimental package
141142
/src/cdk-experimental/* @andrewseguin

src/material-experimental/theming/_config-validation.scss

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
@function validate-theme-config($config) {
99
$err: mat.private-validate-type($config, 'map', 'null');
1010
@if $err {
11-
@return (#{'$config'} #{'should be a color configuraiton object. Got:'} $config);
11+
@return (#{'$config'} #{'should be a color configuration object. Got:'} $config);
1212
}
1313
$err: mat.private-validate-allowed-values(map.keys($config or ()), color, typography, density);
1414
@if $err {
@@ -35,7 +35,7 @@
3535
@function validate-color-config($config) {
3636
$err: mat.private-validate-type($config, 'map', 'null');
3737
@if $err {
38-
@return (#{'$config'} #{'should be a color configuraiton object. Got:'} $config);
38+
@return (#{'$config'} #{'should be a color configuration object. Got:'} $config);
3939
}
4040
$err: mat.private-validate-allowed-values(
4141
map.keys($config or ()), theme-type, primary, secondary, tertiary);
@@ -51,7 +51,7 @@
5151
@function validate-typography-config($config) {
5252
$err: mat.private-validate-type($config, 'map', 'null');
5353
@if $err {
54-
@return (#{'$config'} #{'should be a typography configuraiton object. Got:'} $config);
54+
@return (#{'$config'} #{'should be a typography configuration object. Got:'} $config);
5555
}
5656
$err: mat.private-validate-allowed-values(
5757
map.keys($config or ()), brand-family, plain-family, bold-weight, medium-weight,
@@ -68,7 +68,7 @@
6868
@function validate-density-config($config) {
6969
$err: mat.private-validate-type($config, 'map', 'null');
7070
@if $err {
71-
@return (#{'$config'} #{'should be a density configuraiton object. Got:'} $config);
71+
@return (#{'$config'} #{'should be a density configuration object. Got:'} $config);
7272
}
7373
$err: mat.private-validate-allowed-values(map.keys($config or ()), scale);
7474
@if $err {

src/material-experimental/theming/_definition.scss

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ $theme-version: 1;
7070
@return (
7171
$internals: (
7272
theme-version: $theme-version,
73-
typography-tokens:
74-
m3-tokens.generate-typography-tokens($brand, $plain, $bold, $medium, $regular)
73+
typography-tokens: m3-tokens.generate-typography-tokens(
74+
$brand, $plain, $bold, $medium, $regular)
7575
)
7676
);
7777
}

src/material/card/_card-theme.scss

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@
9797
@mixin theme($theme-or-color-config) {
9898
$theme: theming.private-legacy-get-theme($theme-or-color-config);
9999

100-
@if inspection.get-theme-version($theme-or-color-config) == 1 {
101-
@include _theme-from-tokens(inspection.get-theme-tokens($theme-or-color-config));
102-
}
103-
@else {
104-
@include theming.private-check-duplicate-theme-styles($theme, 'mat-card') {
100+
@include theming.private-check-duplicate-theme-styles($theme, 'mat-card') {
101+
@if inspection.get-theme-version($theme) == 1 {
102+
@include _theme-from-tokens(inspection.get-theme-tokens($theme));
103+
}
104+
@else {
105105
$color: theming.get-color-config($theme);
106106
$density: theming.get-density-config($theme);
107107
$typography: theming.get-typography-config($theme);

src/material/checkbox/_checkbox-theme.scss

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,11 @@
9797
@mixin theme($theme-or-color-config) {
9898
$theme: theming.private-legacy-get-theme($theme-or-color-config);
9999

100-
@if inspection.get-theme-version($theme-or-color-config) == 1 {
101-
@include _theme-from-tokens(inspection.get-theme-tokens($theme-or-color-config));
102-
}
103-
@else {
104-
@include theming.private-check-duplicate-theme-styles($theme, 'mat-checkbox') {
100+
@include theming.private-check-duplicate-theme-styles($theme, 'mat-checkbox') {
101+
@if inspection.get-theme-version($theme) == 1 {
102+
@include _theme-from-tokens(inspection.get-theme-tokens($theme));
103+
}
104+
@else {
105105
$color: theming.get-color-config($theme);
106106
$density: theming.get-density-config($theme);
107107
$typography: theming.get-typography-config($theme);

src/material/core/theming/_theming.scss

Lines changed: 110 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ $_disable-color-backwards-compatibility: false;
3535
$_emitted-color: () !default;
3636
$_emitted-typography: () !default;
3737
$_emitted-density: () !default;
38+
$_emitted-other: () !default;
3839

3940
/// Extracts a color from a palette or throws an error if it doesn't exist.
4041
/// @param {Map} $palette The palette from which to extract a color.
@@ -71,7 +72,6 @@ $_emitted-density: () !default;
7172
lighter: _get-color-from-palette($base-palette, $lighter),
7273
darker: _get-color-from-palette($base-palette, $darker),
7374
text: _get-color-from-palette($base-palette, $text),
74-
7575
default-contrast: get-contrast-color-from-palette($base-palette, $default),
7676
lighter-contrast: get-contrast-color-from-palette($base-palette, $lighter),
7777
darker-contrast: get-contrast-color-from-palette($base-palette, $darker)
@@ -321,6 +321,8 @@ $_emitted-density: () !default;
321321
// Private APIs
322322
//
323323

324+
$_internals: _mat-theming-internals-do-not-access;
325+
324326
// Checks if configurations that have been declared in the given theme have been generated
325327
// before. If so, warnings will be reported. This should notify developers in case duplicate
326328
// styles are accidentally generated due to wrong usage of the all-theme mixins.
@@ -334,6 +336,111 @@ $_emitted-density: () !default;
334336
// (e.g. `all-component-themes` and `all-legacy-component-themes`) without causing any
335337
// style duplication.
336338
@mixin private-check-duplicate-theme-styles($theme-or-color-config, $id) {
339+
// TODO(mmalerba): use get-theme-version for this check when its moved out of experimental.
340+
@if map.get($theme-or-color-config, $_internals, theme-version) == 1 {
341+
@include _check-duplicate-theme-styles-v1($theme-or-color-config, $id) {
342+
// Optionally, consumers of this mixin can wrap contents inside so that nested
343+
// duplicate style checks do not report another warning. e.g. if developers include
344+
// the `all-component-themes` mixin twice, only the top-level duplicate styles check
345+
// should report a warning. Not all individual components should report a warning too.
346+
$orig-mat-theme-ignore-duplication-warnings: $theme-ignore-duplication-warnings;
347+
$theme-ignore-duplication-warnings: true !global;
348+
@content;
349+
$theme-ignore-duplication-warnings: $orig-mat-theme-ignore-duplication-warnings !global;
350+
}
351+
}
352+
@else {
353+
@include _check-duplicate-theme-styles-v0($theme-or-color-config, $id) {
354+
// Optionally, consumers of this mixin can wrap contents inside so that nested
355+
// duplicate style checks do not report another warning. e.g. if developers include
356+
// the `all-component-themes` mixin twice, only the top-level duplicate styles check
357+
// should report a warning. Not all individual components should report a warning too.
358+
$orig-mat-theme-ignore-duplication-warnings: $theme-ignore-duplication-warnings;
359+
$theme-ignore-duplication-warnings: true !global;
360+
@content;
361+
$theme-ignore-duplication-warnings: $orig-mat-theme-ignore-duplication-warnings !global;
362+
}
363+
}
364+
}
365+
366+
/// Strip out any settings map entries that have empty values (null or ()).
367+
@function _strip-empty-settings($settings) {
368+
$result: ();
369+
@each $key, $value in $settings {
370+
@if $value != null and $value != () {
371+
$result: map.set($result, $key, $value);
372+
}
373+
}
374+
@return if($result == (), null, $result);
375+
}
376+
377+
// Checks for duplicate styles in a `theme-version: 1` style theme.
378+
@mixin _check-duplicate-theme-styles-v1($theme-or-color-config, $id) {
379+
$color-settings: _strip-empty-settings((
380+
theme-type: map.get($theme-or-color-config, $_internals, theme-type),
381+
color-tokens: map.get($theme-or-color-config, $_internals, color-tokens),
382+
));
383+
$typography-settings: _strip-empty-settings((
384+
typography-tokens: map.get($theme-or-color-config, $_internals, typography-tokens),
385+
));
386+
$density-settings: _strip-empty-settings((
387+
density-scale: map.get($theme-or-color-config, $_internals, density-scale),
388+
density-tokens: map.get($theme-or-color-config, $_internals, density-tokens),
389+
));
390+
$other-settings: _strip-empty-settings((
391+
other-tokens: map.get($theme-or-color-config, $_internals, other-tokens),
392+
));
393+
$previous-color-settings: map.get($_emitted-color, $id) or ();
394+
$previous-typography-settings: map.get($_emitted-typography, $id) or ();
395+
$previous-density-settings: map.get($_emitted-density, $id) or ();
396+
$previous-other-settings: map.get($_emitted-other, $id) or ();
397+
398+
// Check if the color configuration has been generated before.
399+
@if $color-settings != null {
400+
@if list.index($previous-color-settings, $color-settings) != null and
401+
not $theme-ignore-duplication-warnings {
402+
@warn 'The same color styles are generated multiple times. ' + $_duplicate-warning;
403+
}
404+
$previous-color-settings: list.append($previous-color-settings, $color-settings);
405+
}
406+
407+
// Check if the typography configuration has been generated before.
408+
@if $typography-settings != null {
409+
@if list.index($previous-typography-settings, $typography-settings) != null and
410+
not $theme-ignore-duplication-warnings {
411+
@warn 'The same typography styles are generated multiple times. ' + $_duplicate-warning;
412+
}
413+
$previous-typography-settings: list.append($previous-typography-settings, $typography-settings);
414+
}
415+
416+
// Check if the density configuration has been generated before.
417+
@if $density-settings != null {
418+
@if list.index($previous-density-settings, $density-settings) != null and
419+
not $theme-ignore-duplication-warnings {
420+
@warn 'The same density styles are generated multiple times. ' + $_duplicate-warning;
421+
}
422+
$previous-density-settings: list.append($previous-density-settings, $density-settings);
423+
}
424+
425+
// Check if the other configuration has been generated before.
426+
@if $other-settings != null {
427+
@if list.index($previous-other-settings, $other-settings) != null and
428+
not $theme-ignore-duplication-warnings {
429+
@warn 'The same base theme styles are generated multiple times. ' + $_duplicate-warning;
430+
}
431+
$previous-other-settings: list.append($previous-other-settings, $other-settings);
432+
}
433+
434+
$_emitted-color: map.set($_emitted-color, $id, $previous-color-settings) !global;
435+
$_emitted-density: map.set($_emitted-density, $id, $previous-density-settings) !global;
436+
$_emitted-typography: map.set($_emitted-typography, $id, $previous-typography-settings) !global;
437+
$_emitted-other: map.set($_emitted-other, $id, $previous-other-settings) !global;
438+
439+
@content;
440+
}
441+
442+
// Checks for duplicate styles in a `theme-version: 0` style theme.
443+
@mixin _check-duplicate-theme-styles-v0($theme-or-color-config, $id) {
337444
$theme: private-legacy-get-theme($theme-or-color-config);
338445
$color-config: get-color-config($theme);
339446
$density-config: get-density-config($theme);
@@ -383,13 +490,6 @@ $_emitted-density: () !default;
383490
$_emitted-density: map.merge($_emitted-density, ($id: $previous-density)) !global;
384491
$_emitted-typography: map.merge($_emitted-typography, ($id: $previous-typography)) !global;
385492

386-
// Optionally, consumers of this mixin can wrap contents inside so that nested
387-
// duplicate style checks do not report another warning. e.g. if developers include
388-
// the `all-component-themes` mixin twice, only the top-level duplicate styles check
389-
// should report a warning. Not all individual components should report a warning too.
390-
$orig-mat-theme-ignore-duplication-warnings: $theme-ignore-duplication-warnings;
391-
$theme-ignore-duplication-warnings: true !global;
392-
393493
// If duplicate default density styles would be generated for a legacy constructed theme,
394494
// we adjust the density generation so that no density styles are generated by default.
395495
// If no default density styles have been generated yet, we ensure that the styles
@@ -401,7 +501,6 @@ $_emitted-density: () !default;
401501
compatibility.$private-density-generate-styles: not $duplicate-legacy-density;
402502

403503
@content;
404-
$theme-ignore-duplication-warnings: $orig-mat-theme-ignore-duplication-warnings !global;
405504

406505
compatibility.$private-density-generate-at-root: false;
407506
compatibility.$private-density-generate-styles: true;
@@ -456,7 +555,8 @@ $_emitted-density: () !default;
456555
// TODO(devversion): remove this in the future. Constructing themes manually is rare,
457556
// and the code can be easily updated to the new API.
458557
@function private-legacy-get-theme($theme-or-color-config) {
459-
@if private-is-theme-object($theme-or-color-config) {
558+
@if private-is-theme-object($theme-or-color-config) or
559+
map.get($theme-or-color-config, $_internals, theme-version) == 1 {
460560
@return $theme-or-color-config;
461561
}
462562

0 commit comments

Comments
 (0)