Skip to content

Commit

Permalink
fix(material/theming): disambiguate token names in extend-theme funct…
Browse files Browse the repository at this point in the history 
…ions

Avoids token name collisions between namespaces by assigning string
prefixes to token in namespaces that would otherwise have a collision
  • Loading branch information
@mmalerba
mmalerba committed Oct 10, 2024
1 parent 8efc358 commit 8f9a29c
Show file tree
Hide file tree
Showing 11 changed files with 200 additions and 48 deletions.
40 changes: 32 additions & 8 deletions src/material/button/_button-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
15 changes: 12 additions & 3 deletions src/material/button/_fab-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
18 changes: 12 additions & 6 deletions src/material/card/_card-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}

Expand Down
1 change: 1 addition & 0 deletions src/material/chips/_chips-theme.scss
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down
30 changes: 24 additions & 6 deletions src/material/core/_core-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
10 changes: 8 additions & 2 deletions src/material/core/selection/pseudo-checkbox/_pseudo-checkbox-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
75 changes: 74 additions & 1 deletion src/material/core/theming/tests/m3-theme.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(() =>
Expand Down Expand Up @@ -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();
});
Expand Down
25 changes: 19 additions & 6 deletions src/material/core/tokens/_token-utils.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}
}
}
Expand Down Expand Up @@ -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);
}
10 changes: 8 additions & 2 deletions src/material/form-field/_form-field-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/material/paginator/_paginator-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Expand Down
21 changes: 8 additions & 13 deletions src/material/tabs/_tabs-theme.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
);
}

Expand All @@ -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
);
}

Expand Down

0 comments on commit 8f9a29c

Please sign in to comment.