From 515ff61fcb12972889861dfb47e02632bef083b7 Mon Sep 17 00:00:00 2001 From: Kara Date: Thu, 25 Aug 2016 14:37:57 -0700 Subject: [PATCH] fix(forms): fully support rebinding form group directive (#11051) --- .../form_group_directive.ts | 17 +- .../@angular/forms/src/directives/shared.ts | 10 ++ modules/@angular/forms/src/model.ts | 8 + .../forms/test/reactive_integration_spec.ts | 169 +++++++++++++++++- 4 files changed, 194 insertions(+), 10 deletions(-) diff --git a/modules/@angular/forms/src/directives/reactive_directives/form_group_directive.ts b/modules/@angular/forms/src/directives/reactive_directives/form_group_directive.ts index 1e2e054747..d5d69c5fb4 100644 --- a/modules/@angular/forms/src/directives/reactive_directives/form_group_directive.ts +++ b/modules/@angular/forms/src/directives/reactive_directives/form_group_directive.ts @@ -17,7 +17,7 @@ import {ControlContainer} from '../control_container'; import {Form} from '../form_interface'; import {NgControl} from '../ng_control'; import {ReactiveErrors} from '../reactive_errors'; -import {composeAsyncValidators, composeValidators, setUpControl, setUpFormContainer} from '../shared'; +import {cleanUpControl, composeAsyncValidators, composeValidators, setUpControl, setUpFormContainer} from '../shared'; import {FormArrayName, FormGroupName} from './form_group_name'; @@ -124,11 +124,9 @@ export class FormGroupDirective extends ControlContainer implements Form, var async = composeAsyncValidators(this._asyncValidators); this.form.asyncValidator = Validators.composeAsync([this.form.asyncValidator, async]); - this.form.updateValueAndValidity({onlySelf: true, emitEvent: false}); + this._updateDomValue(changes); } - - this._updateDomValue(); } get submitted(): boolean { return this._submitted; } @@ -189,10 +187,15 @@ export class FormGroupDirective extends ControlContainer implements Form, } /** @internal */ - _updateDomValue() { + _updateDomValue(changes: SimpleChanges) { + const oldForm = changes['form'].previousValue; this.directives.forEach(dir => { - var ctrl: any = this.form.get(dir.path); - dir.valueAccessor.writeValue(ctrl.value); + const newCtrl: any = this.form.get(dir.path); + const oldCtrl = oldForm.get(dir.path); + if (oldCtrl !== newCtrl) { + cleanUpControl(oldCtrl, dir); + if (newCtrl) setUpControl(newCtrl, dir); + } }); } diff --git a/modules/@angular/forms/src/directives/shared.ts b/modules/@angular/forms/src/directives/shared.ts index 2db9e34b6e..0ee8a9d7c9 100644 --- a/modules/@angular/forms/src/directives/shared.ts +++ b/modules/@angular/forms/src/directives/shared.ts @@ -67,6 +67,12 @@ export function setUpControl(control: FormControl, dir: NgControl): void { dir.valueAccessor.registerOnTouched(() => control.markAsTouched()); } +export function cleanUpControl(control: FormControl, dir: NgControl) { + dir.valueAccessor.registerOnChange(() => _noControlError(dir)); + dir.valueAccessor.registerOnTouched(() => _noControlError(dir)); + if (control) control._clearChangeFns(); +} + export function setUpFormContainer( control: FormGroup | FormArray, dir: AbstractFormGroupDirective | FormArrayName) { if (isBlank(control)) _throwError(dir, 'Cannot find control with'); @@ -74,6 +80,10 @@ export function setUpFormContainer( control.asyncValidator = Validators.composeAsync([control.asyncValidator, dir.asyncValidator]); } +function _noControlError(dir: NgControl) { + return _throwError(dir, 'There is no FormControl instance attached to form control element with'); +} + function _throwError(dir: AbstractControlDirective, message: string): void { let messageEnd: string; if (dir.path.length > 1) { diff --git a/modules/@angular/forms/src/model.ts b/modules/@angular/forms/src/model.ts index 6ac18f9f87..f744e51b94 100644 --- a/modules/@angular/forms/src/model.ts +++ b/modules/@angular/forms/src/model.ts @@ -519,6 +519,14 @@ export class FormControl extends AbstractControl { */ registerOnChange(fn: Function): void { this._onChange.push(fn); } + /** + * @internal + */ + _clearChangeFns(): void { + this._onChange = []; + this._onDisabledChange = null; + } + /** * Register a listener for disabled events. */ diff --git a/modules/@angular/forms/test/reactive_integration_spec.ts b/modules/@angular/forms/test/reactive_integration_spec.ts index 9a27d2330d..8c7033f37c 100644 --- a/modules/@angular/forms/test/reactive_integration_spec.ts +++ b/modules/@angular/forms/test/reactive_integration_spec.ts @@ -27,7 +27,8 @@ export function main() { FormControlComp, FormGroupComp, FormArrayComp, FormArrayNestedGroup, FormControlNameSelect, FormControlNumberInput, FormControlRadioButtons, WrappedValue, WrappedValueForm, MyInput, MyInputForm, FormGroupNgModel, FormControlNgModel, - LoginIsEmptyValidator, LoginIsEmptyWrapper, UniqLoginValidator, UniqLoginWrapper + LoginIsEmptyValidator, LoginIsEmptyWrapper, UniqLoginValidator, UniqLoginWrapper, + NestedFormGroupComp ] }); TestBed.compileComponents(); @@ -74,7 +75,11 @@ export function main() { expect(form.value).toEqual({'login': 'updatedValue'}); }); - it('should update DOM elements when rebinding the form group', () => { + }); + + describe('rebound form groups', () => { + + it('should update DOM elements initially', () => { const fixture = TestBed.createComponent(FormGroupComp); fixture.debugElement.componentInstance.form = new FormGroup({'login': new FormControl('oldValue')}); @@ -87,6 +92,148 @@ export function main() { const input = fixture.debugElement.query(By.css('input')); expect(input.nativeElement.value).toEqual('newValue'); }); + + it('should update model when UI changes', () => { + const fixture = TestBed.createComponent(FormGroupComp); + fixture.debugElement.componentInstance.form = + new FormGroup({'login': new FormControl('oldValue')}); + fixture.detectChanges(); + + const newForm = new FormGroup({'login': new FormControl('newValue')}); + fixture.debugElement.componentInstance.form = newForm; + fixture.detectChanges(); + + const input = fixture.debugElement.query(By.css('input')); + input.nativeElement.value = 'Nancy'; + dispatchEvent(input.nativeElement, 'input'); + fixture.detectChanges(); + + expect(newForm.value).toEqual({login: 'Nancy'}); + + newForm.setValue({login: 'Carson'}); + fixture.detectChanges(); + expect(input.nativeElement.value).toEqual('Carson'); + }); + + it('should work with radio buttons when reusing control', () => { + const fixture = TestBed.createComponent(FormControlRadioButtons); + const food = new FormControl('chicken'); + fixture.debugElement.componentInstance.form = + new FormGroup({'food': food, 'drink': new FormControl('')}); + fixture.detectChanges(); + + const newForm = new FormGroup({'food': food, 'drink': new FormControl('')}); + fixture.debugElement.componentInstance.form = newForm; + fixture.detectChanges(); + + newForm.setValue({food: 'fish', drink: ''}); + fixture.detectChanges(); + const inputs = fixture.debugElement.queryAll(By.css('input')); + expect(inputs[0].nativeElement.checked).toBe(false); + expect(inputs[1].nativeElement.checked).toBe(true); + }); + + it('should update nested form group model when UI changes', () => { + const fixture = TestBed.createComponent(NestedFormGroupComp); + fixture.debugElement.componentInstance.form = new FormGroup( + {'signin': new FormGroup({'login': new FormControl(), 'password': new FormControl()})}); + fixture.detectChanges(); + + const newForm = new FormGroup({ + 'signin': new FormGroup( + {'login': new FormControl('Nancy'), 'password': new FormControl('secret')}) + }); + fixture.debugElement.componentInstance.form = newForm; + fixture.detectChanges(); + + const inputs = fixture.debugElement.queryAll(By.css('input')); + expect(inputs[0].nativeElement.value).toEqual('Nancy'); + expect(inputs[1].nativeElement.value).toEqual('secret'); + + inputs[0].nativeElement.value = 'Carson'; + dispatchEvent(inputs[0].nativeElement, 'input'); + fixture.detectChanges(); + + expect(newForm.value).toEqual({signin: {login: 'Carson', password: 'secret'}}); + + newForm.setValue({signin: {login: 'Bess', password: 'otherpass'}}); + fixture.detectChanges(); + expect(inputs[0].nativeElement.value).toEqual('Bess'); + }); + + it('should pick up dir validators from nested form groups', () => { + const fixture = TestBed.createComponent(NestedFormGroupComp); + const form = new FormGroup({ + 'signin': + new FormGroup({'login': new FormControl(''), 'password': new FormControl('')}) + }); + fixture.debugElement.componentInstance.form = form; + fixture.detectChanges(); + expect(form.get('signin').valid).toBe(false); + + const newForm = new FormGroup({ + 'signin': + new FormGroup({'login': new FormControl(''), 'password': new FormControl('')}) + }); + fixture.debugElement.componentInstance.form = newForm; + fixture.detectChanges(); + + expect(form.get('signin').valid).toBe(false); + }); + + it('should strip named controls that are not found', () => { + const fixture = TestBed.createComponent(NestedFormGroupComp); + const form = new FormGroup({ + 'signin': + new FormGroup({'login': new FormControl(''), 'password': new FormControl('')}) + }); + fixture.debugElement.componentInstance.form = form; + fixture.detectChanges(); + + form.addControl('email', new FormControl('email')); + fixture.detectChanges(); + + let emailInput = fixture.debugElement.query(By.css('[formControlName="email"]')); + expect(emailInput.nativeElement.value).toEqual('email'); + + const newForm = new FormGroup({ + 'signin': + new FormGroup({'login': new FormControl(''), 'password': new FormControl('')}) + }); + fixture.debugElement.componentInstance.form = newForm; + fixture.detectChanges(); + + emailInput = fixture.debugElement.query(By.css('[formControlName="email"]')); + expect(emailInput).toBe(null); + }); + + it('should strip array controls that are not found', () => { + const fixture = TestBed.createComponent(FormArrayComp); + const cityArray = new FormArray([new FormControl('SF'), new FormControl('NY')]); + const form = new FormGroup({cities: cityArray}); + fixture.debugElement.componentInstance.form = form; + fixture.debugElement.componentInstance.cityArray = cityArray; + fixture.detectChanges(); + + let inputs = fixture.debugElement.queryAll(By.css('input')); + expect(inputs[2]).not.toBeDefined(); + cityArray.push(new FormControl('LA')); + fixture.detectChanges(); + + inputs = fixture.debugElement.queryAll(By.css('input')); + expect(inputs[2]).toBeDefined(); + + const newArr = new FormArray([new FormControl('SF'), new FormControl('NY')]); + const newForm = new FormGroup({cities: newArr}); + fixture.debugElement.componentInstance.form = newForm; + fixture.debugElement.componentInstance.cityArray = newArr; + fixture.detectChanges(); + + inputs = fixture.debugElement.queryAll(By.css('input')); + expect(inputs[2]).not.toBeDefined(); + }); + + }); describe('form arrays', () => { @@ -1075,7 +1222,7 @@ export function main() { TestBed.overrideComponent(FormGroupComp, { set: { template: ` -
+ hav
` @@ -1191,6 +1338,22 @@ class FormGroupComp { data: string; } +@Component({ + selector: 'nested-form-group-comp', + template: ` +
+
+ + +
+ +
+ ` +}) +class NestedFormGroupComp { + form: FormGroup; +} + @Component({ selector: 'form-control-number-input', template: `