fix(igxGrid): Hide overlays on scroll + special handling for row edit…#17220
Open
MayaKirova wants to merge 8 commits into21.2.xfrom
Open
fix(igxGrid): Hide overlays on scroll + special handling for row edit…#17220MayaKirova wants to merge 8 commits into21.2.xfrom
MayaKirova wants to merge 8 commits into21.2.xfrom
Conversation
MayaKirova
added
❌ status: awaiting-test
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes grid overlay behavior during scrolling to prevent overlays from floating outside their owning (child) grid, and adds special handling for the row-edit overlay to better match pre-refactor behavior.
Changes:
- Updates
IgxGridBaseDirectiveoverlay tracking so overlays are hidden on scroll based on whether their target is within the grid, with exceptions for row-attached overlays. - Adds reconnect/detach handling for row-edit overlay state when the grid is resized/removed from the DOM.
- Updates the hierarchical grid sample to reproduce/validate overlay + row-edit scenarios (action strip, advanced filtering, editing).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/app/hierarchical-grid/hierarchical-grid.sample.ts |
Imports IgxActionStripComponent for the updated sample template. |
src/app/hierarchical-grid/hierarchical-grid.sample.html |
Adjusts sample layout and adds action strip / advanced filtering UI to exercise overlay behaviors. |
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts |
Changes overlay bookkeeping and row-edit overlay lifecycle behavior tied to scroll/resize/DOM attachment. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/5da777fa-12c5-4827-97a7-0bae83db17ae Co-authored-by: MayaKirova <10397980+MayaKirova@users.noreply.github.com>
Agent-Logs-Url: https://github.com/IgniteUI/igniteui-angular/sessions/5da777fa-12c5-4827-97a7-0bae83db17ae Co-authored-by: MayaKirova <10397980+MayaKirova@users.noreply.github.com>
Contributor
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
damyanpetev
reviewed
Apr 22, 2026
Comment on lines
3779
to
3792
| // this sets focus to query builder button for some reason... | ||
| if (this._advancedFilteringOverlayId === event.id) { | ||
| const instance = event.componentRef.instance as IgxAdvancedFilteringDialogComponent; | ||
| if (instance) { | ||
| instance.lastActiveNode = this.navigation.activeNode; | ||
| instance.queryBuilder.setAddButtonFocus(); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // do not hide the overlay if it's attached to a row | ||
| if (this.rowEditingOverlay?.overlayId === event.id) { | ||
| const inRow = (overlaySettings?.target as HTMLElement)?.classList.contains("igx-grid__tr"); | ||
| // do not hide the overlay if it's attached to a row on scroll | ||
| if (inRow) { | ||
| return; | ||
| } |
Member
There was a problem hiding this comment.
These rows should really not need additional changes, just the outlet check below;
Also the row edit already had hide/show handling, so the additional scrollNotify logic is also sus
Contributor
Author
There was a problem hiding this comment.
This is just to match the existing behavior. The row edit overlay did not close on scroll in previous versions. There is just handling to reposition it.
Hristo313
added
💥 status: in-test
damyanpetev
reviewed
Apr 22, 2026
| instance.lastActiveNode = this.navigation.activeNode; | ||
| instance.queryBuilder.setAddButtonFocus(); | ||
| } | ||
| return; |
Member
There was a problem hiding this comment.
This should stay (see https://www.infragistics.com/products/ignite-ui-angular/angular/components/grid/advanced-filtering behavior) and the new test should go
damyanpetev
reviewed
Apr 22, 2026
|
|
||
| if (overlaySettings?.outlet === this.outlet && this.overlayIDs.indexOf(event.id) === -1) { | ||
| // check whole grid, since some overlays like the advanced filtering, are outside the body. | ||
| const isInGrid = this.nativeElement.contains(overlaySettings?.target as Node); |
Member
There was a problem hiding this comment.
Check overlaySettings?.target is Node instead of cast, as contains can throw
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.
… overlay.
Closes #17215
Description
Hide overlays on scroll so that they don't float outside their child grid on parent scroll.
Also add more special handling for row edit overlay so that it matches the old behavior - not closing on scroll, but repositioning to the correct row after grid has been removed from DOM (collapsed, scrolled out of view etc.).
Motivation / Context
Because of the overlay refactoring, bug was introduced.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)