From 11e4385173c43ccd9d0b3000a326b16f7e8a687b Mon Sep 17 00:00:00 2001 From: vsavkin Date: Mon, 11 May 2015 12:00:56 -0700 Subject: [PATCH] feat(forms): improved error messages Closes #1839 --- modules/angular2/src/forms/directives.js | 63 +++++++++---------- modules/angular2/src/forms/model.js | 5 ++ .../angular2/test/forms/directives_spec.js | 26 ++++++++ 3 files changed, 62 insertions(+), 32 deletions(-) create mode 100644 modules/angular2/test/forms/directives_spec.js diff --git a/modules/angular2/src/forms/directives.js b/modules/angular2/src/forms/directives.js index 429297fae5..97df5a9de0 100644 --- a/modules/angular2/src/forms/directives.js +++ b/modules/angular2/src/forms/directives.js @@ -1,11 +1,11 @@ -import {Directive, onChange} from 'angular2/src/core/annotations_impl/annotations'; +import {Directive} from 'angular2/src/core/annotations_impl/annotations'; import {Ancestor} from 'angular2/src/core/annotations_impl/visibility'; import {ElementRef} from 'angular2/src/core/compiler/element_ref'; import {Optional} from 'angular2/src/di/annotations_impl'; import {Renderer} from 'angular2/src/render/api'; -import {isPresent, isString, CONST_EXPR} from 'angular2/src/facade/lang'; +import {isPresent, isString, CONST_EXPR, isBlank, BaseException} from 'angular2/src/facade/lang'; import {ListWrapper} from 'angular2/src/facade/collection'; -import {ControlGroup} from './model'; +import {ControlGroup, Control, isControl} from './model'; import {Validators} from './validators'; //export interface ControlValueAccessor { @@ -13,6 +13,24 @@ import {Validators} from './validators'; // set onChange(fn){} //} +function _lookupControl(groupDirective:ControlGroupDirective, controlOrName:any):any { + if (isControl(controlOrName)) { + return controlOrName; + } + + if (isBlank(groupDirective)) { + throw new BaseException(`No control group found for "${controlOrName}"`); + } + + var control = groupDirective.findControl(controlOrName); + + if (isBlank(control)) { + throw new BaseException(`Cannot find control "${controlOrName}"`); + } + + return control; +} + /** * The default accessor for writing a value and listening to changes that is used by a {@link Control} directive. * @@ -119,7 +137,6 @@ export class CheckboxControlValueAccessor { * @exportedAs angular2/forms */ @Directive({ - lifecycle: [onChange], selector: '[control]', properties: { 'controlOrName' : 'control' @@ -128,25 +145,21 @@ export class CheckboxControlValueAccessor { export class ControlDirective { _groupDirective:ControlGroupDirective; - controlOrName:any; + _controlOrName:any; valueAccessor:any; //ControlValueAccessor validator:Function; constructor(@Optional() @Ancestor() groupDirective:ControlGroupDirective, valueAccessor:DefaultValueAccessor) { this._groupDirective = groupDirective; - this.controlOrName = null; + this._controlOrName = null; this.valueAccessor = valueAccessor; this.validator = Validators.nullValidator; } - // TODO: vsavkin this should be moved into the constructor once static bindings - // are implemented - onChange(_) { - this._initialize(); - } + set controlOrName(controlOrName) { + this._controlOrName = controlOrName; - _initialize() { if(isPresent(this._groupDirective)) { this._groupDirective.addDirective(this); } @@ -167,11 +180,7 @@ export class ControlDirective { } _control() { - if (isString(this.controlOrName)) { - return this._groupDirective.findControl(this.controlOrName); - } else { - return this.controlOrName; - } + return _lookupControl(this._groupDirective, this._controlOrName); } } @@ -218,27 +227,21 @@ export class ControlDirective { @Directive({ selector: '[control-group]', properties: { - 'controlGroup' : 'control-group' + 'controlOrName' : 'control-group' } }) export class ControlGroupDirective { _groupDirective:ControlGroupDirective; - _controlGroupName:string; - - _controlGroup:ControlGroup; _directives:List; + _controlOrName:any; constructor(@Optional() @Ancestor() groupDirective:ControlGroupDirective) { this._groupDirective = groupDirective; this._directives = ListWrapper.create(); } - set controlGroup(controlGroup) { - if (isString(controlGroup)) { - this._controlGroupName = controlGroup; - } else { - this._controlGroup = controlGroup; - } + set controlOrName(controlOrName) { + this._controlOrName = controlOrName; this._updateDomValue(); } @@ -255,11 +258,7 @@ export class ControlGroupDirective { } _getControlGroup():ControlGroup { - if (isPresent(this._controlGroupName)) { - return this._groupDirective.findControl(this._controlGroupName) - } else { - return this._controlGroup; - } + return _lookupControl(this._groupDirective, this._controlOrName); } } diff --git a/modules/angular2/src/forms/model.js b/modules/angular2/src/forms/model.js index 49b107480d..027da5e55c 100644 --- a/modules/angular2/src/forms/model.js +++ b/modules/angular2/src/forms/model.js @@ -29,6 +29,11 @@ export const INVALID = "INVALID"; // setParent(parent){} //} +export function isControl(c:Object):boolean { + return c instanceof AbstractControl; +} + + /** * Omitting from external API doc as this is really an abstract internal concept. */ diff --git a/modules/angular2/test/forms/directives_spec.js b/modules/angular2/test/forms/directives_spec.js new file mode 100644 index 0000000000..703652fa82 --- /dev/null +++ b/modules/angular2/test/forms/directives_spec.js @@ -0,0 +1,26 @@ +import {ddescribe, describe, it, iit, xit, expect, beforeEach, afterEach, el, + AsyncTestCompleter, inject} from 'angular2/test_lib'; +import {ControlGroup, ControlDirective, ControlGroupDirective} from 'angular2/forms'; + +export function main() { + describe("Form Directives", () => { + describe("Control", () => { + it("should throw when the group is not found and the control is not set", () => { + var c = new ControlDirective(null, null); + expect(() => { + c.controlOrName = 'login'; + }).toThrowError(new RegExp('No control group found for "login"')); + }); + + it("should throw when cannot find the control in the group", () => { + var emptyGroup = new ControlGroupDirective(null); + emptyGroup.controlOrName = new ControlGroup({}); + + var c = new ControlDirective(emptyGroup, null); + expect(() => { + c.controlOrName = 'login'; + }).toThrowError(new RegExp('Cannot find control "login"')); + }); + }); + }); +}