fix(slider): fix change & input emit logic. (#6234)

Author: mmalerba

src/lib/slider/slider.spec.ts

@@ -1,6 +1,6 @@
 import {async, ComponentFixture, TestBed} from '@angular/core/testing';
 import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms';
-import {Component, DebugElement} from '@angular/core';
+import {Component, DebugElement, ViewChild} from '@angular/core';
 import {By, HAMMER_GESTURE_CONFIG} from '@angular/platform-browser';
 import {MdSlider, MdSliderModule} from './index';
 import {TestGestureConfig} from './test-gesture-config';
@@ -658,6 +658,7 @@ describe('MdSlider without forms', () => {
 
       testComponent = fixture.debugElement.componentInstance;
       spyOn(testComponent, 'onChange');
+      spyOn(testComponent, 'onInput');
 
       sliderDebugElement = fixture.debugElement.query(By.directive(MdSlider));
       sliderNativeElement = sliderDebugElement.nativeElement;
@@ -691,6 +692,30 @@ describe('MdSlider without forms', () => {
 
       expect(testComponent.onChange).toHaveBeenCalledTimes(1);
     });
+
+    it('should dispatch events when changing back to previously emitted value after ' +
+        'programmatically setting value', () => {
+      expect(testComponent.onChange).not.toHaveBeenCalled();
+      expect(testComponent.onInput).not.toHaveBeenCalled();
+
+      dispatchClickEventSequence(sliderNativeElement, 0.2);
+      fixture.detectChanges();
+
+      expect(testComponent.onChange).toHaveBeenCalledTimes(1);
+      expect(testComponent.onInput).toHaveBeenCalledTimes(1);
+
+      testComponent.slider.value = 0;
+      fixture.detectChanges();
+
+      expect(testComponent.onChange).toHaveBeenCalledTimes(1);
+      expect(testComponent.onInput).toHaveBeenCalledTimes(1);
+
+      dispatchClickEventSequence(sliderNativeElement, 0.2);
+      fixture.detectChanges();
+
+      expect(testComponent.onChange).toHaveBeenCalledTimes(2);
+      expect(testComponent.onInput).toHaveBeenCalledTimes(2);
+    });
   });
 
   describe('slider with input event', () => {
@@ -1351,6 +1376,8 @@ class SliderWithValueGreaterThanMax { }
 class SliderWithChangeHandler {
   onChange() { }
   onInput() { }
+
+  @ViewChild(MdSlider) slider: MdSlider;
 }
 
 @Component({

src/lib/slider/slider.ts

@@ -385,15 +385,12 @@ export class MdSlider extends _MdSliderMixinBase
 
   private _controlValueAccessorChangeFn: (value: any) => void = () => {};
 
-  /** The last value for which a change event was emitted. */
-  private _lastChangeValue: number | null;
-
-  /** The last value for which an input event was emitted. */
-  private _lastInputValue: number | null;
-
   /** Decimal places to round to, based on the step amount. */
   private _roundLabelTo: number;
 
+  /** The value of the slider when the slide start event fires. */
+  private _valueOnSlideStart: number | null;
+
   /**
    * Whether mouse events should be converted to a slider position by calculating their distance
    * from the right or bottom edge of the slider as opposed to the top or left.
@@ -438,13 +435,16 @@ export class MdSlider extends _MdSliderMixinBase
       return;
     }
 
+    let oldValue = this.value;
     this._isSliding = false;
     this._renderer.addFocus();
     this._updateValueFromPosition({x: event.clientX, y: event.clientY});
 
-    /* Emits a change and input event if the value changed. */
-    this._emitInputEvent();
-    this._emitValueIfChanged();
+    /* Emit a change and input event if the value changed. */
+    if (oldValue != this.value) {
+      this._emitInputEvent();
+      this._emitChangeEvent();
+    }
   }
 
   _onSlide(event: HammerInput) {
@@ -460,10 +460,14 @@ export class MdSlider extends _MdSliderMixinBase
 
     // Prevent the slide from selecting anything else.
     event.preventDefault();
+
+    let oldValue = this.value;
     this._updateValueFromPosition({x: event.center.x, y: event.center.y});
 
     // Native range elements always emit `input` events when the value changed while sliding.
-    this._emitInputEvent();
+    if (oldValue != this.value) {
+      this._emitInputEvent();
+    }
   }
 
   _onSlideStart(event: HammerInput | null) {
@@ -476,6 +480,7 @@ export class MdSlider extends _MdSliderMixinBase
 
     this._isSliding = true;
     this._renderer.addFocus();
+    this._valueOnSlideStart = this.value;
 
     if (event) {
       this._updateValueFromPosition({x: event.center.x, y: event.center.y});
@@ -485,7 +490,11 @@ export class MdSlider extends _MdSliderMixinBase
 
   _onSlideEnd() {
     this._isSliding = false;
-    this._emitValueIfChanged();
+
+    if (this._valueOnSlideStart != this.value) {
+      this._emitChangeEvent();
+    }
+    this._valueOnSlideStart = null;
   }
 
   _onFocus() {
@@ -502,6 +511,8 @@ export class MdSlider extends _MdSliderMixinBase
   _onKeydown(event: KeyboardEvent) {
     if (this.disabled) { return; }
 
+    let oldValue = this.value;
+
     switch (event.keyCode) {
       case PAGE_UP:
         this._increment(10);
@@ -540,8 +551,11 @@ export class MdSlider extends _MdSliderMixinBase
         // it.
         return;
     }
-    this._emitInputEvent();
-    this._emitValueIfChanged();
+
+    if (oldValue != this.value) {
+      this._emitInputEvent();
+      this._emitChangeEvent();
+    }
 
     this._isSliding = true;
     event.preventDefault();
@@ -581,22 +595,14 @@ export class MdSlider extends _MdSliderMixinBase
   }
 
   /** Emits a change event if the current value is different from the last emitted value. */
-  private _emitValueIfChanged() {
-    if (this.value != this._lastChangeValue) {
-      let event = this._createChangeEvent();
-      this._lastChangeValue = this.value;
-      this._controlValueAccessorChangeFn(this.value);
-      this.change.emit(event);
-    }
+  private _emitChangeEvent() {
+    this._controlValueAccessorChangeFn(this.value);
+    this.change.emit(this._createChangeEvent());
   }
 
   /** Emits an input event when the current value is different from the last emitted value. */
   private _emitInputEvent() {
-    if (this.value != this._lastInputValue) {
-      let event = this._createChangeEvent();
-      this._lastInputValue = this.value;
-      this.input.emit(event);
-    }
+    this.input.emit(this._createChangeEvent());
   }
 
   /** Updates the amount of space between ticks as a percentage of the width of the slider. */