Skip to content

Commit

Permalink
fix(forms): do not reset the value of the input when it came from the…
Browse files Browse the repository at this point in the history
… view
  • Loading branch information
vsavkin committed Jul 16, 2015
1 parent b1df545 commit b123159
Show file tree
Hide file tree
Showing 11 changed files with 81 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export var uninitialized = new Object();

export class SimpleChange {
constructor(public previousValue: any, public currentValue: any) {}

isFirstChange(): boolean { return this.previousValue === uninitialized; }
}

var _simpleChangesIndex = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {setProperty} from './shared';
host: {
'(change)': 'onChange($event.target.checked)',
'(blur)': 'onTouched()',
'[checked]': 'checked',
'[class.ng-untouched]': 'ngClassUntouched',
'[class.ng-touched]': 'ngClassTouched',
'[class.ng-pristine]': 'ngClassPristine',
Expand All @@ -31,20 +30,14 @@ import {setProperty} from './shared';
}
})
export class CheckboxControlValueAccessor implements ControlValueAccessor {
checked: boolean;
onChange = (_) => {};
onTouched = () => {};

constructor(private cd: NgControl, private renderer: Renderer, private elementRef: ElementRef) {
cd.valueAccessor = this;
}

writeValue(value) {
// both this.checked and setProperty are required at the moment
// remove when a proper imperative API is provided
this.checked = value;
setProperty(this.renderer, this.elementRef, "checked", value);
}
writeValue(value) { setProperty(this.renderer, this.elementRef, "checked", value); }

get ngClassUntouched(): boolean {
return isPresent(this.cd.control) ? this.cd.control.untouched : false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {setProperty} from './shared';
'(change)': 'onChange($event.target.value)',
'(input)': 'onChange($event.target.value)',
'(blur)': 'onTouched()',
'[value]': 'value',
'[class.ng-untouched]': 'ngClassUntouched',
'[class.ng-touched]': 'ngClassTouched',
'[class.ng-pristine]': 'ngClassPristine',
Expand All @@ -32,8 +31,6 @@ import {setProperty} from './shared';
}
})
export class DefaultValueAccessor implements ControlValueAccessor {
value: string = null;

onChange = (_) => {};
onTouched = () => {};

Expand All @@ -44,8 +41,8 @@ export class DefaultValueAccessor implements ControlValueAccessor {
writeValue(value) {
// both this.value and setProperty are required at the moment
// remove when a proper imperative API is provided
this.value = isBlank(value) ? '' : value;
setProperty(this.renderer, this.elementRef, 'value', this.value);
var normalizedValue = isBlank(value) ? '' : value;
setProperty(this.renderer, this.elementRef, 'value', normalizedValue);
}

get ngClassUntouched(): boolean {
Expand Down
13 changes: 9 additions & 4 deletions modules/angular2/src/forms/directives/ng_control_name.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import {CONST_EXPR} from 'angular2/src/facade/lang';
import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async';
import {List, StringMapWrapper, StringMap} from 'angular2/src/facade/collection';
import {List, StringMap} from 'angular2/src/facade/collection';

import {Directive, LifecycleEvent, Query, QueryList} from 'angular2/angular2';
import {forwardRef, Ancestor, Binding, Inject} from 'angular2/di';

import {ControlContainer} from './control_container';
import {NgControl} from './ng_control';
import {NgValidator} from './validators';
import {controlPath, composeNgValidator} from './shared';
import {controlPath, composeNgValidator, isPropertyUpdated} from './shared';
import {Control} from '../model';

const controlNameBinding =
Expand Down Expand Up @@ -82,6 +82,7 @@ export class NgControlName extends NgControl {
_parent: ControlContainer;
update = new EventEmitter();
model: any;
viewModel: any;
ngValidators: QueryList<NgValidator>;
_added = false;

Expand All @@ -98,14 +99,18 @@ export class NgControlName extends NgControl {
this.formDirective.addControl(this);
this._added = true;
}
if (StringMapWrapper.contains(c, "model")) {
if (isPropertyUpdated(c, this.viewModel)) {
this.viewModel = this.model;
this.formDirective.updateModel(this, this.model);
}
}

onDestroy() { this.formDirective.removeControl(this); }

viewToModelUpdate(newValue: any): void { ObservableWrapper.callNext(this.update, newValue); }
viewToModelUpdate(newValue: any): void {
this.viewModel = newValue;
ObservableWrapper.callNext(this.update, newValue);
}

get path(): List<string> { return controlPath(this.name, this._parent); }

Expand Down
11 changes: 7 additions & 4 deletions modules/angular2/src/forms/directives/ng_form_control.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
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, LifecycleEvent, Query, QueryList} from 'angular2/angular2';
Expand All @@ -8,7 +7,7 @@ import {forwardRef, Ancestor, Binding} from 'angular2/di';
import {NgControl} from './ng_control';
import {Control} from '../model';
import {NgValidator} from './validators';
import {setUpControl, composeNgValidator} from './shared';
import {setUpControl, composeNgValidator, isPropertyUpdated} from './shared';

const formControlBinding =
CONST_EXPR(new Binding(NgControl, {toAlias: forwardRef(() => NgFormControl)}));
Expand Down Expand Up @@ -71,6 +70,7 @@ export class NgFormControl extends NgControl {
update = new EventEmitter();
_added = false;
model: any;
viewModel: any;
ngValidators: QueryList<NgValidator>;

// Scope the query once https://github.com/angular/angular/issues/2603 is fixed
Expand All @@ -85,7 +85,7 @@ export class NgFormControl extends NgControl {
this.form.updateValidity();
this._added = true;
}
if (StringMapWrapper.contains(c, "model")) {
if (isPropertyUpdated(c, this.viewModel)) {
this.form.updateValue(this.model);
}
}
Expand All @@ -96,5 +96,8 @@ export class NgFormControl extends NgControl {

get validator(): Function { return composeNgValidator(this.ngValidators); }

viewToModelUpdate(newValue: any): void { ObservableWrapper.callNext(this.update, newValue); }
viewToModelUpdate(newValue: any): void {
this.viewModel = newValue;
ObservableWrapper.callNext(this.update, newValue);
}
}
11 changes: 7 additions & 4 deletions modules/angular2/src/forms/directives/ng_model.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import {CONST_EXPR} from 'angular2/src/facade/lang';
import {EventEmitter, ObservableWrapper} from 'angular2/src/facade/async';
import {StringMapWrapper} from 'angular2/src/facade/collection';

import {Directive, LifecycleEvent, QueryList, Query} from 'angular2/angular2';
import {forwardRef, Ancestor, Binding} from 'angular2/di';

import {NgControl} from './ng_control';
import {Control} from '../model';
import {NgValidator} from './validators';
import {setUpControl, composeNgValidator} from './shared';
import {setUpControl, composeNgValidator, isPropertyUpdated} from './shared';

const formControlBinding = CONST_EXPR(new Binding(NgControl, {toAlias: forwardRef(() => NgModel)}));

Expand Down Expand Up @@ -41,6 +40,7 @@ export class NgModel extends NgControl {
_added = false;
update = new EventEmitter();
model: any;
viewModel: any;
ngValidators: QueryList<NgValidator>;

// Scope the query once https://github.com/angular/angular/issues/2603 is fixed
Expand All @@ -56,7 +56,7 @@ export class NgModel extends NgControl {
this._added = true;
}

if (StringMapWrapper.contains(c, "model")) {
if (isPropertyUpdated(c, this.viewModel)) {
this._control.updateValue(this.model);
}
}
Expand All @@ -67,5 +67,8 @@ export class NgModel extends NgControl {

get validator(): Function { return composeNgValidator(this.ngValidators); }

viewToModelUpdate(newValue: any): void { ObservableWrapper.callNext(this.update, newValue); }
viewToModelUpdate(newValue: any): void {
this.viewModel = newValue;
ObservableWrapper.callNext(this.update, newValue);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export class NgSelectOption {
'(change)': 'onChange($event.target.value)',
'(input)': 'onChange($event.target.value)',
'(blur)': 'onTouched()',
'[value]': 'value',
'[class.ng-untouched]': 'ngClassUntouched',
'[class.ng-touched]': 'ngClassTouched',
'[class.ng-pristine]': 'ngClassPristine',
Expand All @@ -39,7 +38,7 @@ export class NgSelectOption {
}
})
export class SelectControlValueAccessor implements ControlValueAccessor {
value = '';
value: string;
onChange = (_) => {};
onTouched = () => {};

Expand All @@ -51,8 +50,6 @@ export class SelectControlValueAccessor implements ControlValueAccessor {
}

writeValue(value) {
// both this.value and setProperty are required at the moment
// remove when a proper imperative API is provided
this.value = value;
setProperty(this.renderer, this.elementRef, "value", value);
}
Expand Down
14 changes: 11 additions & 3 deletions modules/angular2/src/forms/directives/shared.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {ListWrapper, iterableToList} from 'angular2/src/facade/collection';
import {isBlank, BaseException} from 'angular2/src/facade/lang';
import {ListWrapper, iterableToList, StringMapWrapper} from 'angular2/src/facade/collection';
import {isBlank, BaseException, looseIdentical} from 'angular2/src/facade/lang';

import {ControlContainer} from './control_container';
import {NgControl} from './ng_control';
Expand All @@ -26,7 +26,7 @@ export function setUpControl(c: Control, dir: NgControl) {
// view -> model
dir.valueAccessor.registerOnChange(newValue => {
dir.viewToModelUpdate(newValue);
c.updateValue(newValue);
c.updateValue(newValue, {emitModelToViewChange: false});
c.markAsDirty();
});

Expand All @@ -52,3 +52,11 @@ export function setProperty(renderer: Renderer, elementRef: ElementRef, propName
propValue: any) {
renderer.setElementProperty(elementRef, propName, propValue);
}

export function isPropertyUpdated(changes: StringMap<string, any>, viewModel: any): boolean {
if (!StringMapWrapper.contains(changes, "model")) return false;
var change = changes["model"];

if (change.isFirstChange()) return true;
return !looseIdentical(viewModel, change.currentValue);
}
7 changes: 5 additions & 2 deletions modules/angular2/src/forms/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,13 @@ export class Control extends AbstractControl {
this._valueChanges = new EventEmitter();
}

updateValue(value: any, {onlySelf, emitEvent}: {onlySelf?: boolean, emitEvent?: boolean} = {}):
updateValue(value: any,
{onlySelf, emitEvent, emitModelToViewChange}:
{onlySelf?: boolean, emitEvent?: boolean, emitModelToViewChange?: boolean} = {}):
void {
emitModelToViewChange = isPresent(emitModelToViewChange) ? emitModelToViewChange : true;
this._value = value;
if (isPresent(this._onChange)) this._onChange(this._value);
if (isPresent(this._onChange) && emitModelToViewChange) this._onChange(this._value);
this.updateValueAndValidity({onlySelf: onlySelf, emitEvent: emitEvent});
}

Expand Down
27 changes: 27 additions & 0 deletions modules/angular2/test/forms/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,33 @@ export function main() {
});
}));
});

describe("ng-model corner cases", () => {
it("should not update the view when the value initially came from the view",
inject([TestComponentBuilder], fakeAsync((tcb: TestComponentBuilder) => {
var form = new Control("");

var t =
`<div><input type="text" [ng-form-control]="form" [(ng-model)]="name"></div>`;
var rootTC;
tcb.overrideTemplate(MyComp, t).createAsync(MyComp).then(
(root) => { rootTC = root; });
tick();
rootTC.componentInstance.form = form;
rootTC.detectChanges();

var input = rootTC.query(By.css("input")).nativeElement;
input.value = "aa";
input.selectionStart = 1;
dispatchEvent(input, "change");

tick();
rootTC.detectChanges();

// selection start has not changed because we did not reset the value
expect(input.selectionStart).toEqual(1);
})));
});
});
}

Expand Down
9 changes: 9 additions & 0 deletions modules/angular2/test/forms/model_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,15 @@ export function main() {
expect(onChange).toEqual(["invoked", "newValue"]);
});

it("should not invoke on change when explicitly specified", () => {
var onChange = null;
c.registerOnChange((v) => onChange = ["invoked", v]);

c.updateValue("newValue", {emitModelToViewChange: false});

expect(onChange).toBeNull();
});

it("should update the parent", () => {
c.updateValue("newValue");
expect(g.value).toEqual({"one": "newValue"});
Expand Down

0 comments on commit b123159

Please sign in to comment.