[Windows] - Fix Grid layout not updating when Row/Column changed dynamically#30206
[Windows] - Fix Grid layout not updating when Row/Column changed dynamically#30206prakashKannanSf3972 wants to merge 7 commits intodotnet:mainfrom
Conversation
|
Hey there @@prakashKannanSf3972! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull Request Overview
This PR ensures that dynamic changes to Grid.Row and Grid.Column on Windows correctly invalidate the parent layout so the UI updates without requiring a window resize.
- Added a UI sample page in the HostApp for toggling row/column at runtime.
- Added a cross-platform UI test in TestCases.Shared.Tests to verify dynamic grid updates.
- Updated
Grid.csto propagate measure invalidation to the parent grid on Windows.
Reviewed Changes
Copilot reviewed 3 out of 11 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20404.cs | New UITest verifying dynamic row/column toggles trigger layout updates. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue20404.cs | New sample page with buttons to toggle grid row/column dynamically. |
| src/Controls/src/Core/Layout/Grid.cs | Updated Invalidate method to explicitly invalidate parent grid on Windows. |
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the AI's summary?
274e5b1 to
dc74de7
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 30206Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 30206" |
Validated the AI summary and addressed the concerns accordingly. |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
🚦 Gate — Test Verification📊 Expand Full Gate —
|
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | Issue20404 | Issue20404 |
Verification
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL | ✅ |
| With fix | PASS | FAIL | ❌ |
Details
- ❌ Failed: DynamicGridRowColumnChangeShouldInvalidate [3 s]
- 📋 Error: VisualTestUtils.VisualTestFailedException :
Snapshot different than baseline: AfterGridRowToggled.png (36.09% difference)
If the correct baseline has changed (this isn't a a bug), then update the b... - ❌ Failed: DynamicGridRowColumnChangeShouldInvalidate [2 s]
- 📋 Error: VisualTestUtils.VisualTestFailedException :
Snapshot different than baseline: AfterGridRowToggled.png (6.51% difference)
If the correct baseline has changed (this isn't a a bug), then update the ba...
Fix Files Reverted
eng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/Layout/Grid.cs
Base Branch: main | Merge Base: 720a9d4
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #30206 | #if WINDOWS block: containerGrid.InvalidateMeasure() + return when child is Grid with Grid parent |
❌ FAILED (Gate) | Grid.cs, 8 snapshot PNGs |
Snapshot mismatch in CI; fix too narrow; #if WINDOWS unnecessary |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Remove else from else if in Grid.Invalidate() standalone if; GetRect() test assertions |
PASS | Grid.cs, Issue20404.cs |
Most minimal 1-char change; cross-platform; no #if WINDOWS |
| 2 | try-fix (claude-sonnet-4.6) | Add if (grid.Parent is Grid parentGrid) { parentGrid.InvalidateMeasure(); } inside if (bindable is Grid) block; kept else if; GetRect() assertions |
PASS | Grid.cs, Issue20404.cs |
Cross-platform; preserves existing structure; adds minimal parent check |
| 3 | try-fix (gpt-5.3-codex) | Walk full ancestor chain invalidating ALL ancestor Grids; GetRect() assertions | PASS | Grid.cs, Issue20404.cs |
More thorough but overkill; handles deeply nested Grids |
| 4 | try-fix (gpt-5.1-codex) | Invalidate nearest Layout parent (not just Grid parent); covers non-Grid parents too; GetRect() assertions | PASS | Grid.cs, Issue20404.cs |
Broader fix; handles Grid-in-StackLayout-in-Grid case |
| 5 | try-fix (claude-sonnet-4.6) | Override OnChildAdded/OnChildRemoved to subscribe to PropertyChanged; call InvalidateMeasure() on parent Grid |
PASS | Grid.cs, Issue20404.cs, 2xPublicAPI.Unshipped.txt |
Different paradigm (parent observes children); adds public API surface; most complex |
| PR | PR #30206 | #if WINDOWS block: containerGrid.InvalidateMeasure() + return when child is Grid with Grid parent |
FAILED (Gate) | Grid.cs, 8 snapshot PNGs |
Snapshot mismatch in CI; fix too narrow; #if WINDOWS unnecessary |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Yes | Fix in Windows LayoutHandler/LayoutPanel at platform boundary |
| claude-sonnet-4.6 | 2 | Yes | Override OnChildPropertyChanged in Grid became Attempt 5 |
| gpt-5.3-codex | 2 | Yes | Force Windows arrange invalidation via InvalidateArrange() on parent |
| gpt-5.1-codex | 2 | Yes | Platform-specific handler hook for Windows native panel invalidation |
| claude-opus-4.6 | 3 | No | NO NEW IDEAS "Attempt 1 is the minimal, correct fix" |
| claude-sonnet-4.6 | 3 | Yes | Override InvalidateMeasure() on Grid to chain to Parent |
| gpt-5.3-codex | 3 | Yes | Layout-version invalidation + InvalidateArrange() targeting stale arrange cache |
| gpt-5.1-codex | 3 | Yes | Hook into attached BindableProperty changed callback to walk up immediately |
Exhausted: Yes (max 3 rounds reached)
Selected Fix: Candidate #1 (remove else from else if) Most minimal fix: 1-character change in Grid.cs. Directly fixes the logical root cause (decouples the two independent checks). Cross-platform, no #if WINDOWS, no added public API surface, no event subscriptions. Test: replace VerifyScreenshot with GetRect() position assertions.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #20404, Windows-only layout staleness bug when Grid.Row/Column set dynamically on child Grid |
| Gate | ❌ FAILED | Windows — test fails even with PR's fix (snapshot PNG baseline mismatch: 36.09% / 6.51% diff) |
| Try-Fix | ✅ COMPLETE | 5 attempts, all 5 passing; better fix found (Candidate #1) |
| Report | ✅ COMPLETE |
Summary
PR #30206 fixes a real, reproducible bug: calling Grid.SetRow()/Grid.SetColumn() dynamically on a child Grid element leaves the parent Grid's layout stale on Windows (WinUI) until a window resize forces re-measurement. The PR's fix is on the right track but uses unnecessary #if WINDOWS compile-time guards, includes a confusing return statement, and ships snapshot PNG baselines captured on the author's machine that don't match the CI rendering environment — causing the gate to fail even with the fix applied.
Try-Fix exploration found 5 independent passing fixes. The simplest and best is Candidate #1: remove else from else if in Grid.Invalidate(), making both checks independent. This is a 1-character change that directly fixes the logical root cause, is cross-platform, and requires no #if WINDOWS guard. The test must also be updated to use GetRect() position assertions instead of VerifyScreenshot().
Root Cause
Grid.Invalidate() has a structural asymmetry using else if:
if (bindable is Grid grid)
{
grid.InvalidateMeasure(); // only child is invalidated
}
else if (bindable is Element element && element.Parent is Grid parentGrid)
{
parentGrid.InvalidateMeasure(); // never reached when child IS a Grid!
}Since Grid is also an Element, the else if condition is mutually exclusive with the first if. When a child is a Grid, only that child gets InvalidateMeasure() — the parent Grid never does. On Android/iOS, the native layout system propagates invalidation upward automatically, masking this bug. On Windows/WinUI it doesn't, so the parent Grid never re-measures.
Fix Quality
PR's fix — ❌ FAILED gate:
#if WINDOWScompile-time guard for a cross-platform logical inconsistency — unnecessaryreturnexits theInvalidatemethod early (harmless today, fragile for future changes)- Only handles the Grid-in-Grid case; a Grid nested in other layout types is unaffected
- Snapshot PNG baselines were captured on author's machine; don't match CI environment
Recommended fix — Candidate #1 (✅ PASS):
src/Controls/src/Core/Layout/Grid.cs:
- else if (bindable is Element element && element.Parent is Grid parentGrid)
+
+ if (bindable is Element element && element.Parent is Grid parentGrid)src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20404.cs: Replace VerifyScreenshot() calls with GetRect() position assertions on DynamicLabel:
// After ToggleRow: DynamicGrid moves to row 0 → label Y should decrease
var rectBefore = App.WaitForElement("DynamicLabel").GetRect();
App.Tap("ToggleRowButton");
var rectAfter = App.WaitForElement("DynamicLabel").GetRect();
Assert.That(rectAfter.Y, Is.LessThan(rectBefore.Y));Benefits:
- ✅ 1-character change in Grid.cs (
elseremoved) — minimal, elegant - ✅ Cross-platform — no
#if WINDOWS, noOperatingSystem.IsWindows()check - ✅ Fixes the logical root cause (decouples the two independent checks)
- ✅ Harmless on other platforms (extra
InvalidateMeasure()on parent is a no-op when platform auto-propagates) - ✅ Test uses
GetRect()movement assertions — CI-environment-independent, no PNG baselines needed
Required Changes
src/Controls/src/Core/Layout/Grid.cs— Changeelse iftoif(removeelse)src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20404.cs— ReplaceVerifyScreenshotwithGetRect()position assertions- Remove all snapshot PNG files (
AfterGridRowToggled.png,AfterGridColumnToggled.pngacross Android/Mac/WinUI/iOS test projects) — unused whenVerifyScreenshotis removed and incorrect anyway
kubaflo
left a comment
There was a problem hiding this comment.
Test is failing; could you please verify?
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the AI's summary?
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!
Root Cause
Grid.RoworGrid.Columnproperties are updated on aGridelement nested within anotherGrid, the current implementation only invalidates the element itself and does not propagate the invalidation to its parent. As a result, the parent layout is not re-measured or arranged correctly, leading to inaccurate or incomplete rendering.Description of Change
Gridon Windows to ensure UI updates correctly whenGrid.RoworGrid.Columnproperties change.Issues Fixed
Fixes #20404
Tested the behaviour in the following platforms
Output
BeforeFix_Grid.mp4
AfterFix_Grid.mp4