fix(shape): Fix errors related to multi-value shape categories (#4547)

material-components · Apr 2, 2019 · 9f79d17 · 9f79d17
1 parent f2db177
commit 9f79d17
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 78 deletions.
diff --git a/packages/mdc-notched-outline/_mixins.scss b/packages/mdc-notched-outline/_mixins.scss
@@ -45,6 +45,13 @@
 @mixin mdc-notched-outline-shape-radius($radius, $rtl-reflexive: false) {
   $radius: mdc-shape-prop-value($radius);
 
+  @if (length($radius) > 1) {
+    // stylelint-disable-next-line max-line-length
+    @warn "mdc-notched-outline-shape-radius only supports a single radius; see https://github.com/material-components/material-components-web/issues/4140";
+  }
+
+  $radius: nth($radius, 1);
+
   .mdc-notched-outline__leading {
     @include mdc-shape-radius(mdc-shape-mask-radius($radius, 1 0 0 1), $rtl-reflexive: true);
 

diff --git a/packages/mdc-select/_mixins.scss b/packages/mdc-select/_mixins.scss
@@ -108,7 +108,12 @@
 }
 
 @mixin mdc-select-outline-shape-radius($radius, $rtl-reflexive: false) {
-  $resolved-radius: mdc-shape-resolve-percentage-radius($mdc-select-height, $radius);
+  $resolved-radius: nth(mdc-shape-resolve-percentage-radius($mdc-select-height, mdc-shape-prop-value($radius)), 1);
+
+  @if (length(mdc-shape-prop-value($radius)) > 1) {
+    // stylelint-disable-next-line max-line-length
+    @warn "mdc-select-outline-shape-radius only supports a single radius; see https://github.com/material-components/material-components-web/issues/4140";
+  }
 
   .mdc-notched-outline {
     @include mdc-notched-outline-shape-radius($resolved-radius, $rtl-reflexive);
@@ -118,21 +123,19 @@
     @include mdc-shape-radius($resolved-radius, $rtl-reflexive);
   }
 
-  $radius-pixels: mdc-shape-prop-value($resolved-radius);
-
-  @if ($radius-pixels > $mdc-notched-outline-leading-width) {
+  @if ($resolved-radius > $mdc-notched-outline-leading-width) {
     .mdc-select__native-control {
       @include mdc-rtl-reflexive-property(
         padding,
-        $radius-pixels + $mdc-notched-outline-padding,
+        $resolved-radius + $mdc-notched-outline-padding,
         $mdc-select-arrow-padding
       );
     }
 
     + .mdc-select-helper-text {
       @include mdc-rtl-reflexive-property(
         margin,
-        $radius-pixels + $mdc-notched-outline-padding,
+        $resolved-radius + $mdc-notched-outline-padding,
         $mdc-select-outline-label-offset
       );
     }

diff --git a/packages/mdc-shape/_functions.scss b/packages/mdc-shape/_functions.scss
@@ -57,6 +57,8 @@
 // mdc-shape-resolve-percentage-radius(36px, 50%) => `18px` (i.e., 36px / 2)
 //
 @function mdc-shape-resolve-percentage-radius($component-height, $radius) {
+  $radius: mdc-shape-prop-value($radius);
+
   @if type-of($radius) == "list" {
     $radius-value: ();
 
@@ -70,59 +72,43 @@
   }
 }
 
-@function mdc-shape-resolve-percentage-for-corner_($component-height, $radius) {
-  $radius-value: mdc-shape-prop-corner-value_($radius);
-
-  @if type-of($radius-value) == "number" and unit($radius-value) == "%" {
-    // Converts the percentage to number without unit. Example: 50% => 50.
-    $percentage: $radius-value / ($radius-value * 0 + 1);
-
-    @return $component-height * ($percentage / 100);
-  } @else {
-    @return $radius-value;
-  }
-}
-
-//
-// Strips unit from number. This is accomplished by dividing the value by itself to cancel out units, while resulting
-// in a denominator of 1.
-//
-// Examples:
-//
-// 50% => 50
-//
-@function mdc-shape-strip-unit_($number) {
-  @if type-of($number) == "number" and not unitless($number) {
-    @return $number / ($number * 0 + 1);
-  }
-
-  @return $number;
-}
-
 //
 // Returns $radius value of shape category - `large`, `medium` or `small`.
 // Otherwise, it returns the $radius itself if valid.
-// $radius can be a single value or list of up to 4.
+// $radius can be a single value, or a list of up to 4 values.
 //
 // Examples:
 //
 // mdc-shape-prop-value(small) => 4px
+// mdc-shape-prop-value(small small 0 0) => 4px 4px 0 0
 //
 @function mdc-shape-prop-value($radius) {
   @if type-of($radius) == "list" {
     @if length($radius) > 4 {
       @error "Invalid radius: '#{$radius}' is more than 4 values";
     }
 
-    $radius-value: ();
+    $radius-values: ();
 
-    @each $corner in $radius {
-      $radius-value: append($radius-value, mdc-shape-prop-corner-value_($corner));
+    @for $i from 1 through length($radius) {
+      $corner: nth($radius, $i);
+
+      @if map-has-key($mdc-shape-category-values, $corner) {
+        // If a category is encountered within a list of radii, apply the category's value for the corresponding corner
+        $radius-values:
+          append($radius-values, nth(mdc-shape-unpack-radius_(map-get($mdc-shape-category-values, $corner)), $i));
+      } @else {
+        $radius-values: append($radius-values, mdc-shape-validate-radius-value_($corner));
+      }
     }
 
-    @return $radius-value;
+    @return $radius-values;
   } @else {
-    @return mdc-shape-prop-corner-value_($radius);
+    @if map-has-key($mdc-shape-category-values, $radius) {
+      @return map-get($mdc-shape-category-values, $radius);
+    } @else {
+      @return mdc-shape-validate-radius-value_($radius);
+    }
   }
 }
 
@@ -147,34 +133,58 @@
     @error "Expected masked-corners of length 4 but got '#{length($masked-corners)}'.";
   }
 
-  @if length($radius) == 3 {
-    $radius: nth($radius, 1) nth($radius, 2) nth($radius, 3) nth($radius, 2);
-  } @else if length($radius) == 2 {
-    $radius: nth($radius, 1) nth($radius, 2) nth($radius, 1) nth($radius, 2);
-  } @else if length($radius) == 1 {
-    $radius: $radius $radius $radius $radius;
-  }
+  $radius: mdc-shape-unpack-radius_($radius);
 
   @return if(nth($masked-corners, 1) == 1, nth($radius, 1), 0)
     if(nth($masked-corners, 2) == 1, nth($radius, 2), 0)
     if(nth($masked-corners, 3) == 1, nth($radius, 3), 0)
     if(nth($masked-corners, 4) == 1, nth($radius, 4), 0);
 }
 
-@function mdc-shape-prop-corner-value_($radius) {
-  @if map-has-key($mdc-shape-category-values, $radius) {
-    @return map-get($mdc-shape-category-values, $radius);
-  } @else if mdc-shape-is-valid-radius-value_($radius) {
+//
+// Unpacks shorthand values for border-radius (i.e. lists of 1-3 values).
+// If a list of 4 values is given, it is returned as-is.
+//
+// Examples:
+//
+// 1. mdc-shape-unpack-radius_(4px) => 4px 4px 4px 4px
+// 2. mdc-shape-unpack-radius_(4px 2px) => 4px 2px 4px 2px
+// 3. mdc-shape-unpack-radius_(4px 2px 2px) => 4px 2px 2px 2px
+// 2. mdc-shape-unpack-radius_(4px 2px 0 2px) => 4px 2px 0 2px
+//
+// TODO: This is private for purposes of getting it into a patch; make it public for a future minor/major release.
+//
+@function mdc-shape-unpack-radius_($radius) {
+  @if length($radius) == 4 {
     @return $radius;
-  } @else {
-    @error "Invalid radius: '#{$radius}' radius is not supported";
+  } @else if length($radius) == 3 {
+    @return nth($radius, 1) nth($radius, 2) nth($radius, 3) nth($radius, 2);
+  } @else if length($radius) == 2 {
+    @return nth($radius, 1) nth($radius, 2) nth($radius, 1) nth($radius, 2);
+  } @else if length($radius) == 1 {
+    @return $radius $radius $radius $radius;
   }
 
-  @return map-get($mdc-shape-category-values, $radius);
+  @error "Invalid radius: '#{$radius}' is more than 4 values";
+}
+
+@function mdc-shape-resolve-percentage-for-corner_($component-height, $radius) {
+  @if type-of($radius) == "number" and unit($radius) == "%" {
+    // Converts the percentage to number without unit. Example: 50% => 50.
+    $percentage: $radius / ($radius * 0 + 1);
+
+    @return $component-height * ($percentage / 100);
+  } @else {
+    @return $radius;
+  }
 }
 
-@function mdc-shape-is-valid-radius-value_($radius) {
+@function mdc-shape-validate-radius-value_($radius) {
   $is-number: type-of($radius) == "number";
 
-  @return $is-number or str_index($radius, "var(") or str_index($radius, "calc(");
+  @if not ($is-number or str_index($radius, "var(") or str_index($radius, "calc(")) {
+    @error "Invalid radius: #{$radius}";
+  }
+
+  @return $radius;
 }
diff --git a/packages/mdc-textfield/_mixins.scss b/packages/mdc-textfield/_mixins.scss
@@ -217,21 +217,24 @@
 }
 
 @mixin mdc-text-field-outline-shape-radius($radius, $rtl-reflexive: false) {
-  $resolved-radius: mdc-shape-resolve-percentage-radius($mdc-text-field-height, $radius);
+  $resolved-radius: nth(mdc-shape-resolve-percentage-radius($mdc-text-field-height, mdc-shape-prop-value($radius)), 1);
+
+  @if (length(mdc-shape-prop-value($radius)) > 1) {
+    // stylelint-disable-next-line max-line-length
+    @warn "mdc-text-field-outline-shape-radius only supports a single radius; see https://github.com/material-components/material-components-web/issues/4140";
+  }
 
   .mdc-notched-outline {
     @include mdc-notched-outline-shape-radius($resolved-radius, $rtl-reflexive);
   }
 
-  $radius-pixels: mdc-shape-prop-value($resolved-radius);
-
-  @if ($radius-pixels > $mdc-notched-outline-leading-width) {
+  @if ($resolved-radius > $mdc-notched-outline-leading-width) {
     .mdc-text-field__input {
-      @include mdc-rtl-reflexive-property(padding, $radius-pixels + $mdc-notched-outline-padding, 0);
+      @include mdc-rtl-reflexive-property(padding, $resolved-radius + $mdc-notched-outline-padding, 0);
     }
 
     + .mdc-text-field-helper-line {
-      @include mdc-rtl-reflexive-property(padding, $radius-pixels + $mdc-notched-outline-padding, $radius-pixels);
+      @include mdc-rtl-reflexive-property(padding, $resolved-radius + $mdc-notched-outline-padding, $resolved-radius);
     }
   }
 }

diff --git a/test/screenshot/golden.json b/test/screenshot/golden.json
@@ -1080,11 +1080,11 @@
     }
   },
   "spec/mdc-shape/variables/override.html": {
-    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/03/28/02_09_33_134/spec/mdc-shape/variables/override.html?utm_source=golden_json",
+    "public_url": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/02/18_11_26_590/spec/mdc-shape/variables/override.html?utm_source=golden_json",
     "screenshots": {
-      "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/03/28/02_09_33_134/spec/mdc-shape/variables/override.html.windows_chrome_73.png",
-      "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/03/28/02_09_33_134/spec/mdc-shape/variables/override.html.windows_firefox_65.png",
-      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/03/28/02_09_33_134/spec/mdc-shape/variables/override.html.windows_ie_11.png"
+      "desktop_windows_chrome@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/02/18_11_26_590/spec/mdc-shape/variables/override.html.windows_chrome_73.png",
+      "desktop_windows_firefox@latest": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/02/18_11_26_590/spec/mdc-shape/variables/override.html.windows_firefox_65.png",
+      "desktop_windows_ie@11": "https://storage.googleapis.com/mdc-web-screenshot-tests/travis/2019/04/02/18_11_26_590/spec/mdc-shape/variables/override.html.windows_ie_11.png"
     }
   },
   "spec/mdc-snackbar/classes/baseline-with-action.html": {

diff --git a/test/screenshot/spec/mdc-shape/fixture.js b/test/screenshot/spec/mdc-shape/fixture.js
@@ -22,5 +22,18 @@
  */
 
 window.mdc.testFixture.fontsLoaded.then(() => {
+  [].forEach.call(document.querySelectorAll('.mdc-text-field'), (el) => {
+    mdc.textField.MDCTextField.attachTo(el);
+  });
+
+  // Fixes the wide notched outline issue.
+  [].forEach.call(document.querySelectorAll('.mdc-text-field__input[value=" "]'), (el) => {
+    el.value = '';
+  });
+
+  [].forEach.call(document.querySelectorAll('.mdc-select'), (el) => {
+    mdc.select.MDCSelect.attachTo(el);
+  });
+
   window.mdc.testFixture.notifyDomReady();
 });
diff --git a/test/screenshot/spec/mdc-shape/fixture.scss b/test/screenshot/spec/mdc-shape/fixture.scss
@@ -20,21 +20,32 @@
 // THE SOFTWARE.
 //
 
-// stylelint-disable scss/dollar-variable-pattern
-$mdc-shape-small-component-radius: 50%;
-$mdc-shape-medium-component-radius: 50%;
-$mdc-shape-large-component-radius: 50%;
-// stylelint-enable scss/dollar-variable-pattern
-
-@import "../../../../packages/mdc-button/mdc-button";
-@import "../../../../packages/mdc-fab/mdc-fab";
-@import "../../../../packages/mdc-textfield/mdc-text-field";
-@import "../../../../packages/mdc-select/mdc-select";
 @import "../mixins";
 
 .test-cell--fab,
-.test-cell--button,
+.test-cell--button {
+  // stylelint-disable scss/dollar-variable-pattern
+  $mdc-shape-small-component-radius: 0 50% 0 50%;
+  // stylelint-enable scss/dollar-variable-pattern
+
+  @at-root {
+    @import "../../../../packages/mdc-button/mdc-button";
+    @import "../../../../packages/mdc-fab/mdc-fab";
+  }
+
+  @include test-cell-size(301, 81);
+}
+
 .test-cell--textfield,
 .test-cell--select {
+  // stylelint-disable scss/dollar-variable-pattern
+  $mdc-shape-small-component-radius: 50%;
+  // stylelint-enable scss/dollar-variable-pattern
+
+  @at-root {
+    @import "../../../../packages/mdc-textfield/mdc-text-field";
+    @import "../../../../packages/mdc-select/mdc-select";
+  }
+
   @include test-cell-size(301, 81);
 }