Skip to content

fix(ESF): move custom dialog out of ESF markup #17136

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.

Sign up for GitHub

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

Open
wnvko wants to merge 8 commits into
base: master
Choose a base branch
from
Open

fix(ESF): move custom dialog out of ESF markup #17136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
904b83a
fix(ESF): move custom dialog out of ESF markup
wnvko Mar 31, 2026
ec2a77e
chore(esf): fix imports order
wnvko Mar 31, 2026
4150eb0
Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown
wnvko Mar 31, 2026
0d54fd2
chore(esf): address PR comments
wnvko Mar 31, 2026
e0643fe
Merge branch 'mvenkov/move-esf-dialog-out-of-dropdown' of https://git…
wnvko Mar 31, 2026
4ec7224
Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown
wnvko Apr 1, 2026
173533b
fix(esf): focus value input only when needed
wnvko Apr 1, 2026
740b1ef
Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown
wnvko Apr 1, 2026
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
7 changes: 0 additions & 7 deletions ...ngular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,4 @@
}
</div>
</igx-drop-down>

<igx-excel-style-custom-dialog
#customDialog
[column]="esf.column"
[filteringService]="esf.grid.filteringService"
[overlayComponentId]="esf.overlayComponentId">
</igx-excel-style-custom-dialog>
}
42 changes: 30 additions & 12 deletions ...-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { BaseFilteringComponent } from './base-filtering.component';
import { NgClass } from '@angular/common';
import { IgxDropDownComponent, IgxDropDownItemComponent, IgxDropDownItemNavigationDirective, ISelectionEventArgs } from 'igniteui-angular/drop-down';
import { IgxIconComponent } from 'igniteui-angular/icon';
import { AbsoluteScrollStrategy, AutoPositionStrategy, GridColumnDataType, HorizontalAlignment, IFilteringExpression, IFilteringOperation, OverlaySettings, PlatformUtil, VerticalAlignment } from 'igniteui-angular/core';
import { AbsoluteScrollStrategy, AutoPositionStrategy, ContainerPositionStrategy, GridColumnDataType, HorizontalAlignment, IFilteringExpression, IFilteringOperation, IgxOverlayService, OverlaySettings, PlatformUtil, VerticalAlignment } from 'igniteui-angular/core';


/**
Expand All @@ -15,17 +15,12 @@ import { AbsoluteScrollStrategy, AutoPositionStrategy, GridColumnDataType, Horiz
@Component({
selector: 'igx-excel-style-conditional-filter',
templateUrl: './excel-style-conditional-filter.component.html',
imports: [NgClass, IgxDropDownItemNavigationDirective, IgxIconComponent, IgxDropDownComponent, IgxDropDownItemComponent, IgxExcelStyleCustomDialogComponent]
imports: [NgClass, IgxDropDownItemNavigationDirective, IgxIconComponent, IgxDropDownComponent, IgxDropDownItemComponent]
})
export class IgxExcelStyleConditionalFilterComponent implements OnDestroy {
public esf = inject(BaseFilteringComponent);
protected platform = inject(PlatformUtil);

/**
* @hidden @internal
*/
@ViewChild('customDialog', { read: IgxExcelStyleCustomDialogComponent })
public customDialog: IgxExcelStyleCustomDialogComponent;
private _overlayService = inject<IgxOverlayService>(IgxOverlayService);

