From 8f9a29cec4aa9619a8b34072acd155e4062d2f89 Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Thu, 27 Jun 2024 15:50:20 -0700 Subject: [PATCH] fix(material/theming): disambiguate token names in extend-theme functions Avoids token name collisions between namespaces by assigning string prefixes to token in namespaces that would otherwise have a collision --- src/material/button/_button-theme.scss | 40 ++++++++-- src/material/button/_fab-theme.scss | 15 +++- src/material/card/_card-theme.scss | 18 +++-- src/material/chips/_chips-theme.scss | 1 + src/material/core/_core-theme.scss | 30 ++++++-- .../_pseudo-checkbox-theme.scss | 10 ++- .../core/theming/tests/m3-theme.spec.ts | 75 ++++++++++++++++++- src/material/core/tokens/_token-utils.scss | 25 +++++-- .../form-field/_form-field-theme.scss | 10 ++- src/material/paginator/_paginator-theme.scss | 3 +- src/material/tabs/_tabs-theme.scss | 21 ++---- 11 files changed, 200 insertions(+), 48 deletions(-) diff --git a/src/material/button/_button-theme.scss b/src/material/button/_button-theme.scss index 7b82f8a37dd1..d9c8058b3d68 100644 --- a/src/material/button/_button-theme.scss +++ b/src/material/button/_button-theme.scss @@ -417,14 +417,38 @@ @return token-utils.extend-theme( $theme, sass-utils.list-of( - tokens-mdc-filled-button.$prefix, - tokens-mdc-outlined-button.$prefix, - tokens-mdc-protected-button.$prefix, - tokens-mdc-text-button.$prefix, - tokens-mat-filled-button.$prefix, - tokens-mat-outlined-button.$prefix, - tokens-mat-protected-button.$prefix, - tokens-mat-text-button.$prefix + ( + namespace: tokens-mdc-filled-button.$prefix, + prefix: 'filled-', + ), + ( + namespace: tokens-mdc-outlined-button.$prefix, + prefix: 'outlined-', + ), + ( + namespace: tokens-mdc-protected-button.$prefix, + prefix: 'protected-', + ), + ( + namespace: tokens-mdc-text-button.$prefix, + prefix: 'text-', + ), + ( + namespace: tokens-mat-filled-button.$prefix, + prefix: 'filled-', + ), + ( + namespace: tokens-mat-outlined-button.$prefix, + prefix: 'outlined-', + ), + ( + namespace: tokens-mat-protected-button.$prefix, + prefix: 'protected-', + ), + ( + namespace: tokens-mat-text-button.$prefix, + prefix: 'text-', + ) ), $overrides ); diff --git a/src/material/button/_fab-theme.scss b/src/material/button/_fab-theme.scss index 1acd56128822..c92ba73b680b 100644 --- a/src/material/button/_fab-theme.scss +++ b/src/material/button/_fab-theme.scss @@ -194,10 +194,19 @@ $theme, sass-utils.list-of( tokens-mdc-fab.$prefix, - tokens-mdc-fab-small.$prefix, - tokens-mdc-extended-fab.$prefix, + ( + namespace: tokens-mdc-fab-small.$prefix, + prefix: 'small-', + ), + ( + namespace: tokens-mdc-extended-fab.$prefix, + prefix: 'extended-', + ), tokens-mat-fab.$prefix, - tokens-mat-fab-small.$prefix + ( + namespace: tokens-mat-fab-small.$prefix, + prefix: 'small-', + ) ), $overrides ); diff --git a/src/material/card/_card-theme.scss b/src/material/card/_card-theme.scss index 6cab8352b26f..f993a0d42fdd 100644 --- a/src/material/card/_card-theme.scss +++ b/src/material/card/_card-theme.scss @@ -87,13 +87,19 @@ /// @param {Map} $overrides The token values to override in the theme. @function extend-theme($theme, $overrides: ()) { @return token-utils.extend-theme( - $theme, - sass-utils.list-of( - tokens-mdc-elevated-card.$prefix, - tokens-mdc-outlined-card.$prefix, - tokens-mat-card.$prefix + $theme, + sass-utils.list-of( + ( + namespace: tokens-mdc-elevated-card.$prefix, + prefix: 'elevated-', ), - $overrides + ( + namespace: tokens-mdc-outlined-card.$prefix, + prefix: 'outlined-', + ), + tokens-mat-card.$prefix + ), + $overrides ); } diff --git a/src/material/chips/_chips-theme.scss b/src/material/chips/_chips-theme.scss index e748fd77185d..18165932b6d6 100644 --- a/src/material/chips/_chips-theme.scss +++ b/src/material/chips/_chips-theme.scss @@ -1,4 +1,5 @@ @use 'sass:color'; +@use '../core/style/sass-utils'; @use '../core/tokens/m2/mdc/chip' as tokens-mdc-chip; @use '../core/tokens/m2/mat/chip' as tokens-mat-chip; @use '../core/tokens/token-utils'; diff --git a/src/material/core/_core-theme.scss b/src/material/core/_core-theme.scss index 9c037fb72110..6ac71eb4d38b 100644 --- a/src/material/core/_core-theme.scss +++ b/src/material/core/_core-theme.scss @@ -101,12 +101,30 @@ @return token-utils.extend-theme( $theme, sass-utils.list-of( - tokens-mat-app.$prefix, - tokens-mat-ripple.$prefix, - tokens-mat-option.$prefix, - tokens-mat-optgroup.$prefix, - tokens-mat-full-pseudo-checkbox.$prefix, - tokens-mat-minimal-pseudo-checkbox.$prefix + ( + namespace: tokens-mat-app.$prefix, + prefix: 'app-', + ), + ( + namespace: tokens-mat-ripple.$prefix, + prefix: 'ripple-', + ), + ( + namespace: tokens-mat-option.$prefix, + prefix: 'option-', + ), + ( + namespace: tokens-mat-optgroup.$prefix, + prefix: 'optgroup-', + ), + ( + namespace: tokens-mat-full-pseudo-checkbox.$prefix, + prefix: 'pseudo-checkbox-full-', + ), + ( + namespace: tokens-mat-minimal-pseudo-checkbox.$prefix, + prefix: 'pseudo-checkbox-minimal-', + ) ), $overrides ); diff --git a/src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss b/src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss index 02e0563bb09a..1704d7bdd7f8 100644 --- a/src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss +++ b/src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss @@ -75,8 +75,14 @@ @return token-utils.extend-theme( $theme, sass-utils.list-of( - tokens-mat-full-pseudo-checkbox.$prefix, - tokens-mat-minimal-pseudo-checkbox.$prefix + ( + namespace: tokens-mat-full-pseudo-checkbox.$prefix, + prefix: 'full-', + ), + ( + namespace: tokens-mat-minimal-pseudo-checkbox.$prefix, + prefix: 'minimal-', + ) ), $overrides ); diff --git a/src/material/core/theming/tests/m3-theme.spec.ts b/src/material/core/theming/tests/m3-theme.spec.ts index a2818c6e2fea..d0ee960f79ce 100644 --- a/src/material/core/theming/tests/m3-theme.spec.ts +++ b/src/material/core/theming/tests/m3-theme.spec.ts @@ -262,6 +262,77 @@ describe('M3 theme', () => { ); }); + it('should allow accessing a namespace with a prefix', () => { + const css = transpile(` + @use '../../tokens/token-utils'; + + $theme: token-utils.extend-theme($theme, + ( + (namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'), + (mat, full-pseudo-checkbox) + ), + ( + minimal-selected-checkmark-color: magenta, + selected-checkmark-color: cyan + ) + ); + + html { + @include mat.pseudo-checkbox-theme($theme); + } + `); + + expect(css).toContain('--mat-minimal-pseudo-checkbox-selected-checkmark-color: magenta'); + expect(css).toContain('--mat-full-pseudo-checkbox-selected-checkmark-color: cyan'); + }); + + it('should not allow accessing a prefixed namespace without its prefix', () => { + expect(() => + transpile(` + @use '../../tokens/token-utils'; + + $theme: token-utils.extend-theme($theme, + ( + (namespace: (mat, minimal-pseudo-checkbox), prefix: 'minimal-'), + ), + ( + selected-checkmark-color: magenta + ) + ); + + html { + @include mat.pseudo-checkbox-theme($theme); + } + `), + ).toThrowError( + /Error extending theme: Unrecognized token `selected-checkmark-color`. Allowed tokens are:.* minimal-selected-checkmark-color/, + ); + }); + + it('should detect name collisions that remain after prefixes are applied', () => { + expect(() => + transpile(` + @use '../../tokens/token-utils'; + + $theme: token-utils.extend-theme($theme, + ( + (namespace: (mat, minimal-pseudo-checkbox), prefix: 'both-'), + (namespace: (mat, full-pseudo-checkbox), prefix: 'both-') + ), + ( + both-selected-checkmark-color: magenta + ) + ); + + html { + @include mat.pseudo-checkbox-theme($theme); + } + `), + ).toThrowError( + /Error extending theme: Ambiguous token name `both-selected-checkmark-color` exists in multiple namespaces/, + ); + }); + it('should not error when calling component extend-theme functions', () => { // Ensures that no components have issues with ambiguous token names. expect(() => @@ -307,7 +378,9 @@ describe('M3 theme', () => { $theme: mat.tooltip-extend-theme($theme, ()); $theme: mat.tree-extend-theme($theme, ()); - @include mat.all-component-themes($theme); + html { + @include mat.all-component-themes($theme); + } `), ).not.toThrow(); }); diff --git a/src/material/core/tokens/_token-utils.scss b/src/material/core/tokens/_token-utils.scss index 6785d54b38c5..4d1c40604208 100644 --- a/src/material/core/tokens/_token-utils.scss +++ b/src/material/core/tokens/_token-utils.scss @@ -254,6 +254,11 @@ $_system-fallbacks: m3-system.create-system-fallbacks(); // Returns a theme config with the given tokens overridden. /// @param {Map} $theme The material theme for an application. +/// @param {List} $extend-namespaces The namespaces to extend in the theme. Each item can be either: +/// 1. A list, representing a namespace to match token names against. +/// 2. A map, representing a namespace to match tokens agaisnt + a prefix string that the token +/// name must start with to match this namespace. The prefix will be stripped before matching +/// the token name. /// @param {Map} $overrides The token values to override in the theme. @function extend-theme($theme, $extend-namespaces, $overrides: ()) { $internals: _mat-theming-internals-do-not-access; @@ -267,18 +272,25 @@ $_system-fallbacks: m3-system.create-system-fallbacks(); // Determine which system and namespace each shorthand token belongs to. $seen-tokens: (); @each $namespace in $extend-namespaces { + $prefix: ''; + // Unpack namespaces that come with a prefix. + @if meta.type-of($namespace) == 'map' { + $prefix: map.get($namespace, prefix); + $namespace: map.get($namespace, namespace); + } $namespace-found: false; @each $system in $systems { $tokens: map.get($theme, $internals, $system, $namespace); @if ($tokens != null) { $namespace-found: true; @each $name, $value in $tokens { - @if map.has-key($seen-tokens, $name) { - @error #{'Error extending theme: Ambiguous token name `'}#{$name}#{ - '` exists in multiple namespaces: `('}#{list.nth(map.get($seen-tokens, $name), 1)}#{ - ')` and `('}#{$namespace}#{')`'}; + $prefixed-name: $prefix + $name; + @if map.has-key($seen-tokens, $prefixed-name) { + @error #{'Error extending theme: Ambiguous token name `'}#{$prefixed-name}#{ + '` exists in multiple namespaces: `('}#{ + list.nth(map.get($seen-tokens, $prefixed-name), 1)}#{')` and `('}#{$namespace}#{')`'}; } - $seen-tokens: map.set($seen-tokens, $name, ($namespace, $system)); + $seen-tokens: map.set($seen-tokens, $prefixed-name, ($namespace, $system, $name)); } } } @@ -321,5 +333,6 @@ $_system-fallbacks: m3-system.create-system-fallbacks(); $namespace: list.append($namespace, $variant); } $system: list.nth($token-info, 2); - @return map.set($theme, $internals, $system, $namespace, $token, $value); + $token-name: list.nth($token-info, 3); + @return map.set($theme, $internals, $system, $namespace, $token-name, $value); } diff --git a/src/material/form-field/_form-field-theme.scss b/src/material/form-field/_form-field-theme.scss index 66a706f4a1a2..14a2380ec7e6 100644 --- a/src/material/form-field/_form-field-theme.scss +++ b/src/material/form-field/_form-field-theme.scss @@ -157,8 +157,14 @@ @return token-utils.extend-theme( $theme, sass-utils.list-of( - tokens-mdc-filled-text-field.$prefix, - tokens-mdc-outlined-text-field.$prefix, + ( + namespace: tokens-mdc-filled-text-field.$prefix, + prefix: 'filled', + ), + ( + namespace: tokens-mdc-outlined-text-field.$prefix, + prefix: 'outlined', + ), tokens-mat-form-field.$prefix ), $overrides diff --git a/src/material/paginator/_paginator-theme.scss b/src/material/paginator/_paginator-theme.scss index 1bd96ee79dd7..01750571cc4f 100644 --- a/src/material/paginator/_paginator-theme.scss +++ b/src/material/paginator/_paginator-theme.scss @@ -44,7 +44,8 @@ @mixin density($theme) { $density-scale: inspection.get-theme-density($theme); $form-field-density: if( - (meta.type-of($density-scale) == 'number' and $density-scale >= -4) or $density-scale == maximum, + (meta.type-of($density-scale) == 'number' and $density-scale >= -4) or + ($density-scale == maximum), -4, $density-scale ); diff --git a/src/material/tabs/_tabs-theme.scss b/src/material/tabs/_tabs-theme.scss index 1b87ddff628b..b1d6125ae1e8 100644 --- a/src/material/tabs/_tabs-theme.scss +++ b/src/material/tabs/_tabs-theme.scss @@ -139,11 +139,7 @@ $tokens, (prefix: tokens-mdc-secondary-navigation-tab.$prefix, tokens: $tab-tokens), (prefix: tokens-mdc-tab-indicator.$prefix, tokens: $tab-indicator-tokens), - (prefix: tokens-mat-tab-header.$prefix, tokens: $tab-header-tokens), - ( - prefix: tokens-mat-tab-header-with-background.$prefix, - tokens: $tab-header-with-background-tokens, - ), + (prefix: tokens-mat-tab-header.$prefix, tokens: $tab-header-tokens) ); } @@ -152,14 +148,13 @@ /// @param {Map} $overrides The token values to override in the theme. @function extend-theme($theme, $overrides: ()) { @return token-utils.extend-theme( - $theme, - sass-utils.list-of( - tokens-mdc-tab.$prefix, - tokens-mdc-tab-indicator.$prefix, - tokens-mat-tab-header.$prefix, - tokens-mat-tab-header-with-background.$prefix - ), - $overrides + $theme, + sass-utils.list-of( + tokens-mdc-secondary-navigation-tab.$prefix, + tokens-mdc-tab-indicator.$prefix, + tokens-mat-tab-header.$prefix + ), + $overrides ); }