fix(slide-toggle): stop change event firing upon init (#713)

* test(slide-toggle): add test to confirm that change event don't fires multiple times

* Adds a test which confirms, that the slide-toggle isn't firing the (change) event multiple times.

Closes #709

* update(): prevent change event to be fired at initialization

* update(): remove internal annotation

Author: devversion

src/components/slide-toggle/slide-toggle.spec.ts

@@ -33,11 +33,19 @@ describe('MdSlideToggle', () => {
     beforeEach(async(() => {
       builder.createAsync(SlideToggleTestApp).then(f => {
         fixture = f;
+
+        testComponent = fixture.debugElement.componentInstance;
+
+        // Enable jasmine spies on event functions, which may trigger at initialization
+        // of the slide-toggle component.
+        spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough();
+        spyOn(fixture.debugElement.componentInstance, 'onSlideClick').and.callThrough();
+
+        // Initialize the slide-toggle component, by triggering the first change detection cycle.
         fixture.detectChanges();
 
         let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle'));
 
-        testComponent = fixture.debugElement.componentInstance;
         slideToggle = slideToggleDebug.componentInstance;
         slideToggleElement = slideToggleDebug.nativeElement;
         slideToggleControl = slideToggleDebug.injector.get(NgControl);
@@ -103,8 +111,6 @@ describe('MdSlideToggle', () => {
       // Since we're using a label element and a visual hidden input, this behavior can led
       // to an issue, where the click events on the slide-toggle are getting executed twice.
 
-      spyOn(testComponent, 'onSlideClick');
-
       expect(slideToggle.checked).toBe(false);
       expect(slideToggleElement.classList).not.toContain('md-checked');
 
@@ -117,6 +123,42 @@ describe('MdSlideToggle', () => {
       expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
     });
 
+    it('should not trigger the change event multiple times', async(() => {
+      expect(inputElement.checked).toBe(false);
+      expect(slideToggleElement.classList).not.toContain('md-checked');
+
+      testComponent.slideChecked = true;
+      fixture.detectChanges();
+
+      expect(inputElement.checked).toBe(true);
+      expect(slideToggleElement.classList).toContain('md-checked');
+
+      // Wait for the fixture to become stable, because the EventEmitter for the change event,
+      // will only fire after the zone async change detection has finished.
+      fixture.whenStable().then(() => {
+        expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
+      });
+
+    }));
+
+    it('should not trigger the change event on initialization', async(() => {
+      expect(inputElement.checked).toBe(false);
+      expect(slideToggleElement.classList).not.toContain('md-checked');
+
+      testComponent.slideChecked = true;
+      fixture.detectChanges();
+
+      expect(inputElement.checked).toBe(true);
+      expect(slideToggleElement.classList).toContain('md-checked');
+
+      // Wait for the fixture to become stable, because the EventEmitter for the change event,
+      // will only fire after the zone async change detection has finished.
+      fixture.whenStable().then(() => {
+        expect(testComponent.onSlideChange).toHaveBeenCalledTimes(1);
+      });
+
+    }));
+
     it('should add a suffix to the inputs id', () => {
       testComponent.slideId = 'myId';
       fixture.detectChanges();
@@ -269,6 +311,56 @@ describe('MdSlideToggle', () => {
 
   });
 
+  describe('custom template', () => {
+
+    let testComponent: SlideToggleTestApp;
+    let slideToggle: MdSlideToggle;
+    let slideToggleElement: HTMLElement;
+    let labelElement: HTMLLabelElement;
+    let inputElement: HTMLInputElement;
+
+    it('should not trigger the change event on initialization', async(() => {
+      builder
+        .overrideTemplate(SlideToggleTestApp, `
+          <md-slide-toggle checked="true" (change)="onSlideChange($event)"></md-slide-toggle>
+        `)
+        .createAsync(SlideToggleTestApp)
+        .then(fixture => {
+          // Initialize the variables for our test.
+          initializeTest(fixture);
+
+          // Enable jasmine spies on event functions, which may trigger at initialization
+          // of the slide-toggle component.
+          spyOn(fixture.debugElement.componentInstance, 'onSlideChange').and.callThrough();
+
+          fixture.detectChanges();
+
+          fixture.whenStable().then(() => {
+            expect(testComponent.onSlideChange).not.toHaveBeenCalled();
+          });
+        });
+    }));
+
+    /**
+     * Initializes the suites variables, to allow developers to easily access the several variables
+     * without loading / querying them always again.
+     * @param fixture Custom fixture, which contains the slide-toggle component.
+     */
+    function initializeTest(fixture: ComponentFixture<any>) {
+      testComponent = fixture.debugElement.componentInstance;
+
+      // Initialize the slide-toggle component, by triggering the first change detection cycle.
+      fixture.detectChanges();
+
+      let slideToggleDebug = fixture.debugElement.query(By.css('md-slide-toggle'));
+
+      slideToggle = slideToggleDebug.componentInstance;
+      slideToggleElement = slideToggleDebug.nativeElement;
+      inputElement = fixture.debugElement.query(By.css('input')).nativeElement;
+      labelElement = fixture.debugElement.query(By.css('label')).nativeElement;
+    }
+  });
+
 });
 
 /**
@@ -288,7 +380,7 @@ function dispatchFocusChangeEvent(eventName: string, element: HTMLElement): void
     <md-slide-toggle [(ngModel)]="slideModel" [disabled]="isDisabled" [color]="slideColor" 
                      [id]="slideId" [checked]="slideChecked" [name]="slideName" 
                      [aria-label]="slideLabel" [ariaLabel]="slideLabel" 
-                     [ariaLabelledby]="slideLabelledBy" (change)="lastEvent = $event"
+                     [ariaLabelledby]="slideLabelledBy" (change)="onSlideChange($event)"
                      (click)="onSlideClick($event)">
       <span>Test Slide Toggle</span>
     </md-slide-toggle>
@@ -307,4 +399,7 @@ class SlideToggleTestApp {
   lastEvent: MdSlideToggleChange;
 
   onSlideClick(event: Event) {}
+  onSlideChange(event: MdSlideToggleChange) {
+    this.lastEvent = event;
+  }
 }

src/components/slide-toggle/slide-toggle.ts

@@ -6,7 +6,8 @@ import {
   ChangeDetectionStrategy,
   Input,
   Output,
-  EventEmitter
+  EventEmitter,
+  AfterContentInit
 } from '@angular/core';
 import {
   ControlValueAccessor,
@@ -45,7 +46,7 @@ let nextId = 0;
   providers: [MD_SLIDE_TOGGLE_VALUE_ACCESSOR],
   changeDetection: ChangeDetectionStrategy.OnPush
 })
-export class MdSlideToggle implements ControlValueAccessor {
+export class MdSlideToggle implements AfterContentInit, ControlValueAccessor {
 
   private onChange = (_: any) => {};
   private onTouched = () => {};
@@ -56,6 +57,7 @@ export class MdSlideToggle implements ControlValueAccessor {
   private _color: string;
   private _hasFocus: boolean = false;
   private _isMousedown: boolean = false;
+  private _isInitialized: boolean = false;
 
   @Input() @BooleanFieldValue() disabled: boolean = false;
   @Input() name: string = null;
@@ -74,6 +76,14 @@ export class MdSlideToggle implements ControlValueAccessor {
               private _renderer: Renderer) {
   }
 
+  /** TODO: internal */
+  ngAfterContentInit() {
+    // Mark this component as initialized in AfterContentInit because the initial checked value can
+    // possibly be set by NgModel or the checked attribute. This would cause the change event to
+    // be emitted, before the component is actually initialized.
+    this._isInitialized = true;
+  }
+
   /**
    * The onChangeEvent method will be also called on click.
    * This is because everything for the slide-toggle is wrapped inside of a label,
@@ -163,7 +173,12 @@ export class MdSlideToggle implements ControlValueAccessor {
     if (this.checked !== !!value) {
       this._checked = value;
       this.onChange(this._checked);
-      this._emitChangeEvent();
+
+      // Only fire a change event if the `slide-toggle` is completely initialized and
+      // all attributes / inputs are properly loaded.
+      if (this._isInitialized) {
+        this._emitChangeEvent();
+      }
     }
   }