Skip to content

fix(elements): attach nested elements to their parent's view - #17628

Open
mddragnev wants to merge 6 commits into
masterfrom
mdragnev/fix-elements-children
Open

mddragnev wants to merge 6 commits into
masterfrom
mdragnev/fix-elements-children

Conversation

@mddragnev

@mddragnev mddragnev commented Sep 15, 2026

Copy link
Copy Markdown
Member

Closes #17598

Description

Nested custom elements are now attached into their parent element's view instead of being attached to the ApplicationRef as standalone roots.

IgxCustomNgElementStrategy resolves a ViewContainerRef from the parent component's injector (a container anchored at the parent's host element) and inserts the child's host view there.

Because the anchor is no longer needed, the @igxElementsAnchor plumbing is removed: the anchor property on IgxGridBaseDirective and the <div #sink> markup in the grid, tree grid, hierarchical grid, pivot grid and row island templates.

The DOM position restore that follows the insert now derives its reference node from the last of the host view's root nodes rather than from element.nextSibling. ViewContainerRef.insert() relocates every root node of the view, and some components (igc-action-strip, igc-grid-state) have a trailing anchor comment in addition to the element - using it as the reference node makes insertBefore throw NotFoundError once it has been moved.

Motivation / Context

Elements runs with zoneless change detection, which only walks views reachable from whatever was marked dirty. A detached root is unreachable from its parent, so a grid's own notifyChanges()/markForCheck() never refreshed nested elements and they kept rendering stale state until something ticked their own root.

This surfaced in the grid toolbar actions, which bind directly to live grid state rather than to inputs:

  • igc-grid-toolbar-pinning kept showing the old pinnedColumnsCount after pinning/unpinning a column
  • igc-grid-toolbar-advanced-filtering kept showing the old column count after applying a filter

The same scenarios work under zone.js, where every tick re-checks all attached views regardless of the view hierarchy, and in a plain Angular app, where these components are genuine descendants of the grid's view.

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, Pivot Grid

How Has This Been Tested?

  • Unit tests
  • Manual testing
  • Automated e2e tests

Three tests added to custom-strategy.spec.ts, all verified to fail when the behaviour is reverted:

  • View hierarchy - only the grid's host view is among the ApplicationRef root views; toolbar, title, actions, hiding, pinning, column and paginator are not.
  • DOM position - nothing is stranded next to the grid after the insert, parent/child relationships and sibling order are kept as authored, and the projected paginator keeps its position deeper inside the grid.
  • Change detection - pinning/unpinning a column through the grid API alone refreshes pinnedColumnsCount in the nested toolbar action, with no input or event reaching that element.

Test Configuration:

  • Angular version:
  • Browser(s):
  • OS:

Screenshots / Recordings

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

Copilot AI lite review requested due to automatic review settings September 15, 2026 14:01
@mddragnev mddragnev changed the title Mdragnev/fix elements children fix(elements): attach nested elements to their parent's view Sep 15, 2026

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.

🟡 Changes recommended

Critical and moderate issues remain in parent attachment, reparenting, and row-island projection behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes nested Angular Elements grid views so they attach to parent view containers, preserve DOM order, and update correctly with zoneless change detection.

Changes:

  • Replaces obsolete sink anchors with parent ViewContainerRef attachment.
  • Updates DOM-position restoration and row-island projections.
  • Adds view hierarchy, DOM-position, and change-detection tests.
File summaries
File Summary
projects/igniteui-angular/grids/tree-grid/src/tree-grid.component.html Removes obsolete sink markup.
projects/igniteui-angular/grids/pivot-grid/src/pivot-grid.component.html Removes obsolete sink markup.
projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.component.html Removes obsolete sink markup.
projects/igniteui-angular/grids/grid/src/grid.component.html Removes obsolete sink markup.
projects/igniteui-angular/grids/grid/src/grid-base.directive.ts Removes the anchor property.
projects/igniteui-angular-elements/src/lib/grids/row-island.component.ts Updates projected content; a moderate visibility/overlay issue remains (1 vote).
projects/igniteui-angular-elements/src/app/custom-strategy.ts Changes view attachment and DOM restoration; critical parent-attachment (2 votes), moderate reparenting (1 vote), and trailing-anchor coverage nit (1 vote) remain.
projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts Adds hierarchy, DOM-position, and change-detection regression tests.
Review details

Suppressed comments (3)

projects/igniteui-angular-elements/src/app/custom-strategy.ts:140

  • Once a nested element's view is inserted into this VCR, a DOM move across semantic parents within Angular Elements' delayed-destroy window only cancels the pending destroy; connect() does not rerun this parent lookup or reinsert the view. The element therefore remains attached to the old parent's VCR/injector, so change detection from the new parent cannot reach it (and destroying the old parent can destroy the moved child). Reconcile the logical view attachment when a reconnect resolves a different parent, or do not preserve the instance across such a reparenting move.
                parentAnchor = parentInjector.get(ViewContainerRef);

projects/igniteui-angular-elements/src/app/custom-strategy.ts:198

  • This new reference-node logic is specifically meant to handle views whose rootNodes include a trailing anchor comment (igc-action-strip/igc-grid-state), but the added DOM-position test only creates toolbar, column, and paginator elements. A regression in the reason for replacing element.nextSibling would therefore still pass the suite. Please add an action-strip or grid-state fixture and assert that it remains in its authored position after attachment.
            const nextSibling = (this as any).componentRef.hostView.rootNodes.at(-1).nextSibling;

projects/igniteui-angular-elements/src/lib/grids/row-island.component.ts:32

  • Removing this wrapper changes more than the anchor: row-island content includes igc-action-strip (for example, src/app/index.html:304-318), whose theme makes the host a full-size absolutely positioned overlay (action-strip/themes/_base.scss:31-50). The new template therefore renders that declarative action strip visibly at the row-island declaration instead of keeping it hidden/configuration-only, which can add an extra overlay to the grid. Keep a non-anchoring display: none wrapper around these projections while removing only #sink.
    template: `
    <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>
    `,
  • Files reviewed: 8/8 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.ts Outdated

@damyanpetev damyanpetev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor nits on the tests and comments, overall LGTM;

Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts Outdated
Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts Outdated
Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.spec.ts Outdated
Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.ts Outdated
Comment thread projects/igniteui-angular-elements/src/app/custom-strategy.ts Outdated
@viktorkombov viktorkombov added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Sep 16, 2026
@viktorkombov viktorkombov added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Sep 16, 2026

@damyanpetev damyanpetev left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

Note: This doesn't dismiss the need to resolve the toolbar actions rendering dependency on the grid running full change detection (prob move to signals), it just makes Elements consistent to Angular's runtime.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Elements] Grid column actions indicators are not updated when column state changes

4 participants