From 8a8bedb43c256bf8d62522245423255e66304575 Mon Sep 17 00:00:00 2001
From: Paul Gschwendtner <paulgschwendtner@gmail.com>
Date: Mon, 20 Feb 2017 18:42:57 +0100
Subject: [PATCH] fix(checkbox): create ripple on label mousedown

* Now triggers the ripple on label mousedown (similar as to slide-toggle and radio)
* Only sets cursor for underlying `label` element (similar as for native checkbox inputs)
* Adds a test that confirms that ripples work for checkbox

Fixes #3030
---
 src/lib/checkbox/checkbox.html    |  4 +-
 src/lib/checkbox/checkbox.scss    |  5 ++-
 src/lib/checkbox/checkbox.spec.ts | 75 ++++++++++++++++++++-----------
 src/lib/checkbox/checkbox.ts      |  3 --
 4 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/src/lib/checkbox/checkbox.html b/src/lib/checkbox/checkbox.html
index d9dc0a5a2120..185e28b16ea9 100644
--- a/src/lib/checkbox/checkbox.html
+++ b/src/lib/checkbox/checkbox.html
@@ -1,4 +1,4 @@
-<label class="mat-checkbox-layout">
+<label class="mat-checkbox-layout" #label>
   <div class="mat-checkbox-inner-container">
     <input #input
            class="mat-checkbox-input cdk-visually-hidden" type="checkbox"
@@ -16,7 +16,7 @@
            (change)="_onInteractionEvent($event)"
            (click)="_onInputClick($event)">
     <div md-ripple *ngIf="!_isRippleDisabled()" class="mat-checkbox-ripple"
-         [mdRippleTrigger]="_getHostElement()"
+         [mdRippleTrigger]="label"
          [mdRippleCentered]="true"></div>
     <div class="mat-checkbox-frame"></div>
     <div class="mat-checkbox-background">
diff --git a/src/lib/checkbox/checkbox.scss b/src/lib/checkbox/checkbox.scss
index 66ba03b216eb..26a2d2ea5469 100644
--- a/src/lib/checkbox/checkbox.scss
+++ b/src/lib/checkbox/checkbox.scss
@@ -187,7 +187,6 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default;
 }
 
 .mat-checkbox {
-  cursor: pointer;
   font-family: $mat-font-family;
 
   // Animation
@@ -195,6 +194,10 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default;
               mat-elevation-transition-property-value();
 }
 
