Skip to content

Commit ca4a1ff

Browse files
aefoxyggg
aefox
authored and
yggg
committed
fix(select): allow select to work with empty values in nb-option (#1282)
BREAKING CHANGE: Only 'null' and 'undefined' option values now considered as reset. false and falsy values such as 0, '', NaN don't reset select value anymore.
1 parent e5a7e82 commit ca4a1ff

File tree

6 files changed

+253
-17
lines changed

6 files changed

+253
-17
lines changed

‎src/framework/theme/components/select/option.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ export class NbOptionComponent<T> implements OnDestroy {
8282
* Determines should we render checkbox.
8383
* */
8484
get withCheckbox(): boolean {
85-
return this.multiple && !!this.value;
85+
return this.multiple && this.value != null;
8686
}
8787

8888
get content() {

‎src/framework/theme/components/select/select.component.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export class NbSelectLabelComponent {
7878
*
7979
* @stacked-example(Multiple, select/select-multiple.component)
8080
*
81-
* Items without values will clean selection.
81+
* Items without values will clean the selection. Both `null` and `undefined` values will also clean the selection.
8282
*
8383
* @stacked-example(Clean selection, select/select-clean.component)
8484
*
@@ -384,7 +384,7 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
384384
}
385385

386386
writeValue(value: T | T[]): void {
387-
if (!value || !this.alive) {
387+
if (!this.alive) {
388388
return;
389389
}
390390

@@ -399,10 +399,10 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent
399399
* Selects option or clear all selected options if value is null.
400400
* */
401401
protected handleOptionClick(option: NbOptionComponent<T>) {
402-
if (option.value) {
403-
this.selectOption(option);
404-
} else {
402+
if (option.value == null) {
405403
this.reset();
404+
} else {
405+
this.selectOption(option);
406406
}
407407

408408
this.cd.markForCheck();

‎src/framework/theme/components/select/select.spec.ts

Lines changed: 236 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* Licensed under the MIT License. See License.txt in the project root for license information.
55
*/
66

7-
import { Component, EventEmitter, Input, Output, QueryList, ViewChild, ViewChildren } from '@angular/core';
7+
import { Component, ElementRef, EventEmitter, Input, Output, QueryList, ViewChild, ViewChildren } from '@angular/core';
88
import { ComponentFixture, fakeAsync, flush, TestBed } from '@angular/core/testing';
99
import { FormControl, FormsModule, ReactiveFormsModule } from '@angular/forms';
1010
import { RouterTestingModule } from '@angular/router/testing';
@@ -45,6 +45,15 @@ const TEST_GROUPS = [
4545
{ title: 'Option 33', value: 'Option 33' },
4646
],
4747
},
48+
{
49+
title: 'Group 4',
50+
options: [
51+
{ title: 'Option 41', value: '' },
52+
{ title: 'Option 42', value: '0' },
53+
{ title: 'Option 43', value: 0 },
54+
{ title: 'Option 44'},
55+
],
56+
},
4857
];
4958

5059
@Component({
@@ -139,6 +148,105 @@ export class NbNgModelSelectComponent {
139148
@ViewChild(NbOptionComponent) optionComponent: NbOptionComponent<number>;
140149
}
141150

151+
@Component({
152+
template: `
153+
<nb-layout>
154+
<nb-layout-column>
155+
156+
<nb-select>
157+
<nb-option>No value option</nb-option>
158+
<nb-option [value]="null">undefined value</nb-option>
159+
<nb-option [value]="undefined">undefined value</nb-option>
160+
<nb-option [value]="false">false value</nb-option>
161+
<nb-option [value]="0">0 value</nb-option>
162+
<nb-option [value]="''">empty string value</nb-option>
163+
<nb-option [value]="nanValue">NaN value</nb-option>
164+
<nb-option value="1">truthy value</nb-option>
165+
</nb-select>
166+
167+
</nb-layout-column>
168+
</nb-layout>
169+
`,
170+
})
171+
export class NbSelectWithFalsyOptionValuesComponent {
172+
nanValue = NaN;
173+
174+
@ViewChild(NbSelectComponent) select: NbSelectComponent<any>;
175+
@ViewChildren(NbOptionComponent) options: QueryList<NbOptionComponent<any>>;
176+
@ViewChildren(NbOptionComponent, { read: ElementRef }) optionElements: QueryList<ElementRef<HTMLElement>>;
177+
178+
get noValueOption(): NbOptionComponent<any> {
179+
return this.options.toArray()[0];
180+
}
181+
get noValueOptionElement(): ElementRef<HTMLElement> {
182+
return this.optionElements.toArray()[0];
183+
}
184+
get nullOption(): NbOptionComponent<any> {
185+
return this.options.toArray()[1];
186+
}
187+
get nullOptionElement(): ElementRef<HTMLElement> {
188+
return this.optionElements.toArray()[1];
189+
}
190+
get undefinedOption(): NbOptionComponent<any> {
191+
return this.options.toArray()[2];
192+
}
193+
get undefinedOptionElement(): ElementRef<HTMLElement> {
194+
return this.optionElements.toArray()[2];
195+
}
196+
get falseOption(): NbOptionComponent<any> {
197+
return this.options.toArray()[3];
198+
}
199+
get falseOptionElement(): ElementRef<HTMLElement> {
200+
return this.optionElements.toArray()[3];
201+
}
202+
get zeroOption(): NbOptionComponent<any> {
203+
return this.options.toArray()[4];
204+
}
205+
get zeroOptionElement(): ElementRef<HTMLElement> {
206+
return this.optionElements.toArray()[4];
207+
}
208+
get emptyStringOption(): NbOptionComponent<any> {
209+
return this.options.toArray()[5];
210+
}
211+
get emptyStringOptionElement(): ElementRef<HTMLElement> {
212+
return this.optionElements.toArray()[5];
213+
}
214+
get nanOption(): NbOptionComponent<any> {
215+
return this.options.toArray()[6];
216+
}
217+
get nanOptionElement(): ElementRef<HTMLElement> {
218+
return this.optionElements.toArray()[6];
219+
}
220+
get truthyOption(): NbOptionComponent<any> {
221+
return this.options.toArray()[7];
222+
}
223+
get truthyOptionElement(): ElementRef<HTMLElement> {
224+
return this.optionElements.toArray()[7];
225+
}
226+
}
227+
228+
@Component({
229+
template: `
230+
<nb-layout>
231+
<nb-layout-column>
232+
233+
<nb-select multiple>
234+
<nb-option>No value option</nb-option>
235+
<nb-option [value]="null">undefined value</nb-option>
236+
<nb-option [value]="undefined">undefined value</nb-option>
237+
<nb-option [value]="false">false value</nb-option>
238+
<nb-option [value]="0">0 value</nb-option>
239+
<nb-option [value]="''">empty string value</nb-option>
240+
<nb-option [value]="nanValue">NaN value</nb-option>
241+
<nb-option value="1">truthy value</nb-option>
242+
</nb-select>
243+
244+
</nb-layout-column>
245+
</nb-layout>
246+
`,
247+
})
248+
export class NbMultipleSelectWithFalsyOptionValuesComponent extends NbSelectWithFalsyOptionValuesComponent {}
249+
142250
describe('Component: NbSelectComponent', () => {
143251
let fixture: ComponentFixture<NbSelectTestComponent>;
144252
let overlayContainerService: NbOverlayContainerAdapter;
@@ -290,7 +398,7 @@ describe('Component: NbSelectComponent', () => {
290398

291399
const button = fixture.nativeElement.querySelector('button');
292400
expect(button.textContent).toContain('1 noitpO');
293-
})
401+
});
294402
});
295403

296404
it('should select initially specified value without errors', fakeAsync(() => {
@@ -342,7 +450,7 @@ describe('Component: NbSelectComponent', () => {
342450
selectFixture.detectChanges();
343451
flush();
344452

345-
const optionToSelect = testComponent.options.last;
453+
const optionToSelect = testComponent.options.find(o => o.value != null);
346454
const optionSelectSpy = spyOn(optionToSelect, 'select').and.callThrough();
347455

348456
expect(optionToSelect.selected).toEqual(false);
@@ -417,6 +525,131 @@ describe('Component: NbSelectComponent', () => {
417525
});
418526
});
419527

528+
describe('NbSelectComponent - falsy values', () => {
529+
let fixture: ComponentFixture<NbSelectWithFalsyOptionValuesComponent>;
530+
let testComponent: NbSelectWithFalsyOptionValuesComponent;
531+
let select: NbSelectComponent<any>;
532+
533+
beforeEach(fakeAsync(() => {
534+
TestBed.configureTestingModule({
535+
imports: [
536+
RouterTestingModule.withRoutes([]),
537+
NbThemeModule.forRoot(),
538+
NbLayoutModule,
539+
NbSelectModule,
540+
],
541+
declarations: [
542+
NbSelectWithFalsyOptionValuesComponent,
543+
NbMultipleSelectWithFalsyOptionValuesComponent,
544+
],
545+
});
546+
547+
fixture = TestBed.createComponent(NbSelectWithFalsyOptionValuesComponent);
548+
testComponent = fixture.componentInstance;
549+
select = testComponent.select;
550+
551+
fixture.detectChanges();
552+
flush();
553+
}));
554+
555+
it('should clean selection when selected option does not have a value', fakeAsync(() => {
556+
select.setSelected = testComponent.truthyOption.value;
557+
fixture.detectChanges();
558+
559+
testComponent.noValueOption.onClick();
560+
fixture.detectChanges();
561+
562+
expect(select.selectionModel.length).toEqual(0);
563+
}));
564+
565+
it('should clean selection when selected option has null value', fakeAsync(() => {
566+
select.setSelected = testComponent.truthyOption.value;
567+
fixture.detectChanges();
568+
569+
testComponent.nullOption.onClick();
570+
fixture.detectChanges();
571+
572+
expect(select.selectionModel.length).toEqual(0);
573+
}));
574+
575+
it('should clean selection when selected option has undefined value', fakeAsync(() => {
576+
select.setSelected = testComponent.truthyOption.value;
577+
fixture.detectChanges();
578+
579+
testComponent.undefinedOption.onClick();
580+
fixture.detectChanges();
581+
582+
expect(select.selectionModel.length).toEqual(0);
583+
}));
584+
585+
it('should not reset selection when selected option has false value', fakeAsync(() => {
586+
select.setSelected = testComponent.truthyOption.value;
587+
fixture.detectChanges();
588+
589+
testComponent.falseOption.onClick();
590+
fixture.detectChanges();
591+
592+
expect(select.selectionModel.length).toEqual(1);
593+
}));
594+
595+
it('should not reset selection when selected option has zero value', fakeAsync(() => {
596+
select.setSelected = testComponent.truthyOption.value;
597+
fixture.detectChanges();
598+
599+
testComponent.zeroOption.onClick();
600+
fixture.detectChanges();
601+
602+
expect(select.selectionModel.length).toEqual(1);
603+
}));
604+
605+
it('should not reset selection when selected option has empty string value', fakeAsync(() => {
606+
select.setSelected = testComponent.truthyOption.value;
607+
fixture.detectChanges();
608+
609+
testComponent.emptyStringOption.onClick();
610+
fixture.detectChanges();
611+
612+
expect(select.selectionModel.length).toEqual(1);
613+
}));
614+
615+
it('should not reset selection when selected option has NaN value', fakeAsync(() => {
616+
select.setSelected = testComponent.truthyOption.value;
617+
fixture.detectChanges();
618+
619+
testComponent.nanOption.onClick();
620+
fixture.detectChanges();
621+
622+
expect(select.selectionModel.length).toEqual(1);
623+
}));
624+
625+
describe('multiple', () => {
626+
beforeEach(fakeAsync(() => {
627+
fixture = TestBed.createComponent(NbMultipleSelectWithFalsyOptionValuesComponent);
628+
testComponent = fixture.componentInstance;
629+
select = testComponent.select;
630+
631+
fixture.detectChanges();
632+
flush();
633+
select.show();
634+
fixture.detectChanges();
635+
}));
636+
637+
it('should not render checkbox on options with reset values', () => {
638+
expect(testComponent.noValueOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null);
639+
expect(testComponent.nullOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null);
640+
expect(testComponent.undefinedOptionElement.nativeElement.querySelector('nb-checkbox')).toEqual(null);
641+
});
642+
643+
it('should render checkbox on options with falsy non-reset values', () => {
644+
expect(testComponent.falseOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null);
645+
expect(testComponent.zeroOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null);
646+
expect(testComponent.emptyStringOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null);
647+
expect(testComponent.nanOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null);
648+
expect(testComponent.truthyOptionElement.nativeElement.querySelector('nb-checkbox')).not.toEqual(null);
649+
});
650+
});
651+
});
652+
420653
describe('NbOptionComponent', () => {
421654
let fixture: ComponentFixture<NbReactiveFormSelectComponent>;
422655
let testSelectComponent: NbReactiveFormSelectComponent;

‎src/playground/with-layout/select/select-clean.component.html

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
<nb-select placeholder="Cleanable">
2-
<nb-option>Clean</nb-option>
3-
<nb-option value="1">Option 1</nb-option>
4-
<nb-option value="2">Option 2</nb-option>
5-
<nb-option value="3">Option 3</nb-option>
6-
<nb-option value="4">Option 4</nb-option>
2+
<nb-option>Clean with option without value</nb-option>
3+
<nb-option [value]="null">Clean with null value</nb-option>
4+
<nb-option [value]="undefined">Clean with null value</nb-option>
5+
<nb-option [value]="0">Option 0</nb-option>
6+
<nb-option [value]="1">Option 1</nb-option>
7+
<nb-option [value]="2">Option 2</nb-option>
8+
<nb-option [value]="3">Option 3</nb-option>
9+
<nb-option [value]="4">Option 4</nb-option>
710
</nb-select>

‎src/playground/with-layout/select/select-clean.component.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,4 @@ import { Component } from '@angular/core';
2222
}
2323
`],
2424
})
25-
export class SelectCleanComponent {
26-
}
25+
export class SelectCleanComponent {}

‎src/playground/with-layout/select/select-showcase.component.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
<nb-select placeholder="Select Showcase" [(selected)]="selectedItem">
2+
<nb-option value="">Option empty</nb-option>
3+
<nb-option value="0">Option 0</nb-option>
24
<nb-option value="1">Option 1</nb-option>
35
<nb-option value="2">Option 2</nb-option>
46
<nb-option value="3">Option 3</nb-option>
57
<nb-option value="4">Option 4</nb-option>
68
</nb-select>
7-

0 commit comments

Comments
 (0)