Fix CollectionView grid spacing updates for first row and column#34527
Fix CollectionView grid spacing updates for first row and column#34527kubaflo merged 5 commits intodotnet:inflight/currentfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34527Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34527" |
There was a problem hiding this comment.
Pull request overview
Fixes a CollectionView GridItemsLayout runtime spacing update issue where the first row/column didn’t consistently participate in spacing changes, primarily affecting iOS/MacCatalyst (Items2 compositional layout) and Android (RecyclerView padding vs item decoration).
Changes:
- iOS/MacCatalyst (Items2): apply spacing via per-item
ContentInsets(half-spacing) and sectionContentInsets, settingInterItemSpacingto 0 for consistent edge participation. - Android (Items): stop applying negative RecyclerView padding for
GridItemsLayoutso the grid’s outer half-spacing remains visible. - Add a new HostApp repro page and corresponding Appium UI tests for Issue #34257.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34257.cs | Adds UI tests validating that spacing updates move the first row/column (needs a fix to correctly wait for updated status text). |
| src/Controls/tests/TestCases.HostApp/Issues/Issue34257.cs | Adds a repro page with a 2-column vertical grid and buttons to apply spacing changes. |
| src/Controls/src/Core/Handlers/Items2/iOS/LayoutFactory2.cs | Adjusts Items2 compositional grid spacing logic using item/section insets to ensure first row/column updates. |
| src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs | Changes grid padding behavior to preserve outer half-spacing applied by SpacingItemDecoration. |
| var firstColumnBefore = App.WaitForElement("FirstColumnTopItem").GetRect(); | ||
| App.Tap("ApplyHorizontalSpacingButton"); | ||
| App.WaitForElement("StatusLabel", "Spacing=0,80"); | ||
| var firstColumnAfter = App.WaitForElement("FirstColumnTopItem").GetRect(); |
| var firstColumnBefore = App.WaitForElement("FirstColumnTopItem").GetRect(); | ||
| App.Tap("ApplyVerticalSpacingButton"); | ||
| App.WaitForElement("StatusLabel", "Spacing=40,0"); | ||
| var firstColumnAfter = App.WaitForElement("FirstColumnTopItem").GetRect(); |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #34527 | Android keeps outer half-spacing for GridItemsLayout by disabling negative RecyclerView edge padding; iOS/MacCatalyst switches compositional grid spacing to item/section half-insets so first row/column participate during spacing updates. |
⏳ PENDING (Gate) | src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs, src/Controls/src/Core/Handlers/Items2/iOS/LayoutFactory2.cs, src/Controls/tests/TestCases.HostApp/Issues/Issue34257.cs, src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue34257.cs |
Original PR; test wait logic likely needs scrutiny because current waits do not verify label text updates. |
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: android
Mode: Full Verification
- Tests FAIL without fix: ✅
- Tests PASS with fix: ✅
Notes
- Verification ran through the
verify-tests-fail-without-fixworkflow and logged the expected sequence: theIssue34257UI test failed after reverting the fix files to merge-base state, then passed after restoring the PR changes. - The verification log also shows the script treated
eng/pipelines/ci-copilot.ymlas a fix file; that file is unrelated to the CollectionView fix and does not affect the review conclusion because the Android handler and UI test still produced the expected fail/pass behavior. - Unresolved PR review feedback still notes that the test code waits for
StatusLabelelement existence rather than waiting for its text to change, so the test coverage is validated but remains potentially flaky.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) |
Introduce a position-aware Android GridSpacingItemDecoration returned from CreateSpacingDecoration() so original negative RecyclerView padding remains, but grid edge items receive larger offsets that still leave visible outer half-spacing. |
✅ PASS | src/Controls/src/Core/Handlers/Items/Android/GridSpacingItemDecoration.cs, src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs |
Valid Android pass, but the attempt artifacts also picked up an unrelated HybridWebView.js diff from workspace state, so it is not the cleanest candidate. |
| 2 | try-fix (claude-sonnet-4.6) |
Make SpacingItemDecoration edge-aware for grid layouts so outer edges get no decoration offset, then use positive RecyclerView padding for grids to reserve the outside spacing explicitly. |
✅ PASS | src/Controls/src/Core/Handlers/Items/Android/SpacingItemDecoration.cs, src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs |
Cleanly validated on Android, but more invasive than the PR because it changes shared decoration behavior plus padding semantics. |
| 3 | try-fix (gpt-5.3-codex) |
Keep spacing math unchanged and force a full RecyclerView spacing recomputation/layout invalidation after spacing updates. | ❌ FAIL | src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs |
Unvalidated because BuildAndRunHostApp.ps1 failed before tests ran, reporting out-of-date MSBuild tasks in the detached worktree. |
| 5 | try-fix (gemini-3-pro-preview, retry) |
Keep the PR-style grid zero-padding change and additionally clear the recycled view pool plus invalidate/request layout after spacing updates. | ✅ PASS | src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs |
Passed, but it is effectively a more invasive superset of the PR’s Android fix and adds explicit RecyclerView churn the PR does not need. |
| PR | PR #34527 | Disable negative RecyclerView edge padding for Android GridItemsLayout; on iOS/MacCatalyst replace compositional grid inter-item spacing with item/section half-insets. |
✅ PASSED (Gate) | src/Controls/src/Core/Handlers/Items/Android/MauiRecyclerView.cs, src/Controls/src/Core/Handlers/Items2/iOS/LayoutFactory2.cs, tests |
Original PR; Gate verified fail-without-fix and pass-with-fix on Android. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
| claude-sonnet-4.6 | 2 | No | NO NEW IDEAS |
| gpt-5.3-codex | 2 | No | NO NEW IDEAS |
| gemini-3-pro-preview | 2 | No | NO NEW IDEAS |
Best Fix Comparison
| Candidate | Simplicity | Robustness | Scope | Verdict |
|---|---|---|---|---|
| PR | Best | Good | Minimal Android change + required iOS companion change | Selected |
| Attempt 1 | Low | Good | New class + contaminated artifacts | Not selected |
| Attempt 2 | Medium | Good | More invasive shared decoration changes | Not selected |
| Attempt 5 | Medium | Good | PR behavior plus extra RecyclerView invalidation churn | Not selected |
Exhausted: Yes
Selected Fix: PR's fix — it passed Gate, is the simplest cleanly justified Android solution among the passing candidates, and avoids the extra complexity/invasiveness introduced by the alternatives.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Linked issue #34257, PR scope, affected platforms, and unresolved review feedback were gathered. |
| Gate | ✅ PASSED | Android full verification showed Issue34257 fails without the fix and passes with the fix. |
| Try-Fix | ✅ COMPLETE | 4 models queried; 3 passing alternatives found, 1 unvalidated failure; cross-pollination exhausted with no new ideas. |
| Report | ✅ COMPLETE |
Summary
PR #34527 fixes the CollectionView grid-spacing regression for the supported paths it touches, and the Android UI test added in the PR was empirically verified by Gate: it fails against the broken baseline and passes with the PR applied. Mandatory try-fix exploration found multiple alternative Android implementations that can also satisfy the test, but none beat the PR on simplicity or overall fit.
The strongest alternatives either add a new grid-specific decoration algorithm, alter shared decoration behavior more broadly, or add extra RecyclerView invalidation/pool-clearing work on top of the PR behavior. By comparison, the PR's Android change is the most direct expression of the tested behavior: keep the outer half-spacing for grids so the first row and column participate when spacing changes at runtime.
Root Cause
On Android, SpacingItemDecoration applies half-spacing to all item edges, while UpdateItemSpacing() previously applied matching negative RecyclerView padding. For grid layouts, that cancelled the outer edge offsets, so the first row/column did not visibly move when spacing changed at runtime. On iOS/MacCatalyst, the compositional layout path needed spacing to be distributed through item and section insets so edge items participated consistently.
Fix Quality
The selected PR fix is the best candidate from review because it is minimal, passes the fail-without-fix / pass-with-fix gate, and scales across the active platform-specific implementations that the issue affects. One non-blocking weakness remains in the new UI tests: Issue34257 currently calls App.WaitForElement("StatusLabel", "Spacing=0,80") / ("Spacing=40,0"), which does not actually wait for the label text to change and could make the test race-prone. That does not overturn the review result because Gate still validated the bug-catching behavior, but it is worth tightening in a follow-up.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please make this Android only and resolve conflicts with inflight current?
And create a new PR for iOS?
Thanks!
48aab99 to
689e06f
Compare
I have created a separate PR (#34598) for iOS. The Android PR needs to be merged first; once that is done, we will enable the test in the iOS PR so that it can run successfully on iOS. |
…net#34527) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes dotnet#34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes #34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes #34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
…net#34527) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes dotnet#34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
…net#34527) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes dotnet#34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
…net#34527) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac N/A Fixes dotnet#34257 | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes #34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Issue Details: Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform. ### Root Cause: The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime. ### Description of Change: - On Android, the fix in MauiRecyclerView.cs changes how RecyclerView padding is handled for GridItemsLayout. Android was already using SpacingItemDecoration, which applies half-spacing on all four sides of each item. Previously, negative RecyclerView padding canceled that spacing at the control edges. The branch keeps that negative-padding behavior for non-grid layouts, but disables it for GridItemsLayout, allowing the grid’s half-spacing to remain visible at the outer perimeter. This makes the first row and first column visually respond when spacing changes, but it also changes the grid behavior from spacing only between items to spacing around the outside edges as well. **Tested the behavior in the following platforms:** - [x] Android - [x] Windows - [ ] iOS - [ ] Mac ### Reference: N/A ### Issues Fixed: Fixes #34257 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/578dda69-1d60-474c-a6d8-23b3f9d29a50" Width="300" Height="600"> | <Video src="https://github.com/user-attachments/assets/7f3826e6-5922-4b6f-a6b9-de581b7db6c3" Width="300" Height="600"> |
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details:
Horizontalspacing / Verticalspacing is not not applied to the first column in GridItemLayout using CollectionView on Android platform.
Root Cause:
The grid spacing was not being distributed symmetrically across the active layout implementations, so edge items did not fully participate when spacing changed at runtime.
Description of Change:
Tested the behavior in the following platforms:
Reference:
N/A
Issues Fixed:
Fixes #34257
Screenshots
Screen.Recording.2026-03-18.at.4.25.45.PM.mov
Screen.Recording.2026-03-18.at.4.21.12.PM.mov