fix: visually hidden inputs should not bubble change event (#551)

* fix(): visual hidden inputs should not bubble change event

* Currently the change event of the visual hidden inputs will bubble up and emit its event object to the component's `change` output.

This is currently an issue of Angular 2
- See angular/angular#4059

To prevent the events from bubbling up, we have to stop propagation on
change.

Fixes #544.

Author: devversion

src/components/checkbox/checkbox.spec.ts

@@ -286,7 +286,27 @@ describe('MdCheckbox', () => {
       fixture.detectChanges();
 
       tick();
-      expect(testComponent.handleChange).toHaveBeenCalled();
+
+      expect(testComponent.handleChange).toHaveBeenCalledTimes(1);
+      expect(testComponent.handleChange).toHaveBeenCalledWith(true);
+    }));
+
+    it('should not emit a DOM event to the change output', async(() => {
+      expect(testComponent.handleChange).not.toHaveBeenCalled();
+
+      // Trigger the click on the inputElement, because the input will probably
+      // emit a DOM event to the change output.
+      inputElement.click();
+      fixture.detectChanges();
+
+      fixture.whenStable().then(() => {
+        // We're checking the arguments type / emitted value to be a boolean, because sometimes the
+        // emitted value can be a DOM Event, which is not valid.
+        // See angular/angular#4059
+        expect(testComponent.handleChange).toHaveBeenCalledTimes(1);
+        expect(testComponent.handleChange).toHaveBeenCalledWith(true);
+      });
+
     }));
   });
 
@@ -521,8 +541,8 @@ class CheckboxWithNameAttribute {}
 /** Simple test component with change event */
 @Component({
   directives: [MdCheckbox],
-  template: `<md-checkbox (change)="handleChange()"></md-checkbox>`
+  template: `<md-checkbox (change)="handleChange($event)"></md-checkbox>`
 })
 class CheckboxWithChangeEvent {
-  handleChange(): void {}
+  handleChange(value: boolean): void {}
 }

src/components/checkbox/checkbox.ts

@@ -250,14 +250,18 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor {
   }
 
   /**
-   * Event handler for checkbox input element.  Toggles checked state if element is not disabled.
+   * Event handler for checkbox input element.
+   * Toggles checked state if element is not disabled.
    * @param event
    * @internal
    */
   onInteractionEvent(event: Event) {
-    if (this.disabled) {
-      event.stopPropagation();
-    } else {
+    // We always have to stop propagation on the change event.
+    // Otherwise the change event, from the input element, will bubble up and
+    // emit its event object to the `change` output. 
+    event.stopPropagation();
+
+    if (!this.disabled) {
       this.toggle();
     }
   }

src/components/radio/radio.html

@@ -13,7 +13,7 @@
           [checked]="checked"
           [disabled]="disabled"
           [name]="name"
-          (change)="onInputChange()"
+          (change)="onInputChange($event)"
           (focus)="onInputFocus()"
           (blur)="onInputBlur()" />
 

src/components/radio/radio.spec.ts

@@ -12,7 +12,7 @@ import {FORM_DIRECTIVES, NgControl} from '@angular/common';
 import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing';
 import {Component, DebugElement, provide} from '@angular/core';
 import {By} from '@angular/platform-browser';
-import {MD_RADIO_DIRECTIVES, MdRadioGroup, MdRadioButton} from './radio';
+import {MD_RADIO_DIRECTIVES, MdRadioGroup, MdRadioButton, MdRadioChange} from './radio';
 import {MdRadioDispatcher} from './radio_dispatcher';
 
 
@@ -446,7 +446,7 @@ class RadioGroupWithNgModel {
     {label: 'Chocolate', value: 'chocolate'},
     {label: 'Strawberry', value: 'strawberry'},
   ];
-  onChange(value: string) {}
+  onChange(value: MdRadioChange) {}
 }
 
 // TODO(jelbourn): remove eveything below when Angular supports faking events.

src/components/radio/radio.ts

@@ -391,7 +391,12 @@ export class MdRadioButton implements OnInit {
    * Checks the radio due to an interaction with the underlying native <input type="radio">
    * @internal
    */
-  onInputChange() {
+  onInputChange(event: Event) {
+    // We always have to stop propagation on the change event.
+    // Otherwise the change event, from the input element, will bubble up and
+    // emit its event object to the `change` output.
+    event.stopPropagation();
+
     this.checked = true;
     if (this.radioGroup) {
       this.radioGroup.touch();

src/components/slide-toggle/slide-toggle.html

@@ -17,7 +17,7 @@
            [attr.aria-labelledby]="ariaLabelledby"
            (blur)="onInputBlur()"
            (focus)="onInputFocus()"
-           (change)="onChangeEvent()">
+           (change)="onChangeEvent($event)">
   </div>
   <span class="md-slide-toggle-content">
     <ng-content></ng-content>

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

@@ -5,6 +5,7 @@ import {
   beforeEach,
   inject,
   async,
+  fakeAsync,
 } from '@angular/core/testing';
 import {TestComponentBuilder, ComponentFixture} from '@angular/compiler/testing';
 import {By} from '@angular/platform-browser';
@@ -161,14 +162,17 @@ describe('MdSlideToggle', () => {
       expect(slideToggleElement.classList).not.toContain('ng-dirty');
     });
 
-    it('should emit the new values', () => {
-      expect(testComponent.changeCount).toBe(0);
+    it('should emit the new values properly', fakeAsync(() => {
+      spyOn(testComponent, 'onValueChange');
 
       labelElement.click();
       fixture.detectChanges();
 
-      expect(testComponent.changeCount).toBe(1);
-    });
+      fixture.whenStable().then(() => {
+        expect(testComponent.onValueChange).toHaveBeenCalledTimes(1);
+        expect(testComponent.onValueChange).toHaveBeenCalledWith(true);
+      });
+    }));
 
     it('should support subscription on the change observable', () => {
       slideToggle.change.subscribe(value => {
@@ -265,7 +269,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)="changeCount = changeCount + 1">
+                     [ariaLabelledby]="slideLabelledBy" (change)="onValueChange($event)">
       <span>Test Slide Toggle</span>
     </md-slide-toggle>
   `,
@@ -280,5 +284,6 @@ class SlideToggleTestApp {
   slideName: string;
   slideLabel: string;
   slideLabelledBy: string;
-  changeCount: number = 0;
+
+  onValueChange(value: boolean): void {};
 }

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

@@ -75,7 +75,12 @@ export class MdSlideToggle implements ControlValueAccessor {
    * which triggers a onChange event on click.
    * @internal
    */
-  onChangeEvent() {
+  onChangeEvent(event: Event) {
+    // We always have to stop propagation on the change event.
+    // Otherwise the change event, from the input element, will bubble up and
+    // emit its event object to the component's `change` output.
+    event.stopPropagation();
+
     if (!this.disabled) {
       this.toggle();
     }