fix(elements): Ensure ForOF out of bound row gets deleted before moving to opposite side.#17334
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets a virtualization issue affecting Elements scenarios (reported in #17179), where view recycling/disconnection can lead to incorrect DOM/state updates and, in turn, incorrect scroll sizing/behavior. It also adjusts a grid grouping virtualization test to better stabilize/assert horizontal virtualization state after vertical scrolling.
Changes:
- Updates
IgxForOfDirectiveview-reuse logic to guard against missing embedded views and to force a viewdetectChanges()before reinsertion (Elements-specific behavior). - Tweaks an
IgxGridGroupBy spec to restore virtualization state after vertical scrolling and reduce timing-related flakiness.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| projects/igniteui-angular/directives/src/directives/for-of/for_of.directive.ts | Adjusts recycled view handling during scroll-based view rearrangement (Elements/disconnection scenario). |
| projects/igniteui-angular/grids/grid/src/grid.groupby.spec.ts | Updates a virtualization regression test to stabilize state restoration after scroll. |
Comment on lines
1063
to
1068
| for (let i = start; i < end && this.igxForOf[i] !== undefined; i++) { | ||
| const embView = this._embeddedViews.shift(); | ||
| if (!embView.destroyed) { | ||
| if (embView && !embView.destroyed) { | ||
| this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE) | ||
| || embView.rootNodes[0].nextElementSibling); | ||
| const view = container.detach(0); |
Comment on lines
1088
to
1095
| for (let i = prevIndex - 1; i >= this.state.startIndex && this.igxForOf[i] !== undefined; i--) { | ||
| const embView = this._embeddedViews.pop(); | ||
| if (!embView.destroyed) { | ||
| if (embView && !embView.destroyed) { | ||
| this.scrollFocus(embView.rootNodes.find(node => node.nodeType === Node.ELEMENT_NODE) | ||
| || embView.rootNodes[0].nextElementSibling); | ||
| // embView and view both refer to the same collections | ||
| const view = container.detach(container.length - 1); | ||
|
|
Comment on lines
+2666
to
2676
| await wait(100); // Triggers onStable | ||
| fix.detectChanges(); | ||
|
|
||
| dataRows = grid.dataRowList.toArray(); | ||
| // Workaround for await wait triggering onStable prematurely | ||
| (grid as any)._restoreVirtState(dataRows[7]); | ||
| await wait(100); | ||
| fix.detectChanges(); | ||
|
|
||
| // verify rows are scrolled to the right | ||
| dataRows = grid.dataRowList.toArray(); | ||
| dataRows.forEach(dr => { |
Comment on lines
+1072
to
1076
| // Because in Elements the whole parent div (containing data-index) gets removed (possibly due to being disconnected). In Angular it just gets moved. | ||
| // This ensures to update it with the new context and remove it first from DOM because of detach action before inserting it manually. | ||
| view.detectChanges(); | ||
|
|
||
| container.insert(view); |
dkamburov
changed the title
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 #17179
Description
Motivation / Context
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
How Has This Been Tested?
Test Configuration:
Screenshots / Recordings
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)