fix(material/checkbox): refactor to depend on @angular/material/core/tokens #26744
Conversation
mmalerba
force-pushed
the
mdc-checkbox-tokens
branch
3 times, most recently
from
3fc744e to
11be168
Compare
mmalerba
force-pushed
the
mdc-checkbox-tokens
branch
3 times, most recently
from
2f3019c to
77d5150
Compare
mmalerba
force-pushed
the
mdc-checkbox-tokens
branch
from
77d5150 to
393fa65
Compare
| @include _selected-ripple-colors($primary, primary); | ||
| } | ||
| &.mat-primary { | ||
| $primary-config: map.merge($config, (accent: $primary)); |
There was a problem hiding this comment.
I don't quite understand why we're passing in a primary palette into a key called accent here and in the warn theme.
There was a problem hiding this comment.
Our .mat-primary, .mat-accent, .mat-warn classes don't really fit well with the token-based theming. There aren't separate tokens for e.g. checkbox-color-when-primary, checkbox-color-when-accent. Instead I set up the tokens to draw their values from the default flavor of checkbox (accent). In order to still support the .mat-primary and .mat-warn, I just call get-color-tokens passing the desired palette as the accent palette to get the styles we want.
Ultimately I think we'll want to deprecate and remove the class based theming, but this seemed like a decent solution for now
| @include mdc-helpers.disable-mdc-fallback-declarations { | ||
| @include mdc-checkbox-theme.theme-styles(checkbox-private.$private-checkbox-theme-config); | ||
| } | ||
| .mdc-checkbox__native-control:focus { |
There was a problem hiding this comment.
nit: it seems like the & ~ .mdc-checkbox__ripple and & ~ .mat-mdc-checkbox-ripple .mat-ripple-element are repeated a few times. Maybe we can clean this up a bit with a mixin?
There was a problem hiding this comment.
I did try to extract it, but I felt like it just made the code harder to reason about. I'll give it another try to see if I can come up with something that actually simplifies it instead of just making it harder to follow.
I think the real solution to this is to just do what MDC exepcts w.r.t. the state layer and stop trying to do our own thing. Then we can remove all this code and rely on the MDC mixin
crisbeto
added
the
merge: fix commit message
|
Merging this one in as discussed over DM. |
|
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. |
No description provided.