Repository navigation
fix(grid): avoid double detach on destroy with adv. filtering open - … - #17966
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
No active review issues were identified for the change preventing duplicate overlay detachment during grid destruction.
No open issues 🎉
What changed in this PR
Fixes #17962 for the 22.2.x branch by preventing duplicate overlay detachment when a grid is destroyed with its advanced filtering dialog open. The change applies to shared grid cleanup without altering the overlay service.
Changes:
- Checks that the advanced filtering overlay is still registered before detaching it.
- Always clears the stored overlay ID during grid destruction.
- Adds a regression test for removing a grid through
@ifwhile its dialog is open, checking dialog removal and absence of the warning.
| File | Description |
|---|---|
projects/igniteui-angular/grids/grid/src/grid-filtering-advanced.spec.ts |
Adds a conditional-grid fixture and regression test for dialog cleanup. |
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts |
Guards overlay detachment and unconditionally resets the stored ID. |
Review effort: Balanced
ivanvpetrov
marked this pull request as ready for review
November 12, 2026 08:09
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.This is the 22.2.x version of the fix.
Motivation / Context
This is the same issue as #17938 (date pickers). 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 and passes with it.Runs:
grid-filtering-advanced,grid-toolbar,grid-keyBoardNav-headers,hierarchical-grid.selectionandstate.*specs: 143 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)