Repository navigation
fix(grid): avoid double overlay detach on destroy with adv. filtering open - #17963
Open
ivanvpetrov wants to merge 2 commits into
Open
ivanvpetrov wants to merge 2 commits into
ivanvpetrov wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The review reports no active issues with the advanced-filtering dialog teardown fix.
No open issues 🎉
What changed in this PR
Fixes #17962 by preventing duplicate overlay cleanup when a grid is destroyed with its advanced filtering dialog open. The change applies to the shared base used by Grid, Tree Grid, and Hierarchical Grid.
Changes:
- Detaches the dialog during grid teardown only if its overlay remains registered, and always clears the stored overlay ID.
- Adds a regression test that removes an
@if-controlled grid and checks dialog removal and absence of the warning. - Documents the fix in the changelog.
| File | Description |
|---|---|
projects/igniteui-angular/grids/grid/src/grid-filtering-advanced.spec.ts |
Adds conditional grid teardown coverage for the overlay warning. |
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts |
Guards overlay detachment and unconditionally clears the stored ID. |
CHANGELOG.md |
Records the advanced filtering teardown fix. |
Review effort: Balanced
ivanvpetrov
marked this pull request as ready for review
November 12, 2026 08:09
viktorkombov
approved these changes
Nov 12, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #17962
Description
Destroying a grid while its advanced filtering dialog is open no longer logs
igxOverlay.detach was called with wrong idin the console.ngOnDestroy, the grid now detaches the advanced filtering dialog only if the overlay is still registered (getOverlayById). The grid already guards its other overlays the same way a few lines below.ngOnDestroynow always resets_advancedFilteringOverlayId.Motivation / Context
This is the same issue as #17938 (date pickers, fixed in #17958). The advanced filtering dialog is created in the grid's own
ViewContainerRef. When the grid is destroyed, Angular destroys that container, and the dialog with it, before it runs the grid'sngOnDestroy. The overlay service registers a destroy callback on the components it attaches, so it detaches the dialog at that point. It doesn't emitclosed, so the grid still holds the old overlay id. The grid'sngOnDestroythen callsdetachwith an id that no longer exists, and the service logs the warning.The overlay service's own detach-on-destroy behaviour is correct and stays as is.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
Grid, Tree Grid, Hierarchical Grid (advanced filtering dialog)
How Has This Been Tested?
A new spec in
grid-filtering-advanced.spec.tsfollows the steps from the issue. It renders the grid inside an@if, opens the advanced filtering dialog, removes the grid, and checks that the dialog is gone and that the detach warning wasn't logged. It fails without the fix (igxOverlay.detach was called with wrong id: 0) and passes with it.Runs:
grid-filtering-advanced,grid-toolbar,grid-keyBoardNav-headers,hierarchical-grid.selectionandstate.*specs: 234 passednpm run lint:lib: no new errors or warningsTest Configuration:
Screenshots / Recordings
N/A: the only visible effect was a console warning.
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)