From 673de004d2fe054a96c02eafcf2e6836bf5dc117 Mon Sep 17 00:00:00 2001 From: Kara Date: Fri, 9 Sep 2016 12:00:38 -0700 Subject: [PATCH] fix(forms): clear errors on disable (#11463) Closes #11287 --- modules/@angular/forms/src/model.ts | 23 +++--- .../@angular/forms/test/form_array_spec.ts | 71 ++++++++++++++++-- .../@angular/forms/test/form_control_spec.ts | 51 +++++++++++-- .../@angular/forms/test/form_group_spec.ts | 73 +++++++++++++++++-- 4 files changed, 188 insertions(+), 30 deletions(-) diff --git a/modules/@angular/forms/src/model.ts b/modules/@angular/forms/src/model.ts index 49c15edcdc02d..c2ab359d6527c 100644 --- a/modules/@angular/forms/src/model.ts +++ b/modules/@angular/forms/src/model.ts @@ -190,6 +190,7 @@ export abstract class AbstractControl { emitEvent = isPresent(emitEvent) ? emitEvent : true; this._status = DISABLED; + this._errors = null; this._forEachChild((control: AbstractControl) => { control.disable({onlySelf: true}); }); this._updateValue(); @@ -232,17 +233,16 @@ export abstract class AbstractControl { onlySelf = normalizeBool(onlySelf); emitEvent = isPresent(emitEvent) ? emitEvent : true; + this._setInitialStatus(); this._updateValue(); - this._errors = this._runValidator(); - const originalStatus = this._status; - this._status = this._calculateStatus(); - if (this._status == VALID || this._status == PENDING) { - this._runAsyncValidator(emitEvent); - } + if (this.enabled) { + this._errors = this._runValidator(); + this._status = this._calculateStatus(); - if (this._disabledChanged(originalStatus)) { - this._updateValue(); + if (this._status === VALID || this._status === PENDING) { + this._runAsyncValidator(emitEvent); + } } if (emitEvent) { @@ -261,6 +261,8 @@ export abstract class AbstractControl { this.updateValueAndValidity({onlySelf: true, emitEvent}); } + private _setInitialStatus() { this._status = this._allControlsDisabled() ? DISABLED : VALID; } + private _runValidator(): {[key: string]: any} { return isPresent(this.validator) ? this.validator(this) : null; } @@ -281,11 +283,6 @@ export abstract class AbstractControl { } } - private _disabledChanged(originalStatus: string): boolean { - return this._status !== originalStatus && - (this._status === DISABLED || originalStatus === DISABLED); - } - /** * Sets errors on a form control. * diff --git a/modules/@angular/forms/test/form_array_spec.ts b/modules/@angular/forms/test/form_array_spec.ts index d4ed1abc7d254..c5fadaa8054ab 100644 --- a/modules/@angular/forms/test/form_array_spec.ts +++ b/modules/@angular/forms/test/form_array_spec.ts @@ -14,13 +14,11 @@ import {isPresent} from '../src/facade/lang'; import {Validators} from '../src/validators'; export function main() { - function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) { - return (c: any /** TODO #9100 */) => { + function asyncValidator(expected: string, timeouts = {}) { + return (c: AbstractControl) => { var resolve: (result: any) => void; var promise = new Promise(res => { resolve = res; }); - var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ? - (timeouts as any /** TODO #9100 */)[c.value] : - 0; + var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0; var res = c.value != expected ? {'async': true} : null; if (t == 0) { @@ -799,6 +797,69 @@ export function main() { expect(arr.status).toEqual('VALID'); }); + it('should not run validators on disabled controls', () => { + const validator = jasmine.createSpy('validator'); + const arr = new FormArray([new FormControl()], validator); + expect(validator.calls.count()).toEqual(1); + + arr.disable(); + expect(validator.calls.count()).toEqual(1); + + arr.setValue(['value']); + expect(validator.calls.count()).toEqual(1); + + arr.enable(); + expect(validator.calls.count()).toEqual(2); + }); + + describe('disabled errors', () => { + it('should clear out array errors when disabled', () => { + const arr = new FormArray([new FormControl()], () => { return {'expected': true}; }); + expect(arr.errors).toEqual({'expected': true}); + + arr.disable(); + expect(arr.errors).toEqual(null); + + arr.enable(); + expect(arr.errors).toEqual({'expected': true}); + }); + + it('should re-populate array errors when enabled from a child', () => { + const arr = new FormArray([new FormControl()], () => { return {'expected': true}; }); + arr.disable(); + expect(arr.errors).toEqual(null); + + arr.push(new FormControl()); + expect(arr.errors).toEqual({'expected': true}); + }); + + it('should clear out async array errors when disabled', fakeAsync(() => { + const arr = new FormArray([new FormControl()], null, asyncValidator('expected')); + tick(); + expect(arr.errors).toEqual({'async': true}); + + arr.disable(); + expect(arr.errors).toEqual(null); + + arr.enable(); + tick(); + expect(arr.errors).toEqual({'async': true}); + })); + + it('should re-populate async array errors when enabled from a child', fakeAsync(() => { + const arr = new FormArray([new FormControl()], null, asyncValidator('expected')); + tick(); + expect(arr.errors).toEqual({'async': true}); + + arr.disable(); + expect(arr.errors).toEqual(null); + + arr.push(new FormControl()); + tick(); + expect(arr.errors).toEqual({'async': true}); + })); + }); + describe('disabled events', () => { let logger: string[]; let c: FormControl; diff --git a/modules/@angular/forms/test/form_control_spec.ts b/modules/@angular/forms/test/form_control_spec.ts index bbd735287ad82..2c3471a03b74e 100644 --- a/modules/@angular/forms/test/form_control_spec.ts +++ b/modules/@angular/forms/test/form_control_spec.ts @@ -15,13 +15,11 @@ import {isPresent} from '../src/facade/lang'; import {FormArray} from '../src/model'; export function main() { - function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) { - return (c: any /** TODO #9100 */) => { + function asyncValidator(expected: string, timeouts = {}) { + return (c: FormControl) => { var resolve: (result: any) => void; var promise = new Promise(res => { resolve = res; }); - var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ? - (timeouts as any /** TODO #9100 */)[c.value] : - 0; + var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0; var res = c.value != expected ? {'async': true} : null; if (t == 0) { @@ -573,7 +571,7 @@ export function main() { }); describe('valueChanges & statusChanges', () => { - var c: any /** TODO #9100 */; + let c: FormControl; beforeEach(() => { c = new FormControl('old', Validators.required); }); @@ -892,6 +890,47 @@ export function main() { expect(g.touched).toBe(true); }); + it('should not run validators on disabled controls', () => { + const validator = jasmine.createSpy('validator'); + const c = new FormControl('', validator); + expect(validator.calls.count()).toEqual(1); + + c.disable(); + expect(validator.calls.count()).toEqual(1); + + c.setValue('value'); + expect(validator.calls.count()).toEqual(1); + + c.enable(); + expect(validator.calls.count()).toEqual(2); + }); + + describe('disabled errors', () => { + it('should clear out the errors when disabled', () => { + const c = new FormControl('', Validators.required); + expect(c.errors).toEqual({required: true}); + + c.disable(); + expect(c.errors).toEqual(null); + + c.enable(); + expect(c.errors).toEqual({required: true}); + }); + + it('should clear out async errors when disabled', fakeAsync(() => { + const c = new FormControl('', null, asyncValidator('expected')); + tick(); + expect(c.errors).toEqual({'async': true}); + + c.disable(); + expect(c.errors).toEqual(null); + + c.enable(); + tick(); + expect(c.errors).toEqual({'async': true}); + })); + }); + describe('disabled events', () => { let logger: string[]; let c: FormControl; diff --git a/modules/@angular/forms/test/form_group_spec.ts b/modules/@angular/forms/test/form_group_spec.ts index bddee4e7f8097..3e3e2d32d66e0 100644 --- a/modules/@angular/forms/test/form_group_spec.ts +++ b/modules/@angular/forms/test/form_group_spec.ts @@ -8,19 +8,17 @@ import {async, fakeAsync, tick} from '@angular/core/testing'; import {AsyncTestCompleter, beforeEach, ddescribe, describe, iit, inject, it, xit} from '@angular/core/testing/testing_internal'; -import {FormControl, FormGroup, Validators} from '@angular/forms'; +import {AbstractControl, FormControl, FormGroup, Validators} from '@angular/forms'; import {EventEmitter} from '../src/facade/async'; import {isPresent} from '../src/facade/lang'; export function main() { - function asyncValidator(expected: any /** TODO #9100 */, timeouts = {}) { - return (c: any /** TODO #9100 */) => { + function asyncValidator(expected: string, timeouts = {}) { + return (c: AbstractControl) => { var resolve: (result: any) => void; var promise = new Promise(res => { resolve = res; }); - var t = isPresent((timeouts as any /** TODO #9100 */)[c.value]) ? - (timeouts as any /** TODO #9100 */)[c.value] : - 0; + var t = isPresent((timeouts as any)[c.value]) ? (timeouts as any)[c.value] : 0; var res = c.value != expected ? {'async': true} : null; if (t == 0) { @@ -801,6 +799,69 @@ export function main() { expect(group.status).toEqual('VALID'); }); + it('should not run validators on disabled controls', () => { + const validator = jasmine.createSpy('validator'); + const g = new FormGroup({'one': new FormControl()}, validator); + expect(validator.calls.count()).toEqual(1); + + g.disable(); + expect(validator.calls.count()).toEqual(1); + + g.setValue({one: 'value'}); + expect(validator.calls.count()).toEqual(1); + + g.enable(); + expect(validator.calls.count()).toEqual(2); + }); + + describe('disabled errors', () => { + it('should clear out group errors when disabled', () => { + const g = new FormGroup({'one': new FormControl()}, () => { return {'expected': true}; }); + expect(g.errors).toEqual({'expected': true}); + + g.disable(); + expect(g.errors).toEqual(null); + + g.enable(); + expect(g.errors).toEqual({'expected': true}); + }); + + it('should re-populate group errors when enabled from a child', () => { + const g = new FormGroup({'one': new FormControl()}, () => { return {'expected': true}; }); + g.disable(); + expect(g.errors).toEqual(null); + + g.addControl('two', new FormControl()); + expect(g.errors).toEqual({'expected': true}); + }); + + it('should clear out async group errors when disabled', fakeAsync(() => { + const g = new FormGroup({'one': new FormControl()}, null, asyncValidator('expected')); + tick(); + expect(g.errors).toEqual({'async': true}); + + g.disable(); + expect(g.errors).toEqual(null); + + g.enable(); + tick(); + expect(g.errors).toEqual({'async': true}); + })); + + it('should re-populate async group errors when enabled from a child', fakeAsync(() => { + const g = new FormGroup({'one': new FormControl()}, null, asyncValidator('expected')); + tick(); + expect(g.errors).toEqual({'async': true}); + + g.disable(); + expect(g.errors).toEqual(null); + + g.addControl('two', new FormControl()); + tick(); + expect(g.errors).toEqual({'async': true}); + })); + }); + describe('disabled events', () => { let logger: string[]; let c: FormControl;