Skip to content

Commit

Permalink
refactor(*): Remove the need of hardcoded item heights
Browse files Browse the repository at this point in the history
chore(*): fix lint

chore(*): fix another lint

chore(*): Clean up code. Change some tests and remove some

chore(*): Some more cleanup. Fix tests
  • Loading branch information
mddragnev committed Jan 9, 2025
1 parent 509f5a6 commit 7b9d9fc
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}

/**
Expand Down
50 changes: 21 additions & 29 deletions projects/igniteui-angular/src/lib/combo/combo.common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IgxComboBase>('IgxComboComponentToken');

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}
}

/**
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 = '';
Expand All @@ -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<any>;
Expand Down Expand Up @@ -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 */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
<igx-combo-item [itemHeight]="itemHeight" *igxFor="let item of data
| comboFiltering:filterValue:displayKey:filteringOptions:filterFunction:disableFiltering
| 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" [role]="item?.isHeader? 'group' : 'option'">
<ng-container *ngIf="item?.isHeader">
<ng-container
Expand Down
28 changes: 3 additions & 25 deletions projects/igniteui-angular/src/lib/combo/combo.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,6 @@ describe('igxCombo', () => {
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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
});
});
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@
%igx-combo__content {
position: relative;
overflow: hidden;
max-height: calc(var(--size) * 10);

&:focus {
outline: transparent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,14 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
@Input()
public igxForContainerSize: any;

/**
* @hidden
* @internal
* Initial chunk size if no container size is passed. If container size is passed then the igxForOf calculates its chunk size
*/
@Input()
public igxForInitialChunkSize: any;

/**
* Sets the px-affixed size of the item along the axis of scrolling.
* For "horizontal" orientation this value is the width of the column and for "vertical" is the height or the row.
Expand Down Expand Up @@ -1207,7 +1215,7 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
}
} else {
if (this.igxForOf) {
chunkSize = this.igxForOf.length;
chunkSize = Math.min(this.igxForInitialChunkSize || this.igxForOf.length, this.igxForOf.length);
}
}
return chunkSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,13 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy {
@Output()
public appended = new EventEmitter<ToggleViewEventArgs>();

/**
* @hidden @internal
* Emitted just before the overlay animation start.
*/
@Output()
public animationStarting = new EventEmitter();

/**
* @hidden
*/
Expand Down Expand Up @@ -193,6 +200,7 @@ export class IgxToggleDirective implements IToggleView, OnInit, OnDestroy {
private _overlayClosingSub: Subscription;
private _overlayClosedSub: Subscription;
private _overlayContentAppendedSub: Subscription;
private _overlayAnimationStartingSub: Subscription;

/**
* @hidden
Expand Down Expand Up @@ -387,13 +395,18 @@ 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() {
this.clearSubscription(this._overlayOpenedSub);
this.clearSubscription(this._overlayClosingSub);
this.clearSubscription(this._overlayClosedSub);
this.clearSubscription(this._overlayContentAppendedSub);
this.clearSubscription(this._overlayAnimationStartingSub);
}

private clearSubscription(subscription: Subscription) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,14 @@ export class IgxDropDownComponent extends IgxDropDownBaseDirective implements ID
@Output()
public closed = new EventEmitter<IBaseEventArgs>();

/**
* @hidden
* @internal
* Emitted just before the overlay animation start.
*/
@Output()
public animationStarting = new EventEmitter<ToggleViewEventArgs>();

/**
* Gets/sets whether items take focus. Disabled by default.
* When enabled, drop down items gain tab index and are focused when active -
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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">
<ng-container *ngIf="item?.isHeader">
<ng-container
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,6 @@ describe('IgxSimpleCombo', () => {
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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 7b9d9fc

Please sign in to comment.