/**
* @hidden @internal
Expand Down Expand Up @@ -133,20 +128,43 @@ export class IgxExcelStyleConditionalFilterComponent implements OnDestroy {
* @hidden @internal
*/
public onSubMenuSelection(eventArgs: ISelectionEventArgs) {
const overlaySettings: OverlaySettings = {
modal: false,
closeOnOutsideClick: true,
positionStrategy: new ContainerPositionStrategy()
};
const overlayId = this._overlayService.attach(IgxExcelStyleCustomDialogComponent, this.esf.grid.viewRef, overlaySettings);
const overlayInfo = this._overlayService.getOverlayById(overlayId);
const customDialog = overlayInfo.componentRef.instance as IgxExcelStyleCustomDialogComponent;
this.esf.grid.tbody.nativeElement.appendChild(overlayInfo.wrapperElement.parentElement);
Copy link
Copy Markdown
Member

@damyanpetev damyanpetev Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is needed atm, but we could consider leaving the dialog on the grid container instead as well (separate from the PR);

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appending the overlay container (overlayInfo.wrapperElement.parentElement) directly into grid.tbody results in invalid table DOM (a <div> under <tbody>). It also interferes with IgxOverlayService.hide() cleanup (hide does not remove the wrapper/container), so closing on outside click can leave orphaned overlay containers in the table. Prefer providing a proper overlay container via overlaySettings.outlet (e.g. a positioned grid container element) or appending to a non-table container element, and ensure the container is removed on close.

Suggested change
this.esf.grid.tbody.nativeElement.appendChild(overlayInfo.wrapperElement.parentElement);

Copilot uses AI. Check for mistakes.

customDialog.esf = this.esf;
customDialog.column = this.esf.column;
customDialog.filteringService = this.esf.grid.filteringService;
customDialog.overlayComponentId = overlayId;
if (this.esf.expressionsList && this.esf.expressionsList.length &&
this.esf.expressionsList[0].expression.condition.name !== 'in') {
this.customDialog.expressionsList = this.esf.expressionsList;
customDialog.expressionsList = this.esf.expressionsList;
} else {
this.customDialog.expressionsList = this.customDialog.expressionsList.filter(e => e.expression.fieldName === this.esf.column.field && e.expression.condition);
customDialog.expressionsList = customDialog.expressionsList.filter(e => e.expression.fieldName === this.esf.column.field && e.expression.condition);
}
customDialog.selectedOperator = eventArgs.newSelection.value;

this._overlayService.opening.pipe(takeUntil(this.destroy$)).subscribe((args) => {
if (args.id === overlayId)
customDialog.onCustomDialogOpening();
});
this._overlayService.opened.pipe(takeUntil(this.destroy$)).subscribe((args) => {
if (args.id === overlayId)
customDialog.onCustomDialogOpened();
});
Comment on lines +131 to +160
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onSubMenuSelection creates a new overlay and subscribes to overlayService.opening/opened every time the submenu selection changes, but never detaches the overlay when it is closed via outside click (and doesn’t keep the overlayId to clean it up on destroy). This can accumulate component instances/subscriptions over time. Consider reusing a single attached overlay (cache overlayId/componentRef), and/or subscribe to overlayService.closed for this overlayId (with take(1)) to detach after hide, and detach any remaining overlay in ngOnDestroy.

Copilot uses AI. Check for mistakes.

this.customDialog.selectedOperator = eventArgs.newSelection.value;
eventArgs.cancel = true;
if (this.esf.overlayComponentId) {
this.esf.hide();
}
this.subMenu.close();
this.customDialog.open(this.esf.mainDropdown.nativeElement);
this._overlayService.show(overlayId);
Comment on lines 163 to +167
Copy link
Copy Markdown
Member

@damyanpetev damyanpetev Mar 31, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this PR merely moves things around to remove the outlet usage, but this component can really use some refactoring where the onSubMenuSelection is emitted upwards for the root ESF component to handle, close itself and spawn the dialog. And possibly some actual dialog and/or dialog ARIA would be nice. Just 2c

}

