fix(ESF): move custom dialog out of ESF markup by wnvko · Pull Request #17136 · IgniteUI/igniteui-angular

wnvko · 2026-03-31T08:46:45Z

Right now the custom dialog shown in ESF is a child element of the excel-style-conditional-filter component. When custom dialog is shown its parent is closed. This makes the parent of the dialog invisible, marked with display: none. This effectively hides and the dialog. To fix this custom dialog is now removed from excel-style-conditional-filter component. Custom dialog is shown directly via overlay service and is provided as component.

Next step will be to remove the grid outlet used to show the custom dialog in the grid as this will not be needed anymore.

Closes #17039

Additional information (check all that apply):

Checklist:

All relevant tags have been applied to this PR
This PR includes unit tests covering all the new code (test guidelines)
This PR includes API docs for newly added methods/properties (api docs guidelines)
This PR includes feature/README.MD updates for the feature docs
This PR includes general feature table updates in the root README.MD
This PR includes CHANGELOG.MD updates for newly added functionality
This PR contains breaking changes
This PR includes ng update migrations for the breaking changes (migrations guidelines)
This PR includes behavioral changes and the feature specification has been updated with them

damyanpetev · 2026-03-31T10:33:12Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

+        this._overlayService.opening.pipe(takeUntil(this.destroy$)).subscribe((args) => {
+            if (args.id === overlayId) {
+                customDialog.esf = this.esf;
+                customDialog.column = this.esf.column;
+                customDialog.filteringService = this.esf.grid.filteringService;
+                customDialog.overlayComponentId = overlayId;
+                customDialog.selectedOperator = eventArgs.newSelection.value;


I don't see a reason for the opening here, can we have mostly the same logic as before that just assigned the customDialog props

damyanpetev · 2026-03-31T10:42:42Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

+            modal: false,
+            closeOnOutsideClick: true,
+            positionStrategy: new ContainerPositionStrategy(),
+            scrollStrategy: new AbsoluteScrollStrategy()


This does.. nothing for container strategy?

damyanpetev · 2026-03-31T10:43:28Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

+        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);


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

…hub.com/IgniteUI/igniteui-angular into mvenkov/move-esf-dialog-out-of-dropdown

damyanpetev · 2026-03-31T13:45:47Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

        if (this.esf.overlayComponentId) {
            this.esf.hide();
        }
        this.subMenu.close();
-        this.customDialog.open(this.esf.mainDropdown.nativeElement);
+        this._overlayService.show(overlayId);


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

Copilot

Pull request overview

This PR addresses an Excel Style Filtering (ESF) rendering issue where the custom filter dialog could become hidden when its parent ESF component is closed (parent becomes display: none). The fix moves the custom dialog out of the igx-excel-style-conditional-filter markup and shows it via IgxOverlayService.

Changes:

Remove <igx-excel-style-custom-dialog> from excel-style-conditional-filter template and create/show it dynamically through IgxOverlayService.
Refactor the custom dialog component to no longer rely on IgxToggleDirective for open/close lifecycle.
Update affected grid filtering UI tests and helper utilities to match the new overlay behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
projects/igniteui-angular/test-utils/grid-functions.spec.ts	Adjusts operator dropdown indexing used by ESF-related test helpers.
projects/igniteui-angular/grids/grid/src/grid-filtering-ui.spec.ts	Updates ESF size-related tests to align with the new custom dialog hosting/overlay behavior.
projects/igniteui-angular/grids/core/src/filtering/excel-style/excel-style-custom-dialog.component.ts	Removes toggle-based overlay setup and switches to overlay-service-driven close behavior (incl. detach).
projects/igniteui-angular/grids/core/src/filtering/excel-style/excel-style-custom-dialog.component.html	Removes `igxToggle` host/event bindings from the dialog root element.
projects/igniteui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts	Dynamically attaches/initializes the custom dialog via `IgxOverlayService` instead of using a child component reference.
projects/igniteui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.html	Removes the embedded custom dialog element from the component template.

Copilot · 2026-04-01T12:21:44Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

+        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);


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 · 2026-04-01T12:21:45Z

...eui-angular/grids/core/src/filtering/excel-style/excel-style-conditional-filter.component.ts

+        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);
+
+        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();
+        });


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 · 2026-04-01T12:21:45Z

projects/igniteui-angular/grids/grid/src/grid-filtering-ui.spec.ts

 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');


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.

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

904b83a

wnvko added ❌ status: awaiting-test PRs awaiting manual verification overlay grid: excel-style-filtering version: 21.1.x labels Mar 31, 2026

wnvko requested a review from damyanpetev March 31, 2026 08:48

wnvko added 2 commits March 31, 2026 11:49

chore(esf): fix imports order

ec2a77e

Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown

4150eb0

damyanpetev reviewed Mar 31, 2026

View reviewed changes

wnvko added 🛠️ status: in-development Issues and PRs with active development on them and removed ❌ status: awaiting-test PRs awaiting manual verification labels Mar 31, 2026

wnvko added 2 commits March 31, 2026 14:49

chore(esf): address PR comments

0d54fd2

Merge branch 'mvenkov/move-esf-dialog-out-of-dropdown' of https://git…

e0643fe

…hub.com/IgniteUI/igniteui-angular into mvenkov/move-esf-dialog-out-of-dropdown

wnvko added ❌ status: awaiting-test PRs awaiting manual verification version: 21.2.x and removed 🛠️ status: in-development Issues and PRs with active development on them version: 21.1.x labels Mar 31, 2026

dkamburov self-assigned this Mar 31, 2026

damyanpetev reviewed Mar 31, 2026

View reviewed changes

damyanpetev previously approved these changes Mar 31, 2026

View reviewed changes

dkamburov assigned skrustev Apr 1, 2026

skrustev added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Apr 1, 2026

Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown

4ec7224

Copilot AI review requested due to automatic review settings April 1, 2026 12:13

Copilot started reviewing on behalf of wnvko April 1, 2026 12:13 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

fix(esf): focus value input only when needed

173533b

wnvko dismissed damyanpetev’s stale review via 173533b April 1, 2026 14:21

wnvko requested a review from damyanpetev April 1, 2026 14:22

Merge branch 'master' into mvenkov/move-esf-dialog-out-of-dropdown

740b1ef

damyanpetev approved these changes Apr 1, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

fix(ESF): move custom dialog out of ESF markup#17136
wnvko wants to merge 8 commits intomasterfrom
mvenkov/move-esf-dialog-out-of-dropdown

wnvko commented Mar 31, 2026 •

edited

Loading

Uh oh!

damyanpetev Mar 31, 2026

Uh oh!

damyanpetev Mar 31, 2026

Uh oh!

damyanpetev Mar 31, 2026

Uh oh!

damyanpetev Mar 31, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

wnvko commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional information (check all that apply):

Checklist:

Uh oh!

damyanpetev Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

damyanpetev Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

damyanpetev Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

damyanpetev Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wnvko commented Mar 31, 2026 •

edited

Loading

damyanpetev Mar 31, 2026 •

edited

Loading