From 7b9d9fca3b1a87038522216072e19aeec08e6f16 Mon Sep 17 00:00:00 2001 From: Martin Dragnev Date: Thu, 3 Oct 2024 16:46:18 +0300 Subject: [PATCH] refactor(*): Remove the need of hardcoded item heights chore(*): fix lint chore(*): fix another lint chore(*): Clean up code. Change some tests and remove some chore(*): Some more cleanup. Fix tests --- .../src/lib/combo/combo-dropdown.component.ts | 3 ++ .../src/lib/combo/combo.common.ts | 50 ++++++++----------- .../src/lib/combo/combo.component.html | 2 +- .../src/lib/combo/combo.component.spec.ts | 28 ++--------- .../styles/components/combo/_combo-theme.scss | 1 + .../lib/directives/for-of/for_of.directive.ts | 10 +++- .../lib/directives/toggle/toggle.directive.ts | 13 +++++ .../src/lib/drop-down/drop-down.component.ts | 8 +++ .../simple-combo/simple-combo.component.html | 2 +- .../simple-combo.component.spec.ts | 33 ------------ 10 files changed, 60 insertions(+), 90 deletions(-) diff --git a/projects/igniteui-angular/src/lib/combo/combo-dropdown.component.ts b/projects/igniteui-angular/src/lib/combo/combo-dropdown.component.ts index bb2452ce6da..040860d7886 100644 --- a/projects/igniteui-angular/src/lib/combo/combo-dropdown.component.ts +++ b/projects/igniteui-angular/src/lib/combo/combo-dropdown.component.ts @@ -184,6 +184,9 @@ export class IgxComboDropDownComponent extends IgxDropDownComponent implements I public override ngAfterViewInit() { this.virtDir.getScroll().addEventListener('scroll', this.scrollHandler); + this.toggleDirective?.animationStarting.subscribe((_args: any) => { + this.animationStarting.emit(_args); + }); } /** diff --git a/projects/igniteui-angular/src/lib/combo/combo.common.ts b/projects/igniteui-angular/src/lib/combo/combo.common.ts index 5822be6ce78..4b0039ea4ad 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.common.ts +++ b/projects/igniteui-angular/src/lib/combo/combo.common.ts @@ -45,8 +45,8 @@ import { IComboItemAdditionEvent, IComboSearchInputEventArgs } from './public_ap import { ComboResourceStringsEN, IComboResourceStrings } from '../core/i18n/combo-resources'; import { getCurrentResourceStrings } from '../core/i18n/resources'; import { DOCUMENT } from '@angular/common'; -import { Size } from '../grids/common/enums'; import { isEqual } from 'lodash-es'; +import { ToggleViewEventArgs } from '../directives/toggle/toggle.directive'; export const IGX_COMBO_COMPONENT = /*@__PURE__*/new InjectionToken('IgxComboComponentToken'); @@ -82,19 +82,6 @@ export interface IgxComboBase { let NEXT_ID = 0; -/** - * @hidden - * The default number of items that should be in the combo's - * drop-down list if no `[itemsMaxHeight]` is specified - */ -const itemsInContainer = 10; // TODO: make private readonly - -/** @hidden @internal */ -const ItemHeights = { - "3": 40, - "2": 32, - "1": 28 -}; /** @hidden @internal */ export const enum DataTypes { @@ -234,8 +221,8 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh */ @Input() public get itemsMaxHeight(): number { - if (this._itemsMaxHeight === null || this._itemsMaxHeight === undefined) { - return this.itemHeight * itemsInContainer; + if (this.itemHeight && !this._itemsMaxHeight) { + return this.itemHeight * this.itemsInCointaner; } return this._itemsMaxHeight; } @@ -246,15 +233,9 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh /** @hidden */ public get itemsMaxHeightInRem() { - return rem(this.itemsMaxHeight); - } - - /** - * @hidden - * @internal - */ - public get comboSize(): Size { - return this.computedStyles?.getPropertyValue('--ig-size') || Size.Large; + if (this.itemsMaxHeight) { + return rem(this.itemsMaxHeight); + } } /** @@ -272,9 +253,6 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh */ @Input() public get itemHeight(): number { - if (this._itemHeight === null || this._itemHeight === undefined) { - return ItemHeights[this.comboSize]; - } return this._itemHeight; } @@ -943,6 +921,9 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh public set filteringOptions(value: IComboFilteringOptions) { this._filteringOptions = value; } + + protected containerSize = undefined; + protected itemSize = undefined; protected _data = []; protected _value = []; protected _displayValue = ''; @@ -963,12 +944,13 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh private _id: string = `igx-combo-${NEXT_ID++}`; private _type = null; private _dataType = ''; - private _itemHeight = null; + private _itemHeight = undefined; private _itemsMaxHeight = null; private _overlaySettings: OverlaySettings; private _groupSortingDirection: SortingDirection = SortingDirection.Asc; private _filteringOptions: IComboFilteringOptions; private _defaultFilteringOptions: IComboFilteringOptions = { caseSensitive: false }; + private itemsInCointaner = 10; public abstract dropdown: IgxComboDropDownComponent; public abstract selectionChanging: EventEmitter; @@ -1029,6 +1011,16 @@ export abstract class IgxComboBaseDirective implements IgxComboBase, AfterViewCh const eventArgs: IForOfState = Object.assign({}, e, { owner: this }); this.dataPreLoad.emit(eventArgs); }); + this.dropdown?.animationStarting.subscribe((_args: ToggleViewEventArgs) => { + // calculate the container size and item size based on the sizes from the DOM + const dropdownContainerHeight = this.dropdownContainer.nativeElement.getBoundingClientRect().height; + if (dropdownContainerHeight) { + this.containerSize = parseFloat(dropdownContainerHeight); + } + if (this.dropdown.children?.first) { + this.itemSize = this.dropdown.children.first.element.nativeElement.getBoundingClientRect().height; + } + }); } /** @hidden @internal */ diff --git a/projects/igniteui-angular/src/lib/combo/combo.component.html b/projects/igniteui-angular/src/lib/combo/combo.component.html index 33434403200..2b8d3fc3255 100644 --- a/projects/igniteui-angular/src/lib/combo/combo.component.html +++ b/projects/igniteui-angular/src/lib/combo/combo.component.html @@ -61,7 +61,7 @@ { expect(combo.displayKey).toEqual('field'); expect(combo.groupKey).toEqual('region'); expect(combo.width).toEqual('400px'); - expect(combo.itemsMaxHeight).toEqual(320); - expect(combo.itemHeight).toEqual(32); expect(combo.placeholder).toEqual('Location'); expect(combo.disableFiltering).toEqual(false); expect(combo.allowCustomValues).toEqual(false); @@ -1125,9 +1123,7 @@ describe('igxCombo', () => { const dropdownList = fixture.debugElement.query(By.css(`.${CSS_CLASS_CONTENT}`)); const verifyDropdownItemHeight = () => { - expect(combo.itemHeight).toEqual(itemHeight); expect(dropdownItems[0].nativeElement.clientHeight).toEqual(itemHeight); - expect(combo.itemsMaxHeight).toEqual(itemMaxHeight); expect(dropdownList.nativeElement.clientHeight).toEqual(itemMaxHeight); }; verifyDropdownItemHeight(); @@ -1764,11 +1760,10 @@ describe('igxCombo', () => { }); it('should focus item when onFocus and onBlur are called', () => { expect(dropdown.focusedItem).toEqual(null); - expect(dropdown.items.length).toEqual(9); dropdown.toggle(); fixture.detectChanges(); expect(dropdown.items).toBeDefined(); - expect(dropdown.items.length).toBeTruthy(); + expect(dropdown.items.length).toEqual(9); dropdown.onFocus(); expect(dropdown.focusedItem).toEqual(dropdown.items[0]); expect(dropdown.focusedItem.focused).toEqual(true); @@ -2015,7 +2010,8 @@ describe('igxCombo', () => { expect(combo.dropdown.onToggleOpened).toHaveBeenCalledTimes(1); let vContainerScrollHeight = virtDir.getScroll().scrollHeight; expect(virtDir.getScroll().scrollTop).toEqual(0); - expect(vContainerScrollHeight).toBeGreaterThan(combo.itemHeight); + const itemHeight = parseFloat(combo.dropdown.children.first.element.nativeElement.getBoundingClientRect().height); + expect(vContainerScrollHeight).toBeGreaterThan(itemHeight); virtDir.getScroll().scrollTop = Math.floor(vContainerScrollHeight / 2); await firstValueFrom(combo.virtualScrollContainer.chunkLoad); fixture.detectChanges(); @@ -3573,24 +3569,6 @@ describe('igxCombo', () => { })); }); }); - describe('Display density', () => { - beforeEach(() => { - fixture = TestBed.createComponent(IgxComboSampleComponent); - fixture.detectChanges(); - combo = fixture.componentInstance.combo; - }); - it('should scale items container depending on size (itemHeight * 10)', () => { - combo.toggle(); - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(320); - fixture.componentInstance.size = 'small'; - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(280); - fixture.componentInstance.size = 'large'; - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(400); - }); - }); }); }); diff --git a/projects/igniteui-angular/src/lib/core/styles/components/combo/_combo-theme.scss b/projects/igniteui-angular/src/lib/core/styles/components/combo/_combo-theme.scss index 8bfc4c2fab8..604086cca99 100644 --- a/projects/igniteui-angular/src/lib/core/styles/components/combo/_combo-theme.scss +++ b/projects/igniteui-angular/src/lib/core/styles/components/combo/_combo-theme.scss @@ -189,6 +189,7 @@ %igx-combo__content { position: relative; overflow: hidden; + max-height: calc(var(--size) * 10); &:focus { outline: transparent; diff --git a/projects/igniteui-angular/src/lib/directives/for-of/for_of.directive.ts b/projects/igniteui-angular/src/lib/directives/for-of/for_of.directive.ts index f1b78e8166a..672d2fa9c66 100644 --- a/projects/igniteui-angular/src/lib/directives/for-of/for_of.directive.ts +++ b/projects/igniteui-angular/src/lib/directives/for-of/for_of.directive.ts @@ -168,6 +168,14 @@ export class IgxForOfDirective extends IgxForOfToken extends IgxForOfToken(); + /** + * @hidden @internal + * Emitted just before the overlay animation start. + */ + @Output() + public animationStarting = new EventEmitter(); + /** * @hidden */ @@ -193,6 +200,7 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy { private _overlayClosingSub: Subscription; private _overlayClosedSub: Subscription; private _overlayContentAppendedSub: Subscription; + private _overlayAnimationStartingSub: Subscription; /** * @hidden @@ -387,6 +395,10 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy { .closed .pipe(...this._overlaySubFilter) .subscribe(this.overlayClosed); + + this._overlayAnimationStartingSub = this.overlayService.animationStarting.pipe(first(), takeUntil(this.destroy$)).subscribe(() => { + this.animationStarting.emit(); + }); } private unsubscribe() { @@ -394,6 +406,7 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy { this.clearSubscription(this._overlayClosingSub); this.clearSubscription(this._overlayClosedSub); this.clearSubscription(this._overlayContentAppendedSub); + this.clearSubscription(this._overlayAnimationStartingSub); } private clearSubscription(subscription: Subscription) { diff --git a/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts b/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts index a70438d8a88..c0c889130ce 100644 --- a/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts +++ b/projects/igniteui-angular/src/lib/drop-down/drop-down.component.ts @@ -104,6 +104,14 @@ export class IgxDropDownComponent extends IgxDropDownBaseDirective implements ID @Output() public closed = new EventEmitter(); + /** + * @hidden + * @internal + * Emitted just before the overlay animation start. + */ + @Output() + public animationStarting = new EventEmitter(); + /** * Gets/sets whether items take focus. Disabled by default. * When enabled, drop down items gain tab index and are focused when active - diff --git a/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.html b/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.html index 8de7190212c..875c9f0f3a0 100644 --- a/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.html +++ b/projects/igniteui-angular/src/lib/simple-combo/simple-combo.component.html @@ -62,7 +62,7 @@ [itemHeight]="itemHeight" (click)="handleItemClick()" *igxFor="let item of data | comboFiltering:filterValue:displayKey:filteringOptions:filterFunction | comboGrouping:groupKey:valueKey:groupSortingDirection:compareCollator; - index as rowIndex; containerSize: itemsMaxHeight; scrollOrientation: 'vertical'; itemSize: itemHeight" + index as rowIndex; initialChunkSize: 10; containerSize: itemsMaxHeight || containerSize; itemSize: itemHeight || itemSize; scrollOrientation: 'vertical';" [value]="item" [isHeader]="item?.isHeader" [index]="rowIndex"> { expect(combo.displayKey).toEqual('field'); expect(combo.groupKey).toEqual('region'); expect(combo.width).toEqual('400px'); - expect(combo.itemsMaxHeight).toEqual(320); - expect(combo.itemHeight).toEqual(32); expect(combo.placeholder).toEqual('Location'); expect(combo.allowCustomValues).toEqual(false); expect(combo.cssClass).toEqual(CSS_CLASS_COMBO); @@ -671,9 +669,7 @@ describe('IgxSimpleCombo', () => { const dropdownList = fixture.debugElement.query(By.css(`.${CSS_CLASS_CONTENT}`)); const verifyDropdownItemHeight = () => { - expect(combo.itemHeight).toEqual(itemHeight); expect(dropdownItems[0].nativeElement.clientHeight).toEqual(itemHeight); - expect(combo.itemsMaxHeight).toEqual(itemMaxHeight); expect(dropdownList.nativeElement.clientHeight).toEqual(itemMaxHeight); }; verifyDropdownItemHeight(); @@ -2115,35 +2111,6 @@ describe('IgxSimpleCombo', () => { })); }); - describe('Display density', () => { - beforeAll(waitForAsync(() => { - TestBed.configureTestingModule({ - imports: [ - NoopAnimationsModule, - ReactiveFormsModule, - FormsModule, - IgxSimpleComboSampleComponent - ] - }).compileComponents(); - })); - beforeEach(() => { - fixture = TestBed.createComponent(IgxSimpleComboSampleComponent); - fixture.detectChanges(); - combo = fixture.componentInstance.combo; - }); - it('should scale items container depending on component size (itemHeight * 10)', () => { - combo.toggle(); - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(320); - fixture.componentInstance.size = 'small'; - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(280); - fixture.componentInstance.size = 'large'; - fixture.detectChanges(); - expect(combo.itemsMaxHeight).toEqual(400); - }); - }); - describe('Form control tests: ', () => { describe('Template form tests: ', () => { let inputGroupRequired: DebugElement;