Bugfix/26633 grid layout manager

[maonaoda]

Description of Change

When calculating the width and height of the cell, add space to prevent using the wrong view size to calculate the platform size.

iOS:

Before fix	After fix

Android:

Before fix	After fix

maccatalyst:

Before fix	After fix

Issues Fixed

Fixes #26633

[dotnet-policy-service]

Hey there @maonaoda! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[jfversluis]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[maonaoda]

Should I compare the heights of the Labels directly, or use DeviceTest instead of UITest?

[Copilot]

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

Files not reviewed (1)

• src/Controls/tests/TestCases.HostApp/Issues/Issue26633.xaml: Language not supported

[jfversluis]

Sorry that we didn't get to this yet! Let me see if we can get a bit more attention for it :)

[PureWeen]

/rebase

[PureWeen]

/azp run maui-pr-uitests, maui-pr-devicetests

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[MauiBot]

🤖 AI Summary

📊 Expand Full Review — 7fcb93d · Fix unit test expectations: 200px (not 152px)

---

🔍 Pre-Flight — Context & Validation

---

Issue: #26633 - [iOS/maccatalyst/Android] Label height in Grid with ColumnSpacing > 0 incorrect in certain cases
PR: #26641 - Bugfix/26633 grid layout manager
Platforms Affected: iOS, Android, macOS, Windows (issue); PR targets shared grid layout logic
Files Changed: 1 implementation, 1 test

Key Findings

• The linked issue reports incorrect label sizing in Grid when spacing is non-zero and the child spans multiple rows or columns; the reproduction repo is https://github.com/maonaoda/MauiBugs/tree/Issues/26633.
• The PR changes shared layout logic in src/Core/src/Layouts/GridLayoutManager.cs and adds unit coverage in src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs; it does not add UI tests under TestCases.HostApp / TestCases.Shared.Tests.
• PR discussion is light: the contributor asked whether UI measurement should be validated via label height comparison or device tests, and there is one resolved outdated Copilot thread about removing a BOM from a HostApp file that is no longer part of the current diff.
• No prior agent review comment with the Final Recommendation / phase-status table format was found in PR comments.

Edge Cases Mentioned

• The issue lists iOS, Android, macOS, and Windows as affected, while the PR description only demonstrates iOS, Android, and MacCatalyst screenshots.
• The fix adjusts both row-span and column-span measurement paths, so regressions could affect any spanned child measured against mixed auto/star definitions.

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
PR	PR #26641	Add missing inter-column/inter-row spacing back into the width/height constraints used when measuring spanned grid children; add unit tests for spanning all columns and all rows.	⏳ PENDING (Gate)	src/Core/src/Layouts/GridLayoutManager.cs, src/Core/tests/UnitTests/Layouts/GridLayoutManagerTests.cs	Original PR

---

🚦 Gate — Test Verification

---

Gate Result: ⚠️ SKIPPED

Platform: android
Mode: Full Verification requested; formal UI gate skipped

• Tests FAIL without fix: ✅ (supplemental focused unit verification in clean PR-only worktree)
• Tests PASS with fix: ✅ (supplemental focused unit verification in clean PR-only worktree)

Notes

• No tests were added or changed under src/Controls/tests/TestCases.HostApp/ or src/Controls/tests/TestCases.Shared.Tests/, so the formal verify-tests-fail-without-fix UI gate path does not apply to this PR.
• Supplemental validation was performed against src/Core/tests/UnitTests/Core.UnitTests.csproj filtered to SpannedCellMeasurementIncludes* in a clean worktree created from the PR base SHA plus the squashed PR commit.
• In that clean worktree, reverting only src/Core/src/Layouts/GridLayoutManager.cs made the new tests fail (188 instead of 200), and restoring the PR fix made them pass again. The broader GridLayoutManager test suite also passed on the PR version.

---

🔧 Fix — Analysis & Comparison

---

Fix Candidates

