From 7b0a86718cb4227a656359b26890d1f6a48c4a84 Mon Sep 17 00:00:00 2001 From: gary-b Date: Wed, 14 Dec 2016 16:34:19 +0000 Subject: [PATCH] fix (forms): clear selected options when model is not an array (#12519) When an invalid model value (eg empty string) was preset ngModel on select[multiple] would throw an error, which is inconsistent with how it works on other user input elements. Setting the model value to null or undefined would also have no effect on what was already selected in the UI. Fix this by clearing selected options when model set to null, undefined or a type other than Array. Closes #11926 --- .../select_multiple_control_value_accessor.ts | 14 ++-- .../forms/test/template_integration_spec.ts | 76 ++++++++++++++++++- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts b/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts index cd7302f72e..9fe3027615 100644 --- a/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts +++ b/modules/@angular/forms/src/directives/select_multiple_control_value_accessor.ts @@ -66,11 +66,15 @@ export class SelectMultipleControlValueAccessor implements ControlValueAccessor writeValue(value: any): void { this.value = value; - if (value == null) return; - const values: Array = >value; - // convert values to ids - const ids = values.map((v) => this._getOptionId(v)); - this._optionMap.forEach((opt, o) => { opt._setSelected(ids.indexOf(o.toString()) > -1); }); + let optionSelectedStateSetter: (opt: NgSelectMultipleOption, o: any) => void; + if (Array.isArray(value)) { + // convert values to ids + const ids = value.map((v) => this._getOptionId(v)); + optionSelectedStateSetter = (opt, o) => { opt._setSelected(ids.indexOf(o.toString()) > -1); }; + } else { + optionSelectedStateSetter = (opt, o) => { opt._setSelected(false); }; + } + this._optionMap.forEach(optionSelectedStateSetter); } registerOnChange(fn: (value: any) => any): void { diff --git a/modules/@angular/forms/test/template_integration_spec.ts b/modules/@angular/forms/test/template_integration_spec.ts index 41e22a918b..0a6565131a 100644 --- a/modules/@angular/forms/test/template_integration_spec.ts +++ b/modules/@angular/forms/test/template_integration_spec.ts @@ -7,7 +7,7 @@ */ import {Component, Directive, Input, forwardRef} from '@angular/core'; -import {TestBed, async, fakeAsync, tick} from '@angular/core/testing'; +import {ComponentFixture, TestBed, async, fakeAsync, tick} from '@angular/core/testing'; import {AbstractControl, ControlValueAccessor, FormsModule, NG_ASYNC_VALIDATORS, NG_VALUE_ACCESSOR, NgForm, Validator} from '@angular/forms'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; @@ -23,7 +23,7 @@ export function main() { NgModelRadioForm, NgModelRangeForm, NgModelSelectForm, NgNoFormComp, InvalidNgModelNoName, NgModelOptionsStandalone, NgModelCustomComp, NgModelCustomWrapper, NgModelValidationBindings, NgModelMultipleValidators, NgAsyncValidator, - NgModelAsyncValidation, NgModelSelectWithNullForm + NgModelAsyncValidation, NgModelSelectMultipleForm, NgModelSelectWithNullForm ], imports: [FormsModule] }); @@ -740,6 +740,65 @@ export function main() { })); }); + describe('select multiple controls', () => { + let fixture: ComponentFixture; + let comp: NgModelSelectMultipleForm; + + beforeEach(() => { + fixture = TestBed.createComponent(NgModelSelectMultipleForm); + comp = fixture.componentInstance; + comp.cities = [{'name': 'SF'}, {'name': 'NYC'}, {'name': 'Buffalo'}]; + }); + + const setSelectedCities = (selectedCities: any): void => { + comp.selectedCities = selectedCities; + fixture.detectChanges(); + tick(); + }; + + const assertOptionElementSelectedState = (selectedStates: boolean[]): void => { + const options = fixture.debugElement.queryAll(By.css('option')); + if (options.length !== selectedStates.length) { + throw 'the selected state values to assert does not match the number of options'; + } + for (let i = 0; i < selectedStates.length; i++) { + expect(options[i].nativeElement.selected).toBe(selectedStates[i]); + } + }; + + const testNewModelValueUnselectsAllOptions = (modelValue: any): void => { + setSelectedCities([comp.cities[1]]); + assertOptionElementSelectedState([false, true, false]); + + setSelectedCities(modelValue); + + const select = fixture.debugElement.query(By.css('select')); + expect(select.nativeElement.value).toEqual(''); + assertOptionElementSelectedState([false, false, false]); + }; + + it('should support setting value to null and undefined', fakeAsync(() => { + testNewModelValueUnselectsAllOptions(null); + testNewModelValueUnselectsAllOptions(undefined); + })); + + it('should clear all selected option elements when value of wrong type supplied', + fakeAsync(() => { testNewModelValueUnselectsAllOptions(''); })); + + it('should set option elements to selected that are present in model', fakeAsync(() => { + setSelectedCities([comp.cities[0], comp.cities[2]]); + assertOptionElementSelectedState([true, false, true]); + })); + + it('should clear selected option elements since removed from model', fakeAsync(() => { + const selectedCities: [{'name:string': string}] = <[{'name:string': string}]>[]; + selectedCities.push.apply(selectedCities, comp.cities); + setSelectedCities(selectedCities); + setSelectedCities([comp.cities[1]]); + assertOptionElementSelectedState([false, true, false]); + })); + }); + describe('custom value accessors', () => { it('should support standard writing to view and model', async(() => { const fixture = TestBed.createComponent(NgModelCustomWrapper); @@ -1153,6 +1212,19 @@ class NgModelSelectWithNullForm { cities: any[] = []; } +@Component({ + selector: 'ng-model-select-multiple-form', + template: ` + + ` +}) +class NgModelSelectMultipleForm { + selectedCities: any[]; + cities: any[] = []; +} + @Component({ selector: 'ng-model-custom-comp', template: `