Skip to content

refactor(material-experimental/theming): condense matx.theme and matx.retheme into a single mixin #27309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/dev-app/theme-token-api.scss
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,9 @@ $theme: mat.define-light-theme((
// on the page will inherit these tokens.
html {
@include matx.theme(
$tokens: mat.m2-tokens-from-theme($theme),
$components: (
matx.card(),
matx.checkbox(),
)
matx.token-defaults(mat.m2-tokens-from-theme($theme)),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andrewseguin I discovered that you left a comment on an isolated commit that is somehow no longer part of this actual PR, so transferring the comment here:

I'm all in favor of removing retheme and making a single API that users need to call for applying their styles.

However, there's a lot of layers here that I don't expect users to understand, and taking a step back it seems a little odd to read. We created a theme with define-light-theme but we don't pass that to a theme mixin, but instead we pass that to a "get-tokens" that gets passed to "defaults" and finally put into "theme.

It seems like the theme mixin should be getting some kind of first-class theme object here. I think the confusion comes from trying to support our old theme object. E.g. why does our theme have a mixin called define-light-theme when light has nothing to do with density or typography?

Is there a way to at least eliminate the need for calling define-light-theme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for creating a theme and converting it to tokens is because this example shows how an M2 app would use the new API. The only way we really have to get M2 tokens is to generate them from the legacy theme objects. We had to make up our own tokens for MDC, based on the theme since MDC doesn't give us M2 tokens.

For an M3 app, they would supply the tokens directly without the need for creating a theme first. We still need to figure out exactly how users are expected to get those:

  • export from MDC's theme builder (we still need to figure out how that works)
  • generate from a single color swatch (I think we've been told this can't be done in Sass for now)
  • some other way ??

To make the M2 API less clunky, we could make a single function that takes the theme, generates tokens from it, and then returns a configuration for the new theme system as a one-shot:

@include matx.theme(
  matx.token-defaults-from-legacy-theme($app-theme),
  matx.checkbox(),
  ...
);

It's not going to remove the need to call define-light-theme to generate $app-theme though.

matx.card(),
matx.checkbox(),
);
}

Expand All @@ -51,11 +49,11 @@ html {
// rather than the ones for light theme tokens set on `body`. Note that we're not setting *all* of
// the tokens, since many (density, typography, etc) are the same between light and dark theme.
.demo-unicorn-dark-theme {
@include matx.retheme((
@include matx.theme(
// TODO(mmalerba): In the future this should be configured through `matx.system-colors()`
matx.checkbox((theme-type: dark)),
matx.card((theme-type: dark)),
));
);
}

// Apply tokens related to the color palette to any element with `.mat-primary`, `.mat-accent`, or
Expand All @@ -68,31 +66,31 @@ html {
// take precedence.
// (e.g. <div class="mat-warn><mat-checkbox class="mat-primary">I'm primary</mat-checkbox></div>)
.mat-primary {
@include matx.retheme((
@include matx.theme(
matx.checkbox((
color-palette: map.get($theme, color, primary)
)),
));
);
}
.mat-accent {
@include matx.retheme((
@include matx.theme(
matx.checkbox((
color-palette: map.get($theme, color, accent)
)),
));
);
}
.mat-warn {
@include matx.retheme((
@include matx.theme(
matx.checkbox((
color-palette: map.get($theme, color, warn)
)),
));
);
}

// Apply tokens for a completely custom checkbox that appears as an unfilled red box when unchecked,
// and a filled green box when checked.
.demo-traffic-light-checkbox {
@include matx.retheme((
@include matx.theme(
matx.checkbox((
checkmark-color: transparent,
selected-box-color: green,
Expand All @@ -110,5 +108,10 @@ html {
unselected-hover-ring-color: red,
unselected-pressed-ring-color: red,
))
));
);
}

.demo-what-am-i-doing {
// Will not produce any output, should result in a warning.
@include matx.theme(matx.checkbox());
}
2 changes: 1 addition & 1 deletion src/material-experimental/_index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
popover-edit-typography, popover-edit-density, popover-edit-theme;

// Token-based theming API
@forward './theming/theming' show theme, retheme;
@forward './theming/theming' show theme, token-defaults;
@forward './theming/checkbox' show checkbox;
@forward './theming/card' show card;

Expand Down
31 changes: 14 additions & 17 deletions src/material-experimental/theming/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,37 +20,34 @@ $my-theme: mat.define-light-theme(...);
$m2-tokens: mat.m2-tokens-from-theme($my-theme);
```
## Component theme configuration functions
- These functions are used to specify which tokens should be applied by the theming mixins _and_ to customize the tokens used in that component to something other than the value from the token set
- So far the following component theme configuration functions have been implements:
- These functions are used to specify which tokens should be applied by the `matx.theme` mixin _and_ to customize the tokens used in that component to something other than the value from the token set
- `matx.token-defaults` is a special configuration function used to set the default token values that will be used for any components that are configured as part of the same mixin call, if no `matx.token-defaults` config is specified, only tokens for the explicitly customized properties will be emitted
- So far the following component theme configuration functions have been implemented:
- `matx.checkbox` configures tokens for the mat-checkbox to be applied
- `matx.card` configures tokens for the mat-card to be applied
- The returned configurations from these functions are passed to `matx.theme` or `matx.retheme`
- If no arguments are passed, the configuration instructs the mixin to just output the default value for all of the tokens needed by that component
- The functions can also accept a map of customizations as an argument.
- The returned configurations from these functions are passed to `matx.theme`
- The functions optionally accept a map of customizations as an argument which allows overriding properties tha would otherwise be derived from the default tokens.
- Each function has its own set of supported map keys that can be used to customize the value of the underlying tokens
- The map keys are a higher level API then the tokens, some of the keys may result in a single token being change, but some may change multiple tokens
- The map keys are a higher level API then the tokens, some the keys may result in a single token being change, but some may change multiple tokens
- For supported map keys (TODO: have docs for these):
- See `$_customization-resolvers` [here](https://github.com/angular/components/blob/main/src/material-experimental/theming/_checkbox.scss) for `matx.checkbox`
- See `$_customization-resolvers` [here](https://github.com/angular/components/blob/main/src/material-experimental/theming/_card.scss) for `matx.card`

## Theming mixins
- There are 2 mixins used for theming apps
- `matx.theme` is intended to apply the full theme for some components, with all tokens they need to function.
- `matx.retheme` is intended to re-apply specific tokens to change the appearance for some components by overriding the tokens applied by `matx.theme`.
- Both mixins emit *only* CSS variables representing design tokens
- Both mixins emit their tokens directly under the user specified selector. This gives the user complete control over the selector specificity.
- There is a single mixin used for theming apps: `matx.theme` applies the theme for some set of components (specified by passing component configs)
- This mixin will always apply theme values for properties explicitly customized in the individual component configs
- This mixin will apply *all* tokens for the configured component if a `matx.token-defaults` config is specified
- This mixin emits *only* CSS variables representing design tokens
- This mixin emits the CSS vars directly under the user specified selector. This gives the user complete control over the selector specificity.
- Using `matx.theme`
- Takes 2 arguments:
- `$tokens` The set of token defaults that will be used for any tokens not explicitly customized by the component theme config
- `$components` List of component theme configs indicating which components to emit tokens for, and optionally, customizations for some token values
- Outputs *all* tokens used by the configured components
- Using `matx.retheme`
- Takes 1 argument:
- `$components` List of component theme configs to emit customized token values for
- Outputs *only* the explicitly customized tokens, not any of the other tokens used by the component
- Outputs CSS variables for the configured components

## Recommended theming structure
- Apply the base token values using `matx.theme` *once*
- Apply the base token values using `matx.theme` together with `matx.token-defaults` *once* (typically to the document root `html { ... }`)
- Apply incremental overrides to the theme by calling `matx.theme` *without* `matx.token-default` to emit only the properties you want to change
- Choose selectors with minimal specificity when applying tokens
- Prefer to rely on CSS inheritance to apply token overrides rather than specificity.
For example if checkbox tokens are set on the root element (`html`) they will be inherited down
Expand Down
54 changes: 35 additions & 19 deletions src/material-experimental/theming/_theming.scss
Original file line number Diff line number Diff line change
Expand Up @@ -88,27 +88,43 @@ $_error-on-missing-dep: false;
}
}

/// Takes the full list of tokens and a list of components to configure, and outputs all theme
/// tokens for the configured components.
/// @param {Map} $tokens A map of all tokens for the current design system.
/// Takes a list of components to configure, and outputs theme tokens for the configured components.
/// @param {List} $components The list of component configurations to emit tokens for.
/// @output CSS variables representing the theme tokens for the given component configs.
// TODO(mmalerba): Consider an alternate API where `$tokens` is not a separate argument,
// but one of the configs in the `$components` list
@mixin theme($tokens, $components) {
@include _theme($tokens, _get-transitive-deps(mat.private-coerce-to-list($components)));
@mixin theme($components...) {
$filtered-components: ();
$tokens: null;
@each $component in $components {
@if (map.get($component, id) == 'mat.token-defaults') {
@if $tokens == null {
$tokens: map.get($component, tokens);
}
@else {
@error 'mat.token-defaults specified multiple times in call to mat.theme';
}
}
@else {
$filtered-components: list.append($filtered-components, $component);
}
}
$tokens: $tokens or ();
@if $tokens == () {
@each $component in $filtered-components {
@if (map.get($component, customizations) or ()) == () {
$component-id: map.get($component, id);
@warn 'Call to `#{$component-id}()` will produce no output, this is likely a mistake.' +
' Did you mean to:'+
'\a 1. Provide default token values with `mat.token-defaults`?' +
'\a 2. Specify customizations in the call to `#{$component-id}`?';
}
}
}
@include _theme($tokens, $filtered-components);
}

/// Takes a list of components to configure, and outputs only the theme tokens that are explicitly
/// customized by the configurations.
/// @param {List} $components The list of component configurations to emit tokens for.
/// @output CSS variables representing the theme tokens for the given component configs.
// TODO(mmalerba): What should we call this?
// - update-theme
// - adjust-theme
// - edit-theme
// - override-theme
// - retheme
@mixin retheme($components) {
@include _theme((), mat.private-coerce-to-list($components));
@function token-defaults($tokens) {
@return (
id: 'mat.token-defaults',
tokens: $tokens
);
}
10 changes: 6 additions & 4 deletions src/material/card/_card-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@
}

@mixin theme-from-tokens($tokens) {
@include mdc-elevated-card-theme.theme(map.get($tokens, tokens-mdc-elevated-card.$prefix));
@include mdc-outlined-card-theme.theme(map.get($tokens, tokens-mdc-outlined-card.$prefix));
@include token-utils.create-token-values(
tokens-mat-card.$prefix, map.get($tokens, tokens-mat-card.$prefix));
@if ($tokens != ()) {
@include mdc-elevated-card-theme.theme(map.get($tokens, tokens-mdc-elevated-card.$prefix));
@include mdc-outlined-card-theme.theme(map.get($tokens, tokens-mdc-outlined-card.$prefix));
@include token-utils.create-token-values(
tokens-mat-card.$prefix, map.get($tokens, tokens-mat-card.$prefix));
}
}
8 changes: 5 additions & 3 deletions src/material/checkbox/_checkbox-theme.scss
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@
}

@mixin theme-from-tokens($tokens) {
// TODO(mmalerba): Some of the theme styles above are not represented in terms of tokens,
// so this mixin is currently incomplete.
@include mdc-checkbox-theme.theme(map.get($tokens, tokens-mdc-checkbox.$prefix));
@if ($tokens != ()) {
// TODO(mmalerba): Some of the theme styles above are not represented in terms of tokens,
// so this mixin is currently incomplete.
@include mdc-checkbox-theme.theme(map.get($tokens, tokens-mdc-checkbox.$prefix));
}
}