fix(select): Emitting on initial value + inconsistent validity state (#…

…1867) fix #1829 fix #1812
trimox · Apr 20, 2019 · 625e481 · 625e481
1 parent 0da2f23
commit 625e481
Show file tree

Hide file tree

Showing 4 changed files with 97 additions and 59 deletions.
diff --git a/demos/src/app/components/select-demo/examples.html b/demos/src/app/components/select-demo/examples.html
@@ -17,22 +17,28 @@ <h3 class="demo-content__headline">Simple Native - FormControl</h3>
 <div class="demo-content">
   <h3 class="demo-content__headline">Enhanced Select</h3>
   <div class="demo-layout__row">
-    <button mdc-button (click)="enhanced.outlined = !enhanced.outlined">Outlined:
+    <button mdc-button
+      (click)="enhanced.outlined = !enhanced.outlined">Outlined:
       {{enhanced.outlined ? 'On' : 'Off'}}</button>
-    <button mdc-button (click)="enhanced.required = !enhanced.required">Required:
+    <button mdc-button
+      (click)="enhanced.required = !enhanced.required">Required:
       {{enhanced.required ? 'On' : 'Off'}}</button>
-    <button mdc-button (click)="enhanced.disabled = !enhanced.disabled">Disabled:
+    <button mdc-button
+      (click)="enhanced.disabled = !enhanced.disabled">Disabled:
       {{enhanced.disabled ? 'On' : 'Off'}}</button>
-    <button mdc-button (click)="enhanced.floatLabel = !enhanced.floatLabel">Float
+    <button mdc-button
+      (click)="enhanced.floatLabel = !enhanced.floatLabel">Float
       Label: {{enhanced.floatLabel ? 'On' :
       'Off'}}</button>
     <button mdc-button (click)="enhanced.setSelectionByValue('banana')">Select
       Banana</button>
-    <button mdc-button (click)="enhanced.setSelectedIndex(2)">Set Index (2)</button>
+    <button mdc-button (click)="enhanced.setSelectedIndex(2)">Set Index
+      (2)</button>
     <button mdc-button (click)="enhanced.reset()">Reset</button>
   </div>
-  <mdc-select #enhanced placeholder="Fruit" required [helperText]="enhancedHelper"
-    (selectionChange)="onSelectionChange($event)" class="demo-enhanced-width">
+  <mdc-select #enhanced placeholder="Fruit" required
+    [helperText]="enhancedHelper" (selectionChange)="onSelectionChange($event)"
+    class="demo-enhanced-width">
     <mdc-menu>
       <mdc-list>
         <mdc-list-item></mdc-list-item>
@@ -43,7 +49,8 @@ <h3 class="demo-content__headline">Enhanced Select</h3>
       </mdc-list>
     </mdc-menu>
   </mdc-select>
-  <mdc-helper-text #enhancedHelper validation>Field is required</mdc-helper-text>
+  <mdc-helper-text #enhancedHelper validation>Field is required
+  </mdc-helper-text>
   <p> Value: {{ enhanced.value }}</p>
   <p> Index: {{ enhanced.getSelectedIndex() }}</p>
   <example-viewer [example]="exampleEnhanced"></example-viewer>
@@ -87,7 +94,8 @@ <h3 class="demo-content__headline">Native Select - Leading Icon</h3>
   <mdc-select #meal [helperText]="mealHelper" required placeholder="Pick a Meal"
     autosize>
     <mdc-icon mdcSelectIcon>fastfood</mdc-icon>
-    <option *ngFor="let food of foods" [value]="food.value" [disabled]="food.disabled">
+    <option *ngFor="let food of foods" [value]="food.value"
+      [disabled]="food.disabled">
       {{food.viewValue}}
     </option>
   </mdc-select>
@@ -113,7 +121,8 @@ <h3 class="demo-content__headline">Custom Enhanced</h3>
       </mdc-select>
     </div>
     <div class="demo-container">
-      <mdc-select placeholder="Standard" outlined class="custom-select-outline-shape-radius">
+      <mdc-select placeholder="Standard" outlined
+        class="custom-select-outline-shape-radius">
         <mdc-menu>
           <mdc-list>
             <mdc-list-item *ngFor="let food of foods" [value]="food.value"
@@ -125,7 +134,8 @@ <h3 class="demo-content__headline">Custom Enhanced</h3>
       </mdc-select>
     </div>
     <div class="demo-container">
-      <mdc-select placeholder="Standard" outlined class="custom-select-outline-color">
+      <mdc-select placeholder="Standard" outlined
+        class="custom-select-outline-color">
         <mdc-menu>
           <mdc-list>
             <mdc-list-item *ngFor="let food of foods" [value]="food.value"
