feat(ripple): Expose mdc-states-opacity; fix press fallback #2402
Conversation
kfranqueiro
changed the title
kfranqueiro
changed the title
| @@ -145,8 +145,7 @@ | |||
| // Simple mixin for activated states which automatically selects opacity values based on whether the ink color is | |||
| // light or dark. | |||
| @mixin mdc-states-activated($color, $has-nested-focusable-element: false) { | |||
| $opacity-map: mdc-states-opacities_($color); | |||
| $activated-opacity: map-get($opacity-map, "activated"); | |||
| $activated-opacity: mdc-states-opacity($color, activated); | |||
There was a problem hiding this comment.
Do activated and selected need to be surrounded in quotes?
There was a problem hiding this comment.
I wanted to stay consistent with the usage pattern we use for mdc-theme-prop, where primary, secondary, etc. aren't surrounded in quotes when used as values.
Either should technically work, though unquoted values could be seen as a bit sloppy since they may be coerced to color if recognized as one (e.g. black), but even this is harmless as long as the usage is consistent both in the map's keys and in map-get invocations.
If quoting is something we want to eventually encourage instead, we'd want to do so for mdc-theme-prop as well though.
There was a problem hiding this comment.
Ok leaving it without quotes LGTM
|
|
||
| @return $opacity-map; | ||
| } | ||
|
|
||
| @mixin mdc-states-interactions_($color, $has-nested-focusable-element, $opacity-modifier: 0) { |
There was a problem hiding this comment.
Let's use the new public function here too?
There was a problem hiding this comment.
I debated this, but since I'm doing multiple lookups on the same map it seemed like calling the public function repeatedly with the same color is wasted cycles to choose the same map. I'll test whether it actually has any noticeable impact whatsoever on build time though, 'cause I can see the argument that it'd be more readable.
Codecov Report
@@ Coverage Diff @@
## master #2402 +/- ##
=========================================
Coverage ? 98.89%
=========================================
Files ? 100
Lines ? 4145
Branches ? 531
=========================================
Hits ? 4099
Misses ? 46
Partials ? 0Continue to review full report at Codecov.
|
This is needed to support #2354 (see feat/ripple-states-opacity...fix/text-field/no-ripple for how it will be used for that).
This refactors logic out of a private function in mdc-ripple/mixins into a pair of functions (one public, one private) in mdc-ripple/functions (which is a new file). This has no effect on existing code or backwards compatibility, but adds an additional public Sass API.
This also changes the fallback for the
--mdc-ripple-fg-opacityvariable to0in ripple's styles for keyframes, which allows suppressing press ripple by omitting the styles altogether. This has no effect on MDC Web's own use of states which currently always defines press styles, and has no effect on using the states mixins as documented, since in those cases this variable will always be defined.