Skip to content

Commit

Permalink
fix(material/checkbox): native input not in sync if checked state is …
Browse files Browse the repository at this point in the history
…changed inside event

Fixes that the state of the native checkbox might be out of sync if the `checked` property is changed inside the `changed` output.

Fixes angular#22149.
  • Loading branch information
crisbeto committed Mar 23, 2021
1 parent 4316787 commit 3d7005b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 0 deletions.
13 changes: 13 additions & 0 deletions src/material-experimental/mdc-checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,19 @@ describe('MDC-based MatCheckbox', () => {
expect(testComponent.onCheckboxChange).not.toHaveBeenCalled();
}));

fit('should keep the view in sync if the `checked` value changes inside the `change` listener',
fakeAsync(() => {
spyOn(testComponent, 'onCheckboxChange').and.callFake(() => {
checkboxInstance.checked = false;
});

labelElement.click();
fixture.detectChanges();
flush();

expect(inputElement.checked).toBe(false);
}));

it('should forward the required attribute', fakeAsync(() => {
testComponent.isRequired = true;
fixture.detectChanges();
Expand Down
6 changes: 6 additions & 0 deletions src/material-experimental/mdc-checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,12 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements AfterViewInit,
newEvent.checked = this.checked;
this._cvaOnChange(this.checked);
this.change.next(newEvent);

// Assigning the value again here is redundant, but we have to do it in case it was
// changed inside the `change` listener which will cause the input to be out of sync.
if (this._nativeCheckbox) {
this._nativeCheckbox.nativeElement.checked = this.checked;
}
}

/** Gets the value for the `aria-checked` attribute of the native input. */
Expand Down
14 changes: 14 additions & 0 deletions src/material/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,20 @@ describe('MatCheckbox', () => {
expect(testComponent.onCheckboxChange).not.toHaveBeenCalled();
}));

it('should keep the view in sync if the `checked` value changes inside the `change` listener',
fakeAsync(() => {
spyOn(testComponent, 'onCheckboxChange').and.callFake(() => {
checkboxInstance.checked = false;
});

labelElement.click();
fixture.detectChanges();
flush();

expect(inputElement.checked).toBe(false);
expect(checkboxNativeElement.classList).not.toContain('mat-checkbox-checked');
}));

it('should forward the required attribute', () => {
testComponent.isRequired = true;
fixture.detectChanges();
Expand Down
6 changes: 6 additions & 0 deletions src/material/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,12 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc

this._controlValueAccessorChangeFn(this.checked);
this.change.emit(event);

// Assigning the value again here is redundant, but we have to do it in case it was
// changed inside the `change` listener which will cause the input to be out of sync.
if (this._inputElement) {
this._inputElement.nativeElement.checked = this.checked;
}
}

/** Toggles the `checked` state of the checkbox. */
Expand Down

0 comments on commit 3d7005b

Please sign in to comment.