Skip to content

Commit de8b97f

Browse files
tinayuangaojelbourn
authored andcommitted
fix(checkbox): do not set indeterminate when set checked programmatically (#4024)
Only set indeterminate when user clicked on checkbox
1 parent 9f2c7ac commit de8b97f

File tree

2 files changed

+41
-18
lines changed

2 files changed

+41
-18
lines changed

src/lib/checkbox/checkbox.spec.ts

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -101,15 +101,15 @@ describe('MdCheckbox', () => {
101101
expect(inputElement.indeterminate).toBe(false);
102102
});
103103

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

108108
expect(checkboxInstance.indeterminate).toBe(true);
109109
expect(inputElement.indeterminate).toBe(true);
110110
expect(testComponent.isIndeterminate).toBe(true);
111111

112-
testComponent.isChecked = true;
112+
inputElement.click();
113113
fixture.detectChanges();
114114

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

130-
testComponent.isChecked = false;
130+
inputElement.click();
131131
fixture.detectChanges();
132132

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

142142
}));
143143

144+
it('should not set indeterminate to false when checked is set programmatically', async(() => {
145+
testComponent.isIndeterminate = true;
146+
fixture.detectChanges();
147+
148+
expect(checkboxInstance.indeterminate).toBe(true);
149+
expect(inputElement.indeterminate).toBe(true);
150+
expect(testComponent.isIndeterminate).toBe(true);
151+
152+
testComponent.isChecked = true;
153+
fixture.detectChanges();
154+
155+
expect(checkboxInstance.checked).toBe(true);
156+
expect(inputElement.indeterminate).toBe(true);
157+
expect(inputElement.checked).toBe(true);
158+
expect(testComponent.isIndeterminate).toBe(true);
159+
160+
testComponent.isChecked = false;
161+
fixture.detectChanges();
162+
163+
expect(checkboxInstance.checked).toBe(false);
164+
expect(inputElement.indeterminate).toBe(true);
165+
expect(inputElement.checked).toBe(false);
166+
expect(testComponent.isIndeterminate).toBe(true);
167+
}));
168+
144169
it('should change native element checked when check programmatically', () => {
145170
expect(inputElement.checked).toBe(false);
146171

@@ -216,11 +241,11 @@ describe('MdCheckbox', () => {
216241
expect(checkboxInstance.checked).toBe(false);
217242
});
218243

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

223-
testComponent.isChecked = true;
248+
inputElement.click();
224249
fixture.detectChanges();
225250

226251
fixture.whenStable().then(() => {

src/lib/checkbox/checkbox.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -203,21 +203,14 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
203203
}
204204

205205
/**
206-
* Whether the checkbox is checked. Note that setting `checked` will immediately set
207-
* `indeterminate` to false.
206+
* Whether the checkbox is checked.
208207
*/
209208
@Input() get checked() {
210209
return this._checked;
211210
}
212211

213212
set checked(checked: boolean) {
214213
if (checked != this.checked) {
215-
if (this._indeterminate) {
216-
Promise.resolve().then(() => {
217-
this._indeterminate = false;
218-
this.indeterminateChange.emit(this._indeterminate);
219-
});
220-
}
221214
this._checked = checked;
222215
this._changeDetectorRef.markForCheck();
223216
}
@@ -226,11 +219,8 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
226219
/**
227220
* Whether the checkbox is indeterminate. This is also known as "mixed" mode and can be used to
228221
* represent a checkbox with three states, e.g. a checkbox that represents a nested list of
229-
* checkable items. Note that whenever `checked` is set, indeterminate is immediately set to
230-
* false. This differs from the web platform in that indeterminate state on native
231-
* checkboxes is only remove when the user manually checks the checkbox (rather than setting the
232-
* `checked` property programmatically). However, we feel that this behavior is more accommodating
233-
* to the way consumers would envision using this component.
222+
* checkable items. Note that whenever checkbox is manually clicked, indeterminate is immediately
223+
* set to false.
234224
*/
235225
@Input() get indeterminate() {
236226
return this._indeterminate;
@@ -372,6 +362,14 @@ export class MdCheckbox implements ControlValueAccessor, AfterViewInit, OnDestro
372362
this._removeFocusRipple();
373363

374364
if (!this.disabled) {
365+
// When user manually click on the checkbox, `indeterminate` is set to false.
366+
if (this._indeterminate) {
367+
Promise.resolve().then(() => {
368+
this._indeterminate = false;
369+
this.indeterminateChange.emit(this._indeterminate);
370+
});
371+
}
372+
375373
this.toggle();
376374
this._transitionCheckState(
377375
this._checked ? TransitionCheckState.Checked : TransitionCheckState.Unchecked);

0 commit comments

Comments
 (0)