diff --git a/src/framework/theme/components/select/option.component.ts b/src/framework/theme/components/select/option.component.ts index a7c22263f9..72871748f4 100644 --- a/src/framework/theme/components/select/option.component.ts +++ b/src/framework/theme/components/select/option.component.ts @@ -82,7 +82,7 @@ export class NbOptionComponent implements OnDestroy { * Determines should we render checkbox. * */ get withCheckbox(): boolean { - return this.multiple && !!this.value; + return this.multiple && this.value != null; } get content() { diff --git a/src/framework/theme/components/select/select.component.ts b/src/framework/theme/components/select/select.component.ts index a401a6c916..037aed36e1 100644 --- a/src/framework/theme/components/select/select.component.ts +++ b/src/framework/theme/components/select/select.component.ts @@ -78,7 +78,7 @@ export class NbSelectLabelComponent { * * @stacked-example(Multiple, select/select-multiple.component) * - * Items without values will clean selection. + * Items without values will clean the selection. Both `null` and `undefined` values will also clean the selection. * * @stacked-example(Clean selection, select/select-clean.component) * @@ -384,7 +384,7 @@ export class NbSelectComponent implements OnInit, AfterViewInit, AfterContent } writeValue(value: T | T[]): void { - if (!value || !this.alive) { + if (!this.alive) { return; } @@ -399,10 +399,10 @@ export class NbSelectComponent implements OnInit, AfterViewInit, AfterContent * Selects option or clear all selected options if value is null. * */ protected handleOptionClick(option: NbOptionComponent) { - if (option.value) { - this.selectOption(option); - } else { + if (option.value == null) { this.reset(); + } else { + this.selectOption(option); } this.cd.markForCheck(); diff --git a/src/framework/theme/components/select/select.spec.ts b/src/framework/theme/components/select/select.spec.ts index 25f6dbc026..3c09ce6ac0 100644 --- a/src/framework/theme/components/select/select.spec.ts +++ b/src/framework/theme/components/select/select.spec.ts @@ -4,7 +4,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. */ -import { Component, EventEmitter, Input, Output, QueryList, ViewChild, ViewChildren } from '@angular/core'; +import { Component, ElementRef, EventEmitter, Input, Output, QueryList, ViewChild, ViewChildren } from '@angular/core'; import { ComponentFixture, fakeAsync, flush, TestBed } from '@angular/core/testing'; import { FormControl, FormsModule, ReactiveFormsModule } from '@angular/forms'; import { RouterTestingModule } from '@angular/router/testing'; @@ -45,6 +45,15 @@ const TEST_GROUPS = [ { title: 'Option 33', value: 'Option 33' }, ], }, + { + title: 'Group 4', + options: [ + { title: 'Option 41', value: '' }, + { title: 'Option 42', value: '0' }, + { title: 'Option 43', value: 0 }, + { title: 'Option 44'}, + ], + }, ]; @Component({ @@ -139,6 +148,105 @@ export class NbNgModelSelectComponent { @ViewChild(NbOptionComponent) optionComponent: NbOptionComponent; } +@Component({ + template: ` + + + + + No value option + undefined value + undefined value + false value + 0 value + empty string value + NaN value + truthy value + + + + + `, +}) +export class NbSelectWithFalsyOptionValuesComponent { + nanValue = NaN; + + @ViewChild(NbSelectComponent) select: NbSelectComponent; + @ViewChildren(NbOptionComponent) options: QueryList>; + @ViewChildren(NbOptionComponent, { read: ElementRef }) optionElements: QueryList>; + + get noValueOption(): NbOptionComponent { + return this.options.toArray()[0]; + } + get noValueOptionElement(): ElementRef { + return this.optionElements.toArray()[0]; + } + get nullOption(): NbOptionComponent { + return this.options.toArray()[1]; + } + get nullOptionElement(): ElementRef { + return this.optionElements.toArray()[1]; + } + get undefinedOption(): NbOptionComponent { + return this.options.toArray()[2]; + } + get undefinedOptionElement(): ElementRef { + return this.optionElements.toArray()[2]; + } + get falseOption(): NbOptionComponent { + return this.options.toArray()[3]; + } + get falseOptionElement(): ElementRef { + return this.optionElements.toArray()[3]; + } + get zeroOption(): NbOptionComponent { + return this.options.toArray()[4]; + } + get zeroOptionElement(): ElementRef { + return this.optionElements.toArray()[4]; + } + get emptyStringOption(): NbOptionComponent { + return this.options.toArray()[5]; + } + get emptyStringOptionElement(): ElementRef { + return this.optionElements.toArray()[5]; + } + get nanOption(): NbOptionComponent { + return this.options.toArray()[6]; + } + get nanOptionElement(): ElementRef { + return this.optionElements.toArray()[6]; + } + get truthyOption(): NbOptionComponent { + return this.options.toArray()[7]; + } + get truthyOptionElement(): ElementRef { + return this.optionElements.toArray()[7]; + } +} + +@Component({ + template: ` + + + + + No value option + undefined value + undefined value + false value + 0 value + empty string value + NaN value + truthy value + + + + + `, +}) +export class NbMultipleSelectWithFalsyOptionValuesComponent extends NbSelectWithFalsyOptionValuesComponent {} + describe('Component: NbSelectComponent', () => { let fixture: ComponentFixture; let overlayContainerService: NbOverlayContainerAdapter; @@ -290,7 +398,7 @@ describe('Component: NbSelectComponent', () => { const button = fixture.nativeElement.querySelector('button'); expect(button.textContent).toContain('1 noitpO'); - }) + }); }); it('should select initially specified value without errors', fakeAsync(() => { @@ -342,7 +450,7 @@ describe('Component: NbSelectComponent', () => { selectFixture.detectChanges(); flush(); - const optionToSelect = testComponent.options.last; + const optionToSelect = testComponent.options.find(o => o.value != null); const optionSelectSpy = spyOn(optionToSelect, 'select').and.callThrough(); expect(optionToSelect.selected).toEqual(false); @@ -417,6 +525,131 @@ describe('Component: NbSelectComponent', () => { }); }); +describe('NbSelectComponent - falsy values', () => { + let fixture: ComponentFixture; + let testComponent: NbSelectWithFalsyOptionValuesComponent; + let select: NbSelectComponent; + + beforeEach(fakeAsync(() => { + TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([]), + NbThemeModule.forRoot(), + NbLayoutModule, + NbSelectModule, + ], + declarations: [ + NbSelectWithFalsyOptionValuesComponent, + NbMultipleSelectWithFalsyOptionValuesComponent, + ], + }); + + fixture = TestBed.createComponent(NbSelectWithFalsyOptionValuesComponent); + testComponent = fixture.componentInstance; + select = testComponent.select; + + fixture.detectChanges(); + flush(); + })); + + it('should clean selection when selected option does not have a value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.noValueOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(0); + })); + + it('should clean selection when selected option has null value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.nullOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(0); + })); + + it('should clean selection when selected option has undefined value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.undefinedOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(0); + })); + + it('should not reset selection when selected option has false value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.falseOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(1); + })); + + it('should not reset selection when selected option has zero value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.zeroOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(1); + })); + + it('should not reset selection when selected option has empty string value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.emptyStringOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(1); + })); + + it('should not reset selection when selected option has NaN value', fakeAsync(() => { + select.setSelected = testComponent.truthyOption.value; + fixture.detectChanges(); + + testComponent.nanOption.onClick(); + fixture.detectChanges(); + + expect(select.selectionModel.length).toEqual(1); + })); + + describe('multiple', () => { + beforeEach(fakeAsync(() => { + fixture = TestBed.createComponent(NbMultipleSelectWithFalsyOptionValuesComponent); + testComponent = fixture.componentInstance; + select = testComponent.select; + + fixture.detectChanges(); + flush(); + select.show(); + fixture.detectChanges(); + })); + + it('should not render checkbox on options with reset values', () => { + expect(testComponent.noValueOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null); + expect(testComponent.nullOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null); + expect(testComponent.undefinedOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null); + }); + + it('should render checkbox on options with falsy non-reset values', () => { + expect(testComponent.falseOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null); + expect(testComponent.zeroOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null); + expect(testComponent.emptyStringOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null); + expect(testComponent.nanOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null); + expect(testComponent.truthyOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null); + }); + }); +}); + describe('NbOptionComponent', () => { let fixture: ComponentFixture; let testSelectComponent: NbReactiveFormSelectComponent; diff --git a/src/playground/with-layout/select/select-clean.component.html b/src/playground/with-layout/select/select-clean.component.html index 7552737e1d..ee3656b6dc 100644 --- a/src/playground/with-layout/select/select-clean.component.html +++ b/src/playground/with-layout/select/select-clean.component.html @@ -1,7 +1,10 @@ - Clean - Option 1 - Option 2 - Option 3 - Option 4 + Clean with option without value + Clean with null value + Clean with null value + Option 0 + Option 1 + Option 2 + Option 3 + Option 4 diff --git a/src/playground/with-layout/select/select-clean.component.ts b/src/playground/with-layout/select/select-clean.component.ts index 94c5cd058c..97f70f3ff8 100644 --- a/src/playground/with-layout/select/select-clean.component.ts +++ b/src/playground/with-layout/select/select-clean.component.ts @@ -22,5 +22,4 @@ import { Component } from '@angular/core'; } `], }) -export class SelectCleanComponent { -} +export class SelectCleanComponent {} diff --git a/src/playground/with-layout/select/select-showcase.component.html b/src/playground/with-layout/select/select-showcase.component.html index bdae8073a7..f60a93c9bc 100644 --- a/src/playground/with-layout/select/select-showcase.component.html +++ b/src/playground/with-layout/select/select-showcase.component.html @@ -1,7 +1,8 @@ + Option empty + Option 0 Option 1 Option 2 Option 3 Option 4 -