From fb0f80b6a3c5c15e37307b1a60323f858cac18f4 Mon Sep 17 00:00:00 2001 From: Georgi Anastasov Date: Mon, 16 Sep 2024 16:08:07 +0300 Subject: [PATCH 1/4] feat(combo): filter out non-existent items from selection --- .../src/lib/combo/combo.component.spec.ts | 75 +++++++++++++++++-- .../src/lib/combo/combo.component.ts | 62 ++++++++++++--- 2 files changed, 123 insertions(+), 14 deletions(-) diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts index c0c37334f3d..b9963f3512e 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts @@ -105,6 +105,7 @@ describe('igxCombo', () => { spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null); combo.ngOnInit(); expect(mockInjector.get).toHaveBeenCalledWith(NgControl, null); + combo.data = [{ id: 'test', name: 'test' }]; combo.registerOnChange(mockNgControl.registerOnChangeCb); combo.registerOnTouched(mockNgControl.registerOnTouchedCb); @@ -113,8 +114,8 @@ describe('igxCombo', () => { mockSelection.get.and.returnValue(new Set(['test'])); spyOnProperty(combo, 'isRemote').and.returnValue(false); combo.writeValue(['test']); - expect(mockNgControl.registerOnChangeCb).not.toHaveBeenCalled(); - expect(mockSelection.select_items).toHaveBeenCalledWith(combo.id, ['test'], true); + expect(mockNgControl.registerOnChangeCb).toHaveBeenCalled(); + expect(mockSelection.select_items).toHaveBeenCalledWith(combo.id, [], true); expect(combo.displayValue).toEqual('test'); expect(combo.value).toEqual(['test']); @@ -125,10 +126,11 @@ describe('igxCombo', () => { expect(combo.disabled).toBe(false); // OnChange callback + combo.data = [{ id: 'simpleValue', name: 'simpleValue' }]; mockSelection.add_items.and.returnValue(new Set(['simpleValue'])); combo.select(['simpleValue']); - expect(mockSelection.add_items).toHaveBeenCalledWith(combo.id, ['simpleValue'], undefined); - expect(mockSelection.select_items).toHaveBeenCalledWith(combo.id, ['simpleValue'], true); + expect(mockSelection.add_items).toHaveBeenCalledWith(combo.id, [], undefined); + expect(mockSelection.select_items).toHaveBeenCalledWith(combo.id, [], true); expect(mockNgControl.registerOnChangeCb).toHaveBeenCalledWith(['simpleValue']); // OnTouched callback @@ -258,7 +260,8 @@ describe('igxCombo', () => { ); spyOn(mockIconService, 'addSvgIconFromText').and.returnValue(null); combo.ngOnInit(); - combo.data = data; + combo.data = [{ id: 'EXAMPLE', name: 'Example' }]; + combo.valueKey = 'id'; mockSelection.select_items.calls.reset(); spyOnProperty(combo, 'isRemote').and.returnValue(false); combo.writeValue(['EXAMPLE']); @@ -2604,6 +2607,38 @@ describe('igxCombo', () => { expect(selectionSpy).toHaveBeenCalledWith(expectedResults); expect(input.nativeElement.value).toEqual(expectedDisplayText); }); + it('should only select and display valid values when programmatically selecting invalid items with ngModel', fakeAsync(() => { + fixture = TestBed.createComponent(ComboInvalidValuesComponent); + fixture.detectChanges(); + combo = fixture.componentInstance.combo; + const component = fixture.componentInstance; + tick(100); + + component.selectedItems = ['SF', 'LA', 'NY']; // 'SF' is invalid, 'LA' and 'NY' are valid + fixture.detectChanges(); + tick(100); + + expect(combo.selection).toEqual([{ name: 'Los Angeles', id: 'LA' }, { name: 'New York', id: 'NY' }]); + expect(combo.value).toEqual(['LA', 'NY']); + expect(combo.displayValue).toEqual('Los Angeles, New York'); + expect(component.selectedItems).toEqual(['LA', 'NY']); + })); + it('should only select and display valid values when selecting invalid items programmatically using select', fakeAsync(() => { + fixture = TestBed.createComponent(ComboInvalidValuesComponent); + fixture.detectChanges(); + combo = fixture.componentInstance.combo; + const component = fixture.componentInstance; + tick(100); + + combo.select(['SF', 'LA', 'NY']); // 'SF' is invalid, 'LA' and 'NY' are valid + fixture.detectChanges(); + tick(100); + + expect(combo.selection).toEqual([{ name: 'Los Angeles', id: 'LA' }, { name: 'New York', id: 'NY' }]); + expect(combo.value).toEqual(['LA', 'NY']); + expect(combo.displayValue).toEqual('Los Angeles, New York'); + expect(component.selectedItems).toEqual(['LA', 'NY']); + })); }); describe('Grouping tests: ', () => { beforeEach(() => { @@ -4087,3 +4122,33 @@ export class ComboWithIdComponent { ]; } } + +@Component({ + template: ` + + `, + standalone: true, + imports: [IgxComboComponent, FormsModule] +}) +export class ComboInvalidValuesComponent implements OnInit { + @ViewChild(IgxComboComponent, { read: IgxComboComponent, static: true }) + public combo: IgxComboComponent; + + public cities: any[] = []; + public selectedItems: any[] = []; + + constructor(private cdr: ChangeDetectorRef) { } + + public ngOnInit() { + this.cities = [ + { name: 'New York', id: 'NY' }, + { name: 'Los Angeles', id: 'LA' }, + { name: 'Chicago', id: 'CHI' } + ]; + this.cdr.detectChanges(); + } +} diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.ts b/projects/igniteui-angular/src/lib/combo/combo.component.ts index 2d3997eb75c..787283a06fb 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.ts @@ -1,6 +1,6 @@ import { DOCUMENT, NgClass, NgIf, NgTemplateOutlet } from '@angular/common'; import { - AfterViewInit, ChangeDetectorRef, Component, ElementRef, OnInit, OnDestroy, + AfterViewInit, ChangeDetectorRef, Component, ElementRef, OnInit, OnDestroy, OnChanges, SimpleChanges, Optional, Inject, Injector, ViewChild, Input, Output, EventEmitter, HostListener, DoCheck, booleanAttribute } from '@angular/core'; @@ -124,7 +124,7 @@ const diffInSets = (set1: Set, set2: Set): any[] => { ] }) export class IgxComboComponent extends IgxComboBaseDirective implements AfterViewInit, ControlValueAccessor, OnInit, - OnDestroy, DoCheck, EditorProvider { + OnDestroy, DoCheck, EditorProvider, OnChanges { /** * Whether the combo's search box should be focused after the dropdown is opened. * When `false`, the combo's list item container will be focused instead @@ -251,12 +251,12 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie * @hidden @internal */ public writeValue(value: any[]): void { - const selection = Array.isArray(value) ? value.filter(x => x !== undefined) : []; + this._value = Array.isArray(value) ? value.filter(x => x !== undefined) : []; const oldSelection = this.selection; - this.selectionService.select_items(this.id, selection, true); - this.cdr.markForCheck(); - this._displayValue = this.createDisplayText(this.selection, oldSelection); - this._value = this.valueKey ? this.selection.map(item => item[this.valueKey]) : this.selection; + + if (this.data && this.data.length > 0) { + this.applySelection(oldSelection); + } } /** @hidden @internal */ @@ -267,6 +267,14 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie } } + /** @hidden @internal */ + public ngOnChanges(changes: SimpleChanges): void { + if (changes['data'] && this.data && this.data.length > 0 && this._value && this._value.length) { + const oldSelection = this.selection; + this.applySelection(oldSelection); + } + } + /** * @hidden */ @@ -309,8 +317,16 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie * ``` */ public select(newItems: Array, clearCurrentSelection?: boolean, event?: Event) { - if (newItems) { - const newSelection = this.selectionService.add_items(this.id, newItems, clearCurrentSelection); + if (!newItems) { + return; + } + + if (this.isRemote || (this.data && this.data.length)) { + const validItems = !this.isRemote + ? newItems.filter(item => this.isItemInData(item)) + : newItems; + + const newSelection = this.selectionService.add_items(this.id, validItems, clearCurrentSelection); this.setSelection(newSelection, event); } } @@ -469,4 +485,32 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie selection.join(', '); return value; } + + private applySelection(oldSelection: any[]): void { + const selection = this._value || []; + const filteredSelection = this.isRemote ? selection : selection.filter(item => this.isItemInData(item)); + + this.selectionService.select_items(this.id, filteredSelection, true); + this.cdr.markForCheck(); + + this._displayValue = this.createDisplayText(this.selection, oldSelection); + this._value = this.valueKey ? this.selection.map(item => item[this.valueKey]) : this.selection; + + if (this._onChangeCallback) { + this._onChangeCallback(this._value); + } + } + + private isItemInData(item: any): boolean { + return this.data.some(dataItem => { + const dataValue = this.valueKey ? dataItem[this.valueKey] : dataItem; + const itemValue = this.valueKey ? item : item; + + if (Number.isNaN(dataValue) && Number.isNaN(itemValue)) { + return true; + } + + return dataValue === itemValue; + }); + } } From a4ca004b9d77c620775a403ed0d83ade95b49f01 Mon Sep 17 00:00:00 2001 From: Georgi Anastasov Date: Thu, 19 Sep 2024 17:44:14 +0300 Subject: [PATCH 2/4] feat(combo): refine selection logic to handle unmatched values --- .../src/lib/combo/combo.component.spec.ts | 5 +-- .../src/lib/combo/combo.component.ts | 26 ++++++------ .../igniteui-angular/src/lib/core/utils.ts | 41 +++++++++++++++++++ 3 files changed, 55 insertions(+), 17 deletions(-) diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts index b9963f3512e..f424090ed90 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts @@ -114,7 +114,7 @@ describe('igxCombo', () => { mockSelection.get.and.returnValue(new Set(['test'])); spyOnProperty(combo, 'isRemote').and.returnValue(false); combo.writeValue(['test']); - expect(mockNgControl.registerOnChangeCb).toHaveBeenCalled(); + expect(mockNgControl.registerOnChangeCb).not.toHaveBeenCalled(); expect(mockSelection.select_items).toHaveBeenCalledWith(combo.id, [], true); expect(combo.displayValue).toEqual('test'); expect(combo.value).toEqual(['test']); @@ -2621,7 +2621,6 @@ describe('igxCombo', () => { expect(combo.selection).toEqual([{ name: 'Los Angeles', id: 'LA' }, { name: 'New York', id: 'NY' }]); expect(combo.value).toEqual(['LA', 'NY']); expect(combo.displayValue).toEqual('Los Angeles, New York'); - expect(component.selectedItems).toEqual(['LA', 'NY']); })); it('should only select and display valid values when selecting invalid items programmatically using select', fakeAsync(() => { fixture = TestBed.createComponent(ComboInvalidValuesComponent); @@ -3370,7 +3369,7 @@ describe('igxCombo', () => { expect(combo.valid).toEqual(IgxInputState.INVALID); expect(combo.comboInput.valid).toEqual(IgxInputState.INVALID); - combo.select([combo.dropdown.items[0], combo.dropdown.items[1]]); + combo.select([combo.dropdown.items[0].value, combo.dropdown.items[1].value]); expect(combo.valid).toEqual(IgxInputState.VALID); expect(combo.comboInput.valid).toEqual(IgxInputState.VALID); diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.ts b/projects/igniteui-angular/src/lib/combo/combo.component.ts index 787283a06fb..9c9e8f14935 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.ts @@ -7,7 +7,7 @@ import { import { ControlValueAccessor, FormsModule, NG_VALUE_ACCESSOR } from '@angular/forms'; import { IgxSelectionAPIService } from '../core/selection'; -import { IBaseEventArgs, IBaseCancelableEventArgs, CancelableEventArgs } from '../core/utils'; +import { IBaseEventArgs, IBaseCancelableEventArgs, CancelableEventArgs, areObjectsEqual } from '../core/utils'; import { IgxStringFilteringOperand, IgxBooleanFilteringOperand } from '../data-operations/filtering-condition'; import { FilteringLogic } from '../data-operations/filtering-expression.interface'; import { IgxForOfDirective } from '../directives/for-of/for_of.directive'; @@ -253,7 +253,7 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie public writeValue(value: any[]): void { this._value = Array.isArray(value) ? value.filter(x => x !== undefined) : []; const oldSelection = this.selection; - + if (this.data && this.data.length > 0) { this.applySelection(oldSelection); } @@ -320,12 +320,12 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie if (!newItems) { return; } - + if (this.isRemote || (this.data && this.data.length)) { const validItems = !this.isRemote ? newItems.filter(item => this.isItemInData(item)) : newItems; - + const newSelection = this.selectionService.add_items(this.id, validItems, clearCurrentSelection); this.setSelection(newSelection, event); } @@ -489,27 +489,25 @@ export class IgxComboComponent extends IgxComboBaseDirective implements AfterVie private applySelection(oldSelection: any[]): void { const selection = this._value || []; const filteredSelection = this.isRemote ? selection : selection.filter(item => this.isItemInData(item)); - + this.selectionService.select_items(this.id, filteredSelection, true); this.cdr.markForCheck(); - + this._displayValue = this.createDisplayText(this.selection, oldSelection); this._value = this.valueKey ? this.selection.map(item => item[this.valueKey]) : this.selection; - - if (this._onChangeCallback) { - this._onChangeCallback(this._value); - } } private isItemInData(item: any): boolean { - return this.data.some(dataItem => { - const dataValue = this.valueKey ? dataItem[this.valueKey] : dataItem; - const itemValue = this.valueKey ? item : item; + if (!this.valueKey) { + return this.data.some(dataItem => areObjectsEqual(dataItem, item)); + } + return this.data.some(dataItem => { + const dataValue = dataItem[this.valueKey]; + const itemValue = item; if (Number.isNaN(dataValue) && Number.isNaN(itemValue)) { return true; } - return dataValue === itemValue; }); } diff --git a/projects/igniteui-angular/src/lib/core/utils.ts b/projects/igniteui-angular/src/lib/core/utils.ts index 4068f4cb29b..d6f7c9c6da6 100644 --- a/projects/igniteui-angular/src/lib/core/utils.ts +++ b/projects/igniteui-angular/src/lib/core/utils.ts @@ -93,6 +93,47 @@ export const mergeObjects = (obj1: any, obj2: any): any => mergeWith(obj1, obj2, } }); +/** + * Recursively checks if two objects are deeply equal. + * Handles circular references by keeping track of seen objects. + * + * @param obj1 First object to compare + * @param obj2 Second object to compare + * @param seen Set of already visited objects to handle circular references + * @returns true if objects are equal, false otherwise + * @hidden + */ +export const areObjectsEqual = (obj1: any, obj2: any, seen = new Set()): boolean => { + if (obj1 === obj2) return true; + + if ( + typeof obj1 !== 'object' || + obj1 === null || + typeof obj2 !== 'object' || + obj2 === null + ) { + return obj1 === obj2; + } + + if (seen.has(obj1) || seen.has(obj2)) { + return false; + } + seen.add(obj1); + seen.add(obj2); + + const keys1 = Object.keys(obj1); + const keys2 = Object.keys(obj2); + + if (keys1.length !== keys2.length) return false; + + for (const key of keys1) { + if (!Object.prototype.hasOwnProperty.call(obj2, key)) return false; + if (!areObjectsEqual(obj1[key], obj2[key], seen)) return false; + } + + return true; +}; + /** * Creates deep clone of provided value. * Supports primitive values, dates and objects. From 3f10d0902a50ef00acedd6fbe88220dea84195ce Mon Sep 17 00:00:00 2001 From: Georgi Anastasov Date: Thu, 19 Sep 2024 17:47:11 +0300 Subject: [PATCH 3/4] test(combo): remove unnecessary check --- projects/igniteui-angular/src/lib/combo/combo.component.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts index f424090ed90..61781fc0bf5 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts @@ -2636,7 +2636,6 @@ describe('igxCombo', () => { expect(combo.selection).toEqual([{ name: 'Los Angeles', id: 'LA' }, { name: 'New York', id: 'NY' }]); expect(combo.value).toEqual(['LA', 'NY']); expect(combo.displayValue).toEqual('Los Angeles, New York'); - expect(component.selectedItems).toEqual(['LA', 'NY']); })); }); describe('Grouping tests: ', () => { From d37b2881229fc624e66d163ac50ff61141000fe4 Mon Sep 17 00:00:00 2001 From: Georgi Anastasov Date: Thu, 19 Sep 2024 17:57:59 +0300 Subject: [PATCH 4/4] test(combo): remove unused 'component' variable --- projects/igniteui-angular/src/lib/combo/combo.component.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts index 61781fc0bf5..55ecc47c17f 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.component.spec.ts @@ -2626,7 +2626,6 @@ describe('igxCombo', () => { fixture = TestBed.createComponent(ComboInvalidValuesComponent); fixture.detectChanges(); combo = fixture.componentInstance.combo; - const component = fixture.componentInstance; tick(100); combo.select(['SF', 'LA', 'NY']); // 'SF' is invalid, 'LA' and 'NY' are valid