-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Conversation
….retheme into a single mixin Rather than having a separate mixin for "retheming" we have a single mixin `matx.theme` that can either emit fallback token defaults or not depending on how its configured. Added `matx.token-defaults` which can be used to configure the fallbacks.
matx.card(), | ||
matx.checkbox(), | ||
) | ||
matx.token-defaults(mat.m2-tokens-from-theme($theme)), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
….retheme into a single mixin (angular#27309) Rather than having a separate mixin for "retheming" we have a single mixin `matx.theme` that can either emit fallback token defaults or not depending on how its configured. Added `matx.token-defaults` which can be used to configure the fallbacks.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Rather than having a separate mixin for "retheming" we have a single mixin
matx.theme
that can either emit fallback token defaults or not depending on how its configured. Addedmatx.token-defaults
which can be used to configure the fallbacks.