Skip to content

fix(grid): avoid double detach on destroy with adv. filtering open - … - #17966

Open
ivanvpetrov wants to merge 2 commits into
22.2.xfrom
ipetrov/fix-17962-22.2
Open

ivanvpetrov wants to merge 2 commits into
22.2.xfrom
ipetrov/fix-17962-22.2

Conversation

@ivanvpetrov

Copy link
Copy Markdown
Contributor

Closes #17962

Description

Destroying a grid while its advanced filtering dialog is open no longer logs igxOverlay.detach was called with wrong id in the console.

  • In 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.
  • ngOnDestroy now 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's ngOnDestroy. The overlay service registers a destroy callback on the components it attaches, so it detaches the dialog at that point. It doesn't emit closed, so the grid still holds the old overlay id. The grid's ngOnDestroy then calls detach with 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):

  • Bug fix
  • New functionality
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (no functional changes)
  • Documentation
  • Demos
  • CI/CD
  • Tests
  • Changelog
  • Skills/Agents

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.ts follows 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.

  • Unit tests
  • Manual testing
  • Automated e2e tests

Runs:

  • grid-filtering-advanced, grid-toolbar, grid-keyBoardNav-headers, hierarchical-grid.selection and state.* specs: 143 passed
  • npm run lint:lib: no new errors or warnings

Test Configuration:

  • Angular version: 22.2.0
  • Browser(s): Chrome Headless 157 (Karma)
  • OS: Windows 11

Screenshots / Recordings

N/A: the only visible effect was a console warning.

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
  • Accessibility (ARIA, keyboard navigation, focus management) has been verified

@ivanvpetrov
ivanvpetrov requested a balanced review from Copilot November 12, 2026 08:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 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 @if while 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
ivanvpetrov marked this pull request as ready for review November 12, 2026 08:09

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants