Skip to content

Reconcile realized rows after collapsing all groups#320

Open
wieslawsoltes wants to merge 4 commits into
masterfrom
agent/issue-309
Open

Reconcile realized rows after collapsing all groups#320
wieslawsoltes wants to merge 4 commits into
masterfrom
agent/issue-309

Conversation

@wieslawsoltes

@wieslawsoltes wieslawsoltes commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • reconcile the virtualized display list once after CollapseAllGroups() finishes updating every group and collapsed-slot range
  • recycle any leaf rows left outside the final visible slot set before scrollbar computation and arrange invalidation
  • add an 850-row, two-level grouping regression with a dominant first group and many second-level headers

Root cause

CollapseAllGroups() collapses headers one at a time through EnsureRowGroupVisibility and UpdateRowGroupVisibility. The collapse path intentionally performs incremental removals from the circular realized-element buffer with updateSlotInformation: false, then updates _collapsedSlotsTable afterward. A per-group UpdateDisplayedRows reconciliation only occurs for some scroll-anchor positions.

For distribution-sensitive multi-level grouping, those incremental operations can leave a viewport-sized run of realized DataGridRow controls outside the final visible slot set. The group model and collapsed-slot table are already correct, which is why a later scroll, resize, or expansion repairs the display, but CollapseAllGroups() previously had no final display reconciliation of its own.

Fix

After every group has been collapsed and the collapsed-slot table has settled, CollapseAllGroups() performs one final realization rebuild. For legacy scrolling it derives the first visible header and partial-row offset from the adjusted scrollbar offset; logical scrolling continues through its anchor-alignment path. The reconciliation runs once per explicit collapse-all gesture and adds no work to regular scrolling.

Regression coverage

The new headless test creates:

  • 850 rows;
  • a 640-row dominant first top-level group;
  • dense second-level segments spanning the viewport;
  • a constrained 320-pixel grid viewport.

It performs collapse → expand → collapse to exercise display-list reuse, confirms rows are realized after expansion, and then asserts immediately after the final CollapseAllGroups() call that no visible leaf DataGridRow remains.

Validation

  • focused regression: 1 passed
  • complete group expand/collapse suite: 5 passed
  • complete Avalonia.Controls.DataGrid.UnitTests headless suite: 2,251 passed, 0 failed
  • production build (net8.0, net10.0): 0 errors; existing repository warnings only
  • git diff --check: clean

Closes #309

Review hardening

Follow-up review expanded the final reconciliation beyond stale-row removal:

  • top-clamped collapses rebuild from the first visible group header with offset and partial-row state reset;
  • positive legacy offsets are clamped to the collapsed visible extent and mapped back to the corresponding visible header/partial offset before realization;
  • a regression with 80 preceding top-level groups covers collapsing while scrolled near the end of a dominant group.

Current validation: 6/6 focused collapse/expand tests and 2,252/2,252 full unit tests pass. Review threads are resolved.

@wieslawsoltes

Copy link
Copy Markdown
Owner Author

Self-review completed. I reviewed the collapse-all batch reconciliation point, realized-row cleanup behavior, viewport-bounded work, and regression coverage; no additional actionable findings were found. All six CI checks are successful, the branch is synchronized, and git diff --check is clean. This PR is ready for review.

@wieslawsoltes
wieslawsoltes marked this pull request as ready for review July 15, 2026 00:23

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15920190cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ca8d1a0dc0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Avalonia.Controls.DataGrid/DataGrid.Rows.Groups.cs Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c96857fa5e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/Avalonia.Controls.DataGrid/DataGrid.Rows.Groups.cs Outdated
@pull-request-size pull-request-size Bot added size/L and removed size/M labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CollapseAllGroups() leaves orphaned realized rows below collapsed headers for certain multi-level group distributions

1 participant