Skip to content

Commit

Permalink
fix(checkbox): error when disabling while focused
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
devversion committed Jul 25, 2018
1 parent 6aa1a55 commit 6439788
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 44 deletions.
95 changes: 52 additions & 43 deletions src/lib/checkbox/checkbox.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
import {ComponentFixture, fakeAsync, TestBed, tick, flush} from '@angular/core/testing';
import {
ComponentFixture,
fakeAsync,
TestBed,
tick,
flush,
flushMicrotasks,
} from '@angular/core/testing';
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
import {Component, DebugElement, ViewChild, Type} from '@angular/core';
import {By} from '@angular/platform-browser';
Expand Down Expand Up @@ -873,29 +880,63 @@ describe('MatCheckbox', () => {
let checkboxNativeElement: HTMLElement;
let checkboxInstance: MatCheckbox;
let inputElement: HTMLInputElement;
let ngModel: NgModel;

beforeEach(() => {
fixture = createComponent(CheckboxWithFormDirectives);
fixture = createComponent(CheckboxWithNgModel);

fixture.componentInstance.isRequired = false;
fixture.detectChanges();

checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
checkboxNativeElement = checkboxDebugElement.nativeElement;
checkboxInstance = checkboxDebugElement.componentInstance;
inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
ngModel = checkboxDebugElement.injector.get<NgModel>(NgModel);
});

it('should be in pristine, untouched, and valid states initially', fakeAsync(() => {
flush();

let checkboxElement = fixture.debugElement.query(By.directive(MatCheckbox));
let ngModel = checkboxElement.injector.get<NgModel>(NgModel);

it('should be pristine, untouched, and valid initially', () => {
expect(ngModel.valid).toBe(true);
expect(ngModel.pristine).toBe(true);
expect(ngModel.touched).toBe(false);
});

it('should have correct control states after interaction', fakeAsync(() => {
inputElement.click();
fixture.detectChanges();

// Flush the timeout that is being created whenever a `click` event has been fired by
// the underlying input.
flush();

// After the value change through interaction, the control should be dirty, but remain
// untouched as long as the focus is still on the underlying input.
expect(ngModel.pristine).toBe(false);
expect(ngModel.touched).toBe(false);

// TODO(jelbourn): test that `touched` and `pristine` state are modified appropriately.
// This is currently blocked on issues with async() and fakeAsync().
// If the input element loses focus, the control should remain dirty but should
// also turn touched.
dispatchFakeEvent(inputElement, 'blur');
fixture.detectChanges();
flushMicrotasks();

expect(ngModel.pristine).toBe(false);
expect(ngModel.touched).toBe(true);
}));

it('should not throw an error when disabling while focused', fakeAsync(() => {
expect(() => {
// Focus the input element because after disabling, the `blur` event should automatically
// fire and not result in a changed after checked exception. Related: #12323
inputElement.focus();

// Flush the two nested timeouts from the FocusMonitor that are being created on `focus`.
flush();

checkboxInstance.disabled = true;
fixture.detectChanges();
flushMicrotasks();
}).not.toThrow();
}));

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

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

describe('with required ngModel', () => {
let checkboxInstance: MatCheckbox;
let inputElement: HTMLInputElement;
let testComponent: CheckboxWithNgModel;

beforeEach(() => {
fixture = createComponent(CheckboxWithNgModel);
fixture.detectChanges();

let checkboxDebugElement = fixture.debugElement.query(By.directive(MatCheckbox));
let checkboxNativeElement = checkboxDebugElement.nativeElement;
testComponent = fixture.debugElement.componentInstance;
checkboxInstance = checkboxDebugElement.componentInstance;
inputElement = <HTMLInputElement>checkboxNativeElement.querySelector('input');
});

it('should validate with RequiredTrue validator', () => {
let checkboxElement = fixture.debugElement.query(By.directive(MatCheckbox));
let ngModel = checkboxElement.injector.get<NgModel>(NgModel);

testComponent.isRequired = true;
fixture.componentInstance.isRequired = true;
inputElement.click();
fixture.detectChanges();

Expand Down Expand Up @@ -1124,18 +1145,6 @@ class SingleCheckbox {
onCheckboxChange: (event?: MatCheckboxChange) => void = () => {};
}

/** Simple component for testing an MatCheckbox with ngModel in a form. */
@Component({
template: `
<form>
<mat-checkbox name="cb" [(ngModel)]="isGood">Be good</mat-checkbox>
</form>
`,
})
class CheckboxWithFormDirectives {
isGood: boolean = false;
}

/** Simple component for testing an MatCheckbox with required ngModel. */
@Component({
template: `<mat-checkbox [required]="isRequired" [(ngModel)]="isGood">Be good</mat-checkbox>`,
Expand Down
7 changes: 6 additions & 1 deletion src/lib/checkbox/checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,12 @@ export class MatCheckbox extends _MatCheckboxMixinBase implements ControlValueAc
this._focusRipple = null;
}

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

Expand Down

0 comments on commit 6439788

Please sign in to comment.