Skip to content

fix(hgrid): add and use lifecycle placeholder for grid connection eve… #17242

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MayaKirova wants to merge 4 commits into
base: 21.2.x
Choose a base branch
from
Open

fix(hgrid): add and use lifecycle placeholder for grid connection eve… #17242

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
122d801
fix(hgrid): add and use lifecycle placeholder for grid connection eve…
MayaKirova Apr 23, 2026
ad14b90
chore(*): Fix virtualization issue with change detection.
MayaKirova Apr 23, 2026
6281d2b
chore(*): Apply review comments.
MayaKirova Apr 27, 2026
7baf0e3
Merge branch '21.2.x' into mkirova/fix-17229-21.2.x
kdinev Apr 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions projects/igniteui-angular/directives/src/directives/for-of/for_of.directive.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1065,7 +1065,7 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE)
|| embView.rootNodes[0].nextElementSibling);
const view = container.detach(0);

view.detectChanges();
this.updateTemplateContext(embView.context, i);
container.insert(view);
Comment on lines 1067 to 1070
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

view.detectChanges() is called before updateTemplateContext(embView.context, i), so the view will run change detection with the old context. If the goal is to refresh the recycled view immediately (and/or flush DOM connect/disconnect), update the context first and then call detectChanges() (or batch the detection outside the loop).

Copilot uses AI. Check for mistakes.
this._embeddedViews.push(embView);
Expand All @@ -1085,7 +1085,7 @@ export class IgxForOfDirective<T, U extends T[] = T[]> extends IgxForOfToken<T,U
this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE)
|| embView.rootNodes[0].nextElementSibling);
const view = container.detach(container.length - 1);

view.detectChanges();
this.updateTemplateContext(embView.context, i);
container.insert(view, 0);
Comment on lines 1087 to 1090
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

Same issue here: view.detectChanges() runs before updateTemplateContext(...), so the recycled view is checked with stale $implicit/index/count values. Also, the view.detectChanges(); line is mis-indented compared to surrounding code (likely to trip formatting/lint rules).

Copilot uses AI. Check for mistakes.
this._embeddedViews.unshift(embView);
Expand Down
5 changes: 5 additions & 0 deletions projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,3 +276,8 @@
<ng-content select="igx-column,igc-column,igx-column-group,igc-column-group,igx-action-strip,igc-action-strip"></ng-content>
<ng-content select="igx-row-island,igc-row-island"></ng-content>
}

<!-- Lifecycle placeholder used to determine when the grid gets connected/disconnected from the DOM, for example as a result of virtualization or caching. -->
<igc-lifecycle-placeholder (igcConnected)="onLifecyclePlaceholderConnected()"
(igcDisconnected)="onLifecyclePlaceholderDisconnected()">
</igc-lifecycle-placeholder>
44 changes: 34 additions & 10 deletions projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { IgxIconComponent } from 'igniteui-angular/icon';
import { EntityType, FieldType, IFilteringExpressionsTree, IgxActionStripToken, IgxOverlayOutletDirective, flatten, IGridResourceStrings } from 'igniteui-angular/core';
import { IgxPaginatorToken } from 'igniteui-angular/paginator';
import { IgxGridCellMergePipe, IgxGridComponent, IgxGridFilteringPipe, IgxGridSortingPipe, IgxGridUnmergeActivePipe } from 'igniteui-angular/grids/grid';
import { registerLifecyclePlaceholderElement } from './lifecycle-placeholder-element';

let NEXT_ID = 0;

Expand Down Expand Up @@ -662,6 +663,9 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti
* @hidden
*/
public override ngOnInit() {
if (this.platform.isBrowser) {
registerLifecyclePlaceholderElement();
}
// this.expansionStatesChange.pipe(takeUntil(this.destroy$)).subscribe((value: Map<any, boolean>) => {
// const res = Array.from(value.entries()).filter(({1: v}) => v === true).map(([k]) => k);
// });
Expand All @@ -674,6 +678,26 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti
super.ngOnInit();
}

// Event that triggers when element gets connected back to the DOM.
// Used to determine when to reopen a previously closed row editing overlay.
protected onLifecyclePlaceholderConnected(): void {
if (this.rowEditable && this.crudService.rowInEditMode && this.rowEditingOverlay &&
this.rowEditingOverlay.collapsed) {
// Row is in edit mode, but overlay is closed - reopen.
this.openRowOverlay(this.crudService.rowInEditMode.id);
}

}

