Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

feat(ripple): Expose mdc-states-opacity; fix press fallback #2402

Merged
merged 6 commits into from
Mar 16, 2018
Merged

feat(ripple): Expose mdc-states-opacity; fix press fallback #2402

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
26a7cbe
feat(ripple): Expose mdc-states-opacity; fix press fallback
Mar 14, 2018
fb8608f
WIP Fix README
Mar 14, 2018
9bd9adc
WIP Use public function within private mixin
Mar 15, 2018
0345266
Merge remote-tracking branch 'origin/master' into feat/ripple-states-…
Mar 15, 2018
7286577
Merge branch 'master' into feat/ripple-states-opacity
kfranqueiro Mar 15, 2018
7c1e8ab
Merge branch 'master' into feat/ripple-states-opacity
kfranqueiro Mar 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/mdc-ripple/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ CSS Class | Description
`mdc-ripple-surface--primary` | Sets the ripple color to the theme primary color
`mdc-ripple-surface--accent` | Sets the ripple color to the theme secondary color

### Sass Mixins
### Sass APIs

In order to fully style the ripple effect for different states (hover/focus/pressed), the following mixins must be included:

Expand Down Expand Up @@ -115,6 +115,12 @@ Mixin | Description

> _NOTE_: `$has-nested-focusable-element` defaults to `false` but should be set to `true` if the component contains a focusable element (e.g. an input) inside the root element.

#### Sass Functions

Function | Description
--- | ---
`mdc-states-opacity($color, $state)` | Returns the appropriate default opacity to apply to the given color in the given state (hover, focus, press, selected, or activated)

### `MDCRipple`

The `MDCRipple` JavaScript component allows for programmatic activation / deactivation of the ripple, for interdependent interaction between
Expand Down
48 changes: 48 additions & 0 deletions packages/mdc-ripple/_functions.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//
// Copyright 2016 Google Inc. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

@import "@material/theme/functions";
@import "@material/theme/variables"; // for mdc-theme-prop-value
@import "./variables";

//
// Public
//

@function mdc-states-opacity($color, $state) {
$opacity-map: mdc-states-opacities_($color);

@if not map-has-key($opacity-map, $state) {
@error "Invalid state: '#{$state}'. Choose one of: #{map-keys($opacity-map)}";
}

@return map-get($opacity-map, $state);
}

//
// Private
//

@function mdc-states-opacities_($color) {
$color-value: mdc-theme-prop-value($color);
$opacity-map: if(
mdc-theme-tone($color-value) == "light",
$mdc-ripple-light-ink-opacities,
$mdc-ripple-dark-ink-opacities
);

@return $opacity-map;
}
4 changes: 2 additions & 2 deletions packages/mdc-ripple/_keyframes.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@
}

to {
opacity: var(--mdc-ripple-fg-opacity, map-get($mdc-ripple-dark-ink-opacities, "press"));
opacity: var(--mdc-ripple-fg-opacity, 0);
}
}

@keyframes mdc-ripple-fg-opacity-out {
from {
animation-timing-function: linear;
opacity: var(--mdc-ripple-fg-opacity, map-get($mdc-ripple-dark-ink-opacities, "press"));
opacity: var(--mdc-ripple-fg-opacity, 0);
}

to {
Expand Down
34 changes: 9 additions & 25 deletions packages/mdc-ripple/_mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
//

@import "@material/animation/variables";
@import "@material/theme/functions";
@import "@material/theme/mixins";
@import "./functions";
@import "./variables";

@mixin mdc-ripple-surface() {
Expand Down Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do activated and selected need to be surrounded in quotes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok leaving it without quotes LGTM


&--activated {
// Stylelint seems to think that '&' qualifies as a type selector here?
Expand All @@ -162,8 +161,7 @@
// Simple mixin for selected states which automatically selects opacity values based on whether the ink color is
// light or dark.
@mixin mdc-states-selected($color, $has-nested-focusable-element: false) {
$opacity-map: mdc-states-opacities_($color);
$selected-opacity: map-get($opacity-map, "selected");
$selected-opacity: mdc-states-opacity($color, selected);

&--selected {
// stylelint-disable-next-line selector-max-type
Expand Down Expand Up @@ -219,26 +217,12 @@
}
}

//
// Private
//

@function mdc-states-opacities_($color) {
$color-value: mdc-theme-prop-value($color);
$opacity-map: if(
mdc-theme-tone($color-value) == "light",
$mdc-ripple-light-ink-opacities,
$mdc-ripple-dark-ink-opacities
);

@return $opacity-map;
}

@mixin mdc-states-interactions_($color, $has-nested-focusable-element, $opacity-modifier: 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's use the new public function here too?

Copy link
Contributor Author

@kfranqueiro kfranqueiro Mar 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

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.

$opacity-map: mdc-states-opacities_($color);

@include mdc-states-base-color($color);
@include mdc-states-hover-opacity(map-get($opacity-map, "hover") + $opacity-modifier);
@include mdc-states-focus-opacity(map-get($opacity-map, "focus") + $opacity-modifier, $has-nested-focusable-element);
@include mdc-states-press-opacity(map-get($opacity-map, "press") + $opacity-modifier);
@include mdc-states-hover-opacity(mdc-states-opacity($color, hover) + $opacity-modifier);
@include mdc-states-focus-opacity(
mdc-states-opacity($color, focus) + $opacity-modifier,
$has-nested-focusable-element
);
@include mdc-states-press-opacity(mdc-states-opacity($color, press) + $opacity-modifier);
}