+.mat-checkbox-label {
+  cursor: pointer;
+}
+
 .mat-checkbox-layout {
   // `cursor: inherit` ensures that the wrapper element gets the same cursor as the mat-checkbox
   // (e.g. pointer by default, regular when disabled), instead of the browser default.
diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts
index 5133a8df58d5..32d255aa05f5 100644
--- a/src/lib/checkbox/checkbox.spec.ts
+++ b/src/lib/checkbox/checkbox.spec.ts
@@ -190,17 +190,6 @@ describe('MdCheckbox', () => {
       expect(inputElement.disabled).toBe(false);
     });
 
-    it('should not have a ripple when disabled', () => {
-      let rippleElement = checkboxNativeElement.querySelector('[md-ripple]');
-      expect(rippleElement).toBeTruthy('Expected an enabled checkbox to have a ripple');
-
-      testComponent.isDisabled = true;
-      fixture.detectChanges();
-
-      rippleElement = checkboxNativeElement.querySelector('[md-ripple]');
-      expect(rippleElement).toBeFalsy('Expected a disabled checkbox not to have a ripple');
-    });
-
     it('should not toggle `checked` state upon interation while disabled', () => {
       testComponent.isDisabled = true;
       fixture.detectChanges();
@@ -324,6 +313,44 @@ describe('MdCheckbox', () => {
       expect(document.activeElement).toBe(inputElement);
     });
 
+    describe('ripple elements', () => {
+
+      it('should show ripples on label mousedown', () => {
+        expect(checkboxNativeElement.querySelector('.mat-ripple-element')).toBeFalsy();
+
+        dispatchFakeEvent(labelElement, 'mousedown');
+        dispatchFakeEvent(labelElement, 'mouseup');
+
+        expect(checkboxNativeElement.querySelectorAll('.mat-ripple-element').length).toBe(1);
+      });
+
+      it('should not have a ripple when disabled', () => {
+        let rippleElement = checkboxNativeElement.querySelector('[md-ripple]');
+        expect(rippleElement).toBeTruthy('Expected an enabled checkbox to have a ripple');
+
+        testComponent.isDisabled = true;
+        fixture.detectChanges();
+
+        rippleElement = checkboxNativeElement.querySelector('[md-ripple]');
+        expect(rippleElement).toBeFalsy('Expected a disabled checkbox not to have a ripple');
+      });
+
+      it('should remove ripple if mdRippleDisabled input is set', async(() => {
+        testComponent.disableRipple = true;
+        fixture.detectChanges();
+
+        expect(checkboxNativeElement.querySelectorAll('[md-ripple]').length)
+          .toBe(0, 'Expect no [md-ripple] in checkbox');
+
+        testComponent.disableRipple = false;
+        fixture.detectChanges();
+
+        expect(checkboxNativeElement.querySelectorAll('[md-ripple]').length)
+          .toBe(1, 'Expect [md-ripple] in checkbox');
+      }));
+
+    });
+
     describe('color behaviour', () => {
       it('should apply class based on color attribute', () => {
         testComponent.checkboxColor = 'primary';
@@ -544,19 +571,6 @@ describe('MdCheckbox', () => {
       expect(inputElement.tabIndex).toBe(13);
     });
 
-    it('should remove ripple if mdRippleDisabled input is set', async(() => {
-      testComponent.disableRipple = true;
-      fixture.detectChanges();
-
-      expect(checkboxNativeElement.querySelectorAll('[md-ripple]').length)
-        .toBe(0, 'Expect no [md-ripple] in checkbox');
-
-      testComponent.disableRipple = false;
-      fixture.detectChanges();
-
-      expect(checkboxNativeElement.querySelectorAll('[md-ripple]').length)
-        .toBe(1, 'Expect [md-ripple] in checkbox');
-    }));
   });
 
   describe('with multiple checkboxes', () => {
@@ -678,6 +692,7 @@ describe('MdCheckbox', () => {
         [(indeterminate)]="isIndeterminate"
         [disabled]="isDisabled"
         [color]="checkboxColor"
+        [disableRipple]="disableRipple"
         (change)="changeCount = changeCount + 1"
         (click)="onCheckboxClick($event)"
         (change)="onCheckboxChange($event)">
@@ -691,6 +706,7 @@ class SingleCheckbox {
   isRequired: boolean = false;
   isIndeterminate: boolean = false;
   isDisabled: boolean = false;
+  disableRipple: boolean = false;
   parentElementClicked: boolean = false;
   parentElementKeyedUp: boolean = false;
   lastKeydownEvent: Event = null;
@@ -728,14 +744,12 @@ class MultipleCheckboxes { }
   template: `
     <md-checkbox
         [tabIndex]="customTabIndex"
-        [disabled]="isDisabled"
-        [disableRipple]="disableRipple">
+        [disabled]="isDisabled">
     </md-checkbox>`,
 })
 class CheckboxWithTabIndex {
   customTabIndex: number = 7;
   isDisabled: boolean = false;
-  disableRipple: boolean = false;
 }
 
 /** Simple test component with an aria-label set. */
@@ -771,3 +785,10 @@ class CheckboxWithChangeEvent {
 class CheckboxWithFormControl {
   formControl = new FormControl();
 }
+
+// TODO(devversion): replace with global utility once pull request #2943 is merged.
+function dispatchFakeEvent(element: HTMLElement, eventName: string): void {
+  let event  = document.createEvent('Event');
+  event.initEvent(eventName, true, true);
+  element.dispatchEvent(event);
+}
diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts
index ba8fe29e96f3..270161c6d6dd 100644
--- a/src/lib/checkbox/checkbox.ts
+++ b/src/lib/checkbox/checkbox.ts
@@ -397,9 +397,6 @@ export class MdCheckbox implements ControlValueAccessor {
     return `mat-checkbox-anim-${animSuffix}`;
   }
 
-  _getHostElement() {
-    return this._elementRef.nativeElement;
-  }
 }