From 6edf0474cc34a85f04b642f178861229a66ff7d8 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Thu, 23 Jun 2016 09:55:26 -0700 Subject: [PATCH] feat(forms): add support for standalone ngModel dirs inside forms Closes #9230 --- .../forms/src/directives/form_interface.ts | 2 +- .../@angular/forms/src/directives/ng_form.ts | 7 +-- .../@angular/forms/src/directives/ng_model.ts | 34 ++++++++------ .../form_group_directive.ts | 3 +- .../@angular/forms/test/directives_spec.ts | 2 +- .../@angular/forms/test/integration_spec.ts | 45 ++++++++++++++++++- 6 files changed, 68 insertions(+), 25 deletions(-) diff --git a/modules/@angular/forms/src/directives/form_interface.ts b/modules/@angular/forms/src/directives/form_interface.ts index f59972e501..662120a488 100644 --- a/modules/@angular/forms/src/directives/form_interface.ts +++ b/modules/@angular/forms/src/directives/form_interface.ts @@ -24,7 +24,7 @@ export interface Form { /** * Add a control to this form. */ - addControl(dir: NgControl): FormControl; + addControl(dir: NgControl): void; /** * Remove a control from this form. diff --git a/modules/@angular/forms/src/directives/ng_form.ts b/modules/@angular/forms/src/directives/ng_form.ts index 7e864bf010..afa025285b 100644 --- a/modules/@angular/forms/src/directives/ng_form.ts +++ b/modules/@angular/forms/src/directives/ng_form.ts @@ -116,16 +116,13 @@ export class NgForm extends ControlContainer implements Form { get controls(): {[key: string]: AbstractControl} { return this.form.controls; } - addControl(dir: NgModel): FormControl { - const ctrl = new FormControl(); + addControl(dir: NgModel): void { PromiseWrapper.scheduleMicrotask(() => { const container = this._findContainer(dir.path); - dir._control = container.registerControl(dir.name, ctrl); + dir._control = container.registerControl(dir.name, dir.control); setUpControl(dir.control, dir); dir.control.updateValueAndValidity({emitEvent: false}); }); - - return ctrl; } getControl(dir: NgModel): FormControl { return this.form.find(dir.path); } diff --git a/modules/@angular/forms/src/directives/ng_model.ts b/modules/@angular/forms/src/directives/ng_model.ts index 6cfdc2bb9d..e11dbba4f7 100644 --- a/modules/@angular/forms/src/directives/ng_model.ts +++ b/modules/@angular/forms/src/directives/ng_model.ts @@ -55,14 +55,14 @@ export const formControlBinding: any = export class NgModel extends NgControl implements OnChanges, OnDestroy { /** @internal */ - _control: FormControl; + _control = new FormControl(); /** @internal */ - _added = false; + _registered = false; viewModel: any; @Input('ngModel') model: any; @Input() name: string; - @Input('ngModelOptions') options: {name?: string}; + @Input('ngModelOptions') options: {name?: string, standalone?: boolean}; @Output('ngModelChange') update = new EventEmitter(); constructor(@Optional() @Host() private _parent: ControlContainer, @@ -72,12 +72,11 @@ export class NgModel extends NgControl implements OnChanges, valueAccessors: ControlValueAccessor[]) { super(); this.valueAccessor = selectValueAccessor(this, valueAccessors); - if (!this._parent) this._control = new FormControl(); } ngOnChanges(changes: SimpleChanges) { this._checkName(); - if (!this._added) this._addControl(); + if (!this._registered) this._setUpControl(); if (isPropertyUpdated(changes, this.viewModel)) { this._control.updateValue(this.model); @@ -106,25 +105,32 @@ export class NgModel extends NgControl implements OnChanges, ObservableWrapper.callEmit(this.update, newValue); } - private _addControl(): void { - this._control = this.formDirective ? this.formDirective.addControl(this) : - this._addStandaloneControl(); - this._added = true; + private _setUpControl(): void { + this._isStandalone() ? this._setUpStandalone() : + this.formDirective.addControl(this); + this._registered = true; } - private _addStandaloneControl(): FormControl { + private _isStandalone(): boolean { + return !this._parent || (this.options && this.options.standalone); + } + + private _setUpStandalone(): void { setUpControl(this._control, this); this._control.updateValueAndValidity({emitEvent: false}); - return this._control; } private _checkName() { if (this.options && this.options.name) this.name = this.options.name; - if (this._parent && !this.name) { + if (!this._isStandalone() && !this.name) { throw new BaseException( - `Name attribute must be set if ngModel is used within a form. - Example: `); + `If ngModel is used within a form tag, either the name attribute must be set + or the form control must be defined as 'standalone' in ngModelOptions. + + Example 1: + Example 2: + `); } } } 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 95f359e23b..a22284dcac 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 @@ -144,12 +144,11 @@ export class FormGroupDirective extends ControlContainer implements Form, get path(): string[] { return []; } - addControl(dir: NgControl): FormControl { + addControl(dir: NgControl): void { const ctrl: any = this.form.find(dir.path); setUpControl(ctrl, dir); ctrl.updateValueAndValidity({emitEvent: false}); this.directives.push(dir); - return ctrl; } getControl(dir: NgControl): FormControl { return this.form.find(dir.path); } diff --git a/modules/@angular/forms/test/directives_spec.ts b/modules/@angular/forms/test/directives_spec.ts index 2454c1cb47..5d70b81e6b 100644 --- a/modules/@angular/forms/test/directives_spec.ts +++ b/modules/@angular/forms/test/directives_spec.ts @@ -281,7 +281,7 @@ export function main() { personControlGroupDir = new NgModelGroup(form, [], []); personControlGroupDir.name = 'person'; - loginControlDir = new FormControlName(personControlGroupDir, null, null, [defaultAccessor]); + loginControlDir = new NgModel(personControlGroupDir, null, null, [defaultAccessor]); loginControlDir.name = 'login'; loginControlDir.valueAccessor = new DummyControlValueAccessor(); }); diff --git a/modules/@angular/forms/test/integration_spec.ts b/modules/@angular/forms/test/integration_spec.ts index 2cf928dffb..9d72aa4af2 100644 --- a/modules/@angular/forms/test/integration_spec.ts +++ b/modules/@angular/forms/test/integration_spec.ts @@ -1371,7 +1371,7 @@ export function main() { }))); - it('should throw if ngModel has a parent form but no name attr', + it('should throw if ngModel has a parent form but no name attr or standalone label', inject( [TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { @@ -1384,11 +1384,29 @@ export function main() { .createAsync(MyComp8) .then((fixture) => { expect(() => fixture.detectChanges()) - .toThrowError(new RegExp(`Name attribute must be set`)); + .toThrowError(new RegExp(`name attribute must be set`)); async.done(); }); })); + it('should not throw if ngModel has a parent form, no name attr, and a standalone label', + inject( + [TestComponentBuilder, AsyncTestCompleter], + (tcb: TestComponentBuilder, async: AsyncTestCompleter) => { + const t = `
+ +
`; + + tcb.overrideTemplate(MyComp8, t) + .overrideProviders(MyComp8, providerArr) + .createAsync(MyComp8) + .then((fixture) => { + expect(() => fixture.detectChanges()).not.toThrow(); + async.done(); + }); + })); + + it('should override name attribute with ngModelOptions name if provided', fakeAsync(inject([TestComponentBuilder], (tcb: TestComponentBuilder) => { const t = ` @@ -1407,6 +1425,29 @@ export function main() { expect(form.value).toEqual({two: 'some data'}); }))); + it('should not register standalone ngModels with parent form', + fakeAsync(inject([TestComponentBuilder], (tcb: TestComponentBuilder) => { + const t = ` +
+ + +
+ `; + + const fixture = tcb.overrideTemplate(MyComp8, t).createFakeAsync(MyComp8); + tick(); + fixture.debugElement.componentInstance.data = 'some data'; + fixture.debugElement.componentInstance.list = 'should not show'; + fixture.detectChanges(); + const form = fixture.debugElement.children[0].injector.get(NgForm); + const inputs = fixture.debugElement.queryAll(By.css('input')); + + tick(); + expect(form.value).toEqual({one: 'some data'}); + expect(inputs[1].nativeElement.value).toEqual('should not show'); + }))); + + it('should support ', fakeAsync(inject([TestComponentBuilder], (tcb: TestComponentBuilder) => { const t = `