Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,15 @@ describe('MdCheckbox', () => {
expect(inputElement.indeterminate).toBe(false);
});

it('should set indeterminate to false when set checked', async(() => {
it('should set indeterminate to false when input clicked', async(() => {
testComponent.isIndeterminate = true;
fixture.detectChanges();

expect(checkboxInstance.indeterminate).toBe(true);
expect(inputElement.indeterminate).toBe(true);
expect(testComponent.isIndeterminate).toBe(true);

testComponent.isChecked = true;
inputElement.click();
fixture.detectChanges();

fixture.whenStable().then(() => {
Expand All @@ -127,7 +127,7 @@ describe('MdCheckbox', () => {
expect(inputElement.checked).toBe(true);
expect(testComponent.isIndeterminate).toBe(true);

testComponent.isChecked = false;
inputElement.click();
fixture.detectChanges();

fixture.whenStable().then(() => {
Expand All @@ -141,6 +141,31 @@ describe('MdCheckbox', () => {

}));

it('should not set indeterminate to false when checked is set programmatically', async(() => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may be able to avoid using async here as well.

testComponent.isIndeterminate = true;
fixture.detectChanges();

expect(checkboxInstance.indeterminate).toBe(true);
expect(inputElement.indeterminate).toBe(true);
expect(testComponent.isIndeterminate).toBe(true);

testComponent.isChecked = true;
fixture.detectChanges();

expect(checkboxInstance.checked).toBe(true);
expect(inputElement.indeterminate).toBe(true);
expect(inputElement.checked).toBe(true);
expect(testComponent.isIndeterminate).toBe(true);

testComponent.isChecked = false;
fixture.detectChanges();

expect(checkboxInstance.checked).toBe(false);
expect(inputElement.indeterminate).toBe(true);
expect(inputElement.checked).toBe(false);
expect(testComponent.isIndeterminate).toBe(true);
}));

it('should change native element checked when check programmatically', () => {
expect(inputElement.checked).toBe(false);

Expand Down Expand Up @@ -216,11 +241,11 @@ describe('MdCheckbox', () => {
expect(checkboxInstance.checked).toBe(false);
});

it('should overwrite indeterminate state when checked is re-set', async(() => {
it('should overwrite indeterminate state when clicked', async(() => {
testComponent.isIndeterminate = true;
fixture.detectChanges();

testComponent.isChecked = true;
inputElement.click();
fixture.detectChanges();

fixture.whenStable().then(() => {
Expand Down
24 changes: 11 additions & 13 deletions src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,14 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
}

/**
* Whether the checkbox is checked. Note that setting `checked` will immediately set
* `indeterminate` to false.
* Whether the checkbox is checked.
*/
@Input() get checked() {
return this._checked;
}

set checked(checked: boolean) {
if (checked != this.checked) {
if (this._indeterminate) {
Promise.resolve().then(() => {
this._indeterminate = false;
this.indeterminateChange.emit(this._indeterminate);
});
}
this._checked = checked;
this._changeDetectorRef.markForCheck();
}
Expand All @@ -226,11 +219,8 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
/**
* Whether the checkbox is indeterminate. This is also known as "mixed" mode and can be used to
* represent a checkbox with three states, e.g. a checkbox that represents a nested list of
* checkable items. Note that whenever `checked` is set, indeterminate is immediately set to
* false. This differs from the web platform in that indeterminate state on native
* checkboxes is only remove when the user manually checks the checkbox (rather than setting the
* `checked` property programmatically). However, we feel that this behavior is more accommodating
* to the way consumers would envision using this component.
* checkable items. Note that whenever checkbox is manually clicked, indeterminate is immediately
* set to false.
*/
@Input() get indeterminate() {
return this._indeterminate;
Expand Down Expand Up @@ -371,6 +361,14 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
this._removeFocusRipple();

if (!this.disabled) {
// When user manually click on the checkbox, `indeterminate` is set to false.
if (this._indeterminate) {
Promise.resolve().then(() => {
this._indeterminate = false;
this.indeterminateChange.emit(this._indeterminate);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can go in a follow-up PR, but this should only emit when the value is different

});
}

this.toggle();
this._transitionCheckState(
this._checked ? TransitionCheckState.Checked : TransitionCheckState.Unchecked);
Expand Down