/**
Expand Down
6 changes: 2 additions & 4 deletions ...eui-angular/grids/core/src/filtering/excel-style/excel-style-custom-dialog.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
<article #toggle igxToggle
<article
class="igx-excel-filter__secondary"
[ngClass]="{
'igx-excel-filter__secondary--cosy': grid.gridSize === '2',
'igx-excel-filter__secondary--compact': grid.gridSize === '1'
}"
(keydown)="onKeyDown($event)"
(opening)="onCustomDialogOpening()"
(opened)="onCustomDialogOpened()">
(keydown)="onKeyDown($event)">
<header class="igx-excel-filter__secondary-header">
<h4 class="ig-typography__h6">
{{ grid.resourceStrings.igx_grid_excel_custom_dialog_header }}{{ column.header || column.field }}
Expand Down
55 changes: 12 additions & 43 deletions ...iteui-angular/grids/core/src/filtering/excel-style/excel-style-custom-dialog.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
import { Component, Input, ChangeDetectorRef, ViewChild, AfterViewInit, TemplateRef, ViewChildren, QueryList, ElementRef, inject } from '@angular/core';
import { IgxFilteringService } from '../grid-filtering.service';
import { ILogicOperatorChangedArgs, IgxExcelStyleDefaultExpressionComponent } from './excel-style-default-expression.component';
import { IgxExcelStyleDateExpressionComponent } from './excel-style-date-expression.component';
import { ExpressionUI } from './common';
import { NgClass } from '@angular/common';
import { BaseFilteringComponent } from './base-filtering.component';
import { IgxButtonDirective, IgxToggleDirective } from 'igniteui-angular/directives';
import { ChangeDetectorRef, Component, ElementRef, Input, QueryList, TemplateRef, ViewChild, ViewChildren, inject } from '@angular/core';
import { ColumnType, FilteringLogic, GridColumnDataType, IgxBooleanFilteringOperand, IgxDateFilteringOperand, IgxDateTimeFilteringOperand, IgxNumberFilteringOperand, IgxOverlayService, IgxStringFilteringOperand, IgxTimeFilteringOperand, PlatformUtil } from 'igniteui-angular/core';
import { IgxButtonDirective } from 'igniteui-angular/directives';
import { IgxIconComponent } from 'igniteui-angular/icon';
import { AbsoluteScrollStrategy, AutoPositionStrategy, ColumnType, FilteringLogic, GridColumnDataType, HorizontalAlignment, IgxBooleanFilteringOperand, IgxDateFilteringOperand, IgxDateTimeFilteringOperand, IgxNumberFilteringOperand, IgxOverlayService, IgxStringFilteringOperand, IgxTimeFilteringOperand, OverlaySettings, PlatformUtil, PositionSettings, VerticalAlignment } from 'igniteui-angular/core';
import { IgxFilteringService } from '../grid-filtering.service';
import { BaseFilteringComponent } from './base-filtering.component';
import { ExpressionUI } from './common';
import { IgxExcelStyleDateExpressionComponent } from './excel-style-date-expression.component';
import { ILogicOperatorChangedArgs, IgxExcelStyleDefaultExpressionComponent } from './excel-style-default-expression.component';

/**
* @hidden
*/
@Component({
selector: 'igx-excel-style-custom-dialog',
templateUrl: './excel-style-custom-dialog.component.html',
imports: [IgxToggleDirective, NgClass, IgxExcelStyleDateExpressionComponent, IgxExcelStyleDefaultExpressionComponent, IgxButtonDirective, IgxIconComponent]
imports: [NgClass, IgxExcelStyleDateExpressionComponent, IgxExcelStyleDefaultExpressionComponent, IgxButtonDirective, IgxIconComponent]
})
export class IgxExcelStyleCustomDialogComponent implements AfterViewInit {
export class IgxExcelStyleCustomDialogComponent {
protected overlayService = inject(IgxOverlayService);
private cdr = inject(ChangeDetectorRef);
protected platform = inject(PlatformUtil);
public esf = inject(BaseFilteringComponent);
public esf: BaseFilteringComponent;

@Input()
public expressionsList = new Array<ExpressionUI>();
Expand All @@ -38,9 +38,6 @@ export class IgxExcelStyleCustomDialogComponent implements AfterViewInit {
@Input()
public overlayComponentId: string;

@ViewChild('toggle', { read: IgxToggleDirective, static: true })
public toggle: IgxToggleDirective;

@ViewChild('defaultExpressionTemplate', { read: TemplateRef })
protected defaultExpressionTemplate: TemplateRef<any>;

Expand All @@ -56,24 +53,6 @@ export class IgxExcelStyleCustomDialogComponent implements AfterViewInit {
@ViewChildren(IgxExcelStyleDateExpressionComponent)
private expressionDateComponents: QueryList<IgxExcelStyleDateExpressionComponent>;

private _customDialogPositionSettings: PositionSettings = {
verticalDirection: VerticalAlignment.Middle,
horizontalDirection: HorizontalAlignment.Center,
horizontalStartPoint: HorizontalAlignment.Center,
verticalStartPoint: VerticalAlignment.Middle
};

private _customDialogOverlaySettings: OverlaySettings = {
closeOnOutsideClick: true,
modal: false,
positionStrategy: new AutoPositionStrategy(this._customDialogPositionSettings),
scrollStrategy: new AbsoluteScrollStrategy()
};

public ngAfterViewInit(): void {
this._customDialogOverlaySettings.outlet = this.grid.outlet;
}

public get template(): TemplateRef<any> {
if (this.column.dataType === GridColumnDataType.Date) {
return this.dateExpressionTemplate;
Expand All @@ -98,15 +77,6 @@ export class IgxExcelStyleCustomDialogComponent implements AfterViewInit {
}
}

public open(esf) {
this._customDialogOverlaySettings.target =
this.overlayComponentId ?
this.grid.rootGrid ? this.grid.rootGrid.nativeElement : this.grid.nativeElement :
esf;
this.toggle.open(this._customDialogOverlaySettings);
this.overlayComponentId = this.toggle.overlayId;
}

public onClearButtonClick() {
this.filteringService.clearFilter(this.column.field);
this.selectedOperator = null;
Expand All @@ -117,9 +87,8 @@ export class IgxExcelStyleCustomDialogComponent implements AfterViewInit {
public closeDialog() {
if (this.overlayComponentId) {
this.overlayService.hide(this.overlayComponentId);
this.overlayService.detach(this.overlayComponentId);
this.overlayComponentId = null;
} else {
this.toggle.close();
}
}

Expand Down
4 changes: 3 additions & 1 deletion ...-angular/grids/core/src/filtering/excel-style/excel-style-default-expression.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,9 @@ export class IgxExcelStyleDefaultExpressionComponent implements AfterViewInit {
this.expressionUI.expression.condition = this.getCondition(value);
this.expressionUI.expression.conditionName = value;

this.focus();
if (!this.expressionUI.expression.condition.isUnary) {
this.focus();
}
}

public getCondition(value: string): IFilteringOperation {
Expand Down
3 changes: 2 additions & 1 deletion projects/igniteui-angular/grids/grid/src/grid-filtering-ui.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4203,6 +4203,7 @@ describe('IgxGrid - Filtering actions - Excel style filtering #grid', () => {
fix.detectChanges();

// Open excel style custom filtering dialog and verify its size
setElementSize(grid.nativeElement, ɵSize.Large);
GridFunctions.clickExcelFilterIconFromCode(fix, grid, 'ProductName');

GridFunctions.clickExcelFilterCascadeButton(fix);
Expand Down Expand Up @@ -7563,7 +7564,7 @@ const verifyExcelCustomFilterSize = (fix: ComponentFixture<any>, expectedSize:
const verifyGridSubmenuSize = (gridNativeElement: HTMLElement, expectedSize: ɵSize) => {
const outlet = gridNativeElement.querySelector('.igx-grid__outlet');
const dropdowns = Array.from(outlet.querySelectorAll('.igx-drop-down__list'));
const visibleDropdown: any = dropdowns.find((d) => !d.classList.contains('igx-toggle--hidden'));
const visibleDropdown: any = dropdowns[0];
const dropdownItems = visibleDropdown.querySelectorAll('igx-drop-down-item');
Comment on lines 7564 to 7568
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifyGridSubmenuSize now unconditionally uses dropdowns[0]. In the scenarios where multiple dropdown lists exist in the grid outlet (e.g. cascade submenu list + expression operator dropdown), the first list can be hidden (igx-toggle--hidden) and not the one currently opened, making the assertions target the wrong dropdown. Restoring the previous logic to pick the visible list (e.g. find the one without igx-toggle--hidden) would make the test robust.

Copilot uses AI. Check for mistakes.

dropdownItems.forEach((dropdownItem) => {
Expand Down
2 changes: 1 addition & 1 deletion projects/igniteui-angular/test-utils/grid-functions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1404,7 +1404,7 @@ export class GridFunctions {
input.click();
fix.detectChanges();

const operators = fix.nativeElement.querySelectorAll('.igx-drop-down__list-scroll')[expressionIndex + 1];
const operators = fix.nativeElement.querySelectorAll('.igx-drop-down__list-scroll')[expressionIndex];
const operator = operators.children[itemIndex].children[0];
operator.click();
tick();
Expand Down
Loading