Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(*): Remove the need of hardcoded item heights in combo #14871

Open
wants to merge 19 commits into
base: 19.0.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
3c2cbfc
refactor(*): Remove the need of hardcoded item heights
mddragnev Oct 3, 2024
514829b
chore(*): fix lint
mddragnev Oct 3, 2024
128acac
chore(*): fix another lint
mddragnev Oct 3, 2024
1919412
chore(*): Clean up code. Change some tests and remove some
mddragnev Oct 10, 2024
bb41ed8
chore(*): Some more cleanup. Fix tests
mddragnev Oct 10, 2024
dfbcd39
Merge branch 'master' of https://github.com/IgniteUI/igniteui-angular…
mddragnev Oct 17, 2024
29b249d
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 6, 2024
b373f72
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 9, 2024
4ae12c1
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 9, 2024
e8ab86b
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 11, 2024
977a354
Merge branch 'master' into mdragnev/combo-itemheight
simeonoff Nov 12, 2024
bcce18c
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 14, 2024
404c6c7
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 14, 2024
f5688f9
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 14, 2024
95484b1
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 15, 2024
0288ec0
Merge branch 'master' into mdragnev/combo-itemheight
simeonoff Nov 15, 2024
70e3fb5
Merge branch 'master' into mdragnev/combo-itemheight
kdinev Nov 19, 2024
884b26b
Merge branch '19.0.x' into mdragnev/combo-itemheight
kdinev Nov 26, 2024
ba92edb
Merge branch '19.0.x' into mdragnev/combo-itemheight
kdinev Nov 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,11 +944,12 @@ 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 itemsInCointaner = 10;
private _defaultFilteringOptions: IComboFilteringOptions = { caseSensitive: false };

public abstract dropdown: IgxComboDropDownComponent;
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 @@ -1072,8 +1072,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 @@ -1172,9 +1170,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 @@ -1811,11 +1807,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 @@ -2062,7 +2057,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 @@ -3620,24 +3616,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 @@ -175,6 +175,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 @@ -603,8 +603,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 @@ -686,9 +684,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 @@ -2130,35 +2126,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
Loading