Skip to content

Commit

Permalink
fix(groupBy): treat empty string as valid group
Browse files Browse the repository at this point in the history
  • Loading branch information
@varnastadeus
varnastadeus committed Jun 1, 2018
1 parent df3475c commit acef714
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 46 deletions.
35 changes: 21 additions & 14 deletions demo/app/examples/groups.component.ts
View file
Expand Up @@ -11,11 +11,14 @@ import { Component, ChangeDetectionStrategy } from '@angular/core';
<label>Default group by key</label>
---html,true
<ng-select [items]="accounts"
bindLabel="name"
bindValue="name"
groupBy="country"
[multiple]="true"
[(ngModel)]="selectedAccount">
bindLabel="name"
bindValue="name"
groupBy="country"
[multiple]="true"
[(ngModel)]="selectedAccount">
<ng-template ng-optgroup-tmp let-item="item">
{{item.country || 'Unnamed group'}}
</ng-template>
</ng-select>
---
<p>
Expand All @@ -25,11 +28,11 @@ import { Component, ChangeDetectionStrategy } from '@angular/core';
<label>Group by function expression</label>
---html,true
<ng-select [items]="accounts2"
bindLabel="name"
bindValue="name"
[groupBy]="groupByFn"
[multiple]="true"
[(ngModel)]="selectedAccount2">
bindLabel="name"
bindValue="name"
[groupBy]="groupByFn"
[multiple]="true"
[(ngModel)]="selectedAccount2">
</ng-select>
---
<p>
Expand All @@ -40,10 +43,13 @@ import { Component, ChangeDetectionStrategy } from '@angular/core';
<label>With selectable groups</label>
---html,true
<ng-select [items]="accounts3"
bindLabel="name"
groupBy="country"
[selectableGroup]="true"
[(ngModel)]="selectedAccount3">
bindLabel="name"
groupBy="country"
[selectableGroup]="true"
[(ngModel)]="selectedAccount3">
<ng-template ng-optgroup-tmp let-item="item">
{{item.country || 'Unnamed group'}}
</ng-template>
</ng-select>
---
<p>
Expand All @@ -58,6 +64,7 @@ export class SelectGroupsComponent {
{ name: 'Jill', email: 'jill@email.com', age: 15, child: { state: 'Active' } },
{ name: 'Henry', email: 'henry@email.com', age: 10, child: { state: 'Active' } },
{ name: 'Meg', email: 'meg@email.com', age: 7, country: null, child: { state: 'Active' } },
{ name: 'Homer', email: 'homer@email.com', age: 47, country: '', child: { state: 'Active' } },
{ name: 'Samantha', email: 'samantha@email.com', age: 30, country: 'United States', child: { state: 'Active' } },
{ name: 'Amalie', email: 'amalie@email.com', age: 12, country: 'Argentina', child: { state: 'Active' } },
{ name: 'Estefanía', email: 'estefania@email.com', age: 21, country: 'Argentina', child: { state: 'Active' } },
Expand Down
14 changes: 7 additions & 7 deletions src/ng-select/items-list.ts
View file
Expand Up @@ -271,17 +271,17 @@ export class ItemsList {

private _groupBy(items: NgOption[], prop: string | Function): OptionGroups {
const isFn = isFunction(this._ngSelect.groupBy);
const groups = items.reduce((grouped, item) => {
const groups = new Map<string, NgOption[]>();
for (const item of items) {
let key = isFn ? (<Function>prop).apply(this, [item.value]) : item.value[<string>prop];
key = key || undefined;
const group = grouped.get(key);
key = isDefined(key) ? key : undefined;
const group = groups.get(key);
if (group) {
group.push(item);
} else {
grouped.set(key, [item]);
groups.set(key, [item]);
}
return grouped;
}, new Map<string, NgOption[]>());
}
return groups;
}

Expand All @@ -292,7 +292,7 @@ export class ItemsList {
items.push(...withoutGroup);
let i = withoutGroup.length;
for (const key of Array.from(groups.keys())) {
if (!key) {
if (!isDefined(key)) {
continue;
}
const parent: NgOption = {
Expand Down
53 changes: 28 additions & 25 deletions src/ng-select/ng-select.component.spec.ts
View file
Expand Up @@ -424,7 +424,7 @@ describe('NgSelectComponent', function () {

fixture.componentInstance.selectedCity = { name: 'New city', id: 5 };
tickAndDetectChanges(fixture);
fixture.componentInstance.cities = [...fixture.componentInstance.cities]
fixture.componentInstance.cities = [...fixture.componentInstance.cities];
tickAndDetectChanges(fixture);
expect(fixture.componentInstance.select.itemsList.markedItem.value).toEqual({ name: 'Vilnius', id: 1 });
}));
Expand Down Expand Up @@ -552,7 +552,7 @@ describe('NgSelectComponent', function () {
expect(fixture.componentInstance.selectedCityId).toEqual(1);

// from model to component
fixture.componentInstance.selectedCityId = 2
fixture.componentInstance.selectedCityId = 2;
tickAndDetectChanges(fixture);

expect(fixture.componentInstance.select.selectedItems).toEqual([jasmine.objectContaining({
Expand Down Expand Up @@ -741,8 +741,7 @@ describe('NgSelectComponent', function () {
</ng-select>`);

const cmp = fixture.componentInstance;
const cityId = cmp.cities[1].id.toString();
cmp.selectedCityId = cityId;
cmp.selectedCityId = cmp.cities[1].id.toString();

cmp.compareWith = (city, model: string) => city.id === +model;

Expand Down Expand Up @@ -1022,7 +1021,7 @@ describe('NgSelectComponent', function () {

it('should stop marked loop if all items disabled', fakeAsync(() => {
fixture.componentInstance.cities[0].disabled = true;
fixture.componentInstance.cities = [...fixture.componentInstance.cities]
fixture.componentInstance.cities = [...fixture.componentInstance.cities];
tickAndDetectChanges(fixture);
select.filter('vil');
tickAndDetectChanges(fixture);
Expand Down Expand Up @@ -1119,7 +1118,7 @@ describe('NgSelectComponent', function () {
const result = jasmine.objectContaining({
value: fixture.componentInstance.cities[2]
});
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result)
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result);
triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Tab);
expect(fixture.componentInstance.select.selectedItems).toEqual([result]);
}));
Expand Down Expand Up @@ -1201,7 +1200,7 @@ describe('NgSelectComponent', function () {
tick(200);

expect(fixture.componentInstance.selectedCity).toBeUndefined();
expect(select.itemsList.markedItem.label).toBe('Vilnius')
expect(select.itemsList.markedItem.label).toBe('Vilnius');
expect(findByLabel).toHaveBeenCalledWith('vil')
}));
});
Expand All @@ -1215,7 +1214,7 @@ describe('NgSelectComponent', function () {
it('should select option and close dropdown', () => {
triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Space);
triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Enter);
expect(select.selectedItems[0].value).toEqual(fixture.componentInstance.cities[0])
expect(select.selectedItems[0].value).toEqual(fixture.componentInstance.cities[0]);
expect(select.isOpen).toBe(false);
});
});
Expand Down Expand Up @@ -1347,7 +1346,7 @@ describe('NgSelectComponent', function () {
fixture.detectChanges();
expect(fixture.componentInstance.select.selectedItems.length).toBe(1);

fixture.componentInstance.select.clearItem(fixture.componentInstance.cities[0])
fixture.componentInstance.select.clearItem(fixture.componentInstance.cities[0]);
expect(fixture.componentInstance.select.selectedItems.length).toBe(0);
tick();
}));
Expand All @@ -1367,7 +1366,7 @@ describe('NgSelectComponent', function () {
fixture.componentInstance.cities = [];
fixture.detectChanges();

fixture.componentInstance.select.clearItem(selected)
fixture.componentInstance.select.clearItem(selected);
expect(fixture.componentInstance.select.selectedItems.length).toBe(0);
tick();
}));
Expand Down Expand Up @@ -1671,9 +1670,9 @@ describe('NgSelectComponent', function () {
it('should keep same ordering while unselecting', fakeAsync(() => {
fixture.componentInstance.selectedCities = [...fixture.componentInstance.cities.reverse()];
tickAndDetectChanges(fixture);
select.unselect(select.selectedItems[0])
select.unselect(select.selectedItems[0])
select.unselect(select.selectedItems[0])
select.unselect(select.selectedItems[0]);
select.unselect(select.selectedItems[0]);
select.unselect(select.selectedItems[0]);
expect(select.selectedItems.length).toBe(0);
expect(select.itemsList.filteredItems.length).toBe(3);
expect(select.itemsList.filteredItems[0].label).toBe('Vilnius');
Expand Down Expand Up @@ -1840,7 +1839,7 @@ describe('NgSelectComponent', function () {
tickAndDetectChanges(fixture);
tickAndDetectChanges(fixture);
const selectEl: HTMLElement = select.elementRef.nativeElement;
const ngControl = selectEl.querySelector('.ng-select-container')
const ngControl = selectEl.querySelector('.ng-select-container');
const placeholder: any = selectEl.querySelector('.ng-placeholder');
expect(ngControl.classList.contains('ng-has-value')).toBeTruthy();

Expand All @@ -1855,7 +1854,7 @@ describe('NgSelectComponent', function () {
it('should contain .ng-has-value when value was selected', fakeAsync(() => {
tickAndDetectChanges(fixture);
const selectEl: HTMLElement = fixture.componentInstance.select.elementRef.nativeElement;
const ngControl = selectEl.querySelector('.ng-select-container')
const ngControl = selectEl.querySelector('.ng-select-container');
selectOption(fixture, KeyCode.ArrowDown, 2);
tickAndDetectChanges(fixture);
expect(ngControl.classList.contains('ng-has-value')).toBeTruthy();
Expand Down Expand Up @@ -1963,7 +1962,7 @@ describe('NgSelectComponent', function () {
const result = jasmine.objectContaining({
value: fixture.componentInstance.cities[2]
});
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result)
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result);
triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Enter);
expect(fixture.componentInstance.select.selectedItems).toEqual([result]);
}));
Expand All @@ -1984,7 +1983,7 @@ describe('NgSelectComponent', function () {
const result = jasmine.objectContaining({
value: fixture.componentInstance.cities[2]
});
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result)
expect(fixture.componentInstance.select.itemsList.markedItem).toEqual(result);
triggerKeyDownEvent(getNgSelectElement(fixture), KeyCode.Enter);
expect(fixture.componentInstance.select.selectedItems).toEqual([result]);
}));
Expand Down Expand Up @@ -2097,7 +2096,7 @@ describe('NgSelectComponent', function () {
}));

describe('with typeahead', () => {
let fixture: ComponentFixture<NgSelectTestCmp>
let fixture: ComponentFixture<NgSelectTestCmp>;
beforeEach(() => {
fixture = createTestingModule(
NgSelectTestCmp,
Expand Down Expand Up @@ -2447,7 +2446,7 @@ describe('NgSelectComponent', function () {
tickAndDetectChanges(fixture);
tickAndDetectChanges(fixture);
triggerMousedown = () => {
const control = fixture.debugElement.query(By.css('.ng-select-container'))
const control = fixture.debugElement.query(By.css('.ng-select-container'));
control.triggerEventHandler('mousedown', createEvent({ target: { className: 'ng-control' } }));
};
}));
Expand Down Expand Up @@ -2476,7 +2475,7 @@ describe('NgSelectComponent', function () {
tickAndDetectChanges(fixture);
tickAndDetectChanges(fixture);
triggerMousedown = () => {
const control = fixture.debugElement.query(By.css('.ng-select-container'))
const control = fixture.debugElement.query(By.css('.ng-select-container'));
control.triggerEventHandler('mousedown', createEvent({ target: { className: 'ng-clear' } }));
};
}));
Expand Down Expand Up @@ -2528,7 +2527,7 @@ describe('NgSelectComponent', function () {
tickAndDetectChanges(fixture);
tickAndDetectChanges(fixture);
triggerMousedown = () => {
const control = fixture.debugElement.query(By.css('.ng-select-container'))
const control = fixture.debugElement.query(By.css('.ng-select-container'));
control.triggerEventHandler('mousedown', createEvent({ target: { className: 'ng-value-icon' } }));
};
}));
Expand Down Expand Up @@ -2559,7 +2558,7 @@ describe('NgSelectComponent', function () {
fixture.componentInstance.selectedCity = fixture.componentInstance.cities[0];
tickAndDetectChanges(fixture);
triggerMousedown = () => {
const control = fixture.debugElement.query(By.css('.ng-select-container'))
const control = fixture.debugElement.query(By.css('.ng-select-container'));
control.triggerEventHandler('mousedown', createEvent({ target: { className: 'ng-arrow' } }));
};
}));
Expand Down Expand Up @@ -2670,16 +2669,20 @@ describe('NgSelectComponent', function () {
fixture.componentInstance.accounts.push(
<any>{ name: 'Henry', email: 'henry@email.com', age: 10 },
<any>{ name: 'Meg', email: 'meg@email.com', age: 7, country: null },
<any>{ name: 'Meg', email: 'meg@email.com', age: 7, country: '' },
);
fixture.componentInstance.accounts = [...fixture.componentInstance.accounts]
fixture.componentInstance.accounts = [...fixture.componentInstance.accounts];
tickAndDetectChanges(fixture);

const items = fixture.componentInstance.select.itemsList.items;
expect(items.length).toBe(16);
const items: NgOption[] = fixture.componentInstance.select.itemsList.items;
expect(items.length).toBe(18);
expect(items[0].hasChildren).toBeUndefined();
expect(items[0].parent).toBeUndefined();
expect(items[1].hasChildren).toBeUndefined();
expect(items[1].parent).toBeUndefined();
expect(items[16].hasChildren).toBeTruthy();
expect(items[16].label).toBe('');
expect(items[17].parent).toBeDefined();
}));

it('should group by group fn', fakeAsync(() => {
Expand Down

0 comments on commit acef714

Please sign in to comment.