#	Source	Approach	Test Result	Files Changed	Notes
1	try-fix (claude-opus-4.6)	Replace the duplicated second-pass span measurement loops with UpdateKnownMeasureHeight / UpdateKnownMeasureWidth, which already include spacing.	✅ PASS	1 file	Removes duplication by reusing existing helper logic.
2	try-fix (claude-sonnet-4.6)	Stop recomputing span sizes in SecondMeasurePass and trust the already-populated cell.MeasureWidth / cell.MeasureHeight values.	✅ PASS	1 file	Biggest simplification, but relies heavily on earlier pipeline stages populating cached measures correctly.
3	try-fix (gpt-5.3-codex)	Use GetCellBoundsFor(view, 0, 0) in SecondMeasurePass to obtain the spanned geometry including spacing.	✅ PASS	1 file	Reuses an existing geometry helper instead of rebuilding width/height locally.
4	try-fix (gemini-3-pro-preview)	Introduce a reusable CalculateSpanSize helper and call it from SecondMeasurePass.	✅ PASS	1 file	Centralizes the span-size math and keeps the call site straightforward.
5	try-fix (claude-opus-4.6 round 2)	Derive span size from LeftEdgeOfColumn / TopEdgeOfRow edge subtraction.	✅ PASS	1 file	Uses existing positional math so spacing is implicit.
6	try-fix (claude-sonnet-4.6 round 2)	Add a dedicated lifecycle step that refreshes deferred cell constraints before SecondMeasurePass, which then reads cell.MeasureWidth / cell.MeasureHeight directly.	✅ PASS	1 file	Works, but is notably broader than the others because it restructures the measurement pipeline.
PR	PR #26641	Add missing _rowSpacing / _columnSpacing terms inline when summing spanned rows and columns in SecondMeasurePass.	✅ PASS	2 files	Original PR; explicit fix plus issue-specific tests.

Cross-Pollination

Model	Round	New Ideas?	Details
claude-opus-4.6	1	Yes	Reuse existing helper methods that already handle spacing, instead of inlining spacing math in SecondMeasurePass.
claude-sonnet-4.6	1	Yes	Remove the redundant recomputation entirely and use cell.MeasureWidth / cell.MeasureHeight as the second-pass constraints.
gpt-5.3-codex	1	Yes	Reuse GetCellBoundsFor to centralize span geometry and spacing calculations.
gemini-3-pro-preview	1	Yes	Create a dedicated CalculateSpanSize helper to make the spacing-safe computation reusable and explicit.
claude-opus-4.6	2	Yes	Precompute edge positions and derive span size by subtracting start/end edges.
claude-sonnet-4.6	2	Yes	Defer spanned-auto cells into a single correctly constrained measurement step instead of correcting them in second pass.
gpt-5.3-codex	2	Yes	Use row/column offset subtraction instead of iterative summation.
gemini-3-pro-preview	2	Yes	Compute span size from column/row edge positions so spacing is implicit in positional math.
claude-opus-4.6	3	Yes	Model spacing as synthetic spacer tracks during track resolution.
claude-sonnet-4.6	3	Yes	Pre-calculate and cache the total spacing adjustment per child during initial analysis.
gpt-5.3-codex	3	Yes	Fix the omission through the shared UpdateKnownMeasure* helpers so callers benefit automatically.
gemini-3-pro-preview	3	Yes	Measure deferred cells eagerly during star resolution so they never reach SecondMeasurePass.

Exhausted: Yes (reached the 3-round cross-pollination limit)

Best Fix Selection

Candidate	Simplicity	Robustness	Style match	Verdict
PR	Best	Good	Best	Selected
1	Good	Good	Good	Slightly more implicit due to helper side effects
2	Best	Medium	Medium	Relies on hidden lifecycle invariants
3	Medium	Medium	Medium	More indirect than needed
4	Good	Good	Good	Clean, but adds abstraction without another caller
5	Medium	Good	Medium	Clever, but less obvious than the bug itself
6	Poor	Good	Medium	Broadest behavioral change

Selected Fix: PR — It is the smallest and clearest patch that directly addresses the bug at the point where the incorrect constraint is computed, while the added issue-specific tests demonstrably fail without the fix and pass with it.

---

📋 Report — Final Recommendation

---

✅ Final Recommendation: APPROVE

Phase Status

Phase	Status	Notes
Pre-Flight	✅ COMPLETE	Issue #26633, shared Grid layout fix, 1 implementation file + 1 unit-test file changed
Gate	⚠️ SKIPPED	No HostApp/Shared UI tests, but supplemental focused unit tests failed without fix and passed with fix
Try-Fix	✅ COMPLETE	6 passing alternative attempts explored across required models; PR fix still selected
Report	✅ COMPLETE

Summary

PR #26641 fixes a real bug in GridLayoutManager.SecondMeasurePass() where spanned cells were measured too small because the inter-row/inter-column spacing was omitted from the finite width/height constraints. The new issue-specific unit tests catch the regression: they fail on the broken baseline and pass with the PR applied. I also explored six independent alternatives; several worked, but none beat the PR on overall tradeoff.

Root Cause

The second-pass measurement path rebuilt the available span size by summing the row or column definition sizes only. For cells spanning multiple rows or columns, that omitted the (span - 1) * spacing gap between definitions, so the constraint passed into MeasureCell was smaller than the actual grid space.

Fix Quality

The PR’s implementation is the most pragmatic option: it corrects the miscalculation exactly where it occurs, keeps the behavioral change tiny, and pairs it with focused regression tests for both column spacing and row spacing. Alternative fixes can reduce duplication or further restructure the measurement flow, but they all introduce more indirection or broader lifecycle coupling than this targeted patch needs.

---