From 652ed0cf6df6bd96808bbd6042c2b23a64a9b693 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Mon, 1 Jun 2015 10:41:50 -0700 Subject: [PATCH] feat(form): implemented an imperative way of updating the view by updating the value of a control --- .../directives/form_control_directive.ts | 21 ++-- .../forms/directives/form_model_directive.ts | 3 +- .../forms/directives/ng_model_directive.ts | 3 +- .../angular2/src/forms/directives/shared.ts | 5 + .../template_driven_form_directive.ts | 3 +- modules/angular2/src/forms/model.ts | 100 ++++++++---------- .../angular2/test/forms/directives_spec.ts | 6 +- .../angular2/test/forms/integration_spec.ts | 24 +++++ modules/angular2/test/forms/model_spec.ts | 55 +++++++++- 9 files changed, 145 insertions(+), 75 deletions(-) diff --git a/modules/angular2/src/forms/directives/form_control_directive.ts b/modules/angular2/src/forms/directives/form_control_directive.ts index 0bd1841a83..208ef0d5a1 100644 --- a/modules/angular2/src/forms/directives/form_control_directive.ts +++ b/modules/angular2/src/forms/directives/form_control_directive.ts @@ -1,4 +1,5 @@ import {CONST_EXPR} from 'angular2/src/facade/lang'; +import {StringMapWrapper} from 'angular2/src/facade/collection'; import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async'; import {Directive, Ancestor, onChange} from 'angular2/angular2'; @@ -53,20 +54,24 @@ const formControlBinding = export class FormControlDirective extends ControlDirective { control: Control; ngModel: EventEmitter; + _added: boolean; + model: any; constructor() { super(); this.ngModel = new EventEmitter(); + this._added = false; } - onChange(_) { - setUpControl(this.control, this); - this.control.updateValidity(); - } - - set model(value) { - this.control.updateValue(value); - this.valueAccessor.writeValue(value); + onChange(c) { + if (!this._added) { + setUpControl(this.control, this); + this.control.updateValidity(); + this._added = true; + } + if (StringMapWrapper.contains(c, "model")) { + this.control.updateValue(this.model); + } } viewToModelUpdate(newValue: any): void { ObservableWrapper.callNext(this.ngModel, newValue); } diff --git a/modules/angular2/src/forms/directives/form_model_directive.ts b/modules/angular2/src/forms/directives/form_model_directive.ts index 1413bf174e..ce6816294b 100644 --- a/modules/angular2/src/forms/directives/form_model_directive.ts +++ b/modules/angular2/src/forms/directives/form_model_directive.ts @@ -89,8 +89,7 @@ export class FormModelDirective extends ControlContainerDirective implements For updateModel(dir: ControlDirective, value: any): void { var c  = this.form.find(dir.path); - c.value = value; - dir.valueAccessor.writeValue(value); + c.updateValue(value); } _updateDomValue() { diff --git a/modules/angular2/src/forms/directives/ng_model_directive.ts b/modules/angular2/src/forms/directives/ng_model_directive.ts index c71d28409f..c6b9036ba0 100644 --- a/modules/angular2/src/forms/directives/ng_model_directive.ts +++ b/modules/angular2/src/forms/directives/ng_model_directive.ts @@ -40,8 +40,7 @@ export class NgModelDirective extends ControlDirective { }; if (StringMapWrapper.contains(c, "model")) { - this.control.value = this.model; - this.valueAccessor.writeValue(this.model); + this.control.updateValue(this.model); } } diff --git a/modules/angular2/src/forms/directives/shared.ts b/modules/angular2/src/forms/directives/shared.ts index 476255d790..7b24b8ce11 100644 --- a/modules/angular2/src/forms/directives/shared.ts +++ b/modules/angular2/src/forms/directives/shared.ts @@ -18,10 +18,15 @@ export function setUpControl(c: Control, dir: ControlDirective) { c.validator = Validators.compose([c.validator, dir.validator]); dir.valueAccessor.writeValue(c.value); + + // view -> model dir.valueAccessor.registerOnChange(newValue => { dir.viewToModelUpdate(newValue); c.updateValue(newValue); }); + + // model -> view + c.registerOnChange(newValue => dir.valueAccessor.writeValue(newValue)); } function _throwError(dir: ControlDirective, message: string): void { diff --git a/modules/angular2/src/forms/directives/template_driven_form_directive.ts b/modules/angular2/src/forms/directives/template_driven_form_directive.ts index c04826d323..b601fb6313 100644 --- a/modules/angular2/src/forms/directives/template_driven_form_directive.ts +++ b/modules/angular2/src/forms/directives/template_driven_form_directive.ts @@ -65,8 +65,7 @@ export class TemplateDrivenFormDirective extends ControlContainerDirective imple updateModel(dir: ControlDirective, value: any): void { this._later(_ => { var c = this.form.find(dir.path); - c.value = value; - dir.valueAccessor.writeValue(value); + c.updateValue(value); }); } diff --git a/modules/angular2/src/forms/model.ts b/modules/angular2/src/forms/model.ts index 5aa9ab9d5a..88cbd87506 100644 --- a/modules/angular2/src/forms/model.ts +++ b/modules/angular2/src/forms/model.ts @@ -42,8 +42,6 @@ export class AbstractControl { get value(): any { return this._value; } - set value(v) { this._value = v; } - get status(): string { return this._status; } get valid(): boolean { return this._status === VALID; } @@ -58,19 +56,36 @@ export class AbstractControl { setParent(parent) { this._parent = parent; } - _updateParent() { - if (isPresent(this._parent)) { - this._parent._updateValue(); + updateValidity({onlySelf}: {onlySelf?: boolean} = {}): void { + onlySelf = isPresent(onlySelf) ? onlySelf : false; + + this._errors = this.validator(this); + this._status = isPresent(this._errors) ? INVALID : VALID; + if (isPresent(this._parent) && !onlySelf) { + this._parent.updateValidity({onlySelf: onlySelf}); } } - updateValidity() { + updateValueAndValidity({onlySelf, emitEvent}: {onlySelf?: boolean, + emitEvent?: boolean} = {}): void { + onlySelf = isPresent(onlySelf) ? onlySelf : false; + emitEvent = isPresent(emitEvent) ? emitEvent : true; + + this._updateValue(); + this._pristine = false; + + if (emitEvent) { + ObservableWrapper.callNext(this._valueChanges, this._value); + } + this._errors = this.validator(this); this._status = isPresent(this._errors) ? INVALID : VALID; - if (isPresent(this._parent)) { - this._parent.updateValidity(); + if (isPresent(this._parent) && !onlySelf) { + this._parent.updateValueAndValidity({onlySelf: onlySelf, emitEvent: emitEvent}); } } + + _updateValue(): void {} } /** @@ -83,26 +98,23 @@ export class AbstractControl { * @exportedAs angular2/forms */ export class Control extends AbstractControl { + _onChange: Function; + constructor(value: any, validator: Function = Validators.nullValidator) { super(validator); - this._setValueErrorsStatus(value); + this._value = value; + this.updateValidity({onlySelf: true}); this._valueChanges = new EventEmitter(); } - updateValue(value: any): void { - this._setValueErrorsStatus(value); - this._pristine = false; - - ObservableWrapper.callNext(this._valueChanges, this._value); - - this._updateParent(); - } - - _setValueErrorsStatus(value) { + updateValue(value: any, + {onlySelf, emitEvent}: {onlySelf?: boolean, emitEvent?: boolean} = {}): void { this._value = value; - this._errors = this.validator(this); - this._status = isPresent(this._errors) ? INVALID : VALID; + if (isPresent(this._onChange)) this._onChange(this._value); + this.updateValueAndValidity({onlySelf: onlySelf, emitEvent: emitEvent}); } + + registerOnChange(fn: Function): void { this._onChange = fn; } } /** @@ -136,7 +148,8 @@ export class ControlGroup extends AbstractControl { this._valueChanges = new EventEmitter(); this._setParentForControls(); - this._setValueErrorsStatus(); + this._value = this._reduceValue(); + this.updateValidity({onlySelf: true}); } addControl(name: string, c: AbstractControl) { this.controls[name] = c; } @@ -145,12 +158,12 @@ export class ControlGroup extends AbstractControl { include(controlName: string): void { StringMapWrapper.set(this._optionals, controlName, true); - this._updateValue(); + this.updateValueAndValidity(); } exclude(controlName: string): void { StringMapWrapper.set(this._optionals, controlName, false); - this._updateValue(); + this.updateValueAndValidity(); } contains(controlName: string): boolean { @@ -174,20 +187,7 @@ export class ControlGroup extends AbstractControl { StringMapWrapper.forEach(this.controls, (control, name) => { control.setParent(this); }); } - _updateValue() { - this._setValueErrorsStatus(); - this._pristine = false; - - ObservableWrapper.callNext(this._valueChanges, this._value); - - this._updateParent(); - } - - _setValueErrorsStatus() { - this._value = this._reduceValue(); - this._errors = this.validator(this); - this._status = isPresent(this._errors) ? INVALID : VALID; - } + _updateValue() { this._value = this._reduceValue(); } _reduceValue() { return this._reduceChildren({}, (acc, control, name) => { @@ -239,7 +239,8 @@ export class ControlArray extends AbstractControl { this._valueChanges = new EventEmitter(); this._setParentForControls(); - this._setValueErrorsStatus(); + this._updateValue(); + this.updateValidity({onlySelf: true}); } at(index: number): AbstractControl { return this.controls[index]; } @@ -247,38 +248,25 @@ export class ControlArray extends AbstractControl { push(control: AbstractControl): void { ListWrapper.push(this.controls, control); control.setParent(this); - this._updateValue(); + this.updateValueAndValidity(); } insert(index: number, control: AbstractControl): void { ListWrapper.insert(this.controls, index, control); control.setParent(this); - this._updateValue(); + this.updateValueAndValidity(); } removeAt(index: number): void { ListWrapper.removeAt(this.controls, index); - this._updateValue(); + this.updateValueAndValidity(); } get length(): number { return this.controls.length; } - _updateValue() { - this._setValueErrorsStatus(); - this._pristine = false; - - ObservableWrapper.callNext(this._valueChanges, this._value); - - this._updateParent(); - } + _updateValue() { this._value = ListWrapper.map(this.controls, (c) => c.value); } _setParentForControls() { ListWrapper.forEach(this.controls, (control) => { control.setParent(this); }); } - - _setValueErrorsStatus() { - this._value = ListWrapper.map(this.controls, (c) => c.value); - this._errors = this.validator(this); - this._status = isPresent(this._errors) ? INVALID : VALID; - } } diff --git a/modules/angular2/test/forms/directives_spec.ts b/modules/angular2/test/forms/directives_spec.ts index f84347e473..98b77aa1e7 100644 --- a/modules/angular2/test/forms/directives_spec.ts +++ b/modules/angular2/test/forms/directives_spec.ts @@ -79,7 +79,7 @@ export function main() { }); it("should write value to the DOM", () => { - formModel.find(["login"]).value = "initValue"; + formModel.find(["login"]).updateValue("initValue"); form.addControl(loginControlDir); @@ -104,7 +104,7 @@ export function main() { it("should update dom values of all the directives", () => { form.addControl(loginControlDir); - formModel.find(["login"]).value = "new value"; + formModel.find(["login"]).updateValue("new value"); form.onChange(null); @@ -180,7 +180,7 @@ export function main() { expect(control.valid).toBe(true); // this will add the required validator and recalculate the validity - controlDir.onChange(null); + controlDir.onChange({}); expect(control.valid).toBe(false); }); diff --git a/modules/angular2/test/forms/integration_spec.ts b/modules/angular2/test/forms/integration_spec.ts index bd76722e1f..4798c79bdf 100644 --- a/modules/angular2/test/forms/integration_spec.ts +++ b/modules/angular2/test/forms/integration_spec.ts @@ -114,6 +114,30 @@ export function main() { }); })); + it("should update DOM elements when updating the value of a control", + inject([TestBed, AsyncTestCompleter], (tb, async) => { + var login = new Control("oldValue"); + var form = new ControlGroup({"login": login}); + var ctx = MyComp.create({form: form}); + + var t = `
+ +
`; + + tb.createView(MyComp, {context: ctx, html: t}) + .then((view) => { + view.detectChanges(); + + login.updateValue("newValue"); + + view.detectChanges(); + + var input = view.querySelector("input"); + expect(input.value).toEqual("newValue"); + async.done(); + }); + })); + describe("different control types", () => { it("should support ", inject([TestBed, AsyncTestCompleter], (tb, async) => { var ctx = MyComp.create({form: new ControlGroup({"text": new Control("old")})}); diff --git a/modules/angular2/test/forms/model_spec.ts b/modules/angular2/test/forms/model_spec.ts index 4279a53528..1bd89382b8 100644 --- a/modules/angular2/test/forms/model_spec.ts +++ b/modules/angular2/test/forms/model_spec.ts @@ -9,6 +9,9 @@ import { afterEach, el, AsyncTestCompleter, + fakeAsync, + flushMicrotasks, + tick, inject } from 'angular2/test_lib'; import {ControlGroup, Control, ControlArray, Validators} from 'angular2/forms'; @@ -62,6 +65,54 @@ export function main() { }); }); + describe("updateValue", () => { + var g, c; + beforeEach(() => { + c = new Control("oldValue"); + g = new ControlGroup({"one": c}); + }); + + it("should update the value of the control", () => { + c.updateValue("newValue"); + expect(c.value).toEqual("newValue"); + }); + + it("should invoke onChange if it is present", () => { + var onChange; + c.registerOnChange((v) => onChange = ["invoked", v]); + + c.updateValue("newValue"); + + expect(onChange).toEqual(["invoked", "newValue"]); + }); + + it("should update the parent", () => { + c.updateValue("newValue"); + expect(g.value).toEqual({"one": "newValue"}); + }); + + it("should not update the parent when explicitly specified", () => { + c.updateValue("newValue", {onlySelf: true}); + expect(g.value).toEqual({"one": "oldValue"}); + }); + + it("should fire an event", fakeAsync(() => { + ObservableWrapper.subscribe(c.valueChanges, + (value) => { expect(value).toEqual("newValue"); }); + + c.updateValue("newValue"); + tick(); + })); + + it("should not fire an event when explicitly specified", fakeAsync(() => { + ObservableWrapper.subscribe(c.valueChanges, (value) => { throw "Should not happen"; }); + + c.updateValue("newValue", {emitEvent: false}); + + tick(); + })); + }); + describe("valueChanges", () => { var c; @@ -302,8 +353,8 @@ export function main() { if (loggedValues.length == 2) { expect(loggedValues) - .toEqual([{"one": "new1", "two": "old2"}, {"one": "new1", "two": "new2"}]) - async.done(); + .toEqual([{"one": "new1", "two": "old2"}, {"one": "new1", "two": "new2"}]); + async.done(); } });