Skip to content

Commit c4cacce

Browse files
devversionjelbourn
authored andcommitted
fix(checkbox): prevent error when disabling while focused (#12327)
* Fixes that Angular throws an ExpressionChangedAfterItHasBeenCheckedError when disabling the checkbox while the component has been focused. * Adds missing test for `NgModel` states after value change through view. Related #12323
1 parent a10bfa4 commit c4cacce

File tree

2 files changed

+58
-44
lines changed

2 files changed

+58
-44
lines changed

src/lib/checkbox/checkbox.spec.ts

Lines changed: 52 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
import {ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
1+
import {
2+
ComponentFixture,
3+
fakeAsync,
4+
TestBed,
5+
tick,
6+
flush,
7+
flushMicrotasks,
8+
} from '@angular/core/testing';
29
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
310
import {Component, DebugElement, ViewChild, Type} from '@angular/core';
411
import {By} from '@angular/platform-browser';
@@ -873,29 +880,63 @@ describe('MatCheckbox', () => {
873880
let checkboxNativeElement: HTMLElement;
874881
let checkboxInstance: MatCheckbox;
875882
let inputElement: HTMLInputElement;
883+
let ngModel: NgModel;
876884

877885
beforeEach(() => {
878-
fixture = createComponent(CheckboxWithFormDirectives);
886+
fixture = createComponent(CheckboxWithNgModel);
887+
888+
fixture.componentInstance.isRequired = false;
879889
fixture.detectChanges();
880890

881891
checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
882892
checkboxNativeElement = checkboxDebugElement.nativeElement;
883893
checkboxInstance = checkboxDebugElement.componentInstance;
884894
inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
895+
ngModel = checkboxDebugElement.injector.get<NgModel>(NgModel);
885896
});
886897

887-
it('should be in pristine, untouched, and valid states initially', fakeAsync(() => {
888-
flush();
889-
890-
let checkboxElement = fixture.debugElement.query(By.directive(MatCheckbox));
891-
let ngModel = checkboxElement.injector.get<NgModel>(NgModel);
892-
898+
it('should be pristine, untouched, and valid initially', () => {
893899
expect(ngModel.valid).toBe(true);
894900
expect(ngModel.pristine).toBe(true);
895901
expect(ngModel.touched).toBe(false);
902+
});
903+
904+
it('should have correct control states after interaction', fakeAsync(() => {
905+
inputElement.click();
906+
fixture.detectChanges();
907+
908+
// Flush the timeout that is being created whenever a `click` event has been fired by
909+
// the underlying input.
910+
flush();
911+
912+
// After the value change through interaction, the control should be dirty, but remain
913+
// untouched as long as the focus is still on the underlying input.
914+
expect(ngModel.pristine).toBe(false);
915+
expect(ngModel.touched).toBe(false);
896916

897-
// TODO(jelbourn): test that `touched` and `pristine` state are modified appropriately.
898-
// This is currently blocked on issues with async() and fakeAsync().
917+
// If the input element loses focus, the control should remain dirty but should
918+
// also turn touched.
919+
dispatchFakeEvent(inputElement, 'blur');
920+
fixture.detectChanges();
921+
flushMicrotasks();
922+
923+
expect(ngModel.pristine).toBe(false);
924+
expect(ngModel.touched).toBe(true);
925+
}));
926+
927+
it('should not throw an error when disabling while focused', fakeAsync(() => {
928+
expect(() => {
929+
// Focus the input element because after disabling, the `blur` event should automatically
930+
// fire and not result in a changed after checked exception. Related: #12323
931+
inputElement.focus();
932+
933+
// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
934+
flush();
935+
936+
checkboxInstance.disabled = true;
937+
fixture.detectChanges();
938+
flushMicrotasks();
939+
}).not.toThrow();
899940
}));
900941

901942
it('should toggle checked state on click', () => {
@@ -911,29 +952,9 @@ describe('MatCheckbox', () => {
911952

912953
expect(checkboxInstance.checked).toBe(false);
913954
});
914-
});
915-
916-
describe('with required ngModel', () => {
917-
let checkboxInstance: MatCheckbox;
918-
let inputElement: HTMLInputElement;
919-
let testComponent: CheckboxWithNgModel;
920-
921-
beforeEach(() => {
922-
fixture = createComponent(CheckboxWithNgModel);
923-
fixture.detectChanges();
924-
925-
let checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
926-
let checkboxNativeElement = checkboxDebugElement.nativeElement;
927-
testComponent = fixture.debugElement.componentInstance;
928-
checkboxInstance = checkboxDebugElement.componentInstance;
929-
inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
930-
});
931955

932956
it('should validate with RequiredTrue validator', () => {
933-
let checkboxElement = fixture.debugElement.query(By.directive(MatCheckbox));
934-
let ngModel = checkboxElement.injector.get<NgModel>(NgModel);
935-
936-
testComponent.isRequired = true;
957+
fixture.componentInstance.isRequired = true;
937958
inputElement.click();
938959
fixture.detectChanges();
939960

@@ -1124,18 +1145,6 @@ class SingleCheckbox {
11241145
onCheckboxChange: (event?: MatCheckboxChange) => void = () => {};
11251146
}
11261147

1127-
/** Simple component for testing an MatCheckbox with ngModel in a form. */
1128-
@Component({
1129-
template: `
1130-
<form>
1131-
<mat-checkbox name="cb" [(ngModel)]="isGood">Be good</mat-checkbox>
1132-
</form>
1133-
`,
1134-
})
1135-
class CheckboxWithFormDirectives {
1136-
isGood: boolean = false;
1137-
}
1138-
11391148
/** Simple component for testing an MatCheckbox with required ngModel. */
11401149
@Component({
11411150
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood">Be good</mat-checkbox>`,

src/lib/checkbox/checkbox.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,12 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
341341
this._focusRipple = null;
342342
}
343343

344-
this._onTouched();
344+
// When a focused element becomes disabled, the browser *immediately* fires a blur event.
345+
// Angular does not expect events to be raised during change detection, so any state change
346+
// (such as a form control's 'ng-touched') will cause a changed-after-checked error.
347+
// See https://github.com/angular/angular/issues/17793. To work around this, we defer telling
348+
// the form control it has been touched until the next tick.
349+
Promise.resolve().then(() => this._onTouched());
345350
}
346351
}
347352

0 commit comments

Comments
 (0)