@@ -143,17 +153,21 @@ <h3 class="demo-content__headline">Custom Enhanced</h3>
 <div class="demo-content">
   <h3 class="demo-content__headline">Lazy Load</h3>
   <form [formGroup]="lazyLoadForm" #formDirectiveLazy="ngForm">
-    <mdc-select outlined formControlName="lazySelect" [helperText]="lazyHelper" class="demo-enhanced-width">
-      <option *ngFor="let food of lazyFoods" [value]="food.value" [disabled]="food.disabled">{{food.viewValue}}</option>
+    <mdc-select outlined formControlName="lazySelect" [helperText]="lazyHelper"
+      class="demo-enhanced-width">
+      <option *ngFor="let food of lazyFoods" [value]="food.value"
+        [disabled]="food.disabled">{{food.viewValue}}</option>
     </mdc-select>
     <mdc-helper-text #lazyHelper validation>
-      <span *ngIf="lazyLoadForm.controls['lazySelect'].hasError('required')">Selection
+      <span
+        *ngIf="lazyLoadForm.controls['lazySelect'].hasError('required')">Selection
         is required</span>
     </mdc-helper-text>
 
     <div class="demo-layout__row">
       <button mdc-button (click)="loadFoods()">Load</button>
-      <button mdc-button type="button" (click)="resetLazyLoaded(formDirectiveLazy)">Reset</button>
+      <button mdc-button type="button"
+        (click)="resetLazyLoaded(formDirectiveLazy)">Reset</button>
     </div>
   </form>
   <example-viewer [example]="exampleLazyLoadedNative"></example-viewer>
@@ -184,12 +198,15 @@ <h3 class="demo-content__headline">Select with ngModel</h3>
     <button mdc-button (click)="demoNgModel.reset()">Clear Selection</button>
     <button mdc-button (click)="select.setSelectionByValue('fruit-3')">Select
       Fruit</button>
-    <button mdc-button (click)="select.setSelectedIndex(4)">Set Index (4)</button>
+    <button mdc-button (click)="select.setSelectedIndex(2)">Select index
+      2</button>
   </div>
   <form #demoForm="ngForm">
     <mdc-select #select placeholder="Favorite food" name="food" required
-      outlined ngModel #demoNgModel="ngModel" (selectionChange)="onSelectionChange($event)">
-      <option *ngFor="let food of foods" [value]="food.value" [disabled]="food.disabled">
+      outlined ngModel #demoNgModel="ngModel"
+      (selectionChange)="onSelectionChange($event)">
+      <option *ngFor="let food of foods" [value]="food.value"
+        [disabled]="food.disabled">
         {{food.viewValue}}
       </option>
     </mdc-select>
@@ -205,36 +222,45 @@ <h3 class="demo-content__headline">Select with ngModel</h3>
 <div class="demo-content">
   <h3 class="demo-content__headline">Select with FormControl</h3>
   <div class="demo-layout__row">
-    <button mdc-button (click)="favoriteFood.outlined = !favoriteFood.outlined">Outlined:
+    <button mdc-button
+      (click)="favoriteFood.outlined = !favoriteFood.outlined">Outlined:
       {{favoriteFood.outlined ?
       'On' : 'Off'}}</button>
-    <button mdc-button (click)="favoriteFood.disabled = !favoriteFood.disabled">Disabled:
+    <button mdc-button
+      (click)="favoriteFood.disabled = !favoriteFood.disabled">Disabled:
       {{favoriteFood.disabled ?
       'On' : 'Off'}}</button>
-    <button mdc-button (click)="favoriteFood.floatLabel = !favoriteFood.floatLabel">Float
+    <button mdc-button
+      (click)="favoriteFood.floatLabel = !favoriteFood.floatLabel">Float
       Label:
       {{favoriteFood.floatLabel ? 'On' : 'Off'}}</button>
-    <button mdc-button (click)="favoriteFood.setSelectionByValue('pizza-1')">Select
+    <button mdc-button
+      (click)="favoriteFood.setSelectionByValue('pizza-1')">Select
       Pizza</button>
-    <button mdc-button (click)="foodForm.controls['favoriteFood'].setValue('pizza-1')">Set
+    <button mdc-button
+      (click)="foodForm.controls['favoriteFood'].setValue('pizza-1')">Set
       Form Value</button>
-    <button mdc-button (click)="favoriteFood.setSelectedIndex(1)">Set Index (1)</button>
+    <button mdc-button (click)="favoriteFood.setSelectedIndex(1)">Set Index
+      (1)</button>
   </div>
   <form [formGroup]="foodForm" id="foodForm" (ngSubmit)="submitForm()"
     #formDirective="ngForm">
