From d22eeb70b8eec5ed29606154edb2f8ebe02dbab0 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Mon, 10 Oct 2016 18:17:45 +0200 Subject: [PATCH] fix(forms): allow optional fields with pattern and minlength validators (#12147) --- modules/@angular/forms/src/validators.ts | 25 +++++--- .../forms/test/template_integration_spec.ts | 60 ++++++++++++++++++- .../@angular/forms/test/validators_spec.ts | 59 +++++++----------- 3 files changed, 98 insertions(+), 46 deletions(-) diff --git a/modules/@angular/forms/src/validators.ts b/modules/@angular/forms/src/validators.ts index f0bff67ecf..46fdd09efd 100644 --- a/modules/@angular/forms/src/validators.ts +++ b/modules/@angular/forms/src/validators.ts @@ -11,11 +11,13 @@ import {toPromise} from 'rxjs/operator/toPromise'; import {AsyncValidatorFn, ValidatorFn} from './directives/validators'; import {StringMapWrapper} from './facade/collection'; -import {isBlank, isPresent, isString} from './facade/lang'; +import {isPresent} from './facade/lang'; import {AbstractControl} from './model'; import {isPromise} from './private_import_core'; - +function isEmptyInputValue(value: any) { + return value == null || typeof value === 'string' && value.length === 0; +} /** * Providers for validators to be used for {@link FormControl}s in a form. @@ -60,9 +62,7 @@ export class Validators { * Validator that requires controls to have a non-empty value. */ static required(control: AbstractControl): {[key: string]: boolean} { - return isBlank(control.value) || (isString(control.value) && control.value == '') ? - {'required': true} : - null; + return isEmptyInputValue(control.value) ? {'required': true} : null; } /** @@ -70,6 +70,9 @@ export class Validators { */ static minLength(minLength: number): ValidatorFn { return (control: AbstractControl): {[key: string]: any} => { + if (isEmptyInputValue(control.value)) { + return null; // don't validate empty values to allow optional controls + } const length = typeof control.value === 'string' ? control.value.length : 0; return length < minLength ? {'minlength': {'requiredLength': minLength, 'actualLength': length}} : @@ -94,10 +97,14 @@ export class Validators { */ static pattern(pattern: string): ValidatorFn { return (control: AbstractControl): {[key: string]: any} => { - let regex = new RegExp(`^${pattern}$`); - let v: string = control.value; - return regex.test(v) ? null : - {'pattern': {'requiredPattern': `^${pattern}$`, 'actualValue': v}}; + if (isEmptyInputValue(control.value)) { + return null; // don't validate empty values to allow optional controls + } + const regex = new RegExp(`^${pattern}$`); + const value: string = control.value; + return regex.test(value) ? + null : + {'pattern': {'requiredPattern': `^${pattern}$`, 'actualValue': value}}; }; } diff --git a/modules/@angular/forms/test/template_integration_spec.ts b/modules/@angular/forms/test/template_integration_spec.ts index 90d7f78bfc..7d5a20cffd 100644 --- a/modules/@angular/forms/test/template_integration_spec.ts +++ b/modules/@angular/forms/test/template_integration_spec.ts @@ -22,7 +22,7 @@ export function main() { StandaloneNgModel, NgModelForm, NgModelGroupForm, NgModelValidBinding, NgModelNgIfForm, NgModelRadioForm, NgModelSelectForm, NgNoFormComp, InvalidNgModelNoName, NgModelOptionsStandalone, NgModelCustomComp, NgModelCustomWrapper, - NgModelValidationBindings + NgModelValidationBindings, NgModelMultipleValidators ], imports: [FormsModule] }); @@ -728,6 +728,50 @@ export function main() { expect(form.valid).toEqual(true); })); + it('should support optional fields with pattern validator', fakeAsync(() => { + const fixture = TestBed.createComponent(NgModelMultipleValidators); + fixture.componentInstance.required = false; + fixture.componentInstance.pattern = '[a-z]+'; + fixture.detectChanges(); + tick(); + + const form = fixture.debugElement.children[0].injector.get(NgForm); + const input = fixture.debugElement.query(By.css('input')); + + input.nativeElement.value = ''; + dispatchEvent(input.nativeElement, 'input'); + fixture.detectChanges(); + expect(form.valid).toBeTruthy(); + + input.nativeElement.value = '1'; + dispatchEvent(input.nativeElement, 'input'); + fixture.detectChanges(); + expect(form.valid).toBeFalsy(); + expect(form.control.hasError('pattern', ['tovalidate'])).toBeTruthy(); + })); + + it('should support optional fields with minlength validator', fakeAsync(() => { + const fixture = TestBed.createComponent(NgModelMultipleValidators); + fixture.componentInstance.required = false; + fixture.componentInstance.minLen = 2; + fixture.detectChanges(); + tick(); + + const form = fixture.debugElement.children[0].injector.get(NgForm); + const input = fixture.debugElement.query(By.css('input')); + + input.nativeElement.value = ''; + dispatchEvent(input.nativeElement, 'input'); + fixture.detectChanges(); + expect(form.valid).toBeTruthy(); + + input.nativeElement.value = '1'; + dispatchEvent(input.nativeElement, 'input'); + fixture.detectChanges(); + expect(form.valid).toBeFalsy(); + expect(form.control.hasError('minlength', ['tovalidate'])).toBeTruthy(); + })); + it('changes on bound properties should change the validation state of the form', fakeAsync(() => { const fixture = TestBed.createComponent(NgModelValidationBindings); @@ -1037,6 +1081,20 @@ class NgModelValidationBindings { pattern: string; } +@Component({ + selector: 'ng-model-multiple-validators', + template: ` +
+ +
+ ` +}) +class NgModelMultipleValidators { + required: boolean; + minLen: number; + pattern: string; +} + function sortedClassList(el: HTMLElement) { const l = getDOM().classList(el); l.sort(); diff --git a/modules/@angular/forms/test/validators_spec.ts b/modules/@angular/forms/test/validators_spec.ts index 83b0c2effb..abfbdc28ae 100644 --- a/modules/@angular/forms/test/validators_spec.ts +++ b/modules/@angular/forms/test/validators_spec.ts @@ -44,33 +44,24 @@ export function main() { () => { expect(Validators.required(new FormControl(null))).toEqual({'required': true}); }); it('should not error on a non-empty string', - () => { expect(Validators.required(new FormControl('not empty'))).toEqual(null); }); + () => { expect(Validators.required(new FormControl('not empty'))).toBeNull(); }); it('should accept zero as valid', - () => { expect(Validators.required(new FormControl(0))).toEqual(null); }); + () => { expect(Validators.required(new FormControl(0))).toBeNull(); }); }); describe('minLength', () => { - it('should error on an empty string', () => { - expect(Validators.minLength(2)(new FormControl(''))).toEqual({ - 'minlength': {'requiredLength': 2, 'actualLength': 0} - }); - }); + it('should not error on an empty string', + () => { expect(Validators.minLength(2)(new FormControl(''))).toBeNull(); }); - it('should error on null', () => { - expect(Validators.minLength(2)(new FormControl(null))).toEqual({ - 'minlength': {'requiredLength': 2, 'actualLength': 0} - }); - }); + it('should not error on null', + () => { expect(Validators.minLength(2)(new FormControl(null))).toBeNull(); }); - it('should error on undefined', () => { - expect(Validators.minLength(2)(new FormControl(null))).toEqual({ - 'minlength': {'requiredLength': 2, 'actualLength': 0} - }); - }); + it('should not error on undefined', + () => { expect(Validators.minLength(2)(new FormControl(null))).toBeNull(); }); it('should not error on valid strings', - () => { expect(Validators.minLength(2)(new FormControl('aa'))).toEqual(null); }); + () => { expect(Validators.minLength(2)(new FormControl('aa'))).toBeNull(); }); it('should error on short strings', () => { expect(Validators.minLength(2)(new FormControl('a'))).toEqual({ @@ -81,13 +72,13 @@ export function main() { describe('maxLength', () => { it('should not error on an empty string', - () => { expect(Validators.maxLength(2)(new FormControl(''))).toEqual(null); }); + () => { expect(Validators.maxLength(2)(new FormControl(''))).toBeNull(); }); it('should not error on null', - () => { expect(Validators.maxLength(2)(new FormControl(null))).toEqual(null); }); + () => { expect(Validators.maxLength(2)(new FormControl(null))).toBeNull(); }); it('should not error on valid strings', - () => { expect(Validators.maxLength(2)(new FormControl('aa'))).toEqual(null); }); + () => { expect(Validators.maxLength(2)(new FormControl('aa'))).toBeNull(); }); it('should error on long strings', () => { expect(Validators.maxLength(2)(new FormControl('aaa'))).toEqual({ @@ -98,29 +89,25 @@ export function main() { describe('pattern', () => { it('should not error on an empty string', - () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl(''))).toEqual(null); }); + () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(''))).toBeNull(); }); it('should not error on null', - () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl(null))).toEqual(null); }); + () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(null))).toBeNull(); }); + + it('should not error on undefined', + () => { expect(Validators.pattern('[a-zA-Z ]+')(new FormControl(null))).toBeNull(); }); it('should not error on null value and "null" pattern', - () => { expect(Validators.pattern('null')(new FormControl(null))).toEqual(null); }); + () => { expect(Validators.pattern('null')(new FormControl(null))).toBeNull(); }); - it('should not error on valid strings', () => { - expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaAA'))).toEqual(null); - }); + it('should not error on valid strings', + () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaAA'))).toBeNull(); }); it('should error on failure to match string', () => { expect(Validators.pattern('[a-zA-Z ]*')(new FormControl('aaa0'))).toEqual({ 'pattern': {'requiredPattern': '^[a-zA-Z ]*$', 'actualValue': 'aaa0'} }); }); - - it('should error on failure to match empty string', () => { - expect(Validators.pattern('[a-zA-Z]+')(new FormControl(''))).toEqual({ - 'pattern': {'requiredPattern': '^[a-zA-Z]+$', 'actualValue': ''} - }); - }); }); describe('compose', () => { @@ -139,7 +126,7 @@ export function main() { it('should return null when no errors', () => { var c = Validators.compose([Validators.nullValidator, Validators.nullValidator]); - expect(c(new FormControl(''))).toEqual(null); + expect(c(new FormControl(''))).toBeNull(); }); it('should ignore nulls', () => { @@ -166,7 +153,7 @@ export function main() { } it('should return null when given null', - () => { expect(Validators.composeAsync(null)).toEqual(null); }); + () => { expect(Validators.composeAsync(null)).toBeNull(); }); it('should collect errors from all the validators', fakeAsync(() => { var c = Validators.composeAsync([ @@ -199,7 +186,7 @@ export function main() { (>c(new FormControl('expected'))).then(v => value = v); tick(1); - expect(value).toEqual(null); + expect(value).toBeNull(); })); it('should ignore nulls', fakeAsync(() => {