fix(checkbox): prevent double click events (#672)

* fix(slide-toggle and checkbox): visual hidden input should not bubble click

* Currently the (click) event gets called twice.
  This is caused by the bubbling of the (click) event on the input.

Fixes #671.

* update(): move input click into component source

* Moved input click event into component source file.
* Added comment for the issue cause.

* fix(checkbox): do not trigger the click event twice

Author: devversion

src/components/checkbox/checkbox.html

@@ -11,7 +11,8 @@
            [attr.aria-labelledby]="ariaLabelledby"
            (focus)="onInputFocus()"
            (blur)="onInputBlur()"
-           (change)="onInteractionEvent($event)">
+           (change)="onInteractionEvent($event)"
+           (click)="onInputClick($event)">
     <div class="md-ink-ripple"></div>
     <div class="md-checkbox-frame"></div>
     <div class="md-checkbox-background">

src/components/checkbox/checkbox.spec.ts

@@ -183,6 +183,26 @@ describe('MdCheckbox', () => {
       expect(checkboxNativeElement.classList).toContain('md-checkbox-align-end');
     });
 
+    it('should not trigger the click event multiple times', () => {
+      // By default, when clicking on a label element, a generated click will be dispatched
+      // on the associated input element.
+      // 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 checkbox are getting executed twice.
+
+      spyOn(testComponent, 'onCheckboxClick');
+
+      expect(inputElement.checked).toBe(false);
+      expect(checkboxNativeElement.classList).not.toContain('md-checkbox-checked');
+
+      labelElement.click();
+      fixture.detectChanges();
+
+      expect(checkboxNativeElement.classList).toContain('md-checkbox-checked');
+      expect(inputElement.checked).toBe(true);
+
+      expect(testComponent.onCheckboxClick).toHaveBeenCalledTimes(1);
+    });
+
     it('should emit a change event when the `checked` value changes', () => {
       // TODO(jelbourn): this *should* work with async(), but fixture.whenStable currently doesn't
       // know to look at pending macro tasks.
@@ -463,7 +483,8 @@ describe('MdCheckbox', () => {
         [checked]="isChecked" 
         [indeterminate]="isIndeterminate" 
         [disabled]="isDisabled"
-        (change)="changeCount = changeCount + 1">
+        (change)="changeCount = changeCount + 1"
+        (click)="onCheckboxClick($event)">
       Simple checkbox
     </md-checkbox>
   </div>`
@@ -476,6 +497,8 @@ class SingleCheckbox {
   parentElementClicked: boolean = false;
   parentElementKeyedUp: boolean = false;
   lastKeydownEvent: Event = null;
+
+  onCheckboxClick(event: Event) {}
 }
 
 /** Simple component for testing an MdCheckbox with ngModel and ngControl. */

src/components/checkbox/checkbox.ts

@@ -278,6 +278,18 @@ export class MdCheckbox implements AfterContentInit, ControlValueAccessor {
     }
   }
 
+  /** @internal */
+  onInputClick(event: Event) {
+    // We have to stop propagation for click events on the visual hidden input element.
+    // By default, when a user clicks on a label element, a generated click event will be
+    // dispatched on the associated input element. Since we are using a label element as our
+    // root container, the click event on the `checkbox` will be executed twice.
+    // The real click event will bubble up, and the generated click event also tries to bubble up.
+    // This will lead to multiple click events.
+    // Preventing bubbling for the second event will solve that issue.
+    event.stopPropagation();
+  }
+
   private _getAnimationClassForCheckStateTransition(
       oldState: TransitionCheckState, newState: TransitionCheckState): string {
     var animSuffix: string;

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

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

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

@@ -97,6 +97,26 @@ describe('MdSlideToggle', () => {
       expect(slideToggle.checked).toBe(true);
     });
 
+    it('should not trigger the click event multiple times', () => {
+      // By default, when clicking on a label element, a generated click will be dispatched
+      // on the associated input element.
+      // 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');
+
+      labelElement.click();
+      fixture.detectChanges();
+
+      expect(slideToggleElement.classList).toContain('md-checked');
+      expect(slideToggle.checked).toBe(true);
+
+      expect(testComponent.onSlideClick).toHaveBeenCalledTimes(1);
+    });
+
     it('should add a suffix to the inputs id', () => {
       testComponent.slideId = 'myId';
       fixture.detectChanges();
@@ -268,7 +288,8 @@ 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)="lastEvent = $event"
+                     (click)="onSlideClick($event)">
       <span>Test Slide Toggle</span>
     </md-slide-toggle>
   `,
@@ -284,4 +305,6 @@ class SlideToggleTestApp {
   slideLabel: string;
   slideLabelledBy: string;
   lastEvent: MdSlideToggleChange;
+
+  onSlideClick(event: Event) {}
 }

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

@@ -38,7 +38,6 @@ let nextId = 0;
     '[class.md-disabled]': 'disabled',
     // This md-slide-toggle prefix will change, once the temporary ripple is removed.
     '[class.md-slide-toggle-focused]': '_hasFocus',
-    '(click)': 'onTouched()',
     '(mousedown)': 'setMousedown()'
   },
   templateUrl: 'slide-toggle.html',
@@ -92,6 +91,20 @@ export class MdSlideToggle implements ControlValueAccessor {
     }
   }
 
+  /** @internal */
+  onInputClick(event: Event) {
+    this.onTouched();
+
+    // We have to stop propagation for click events on the visual hidden input element.
+    // By default, when a user clicks on a label element, a generated click event will be
+    // dispatched on the associated input element. Since we are using a label element as our
+    // root container, the click event on the `slide-toggle` will be executed twice.
+    // The real click event will bubble up, and the generated click event also tries to bubble up.
+    // This will lead to multiple click events.
+    // Preventing bubbling for the second event will solve that issue.
+    event.stopPropagation();
+  }
+
   /** @internal */
   setMousedown() {
     // We only *show* the focus style when focus has come to the button via the keyboard.