Skip to content
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

fix(multiple): avoid repeating selectors in m3 themes #30645

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

andrewseguin
Copy link
Contributor

In #29596 we switched to outputting each group of tokens under a different selector in order to work around an Sass deprecation warning. Now that M3 themes are purely token-based, we can switch back to the old approach which produces smaller themes and makes the tokens easier to read in the dev tools.

We have to keep the workaround in place for M2 themes, because they mixed CSS variables with other rules.

@andrewseguin andrewseguin requested a review from a team as a code owner March 18, 2025 14:18
@andrewseguin andrewseguin requested review from crisbeto and mmalerba and removed request for a team March 18, 2025 14:18
@mmalerba mmalerba removed their request for review March 19, 2025 00:34
@@ -123,15 +123,15 @@
@include _theme-from-tokens(inspection.get-theme-tokens($theme, typography));
} @else {
@include sass-utils.current-selector-or-root() {
@include token-utils.create-token-values(
@include token-utils.create-token-values-mixed(
Copy link
Member

Choose a reason for hiding this comment

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

Should we come up some more explicit name for this function? We can do it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we can bikeshed another name. The opposite case, -inline, would be an easy name but I dont know what not-inline would be

@andrewseguin andrewseguin added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release target: major This PR is targeted for the next major release and removed target: patch This PR is targeted for the next patch release labels Mar 19, 2025
@andrewseguin andrewseguin merged commit e1cf284 into angular:main Mar 19, 2025
24 of 26 checks passed
mistrykaran91 pushed a commit to mistrykaran91/components that referenced this pull request Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants