Repository navigation
fix(grid): skip redundant sizing pass after data and column change - 21.2.x - #17802
Open
viktorkombov wants to merge 6 commits into
Open
viktorkombov wants to merge 6 commits into
viktorkombov wants to merge 6 commits into
Conversation
The post-render width callback in calculateGridSizes re-checks the grid after every sizing pass. When the data and columns of a percentage-height grid are replaced, that re-check already sizes the grid for the new columns, and the Promise-deferred repaints from onColumnsAddedOrRemoved and resetSummaryHeight then ran a third, identical sizing pass. Route those repaints through notifyChangesAfterCurrentPass, which drops the request once a post-render check and a new sizing pass have run after it. Closes #17792 Co-Authored-By: Claude Opus 5.5 <[email protected]>
viktorkombov
added
❌ status: awaiting-test
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Shared render timing needs browser validation across both change-detection modes, with replacement tests exercising the column-add/remove path.
1 open issue
What changed in this PR
Backports #17801 to 21.2.x to reduce redundant grid sizing after data and column replacement, addressing #17792.
Changes:
- Skips deferred repaints already covered by post-render sizing.
- Routes column and summary updates through the shared helper.
- Adds zoned and zoneless sizing and geometry tests.
| File | Description |
|---|---|
| projects/igniteui-angular/grids/grid/src/grid.component.spec.ts | Adds replacement and resize regression tests. |
| projects/igniteui-angular/grids/grid/src/grid-base.directive.ts | Tracks sizing progress and skips redundant repaints. |
| projects/igniteui-angular/grids/core/src/summaries/grid-summary.service.ts | Uses the shared deferred repaint helper. |
| projects/igniteui-angular/grids/core/src/common/grid.interface.ts | Declares the helper on the grid interface. |
Review effort: Balanced
Comment on lines
+3785
to
+3790
| ({ key: `g${gen}c${i}`, field: `f${i}`, header: `G${gen} C${i}`, width: `${dynamicColumnWidth(gen)}px` })); | ||
|
|
||
| const makeDynamicData = (gen: number) => Array.from({ length: DYNAMIC_COLUMNS_ROWS }, (_, r) => { | ||
| const row: Record<string, any> = { id: r }; | ||
| for (let c = 0; c < DYNAMIC_COLUMNS_COUNT; c++) { | ||
| row[`f${c}`] = `g${gen} r${r} c${c}`; |
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
The hierarchical grid publishes new columns in a Promise and re-sizes only when their count changes. When a sizing pass ran before the publication, the deferred summary repaint was dropped, so the old headers and widths stayed until the next change detection. Request a repaint after a same-count replacement as well. Co-Authored-By: Claude Opus 5.5 <[email protected]>
In the data switch of #17792 the new data meets the old filters, so the vertical scrollbar hides and shows again, and that repaint request outlives the one the removed columns cancel. Filter an old column in the new-fields case, so it fails without the fix. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…1.2.x # Conflicts: # projects/igniteui-angular/grids/hierarchical-grid/src/hierarchical-grid.spec.ts
This branch has not been deployed
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 #17792
Description
Backport of #17801 to 21.2.x.
Replacing the data and the columns of a percentage-height grid ran three sizing passes instead of two. This is the "Data switch" slowdown reported since 21.2.12 / 22.0.4.
Problem: since #17328,
calculateGridSizes()schedules a post-render check (runAfterRenderOnce) that runs the grid'sngDoCheckagain. Zoneless apps depend on it. On a data and column switch, that check already sizes the grid for the new columns. Then the Promise-deferred repaints fromonColumnsAddedOrRemoved()andIgxGridSummaryService.resetSummaryHeight()(one per new column) ran a third, identical sizing pass, in both zoneless and ZoneJS apps. In the benchmark's switch the fields change and removing the old columns cancels that request, but it is made again in the same pass: the new data first meets the old filters, so the vertical scrollbar hides and shows again.Fix: the post-render check is unchanged. Those deferred repaints now go through a new internal
notifyChangesAfterCurrentPass(). It drops a request only when a post-render check and a new sizing pass have both run after it; otherwise it repaints as before. The hierarchical grid publishes new columns in a Promise, so it now also requests a repaint when they replace columns of the same count; before, it relied on that summary repaint.Motivation / Context
Fixes the regression reported in #17792. Replaying the benchmark's data-source switch with its own component, steps and mutation timer, the switch runs two sizing passes instead of three and takes about 17-25% less time, in line with reverting the width callback. Skipping the post-render width callback for explicit widths is not an option: it breaks existing zoneless tests (aria-colcount with grouped columns, last row after scrolling) and the filter chips indicator after a column resize.
Type of Change (check all that apply):
Component(s) / Area(s) Affected:
Grid, Tree Grid, Hierarchical Grid (shared sizing in the grid base), grid summary service
How Has This Been Tested?
New
IgxGrid - sizing after data and column replacementspecs ingrid.component.spec.ts, run zoneless (with exhaustivecheckNoChanges) and with ZoneJS:100%, px andnullwidthsnullwidth, and resizing the container of a100%gridhierarchical-grid.spec.ts)grid.component,grid-filtering-ui,grid-summary,grid.master-detailand the hierarchical grid specs pass locally.Test Configuration:
Screenshots / Recordings
N/A
Checklist:
feature/README.MDupdates for the feature docsREADME.MDCHANGELOG.MDupdates for newly added functionalityng updatemigrations for the breaking changes (migrations guidelines)🤖 Generated with Claude Code