-    <mdc-select #favoriteFood placeholder="Favorite food" formControlName="favoriteFood"
-      [helperText]="reactiveHelper">
-      <option *ngFor="let food of foods" [value]="food.value" [disabled]="food.disabled">
+    <mdc-select #favoriteFood placeholder="Favorite food"
+      formControlName="favoriteFood" [helperText]="reactiveHelper">
+      <option *ngFor="let food of foods" [value]="food.value"
+        [disabled]="food.disabled">
         {{food.viewValue}}
       </option>
     </mdc-select>
     <mdc-helper-text #reactiveHelper validation>
-      <span *ngIf="foodForm.controls['favoriteFood'].hasError('required')">Selection
+      <span
+        *ngIf="foodForm.controls['favoriteFood'].hasError('required')">Selection
         is required</span>
     </mdc-helper-text>
     <div class="demo-layout__row">
       <button mdc-button>Submit</button>
-      <button mdc-button type="button" (click)="resetForm(formDirective)">Reset</button>
+      <button mdc-button type="button"
+        (click)="resetForm(formDirective)">Reset</button>
     </div>
   </form>
   <p>Control Valid: {{foodForm.controls['favoriteFood'].valid}}</p>
@@ -245,4 +271,4 @@ <h3 class="demo-content__headline">Select with FormControl</h3>
   <p>Value: {{ foodForm.value | json }}</p>
 
   <example-viewer [example]="exampleReactive"></example-viewer>
