[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). |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #30206 | In Grid.Invalidate(), add #if WINDOWS block to call containerGrid.InvalidateMeasure() when parent is Grid |
⏳ PENDING (Gate) | Grid.cs |
Original PR |
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: windows
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | DynamicGridRowColumnChangeShouldInvalidate | Issue20404 |
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL (36.09% screenshot diff) | ✅ |
| With fix | PASS | PASS (1/1 tests passed) | ✅ |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Change else if → separate if — cross-platform, handles Grid parent only |
✅ PASS | 1 file | Logical flow fix, minimal change |
| 2 | try-fix (claude-sonnet-4.6) | Add if (grid.Parent is Layout parentLayout) parentLayout.InvalidateMeasure() — cross-platform, handles any Layout parent |
✅ PASS | 1 file | Most general — handles Grid in StackLayout/FlexLayout etc. |
| 3 | try-fix (gpt-5.3-codex) | Walk full ancestor Grid chain + Windows-specific code | ❌ FAIL | 1 file | 6.51% screenshot diff |
| 4 | try-fix (gpt-5.4) | Child PropertyChanged observer in parent Grid | ❌ FAIL | 1 file | 6.51% screenshot diff |
| PR | PR #30206 | #if WINDOWS block in Invalidate() calls containerGrid.InvalidateMeasure() |
✅ PASSED (Gate) | 1 file | Windows-only compile guard, Grid parent only |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | Solution space covered across all axes |
| gpt-5.3-codex | 2 | Marginal | Suggested also calling InvalidateArrange() — not needed since InvalidateMeasure() triggers re-arrange |
| claude-sonnet-4.6 | 2 | No | Confirmed root cause: (Parent as VisualElement)?.OnChildMeasureInvalidated fires events only, never calls parent's IView.InvalidateMeasure() |
| gpt-5.4 | 2 | Minor | Suggested recomputing child constraints — not addressing parent invalidation root cause |
Exhausted: Yes
Selected Fix: Candidate #2 (Attempt 2, claude-sonnet-4.6) — Cross-platform fix that explicitly calls parentLayout.InvalidateMeasure() for any Layout parent. More general than the PR's Windows-only Grid-only check, no platform guards needed, minimal change.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #20404, Windows-only layout bug |
| Gate | ✅ PASSED | windows — 36.09% diff without fix, 1/1 pass with fix |
| Try-Fix | ✅ COMPLETE | 4 attempts, 2 passing (Attempts 1 & 2 superior to PR fix) |
| Report | ✅ COMPLETE |
Summary
The PR correctly identifies and fixes a real Windows-specific bug: when Grid.SetRow()/Grid.SetColumn() is called on a child Grid nested inside another Grid, the parent Grid does not re-measure on Windows (unlike iOS/Android which auto-propagate). The fix works. However, a cleaner cross-platform alternative was found during Try-Fix exploration that should be preferred over the PR's #if WINDOWS compile guard.
Root Cause
In Grid.Invalidate() (the property-changed callback for Row, Column, RowSpan, ColumnSpan attached properties), when bindable is Grid, the code calls grid.InvalidateMeasure() on the child but never calls InvalidateMeasure() on the parent. The else if branch that does invalidate the parent is skipped because Grid satisfies the first if condition. Windows' WinUI layout engine does not auto-propagate child invalidation to the parent when only the child's position changes (not its size), while iOS/Android native layout engines handle this automatically.
Fix Quality
The PR's fix (#if WINDOWS) works but has drawbacks:
- Uses compile-time platform guard — the bug could theoretically affect other platforms too if their native engines change behavior
- Only handles Grid parent — misses cases like Grid nested in StackLayout or FlexLayout
- Unnecessary
return— inside the#if WINDOWSblock aftercontainerGrid.InvalidateMeasure(), thereturnis harmless but needlessly early-exits from the method
Better alternative found (Attempt 2): Add if (grid.Parent is Layout parentLayout) parentLayout.InvalidateMeasure() inside the existing Grid branch:
if (bindable is Grid grid)
{
grid.InvalidateMeasure();
if (grid.Parent is Layout parentLayout)
parentLayout.InvalidateMeasure();
}
else if (bindable is Element element && element.Parent is Grid parentGrid)
{
parentGrid.InvalidateMeasure();
}This is:
- ✅ Cross-platform (no
#if WINDOWS) - ✅ More general (handles Grid inside any
Layout, not just Grid-in-Grid) - ✅ 2-line addition with no structural changes
- ✅ Passes all tests on Windows
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