fix(select): Do not fire change event on programmatic change (#5255)

material-components · Nov 27, 2019 · ec72968 · ec72968
1 parent 7fd17ce
commit ec72968
Show file tree

Hide file tree

Showing 5 changed files with 23 additions and 21 deletions.
diff --git a/packages/mdc-layout-grid/mdc-layout-grid.scss b/packages/mdc-layout-grid/mdc-layout-grid.scss
@@ -64,8 +64,7 @@
       @for $span from 1 through map-get($mdc-layout-grid-columns, $upper-breakpoint) {
         // Span classes.
         // stylelint-disable max-nesting-depth
-        @at-root .mdc-layout-grid__cell--span-#{$span},
-        .mdc-layout-grid__cell--span-#{$span}-#{$size} {
+        @at-root .mdc-layout-grid__cell--span-#{$span}, .mdc-layout-grid__cell--span-#{$span}-#{$size} {
           @include mdc-layout-grid-cell-span_($size, $span, $gutter);
         }
         // stylelint-enable max-nesting-depth

diff --git a/packages/mdc-select/README.md b/packages/mdc-select/README.md
@@ -462,7 +462,7 @@ If you are using a JavaScript framework, such as React or Angular, you can creat
 | `getSelectedIndex() => number` | Returns the index of the currently selected menu item. |
 | `setSelectedIndex(index: number) => void` | Handles setting the `mdc-select__selected-text` element and closing the menu. Also causes the label to float and outline to notch if needed. |
 | `getValue() => string` | Handles getting the value through the adapter. |
-| `setValue() => string` | Sets the selected index to the index of the menu item with the given value. |
+| `setValue(value: string) => void` | Sets the selected index to the index of the menu item with the given value. |
 | `setValid(isValid: boolean) => void` | Sets the valid state through the adapter. |
 | `isValid() => boolean` | Gets the valid state through the adapter's `checkValidity` API. |
 | `setRequired(isRequired: boolean) => void` | Sets the required state through the adapter. |

diff --git a/packages/mdc-select/foundation.ts b/packages/mdc-select/foundation.ts
@@ -147,13 +147,16 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
       this.adapter_.closeMenu();
     }
 
-    this.handleChange();
+    this.updateLabel_();
+    this.updateValidity_();
   }
 
   setValue(value: string) {
     const index = this.menuItemValues_.indexOf(value);
     this.setSelectedIndex(index);
-    this.handleChange();
+
+    this.updateLabel_();
+    this.updateValidity_();
   }
 
   getValue() {
@@ -229,15 +232,8 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
    */
   handleChange() {
     this.updateLabel_();
+    this.updateValidity_();
     this.adapter_.notifyChange(this.getValue());
-
-    const isRequired = this.adapter_.hasClass(cssClasses.REQUIRED);
-    if (isRequired) {
-      this.setValid(this.isValid());
-      if (this.helperText_) {
-        this.helperText_.setValidity(this.isValid());
-      }
-    }
   }
 
   handleMenuItemAction(index: number) {
@@ -402,13 +398,9 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
   }
 
   /**
-   * Unfocuses the select component.
+   * Updates the valid state when appropriate.
    */
-  private blur_() {
-    this.adapter_.removeClass(cssClasses.FOCUSED);
-    this.updateLabel_();
-    this.adapter_.deactivateBottomLine();
-
+  private updateValidity_() {
     const isRequired = this.adapter_.hasClass(cssClasses.REQUIRED);
     if (isRequired) {
       this.setValid(this.isValid());
@@ -417,6 +409,16 @@ export class MDCSelectFoundation extends MDCFoundation<MDCSelectAdapter> {
       }
     }
   }
+
+  /**
+   * Unfocuses the select component.
+   */
+  private blur_() {
+    this.adapter_.removeClass(cssClasses.FOCUSED);
+    this.updateLabel_();
+    this.updateValidity_();
+    this.adapter_.deactivateBottomLine();
+  }
 }
 
 // tslint:disable-next-line:no-default-export Needed for backward compatibility with MDC Web v0.44.0 and earlier.

diff --git a/packages/mdc-textfield/_mixins.scss b/packages/mdc-textfield/_mixins.scss
@@ -451,6 +451,7 @@
     $property-name: height,
   );
 
+  // stylelint-disable-next-line max-line-length
   $resolved-radius: nth(mdc-shape-resolve-percentage-radius($height, mdc-shape-prop-value($radius)), 1);
 
   @if (length(mdc-shape-prop-value($radius)) > 1) {

diff --git a/test/unit/mdc-select/foundation.test.js b/test/unit/mdc-select/foundation.test.js
@@ -256,11 +256,11 @@ test('#handleChange does not call foundation.notchOutline() when there is no lab
   td.verify(foundation.notchOutline(td.matchers.anything()), {times: 0});
 });
 
-test('#handleChange calls adapter.notifyChange() if didChange is true', () => {
+test('#handleChange calls adapter.notifyChange()', () => {
   const {foundation, mockAdapter} = setupTest();
   td.when(mockAdapter.getMenuItemAttr(td.matchers.anything(), strings.VALUE_ATTR)).thenReturn('value');
 
-  foundation.handleChange(/* didChange */ true);
+  foundation.handleChange();
   td.verify(mockAdapter.notifyChange(td.matchers.anything()), {times: 1});
 });