From d34e1b1fd6bc1ba6fe90aaf8656771f95f620de2 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Mon, 10 Apr 2017 12:02:54 -0700 Subject: [PATCH 1/2] Only set indeterminate when user clicked on checkbox --- src/lib/checkbox/checkbox.spec.ts | 42 +++++++++++++++++++++++++++---- src/lib/checkbox/checkbox.ts | 24 ++++++++---------- 2 files changed, 48 insertions(+), 18 deletions(-) diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts index 180a4003fa7a..300b11de8f70 100644 --- a/src/lib/checkbox/checkbox.spec.ts +++ b/src/lib/checkbox/checkbox.spec.ts @@ -101,7 +101,7 @@ 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(); @@ -109,7 +109,7 @@ describe('MdCheckbox', () => { expect(inputElement.indeterminate).toBe(true); expect(testComponent.isIndeterminate).toBe(true); - testComponent.isChecked = true; + inputElement.click(); fixture.detectChanges(); fixture.whenStable().then(() => { @@ -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(() => { @@ -141,6 +141,38 @@ describe('MdCheckbox', () => { })); + it('should not set indeterminate to false when checked is set programmatically', async(() => { + 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(); + + fixture.whenStable().then(() => { + 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(); + + fixture.whenStable().then(() => { + 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); @@ -216,11 +248,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(() => { diff --git a/src/lib/checkbox/checkbox.ts b/src/lib/checkbox/checkbox.ts index e7a69a412169..92b8a30c8140 100644 --- a/src/lib/checkbox/checkbox.ts +++ b/src/lib/checkbox/checkbox.ts @@ -203,8 +203,7 @@ 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; @@ -212,12 +211,6 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro 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(); } @@ -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; @@ -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); + }); + } + this.toggle(); this._transitionCheckState( this._checked ? TransitionCheckState.Checked : TransitionCheckState.Unchecked); From 9861af9fe1cb48b554d864df9ce478c6a88b7436 Mon Sep 17 00:00:00 2001 From: Yuan Gao Date: Tue, 11 Apr 2017 17:16:42 -0700 Subject: [PATCH 2/2] update test --- src/lib/checkbox/checkbox.spec.ts | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/lib/checkbox/checkbox.spec.ts b/src/lib/checkbox/checkbox.spec.ts index 300b11de8f70..572286922478 100644 --- a/src/lib/checkbox/checkbox.spec.ts +++ b/src/lib/checkbox/checkbox.spec.ts @@ -152,25 +152,18 @@ describe('MdCheckbox', () => { testComponent.isChecked = true; fixture.detectChanges(); - fixture.whenStable().then(() => { - 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(true); + expect(inputElement.indeterminate).toBe(true); + expect(inputElement.checked).toBe(true); + expect(testComponent.isIndeterminate).toBe(true); - fixture.whenStable().then(() => { - fixture.detectChanges(); - expect(checkboxInstance.checked).toBe(false); - expect(inputElement.indeterminate).toBe(true); - expect(inputElement.checked).toBe(false); - 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', () => {