// Event that triggers when element gets disconnected from the DOM, for example as a result of virtualization or caching.
// Used to determine when to close the row editing overlay.
protected onLifecyclePlaceholderDisconnected(): void {
if (this.rowEditable && this.crudService.rowInEditMode && this.rowEditingOverlay) {
// disconnected from DOM (possibly cached) & row was in edit mode - close overlay.
Comment on lines +695 to +696
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

onLifecyclePlaceholderDisconnected() calls closeRowEditingOverlay() whenever a row is in edit mode, but the overlay may already be collapsed/closed as a result of the detach that triggered this event. closeRowEditingOverlay() unconditionally touches rowEditingOverlay.element.parentElement.style, which can throw if the element is already detached (parentElement is null). Add a guard (e.g., only close when not collapsed and/or when rowEditingOverlay.element.parentElement exists) before calling closeRowEditingOverlay().

Suggested change
if (this.rowEditable && this.crudService.rowInEditMode && this.rowEditingOverlay) {
// disconnected from DOM (possibly cached) & row was in edit mode - close overlay.
const overlay = this.rowEditingOverlay;
if (this.rowEditable && this.crudService.rowInEditMode && overlay &&
!overlay.collapsed && overlay.element?.parentElement) {
// disconnected from DOM (possibly cached) & row was in edit mode - close overlay
// only if it is still open and attached.

Copilot uses AI. Check for mistakes.
this.closeRowEditingOverlay();
}
}
Comment on lines +681 to +699
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

New connect/disconnect-driven behavior for row edit overlay state (close on disconnect, reopen on reconnect) is added here, but there are existing hgrid specs (including virtualization ones) and none appear to cover this scenario. Please add a regression test that starts row edit in a child grid, virtualizes/detaches it (scroll/collapse), and verifies the overlay is restored when reconnected.

Copilot uses AI. Check for mistakes.

/**
* @hidden
*/
Expand Down Expand Up @@ -1232,15 +1256,15 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti
{
name: null,
fields: filterableFields.map(f => ({
field: f.field,
dataType: f.dataType,
header: f.header,
editorOptions: f.editorOptions,
filters: f.filters,
pipeArgs: f.pipeArgs,
defaultTimeFormat: f.defaultTimeFormat,
defaultDateTimeFormat: f.defaultDateTimeFormat
})) as FieldType[]
field: f.field,
dataType: f.dataType,
header: f.header,
editorOptions: f.editorOptions,
filters: f.filters,
pipeArgs: f.pipeArgs,
defaultTimeFormat: f.defaultTimeFormat,
defaultDateTimeFormat: f.defaultDateTimeFormat
})) as FieldType[]
}
];

Expand All @@ -1249,7 +1273,7 @@ export class IgxHierarchicalGridComponent extends IgxHierarchicalGridBaseDirecti
this.data[0][rowIsland.key][0] : null;
return acc.concat(this.generateChildEntity(rowIsland, childFirstRowData));
}
, []);
, []);
}

return entities;
Expand Down
23 changes: 23 additions & 0 deletions projects/igniteui-angular/grids/hierarchical-grid/src/lifecycle-placeholder-element.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
const LIFECYCLE_PLACEHOLDER_TAG = 'igc-lifecycle-placeholder';
const LIFECYCLE_CONNECTED_EVENT = 'igcConnected';
const LIFECYCLE_DISCONNECTED_EVENT = 'igcDisconnected';

/** @hidden @internal */
export function registerLifecyclePlaceholderElement(): void {
if (typeof customElements === 'undefined' || typeof HTMLElement === 'undefined' ||
typeof CustomEvent === 'undefined' || customElements.get(LIFECYCLE_PLACEHOLDER_TAG)) {
return;
}

class LifecyclePlaceholderElement extends HTMLElement {
public connectedCallback(): void {
this.dispatchEvent(new CustomEvent(LIFECYCLE_CONNECTED_EVENT));
}

public disconnectedCallback(): void {
this.dispatchEvent(new CustomEvent(LIFECYCLE_DISCONNECTED_EVENT));
}
}

customElements.define(LIFECYCLE_PLACEHOLDER_TAG, LifecyclePlaceholderElement);
}
6 changes: 3 additions & 3 deletions src/app/hierarchical-grid/hierarchical-grid.sample.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,12 @@ <h4 class="sample-title">Sample two</h4>
<igx-grid-toolbar-exporter></igx-grid-toolbar-exporter>
</igx-grid-toolbar-actions>
</igx-grid-toolbar>
<igx-row-island [key]="'childData'" [autoGenerate]="true" [rowSelection]='selectionMode' [batchEditing]="true" [rowEditable]="true"
<igx-row-island [rowEditable]="true" [primaryKey]="'ID'" [key]="'childData'" [autoGenerate]="true" [rowSelection]='selectionMode' [batchEditing]="true" [rowEditable]="true"
[allowFiltering]="true">
<igx-row-island [key]="'childData'" [autoGenerate]="true" [rowSelection]='selectionMode' [batchEditing]="true" [rowEditable]="true"
<igx-row-island [rowEditable]="true" [primaryKey]="'ID'" [key]="'childData'" [autoGenerate]="true" [rowSelection]='selectionMode' [batchEditing]="true" [rowEditable]="true"
[allowFiltering]="true"></igx-row-island>
Comment on lines +149 to 152
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

In this sample, [rowEditable]="true" is specified twice on the same <igx-row-island> (and again on the nested one). This is redundant and can be confusing when editing the sample (and may mask accidental mismatched values). Keep a single [rowEditable] binding per element.

Copilot uses AI. Check for mistakes.
</igx-row-island>
<igx-row-island [key]="'childData2'" [autoGenerate]="true" [allowFiltering]="true"></igx-row-island>
<igx-row-island [rowEditable]="true" [primaryKey]="'ID'" [key]="'childData2'" [autoGenerate]="true" [allowFiltering]="true"></igx-row-island>
</igx-hierarchical-grid>


Expand Down
Loading