From d29a9a99aa2f9054f2d57b962b6f5b0c4f658205 Mon Sep 17 00:00:00 2001 From: mikael Date: Mon, 28 Sep 2015 10:06:52 +1000 Subject: [PATCH] fix(forms): handle control change in NgFormControl when a new Control instance is bound to the directive, use the new instance, not the old one --- .../core/forms/directives/ng_form_control.ts | 7 ++--- .../src/core/forms/directives/shared.ts | 6 ++++ .../test/core/forms/directives_spec.ts | 30 ++++++++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/modules/angular2/src/core/forms/directives/ng_form_control.ts b/modules/angular2/src/core/forms/directives/ng_form_control.ts index 63fd201c4e..4c6fbe60a6 100644 --- a/modules/angular2/src/core/forms/directives/ng_form_control.ts +++ b/modules/angular2/src/core/forms/directives/ng_form_control.ts @@ -8,7 +8,7 @@ import {NgControl} from './ng_control'; import {Control} from '../model'; import {Validators, NG_VALIDATORS} from '../validators'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; -import {setUpControl, isPropertyUpdated, selectValueAccessor} from './shared'; +import {setUpControl, isPropertyUpdated, isControlChanged, selectValueAccessor} from './shared'; const formControlBinding = CONST_EXPR(new Provider(NgControl, {useExisting: forwardRef(() => NgFormControl)})); @@ -70,8 +70,6 @@ const formControlBinding = export class NgFormControl extends NgControl implements OnChanges { form: Control; update = new EventEmitter(); - /** @internal */ - _added = false; model: any; viewModel: any; validators: Function[]; @@ -84,10 +82,9 @@ export class NgFormControl extends NgControl implements OnChanges { } onChanges(changes: {[key: string]: SimpleChange}): void { - if (!this._added) { + if (isControlChanged(changes)) { setUpControl(this.form, this); this.form.updateValidity(); - this._added = true; } if (isPropertyUpdated(changes, this.viewModel)) { this.form.updateValue(this.model); diff --git a/modules/angular2/src/core/forms/directives/shared.ts b/modules/angular2/src/core/forms/directives/shared.ts index 127dea54d6..8cf4b73ab1 100644 --- a/modules/angular2/src/core/forms/directives/shared.ts +++ b/modules/angular2/src/core/forms/directives/shared.ts @@ -60,6 +60,12 @@ export function isPropertyUpdated(changes: {[key: string]: any}, viewModel: any) return !looseIdentical(viewModel, change.currentValue); } +export function isControlChanged(changes: {[key: string]: any}): boolean { + if (!StringMapWrapper.contains(changes, "form")) return false; + var change = changes["form"]; + return change.previousValue !== change.currentValue; +} + // TODO: vsavkin remove it once https://github.com/angular/angular/issues/3011 is implemented export function selectValueAccessor(dir: NgControl, valueAccessors: ControlValueAccessor[]): ControlValueAccessor { diff --git a/modules/angular2/test/core/forms/directives_spec.ts b/modules/angular2/test/core/forms/directives_spec.ts index e53d8e5551..520d616926 100644 --- a/modules/angular2/test/core/forms/directives_spec.ts +++ b/modules/angular2/test/core/forms/directives_spec.ts @@ -38,6 +38,7 @@ import { import {selectValueAccessor} from 'angular2/src/core/forms/directives/shared'; +import {SimpleChange} from 'angular2/src/core/change_detection'; class DummyControlValueAccessor implements ControlValueAccessor { writtenValue; @@ -275,6 +276,16 @@ export function main() { describe("NgFormControl", () => { var controlDir; var control; + var checkProperties = function(control) { + expect(controlDir.control).toBe(control); + expect(controlDir.value).toBe(control.value); + expect(controlDir.valid).toBe(control.valid); + expect(controlDir.errors).toBe(control.errors); + expect(controlDir.pristine).toBe(control.pristine); + expect(controlDir.dirty).toBe(control.dirty); + expect(controlDir.touched).toBe(control.touched); + expect(controlDir.untouched).toBe(control.untouched); + }; beforeEach(() => { controlDir = new NgFormControl([], [defaultAccessor]); @@ -284,15 +295,14 @@ export function main() { controlDir.form = control; }); - it("should reexport control properties", () => { - expect(controlDir.control).toBe(control); - expect(controlDir.value).toBe(control.value); - expect(controlDir.valid).toBe(control.valid); - expect(controlDir.errors).toBe(control.errors); - expect(controlDir.pristine).toBe(control.pristine); - expect(controlDir.dirty).toBe(control.dirty); - expect(controlDir.touched).toBe(control.touched); - expect(controlDir.untouched).toBe(control.untouched); + it("should reexport control properties", () => { checkProperties(control); }); + + it("should reexport new control properties", () => { + var newControl = new Control(null); + controlDir.form = newControl; + controlDir.onChanges({"form": new SimpleChange(control, newControl)}); + + checkProperties(newControl); }); it("should set up validator", () => { @@ -301,7 +311,7 @@ export function main() { expect(control.valid).toBe(true); // this will add the required validator and recalculate the validity - controlDir.onChanges({}); + controlDir.onChanges({"form": new SimpleChange(null, control)}); expect(control.valid).toBe(false); });