-
Notifications
You must be signed in to change notification settings - Fork 6.7k
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
feat(material/schematics): Switch custom theme schematic to use theme mixin instead of define-theme and add high contrast override mixins #29642
base: main
Are you sure you want to change the base?
Conversation
2d7fe98
to
0514fcc
Compare
): DynamicScheme { | ||
return new DynamicScheme({ | ||
sourceColorArgb: primaryPalette.keyColor.toInt(), | ||
variant: 6, // Variant enum is not accessible outside of @material/material-color-utilities |
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.
What is "6"? Should include this in comments
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.
Updated the comment, variant is supposed to take a Variant enum but it isn't exposed outside the package
0514fcc
to
aa3ef13
Compare
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.
I think we can remove the m3
prefix from this schematic. Also I wonder if another name makes more sense now that it isn't creating a config. Maybe theme-color
or just color
Let's also wait on merging this pull request until the theme
mixin is renamed
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 first sentence of the README says it makes theme configurations - let's reword that to say it makes theme colors
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.
I renamed it to palettes since that's what users are importing from the generated file
aa3ef13
to
84cdc16
Compare
… mixin instead of define-theme and add high contrast override mixins
84cdc16
to
a3cbcfc
Compare
I renamed the schematic to |
No description provided.