From 821b8f09d6054261c737f30ce500d912d7d8017e Mon Sep 17 00:00:00 2001 From: gary-b Date: Tue, 1 Nov 2016 22:43:13 +0000 Subject: [PATCH] fix(forms): ensure select[multiple] retains selections If you bound an array to select[multiple] via ngModel and subsequently changed the options to select from, the UI would drop any selections made since by the user. This was due to SelectMultipleControlValueAccessor not keeping a reference to the new model arrays it generated when users interacted with the select control. Update code to keep the reference. Closes #12527 Closes #12654 --- .../select_multiple_control_value_accessor.ts | 1 + .../forms/test/template_integration_spec.ts | 65 ++++++++++--------- 2 files changed, 36 insertions(+), 30 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 9fe3027615..105ead76c1 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 @@ -99,6 +99,7 @@ export class SelectMultipleControlValueAccessor implements ControlValueAccessor } } } + this.value = selected; fn(selected); }; } diff --git a/modules/@angular/forms/test/template_integration_spec.ts b/modules/@angular/forms/test/template_integration_spec.ts index ebfa75848f..02e7bc88ab 100644 --- a/modules/@angular/forms/test/template_integration_spec.ts +++ b/modules/@angular/forms/test/template_integration_spec.ts @@ -765,12 +765,23 @@ export function main() { comp.cities = [{'name': 'SF'}, {'name': 'NYC'}, {'name': 'Buffalo'}]; }); - const setSelectedCities = (selectedCities: any): void => { - comp.selectedCities = selectedCities; + const detectChangesAndTick = (): void => { fixture.detectChanges(); tick(); }; + const setSelectedCities = (selectedCities: any): void => { + comp.selectedCities = selectedCities; + detectChangesAndTick(); + }; + + const selectOptionViaUI = (valueString: string): void => { + const select = fixture.debugElement.query(By.css('select')); + select.nativeElement.value = valueString; + dispatchEvent(select.nativeElement, 'change'); + detectChangesAndTick(); + }; + const assertOptionElementSelectedState = (selectedStates: boolean[]): void => { const options = fixture.debugElement.queryAll(By.css('option')); if (options.length !== selectedStates.length) { @@ -781,36 +792,30 @@ export function main() { } }; - const testNewModelValueUnselectsAllOptions = (modelValue: any): void => { - setSelectedCities([comp.cities[1]]); - assertOptionElementSelectedState([false, true, false]); + it('should reflect state of model after option selected and new options subsequently added', + fakeAsync(() => { + setSelectedCities([]); - 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]]); + selectOptionViaUI('1: Object'); assertOptionElementSelectedState([false, true, false]); + + comp.cities.push({'name': 'Chicago'}); + detectChangesAndTick(); + + assertOptionElementSelectedState([false, true, false, false]); + })); + + it('should reflect state of model after option selected and then other options removed', + fakeAsync(() => { + setSelectedCities([]); + + selectOptionViaUI('1: Object'); + assertOptionElementSelectedState([false, true, false]); + + comp.cities.pop(); + detectChangesAndTick(); + + assertOptionElementSelectedState([false, true]); })); });