From 46023e47925c74028ab2c07eeaab96c9f89443c3 Mon Sep 17 00:00:00 2001 From: Dzmitry Shylovich Date: Sat, 12 Nov 2016 02:58:43 +0300 Subject: [PATCH] fix(select): allow for null values in HTML select options bound with ngValue closes #12829 --- .../select_control_value_accessor.ts | 10 ++--- .../select_multiple_control_value_accessor.ts | 12 +++--- .../forms/test/template_integration_spec.ts | 38 ++++++++++++++++++- 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/modules/@angular/forms/src/directives/select_control_value_accessor.ts b/modules/@angular/forms/src/directives/select_control_value_accessor.ts index 353c1346b0..7af3a24587 100644 --- a/modules/@angular/forms/src/directives/select_control_value_accessor.ts +++ b/modules/@angular/forms/src/directives/select_control_value_accessor.ts @@ -6,13 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, Host, Input, OnDestroy, Optional, Renderer, forwardRef} from '@angular/core'; +import {Directive, ElementRef, Host, Input, OnDestroy, Optional, Provider, Renderer, forwardRef} from '@angular/core'; import {isPrimitive, looseIdentical} from '../facade/lang'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; -export const SELECT_VALUE_ACCESSOR: any = { +export const SELECT_VALUE_ACCESSOR: Provider = { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => SelectControlValueAccessor), multi: true @@ -115,8 +115,8 @@ export class SelectControlValueAccessor implements ControlValueAccessor { /** @internal */ _getOptionValue(valueString: string): any { - let key: string = _extractId(valueString); - return this._optionMap.has(key) ? this._optionMap.get(key) : valueString; + const id: string = _extractId(valueString); + return this._optionMap.has(id) ? this._optionMap.get(id) : valueString; } } @@ -158,7 +158,7 @@ export class NgSelectOption implements OnDestroy { this._renderer.setElementProperty(this._element.nativeElement, 'value', value); } - ngOnDestroy() { + ngOnDestroy(): void { if (this._select) { this._select._optionMap.delete(this.id); this._select.writeValue(this._select.value); 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 e2af99baab..cd7302f72e 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 @@ -6,13 +6,13 @@ * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, Host, Input, OnDestroy, OpaqueToken, Optional, Renderer, Type, forwardRef} from '@angular/core'; +import {Directive, ElementRef, Host, Input, OnDestroy, Optional, Provider, Renderer, forwardRef} from '@angular/core'; import {isPrimitive, looseIdentical} from '../facade/lang'; import {ControlValueAccessor, NG_VALUE_ACCESSOR} from './control_value_accessor'; -export const SELECT_MULTIPLE_VALUE_ACCESSOR = { +export const SELECT_MULTIPLE_VALUE_ACCESSOR: Provider = { provide: NG_VALUE_ACCESSOR, useExisting: forwardRef(() => SelectMultipleControlValueAccessor), multi: true @@ -121,8 +121,8 @@ export class SelectMultipleControlValueAccessor implements ControlValueAccessor /** @internal */ _getOptionValue(valueString: string): any { - let key: string = _extractId(valueString); - return this._optionMap.has(key) ? this._optionMap.get(key)._value : valueString; + const id: string = _extractId(valueString); + return this._optionMap.has(id) ? this._optionMap.get(id)._value : valueString; } } @@ -180,12 +180,10 @@ export class NgSelectMultipleOption implements OnDestroy { this._renderer.setElementProperty(this._element.nativeElement, 'selected', selected); } - ngOnDestroy() { + ngOnDestroy(): void { if (this._select) { this._select._optionMap.delete(this.id); this._select.writeValue(this._select.value); } } } - -export const SELECT_DIRECTIVES = [SelectMultipleControlValueAccessor, NgSelectMultipleOption]; diff --git a/modules/@angular/forms/test/template_integration_spec.ts b/modules/@angular/forms/test/template_integration_spec.ts index a3673a1ed3..4c5468e113 100644 --- a/modules/@angular/forms/test/template_integration_spec.ts +++ b/modules/@angular/forms/test/template_integration_spec.ts @@ -23,7 +23,7 @@ export function main() { NgModelRadioForm, NgModelRangeForm, NgModelSelectForm, NgNoFormComp, InvalidNgModelNoName, NgModelOptionsStandalone, NgModelCustomComp, NgModelCustomWrapper, NgModelValidationBindings, NgModelMultipleValidators, NgAsyncValidator, - NgModelAsyncValidation + NgModelAsyncValidation, NgModelSelectWithNullForm ], imports: [FormsModule] }); @@ -699,6 +699,28 @@ export function main() { expect(select.nativeElement.value).toEqual('2: Object'); expect(secondNYC.nativeElement.selected).toBe(true); })); + + it('should work with null option', fakeAsync(() => { + const fixture = TestBed.createComponent(NgModelSelectWithNullForm); + const comp = fixture.componentInstance; + comp.cities = [{'name': 'SF'}, {'name': 'NYC'}]; + comp.selectedCity = null; + fixture.detectChanges(); + + const select = fixture.debugElement.query(By.css('select')); + + select.nativeElement.value = '2: Object'; + dispatchEvent(select.nativeElement, 'change'); + fixture.detectChanges(); + tick(); + expect(comp.selectedCity['name']).toEqual('NYC'); + + select.nativeElement.value = '0: null'; + dispatchEvent(select.nativeElement, 'change'); + fixture.detectChanges(); + tick(); + expect(comp.selectedCity).toEqual(null); + })); }); describe('custom value accessors', () => { @@ -1078,6 +1100,20 @@ class NgModelSelectForm { cities: any[] = []; } +@Component({ + selector: 'ng-model-select-null-form', + template: ` + + ` +}) +class NgModelSelectWithNullForm { + selectedCity: {[k: string]: string} = {}; + cities: any[] = []; +} + @Component({ selector: 'ng-model-custom-comp', template: `