Skip to content

Commit

Permalink
fix(checkbox): create ripple on label mousedown
Browse files Browse the repository at this point in the history
* 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 angular#3030
  • Loading branch information
devversion committed Feb 20, 2017
1 parent b939cd8 commit 8a8bedb
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 33 deletions.
4 changes: 2 additions & 2 deletions src/lib/checkbox/checkbox.html
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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">
Expand Down
5 changes: 4 additions & 1 deletion src/lib/checkbox/checkbox.scss
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,17 @@ $_mat-checkbox-mark-stroke-size: 2 / 15 * $mat-checkbox-size !default;
}

.mat-checkbox {
cursor: pointer;
font-family: $mat-font-family;

// Animation
transition: background $swift-ease-out-duration $swift-ease-out-timing-function,
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.
Expand Down
75 changes: 48 additions & 27 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -678,6 +692,7 @@ describe('MdCheckbox', () => {
[(indeterminate)]="isIndeterminate"
[disabled]="isDisabled"
[color]="checkboxColor"
[disableRipple]="disableRipple"
(change)="changeCount = changeCount + 1"
(click)="onCheckboxClick($event)"
(change)="onCheckboxChange($event)">
Expand All @@ -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;
Expand Down Expand Up @@ -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. */
Expand Down Expand Up @@ -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);
}
3 changes: 0 additions & 3 deletions src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,6 @@ export class MdCheckbox implements ControlValueAccessor {
return `mat-checkbox-anim-${animSuffix}`;
}

_getHostElement() {
return this._elementRef.nativeElement;
}
}


Expand Down

0 comments on commit 8a8bedb

Please sign in to comment.