-</div>
+</div>
diff --git a/packages/menu-surface/menu-surface-base.ts b/packages/menu-surface/menu-surface-base.ts
@@ -53,8 +53,11 @@ export abstract class MdcMenuSurfaceBase extends MDCComponent<any> {
   @Input()
   get open(): boolean { return this._open; }
   set open(value: boolean) {
-    this._open = toBoolean(value);
-    this.setOpen();
+    const newValue = toBoolean(value);
+    if (newValue !== this._open) {
+      this._open = toBoolean(value);
+      this.setOpen();
+    }
   }
   private _open: boolean = false;
 
@@ -171,7 +174,7 @@ export abstract class MdcMenuSurfaceBase extends MDCComponent<any> {
       },
       isLastElementFocused: () => {
         if (!this.platform.isBrowser) { return false; }
-        return this._lastFocusableElement ? this._lastFocusableElement === document.activeElement :  false;
+        return this._lastFocusableElement ? this._lastFocusableElement === document.activeElement : false;
       },
       focusFirstElement: () => {
         if (!this.platform.isBrowser) { return; }

diff --git a/packages/select/select.ts b/packages/select/select.ts
@@ -78,7 +78,7 @@ export class MdcSelectChange {
   constructor(
     public source: MdcSelect,
     public index: number,
-    public value: any) { }
+    public value: string) { }
 }
 
 @Directive({
@@ -100,7 +100,7 @@ let nextUniqueId = 0;
     '[class.mdc-select--outlined]': 'outlined',
     '[class.mdc-select--required]': 'required',
     '[class.mdc-select--with-leading-icon]': 'leadingIcon',
-    '[class.mdc-select--invalid]': 'errorState'
+    '[class.mdc-select--invalid]': 'errorState && !focused'
   },
   template: `
   <ng-content select="mdc-icon"></ng-content>
@@ -147,6 +147,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
 
   controlType: string = 'mdc-select';
   _enhancedSelectedText: string = '';
+  focused: boolean = false;
 
   @Input() id: string = this._uniqueId;
   @Input() name: string | null = null;
@@ -246,11 +247,11 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
 
   /** Value of the select control. */
   @Input()
-  get value(): any { return this._value; }
-  set value(newValue: any) {
+  get value(): string { return this._value; }
+  set value(newValue: string) {
     this.setSelectionByValue(newValue);
   }
-  private _value: any;
+  private _value: string = '';
 
   @Input()
   get helperText(): MdcHelperText | null { return this._helperText; }
@@ -274,7 +275,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
    * to facilitate the two-way binding for the `value` input.
    */
   @Output() readonly valueChange:
-    EventEmitter<{ index: number, value: any }> = new EventEmitter<any>();
+    EventEmitter<{ index: number, value: string }> = new EventEmitter<any>();
 
   @ViewChild(MdcFloatingLabel) _floatingLabel?: MdcFloatingLabel;
   @ViewChild(MdcLineRipple) _lineRipple?: MdcLineRipple;
@@ -330,7 +331,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
   private _getNativeSelectAdapterMethods() {
     return {
       getValue: () => this._platform.isBrowser ? this._getInputElement().value : '',
-      setValue: (value: any) => this._getInputElement().value = value,
+      setValue: (value: string) => this._getInputElement().value = value,
       isMenuOpen: () => false,
       setSelectedIndex: (index: number) => this._getInputElement().selectedIndex = index,
       setDisabled: (isDisabled: boolean) => this._getInputElement().disabled = isDisabled,
@@ -398,7 +399,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
 
   private _foundation!: {
     setSelectedIndex(index: number): void,
-    setValue(value: any): void,
+    setValue(value: string): void,
     setDisabled(isDisabled: boolean): void,
     notchOutline(openNotch: boolean): void,
     handleChange(didChange?: boolean): void,
@@ -483,7 +484,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
     this._foundation.handleChange(false);
   }
 
-  writeValue(value: any): void {
+  writeValue(value: string): void {
     this.setSelectionByValue(value, false);
   }
 
@@ -504,13 +505,15 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
     if (!this.disabled) {
       this._foundation.handleBlur();
       this._onTouched();
+      this.focused = false;
     }
   }
 
   onFocus(): void {
     if (!this.disabled) {
       this._foundation.handleFocus();
       this._onTouched();
+      this.focused = true;
     }
   }
 
@@ -525,7 +528,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
     this._foundation.handleKeydown(evt);
   }
 
-  getValue(): any {
+  getValue(): string {
     return this._value;
   }
 
@@ -541,34 +544,35 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
    * Sets the selected option based on a value. If no option can be
    * found with the designated value, the select trigger is cleared.
    */
-  setSelectionByValue(value: any, isUserInput: boolean = true): void {
+  setSelectionByValue(value: string, isUserInput: boolean = true): void {
     if (!this._foundation) { return; }
 
-    const newValue = value;
-    this._setEnhancedSelection(newValue); // if enhanced select, perform selection
+    this._setEnhancedSelection(value); // if enhanced select, perform selection
 
-    if (this._value === newValue) {
-      if (newValue === null) {
-        this._valid = true;
-      }
-      return;
+    this._value = value !== null ? value : '';
+    if (!this.isValueEmpty()) {
+      this._foundation.setValue(this._value);
     }
 
-    this._value = newValue;
-    this._foundation.setValue(this._value);
     this.valueChange.emit({ index: this.getSelectedIndex(), value: this._value });
 
     if (isUserInput) {
       this._onChange(this._value);
     }
+
+    if (this.isValueEmpty()) {
+      this._getFloatingLabel().float(false);
+      this._foundation.notchOutline(false);
+    }
     this._changeDetectorRef.markForCheck();
   }
 
   setSelectedIndex(index: number): void {
-    this._foundation.setSelectedIndex(index);
-
     if (this._isEnhancedVariant()) {
       this._list.setSelectedIndex(index);
+      this._getEnhancedSelectAdapterMethods().closeMenu();
+    } else {
+      this._getNativeSelectAdapterMethods().setSelectedIndex(index);
     }
 
     const value = this._isEnhancedVariant() ? this._list.getSelectedValue() : this._getInputElement().value;
@@ -595,10 +599,13 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
       this._enhancedSelectedText = '';
       this._list.reset();
     }
+    this._value = '';
+    this.setSelectedIndex(-1);
+    this._foundation.setValid(true);
+  }
 
-    this._value = null;
-    this.valid = true;
-    this.layout();
+  isValueEmpty(): boolean {
+    return (this._value === undefined || this._value === null) || this._value.length === 0 ? true : false;
   }
 
   /** Initialize Select internal state based on the environment state */
@@ -719,7 +726,7 @@ export class MdcSelect extends _MdcSelectMixinBase implements AfterContentInit,
     }
 
     if (this.required && !this.disabled) {
-      return this.getSelectedIndex() !== -1 && (this.getSelectedIndex() !== 0 || this._value);
+      return this.getSelectedIndex() !== -1 && (this.getSelectedIndex() !== 0 || !this.isValueEmpty());
     }
     return true;
   }

diff --git a/test/select/select.test.ts b/test/select/select.test.ts
@@ -182,9 +182,11 @@ describe('MdcSelectModule', () => {
     }));
 
     it('should be able to focus the select trigger', fakeAsync(() => {
+      expect(testInstance.focused).toBe(false);
       document.body.focus(); // ensure that focus isn't on the trigger already
       testInstance.focus();
 
+      expect(testInstance.focused).toBe(true);
       expect(document.activeElement).toBe(testInstance._nativeSelect.nativeElement,
         'Expected select element to be focused.');
     }));