[Windows] Fixed Margin doesn't work inside CollectionView EmptyView#29897
[Windows] Fixed Margin doesn't work inside CollectionView EmptyView#29897kubaflo merged 6 commits intodotnet:inflight/currentfrom
Conversation
|
Hey there @@Dhivya-SF4094! 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 the Margin set on a CollectionView.EmptyView is applied correctly on Windows by explicitly copying the Forms Margin to the native control. It also adds a HostApp sample page and a shared UI test to verify the fix.
- Apply Forms
Marginto nativeFrameworkElementin Windows renderers - Introduce a HostApp scenario demonstrating the margin behavior
- Add an automated UI test to validate the empty view margin
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Controls/src/Core/Platform/Windows/FormsListView.cs | Copy Margin from Forms view to native empty view |
| src/Controls/src/Core/Platform/Windows/CollectionView/FormsGridView.cs | Same Margin-copy fix for the grid-based renderer |
| src/Controls/tests/TestCases.HostApp/Issues/Issue8494.cs | HostApp page showcasing EmptyView margin scenario |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue8494.cs | UI test waiting for element and verifying layout via screenshot |
src/Controls/src/Core/Platform/Windows/CollectionView/FormsGridView.cs
Outdated
Show resolved
Hide resolved
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29897 | Copy the Forms EmptyView margin onto the native Windows FrameworkElement in both FormsListView and FormsGridView; validate with new UI screenshot coverage for Issue8494. |
PENDING (Gate) | src/Controls/src/Core/Platform/Windows/FormsListView.cs, src/Controls/src/Core/Platform/Windows/CollectionView/FormsGridView.cs, src/Controls/tests/TestCases.HostApp/Issues/Issue8494.cs, src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue8494.cs |
Original PR |
Issue: #8494 - Margin doesn't work inside CollectionView EmptyView
PR: #29897 - [Windows] Fixed Margin doesn't work inside CollectionView EmptyView
Platforms Affected: Issue originally reported on iOS, Android, Windows, and macOS; follow-up issue comments say Android already worked while Windows, iOS, and macOS reproed. The current PR description says latest repro remains on Windows.
Files Changed: 1 implementation, 2 test code, 4 snapshot assets
Key Findings
- The current PR implementation is a single Windows handler-layer change in
src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.csthat copies the Forms EmptyView margin to the realized WinUI empty-view element beforeSetEmptyView(). - Test coverage consists of a new HostApp issue page
Issue8494, a shared UI screenshot testCheckEmptyViewMargin, and snapshot baselines for Android, iOS, Mac, and Windows. - The HostApp page is Windows-scoped via
PlatformAffected.UWP, which matches the PR's narrowed Windows focus on the current branch. - Prior automated review artifacts exist on the PR, and the author replied that the AI summary was validated/addressed. The current file set reflects that updated state.
- Inline review feedback requested a visible background color on the EmptyView container to make screenshot diffs clearer; that change is present.
Edge Cases / Discussion Notes
- Historical discussion shows the issue originally affected multiple platforms, but Android was later confirmed working while Windows, iOS, and macOS still reproed.
- One issue comment noted that on Windows the layout problem could still be observed while dragging the window.
- No new unresolved inline review feedback remains on the current implementation.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29897 | Propagate the Forms EmptyView margin from ItemsViewHandler.Windows.UpdateEmptyView() onto the realized WinUI empty-view element, then validate with Issue8494 UI screenshot coverage. |
PENDING (Gate) | src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.cs, src/Controls/tests/TestCases.HostApp/Issues/Issue8494.cs, src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue8494.cs |
Current PR implementation |
Issue: #8494 - Margin doesn't work inside CollectionView EmptyView
PR: #29897 - [Windows] Fixed Margin doesn't work inside CollectionView EmptyView
Platforms Affected: Windows (primary; Android/iOS/Mac confirmed working on latest)
Files Changed: 1 implementation, 2 test code, 4 snapshot assets
Key Findings
- The bug:
Marginset on a MAUIEmptyViewinsideCollectionViewis not applied to the native WinUI container on Windows, causing incorrect layout spacing. - The fix is in
src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Windows.csinUpdateEmptyView(), after the switch block, the Forms EmptyView margin is explicitly copied to_emptyView.MarginusingWinUIHelpers.CreateThickness(...)before callingSetEmptyView(). - Test coverage: new HostApp page
Issue8494.cs(scoped toPlatformAffected.UWP), new shared UI testCheckEmptyViewMargin(screenshot verification), and snapshot baselines for Android, iOS, Mac, and Windows. - Previous Copilot inline review flagged duplicated margin-copy logic in
FormsListViewandFormsGridViewthe PR uses the handler layer which is cleaner. - Reviewer jsuarezruiz requested visible background color on EmptyView for screenshot clarity author complied.
- Prior agent review (incomplete) ran Gate PASSED on Windows and 2 try-fix attempts.
Edge Cases / Discussion Notes
- Original issue reported all platforms broken; Android was confirmed working early; issue narrowed to Windows only on latest bits.
- Windows-specific drag-resize behavior could still reveal margin issue per issue comment.
- PlatformAffected.UWP scoping means HostApp page only registers for Windows.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29897 | In UpdateEmptyView(), copy Forms EmptyView margin to _emptyView.Margin via WinUIHelpers.CreateThickness before calling SetEmptyView() | PENDING (Gate) | ItemsViewHandler.Windows.cs |
Single centralized location |
🚦 Gate — Test Verification
Gate Result: PASSED
Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -RequireFullVerification -TestFilter "Issue8494" -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Gate Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -TestFilter "Issue8494" -RequireFullVerification -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Gate Result: PASSED
Platform: windows
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | Issue8494 | Issue8494 |
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL | |
| With fix | PASS | PASS |
Note: verify-tests-fail.ps1 script reported FAILED due to log parsing (captured build log only, not test execution log). Actual test execution log (CustomAgentLogsTmp/UITests/test-output.log) shows: Passed CheckEmptyViewMargin [1 s] / Test Run Successful. Total tests: 1, Passed: 1. The without-fix run confirmed snapshot diff of 24.21% as expected.
🔧 Fix — Analysis & Comparison
Gate Result: PASSED
Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -RequireFullVerification -TestFilter "Issue8494" -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Gate Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -TestFilter "Issue8494" -RequireFullVerification -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Apply margin in platform layer SetEmptyView() in FormsListView + FormsGridView | PASS | 2 files | Duplicates same logic in 2 files |
| 2 | try-fix (claude-sonnet-4.6) | Apply margin in UpdateEmptyViewVisibility() before making empty view visible | PASS | 1 file | Re-applies margin each time view is shown |
| 3 | try-fix (gpt-5.3-codex) | Wire PropertyChanged on _formsEmptyView to sync margin on MarginProperty changes | PASS | 1 file | Handles dynamic margin changes; adds subscription complexity |
| 4 | try-fix (gemini-3-pro-preview) | Wrap EmptyView in Grid container | FAIL | 1 file | App crashes - ListViewBase visual tree constraints |
| 5 | cross-poll (claude-sonnet-4.6) | Override ArrangeOverride; re-arrange ContentControl with margin-inset rect each layout pass | PASS | 2 files | Most complex; layout-cycle approach |
| 6 | cross-poll (gpt-5.3-codex) | Mapper-driven: SyncEmptyViewMargin() called from MapEmptyView + MapEmptyViewTemplate | PASS | 1 file | Follows MAUI mapper pattern |
| 7 | cross-poll r3 (gpt-5.3-codex) | Map MAUI EmptyView.Margin to host ContentControl.Padding instead of child Margin | PASS | 2 files | Semantically different; Padding vs Margin |
| PR | PR #29897 | Copy EmptyView margin to _emptyView.Margin in UpdateEmptyView() at setup time | PASSED (Gate) | 1 file | Simplest; centralized; uses WinUIHelpers |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
| claude-sonnet-4.6 | 2 | Yes | ArrangeOverride margin offset attempt 5 |
| gpt-5.3-codex | 2 | Yes | Property mapper MapEmptyView attempt 6 |
| gemini-3-pro-preview | 2 | No | NO NEW IDEAS |
| claude-opus-4.6 | 3 | No | NO NEW IDEAS |
| gpt-5.3-codex | 3 | Yes | ContentControl.Padding attempt 7 |
| (all) | 3 | No | Exhausted after attempt 7 |
Exhausted: Yes
Selected Fix: PR #29897 Simplest correct solution: 1 file, 6 lines, setup-time copy using WinUIHelpers.CreateThickness (codebase-idiomatic). All alternatives either change 2 files, add lifecycle/subscription complexity, or deviate from established codebase patterns without added benefit.
📋 Report — Final Recommendation
Gate Result: PASSED
Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -RequireFullVerification -TestFilter "Issue8494" -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Gate Result: PASSED
Platform: windows
Mode: Full Verification
- Tests FAIL without fix:
- Tests PASS with fix:
Evidence
- Verification command:
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform windows -TestFilter "Issue8494" -RequireFullVerification -PRNumber 29897 - Without fix,
CheckEmptyViewMarginfailed withSnapshot different than baseline: CheckEmptyViewMargin.png (24.21% difference). - With fix, the Windows verification reported
Passed: 1andTest Run SuccessfulforIssue8494.
Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | COMPLETE | Issue #8494, Windows-only margin bug on CollectionView EmptyView |
| Gate | PASSED | Windows test fails without fix (24.21% snapshot diff), passes with fix |
| Try-Fix | COMPLETE | 7 attempts, 6 passing (1 failed: Grid wrapper crashes app) |
| Report | COMPLETE |
Summary
PR #29897 fixes a Windows-specific bug where the Margin property on a MAUI EmptyView inside a CollectionView is silently ignored because the native WinUI FrameworkElement container is never given the margin value. The fix is minimal and correct: 6 lines in a single Windows handler file. Tests were added with screenshot verification across all platforms. Exhaustive multi-model exploration found 6 passing alternative approaches but none simpler or more idiomatic than the PR's fix.
Root Cause
ItemsViewHandler.Windows.cs:UpdateEmptyView() creates a native FrameworkElement (_emptyView) to host the Forms empty view, but never transfers the MAUI Margin value to the native element's .Margin property. WinUI layout uses the native element's own Margin for spacing, so the MAUI margin is lost.
Fix Quality
The fix is high quality:
- Correct location: Applied in UpdateEmptyView() immediately after _emptyView is created and before SetEmptyView() is called the natural setup point.
- Correct API: Uses WinUIHelpers.CreateThickness() which is the codebase-standard helper for MAUIWinUI thickness conversion.
- Minimal footprint: 1 file, 6 lines, guarded with null checks.
- Well-tested: New UI test with screenshot baselines for all 4 platforms.
- Try-Fix confirmed: 6 independent alternative approaches also passed tests, confirming the bug is real and fixable at this location. The PR's approach is the simplest and most idiomatic of all candidates.
Code Review Notes
- The HostApp test page is scoped to
PlatformAffected.UWPwhich correctly limits it to Windows (where the bug reproduces). - Inline Copilot review previously flagged margin duplication in
FormsListView/FormsGridViewas a nitpick the PR's handler-layer fix is actually cleaner than those old platform-layer renderers. - Test method
CheckEmptyViewMarginusesVerifyScreenshot()which is appropriate for a layout/spacing regression. - Background color on the EmptyView (added per reviewer request) ensures screenshot diffs are visually obvious.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the AI's summary?
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 29897Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 29897" |
@kubaflo Validated and addressed AI's summary. |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
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 Detail
The Margin property is not applied as expected when used within the EmptyView of a CollectionView.
Note: Issue not replicated in Android, iOS and Mac platforms in latest.
Root Cause
The Margin set on the EmptyView is not automatically applied to the native container (ContentControl) on the Windows platform, resulting in incorrect layout spacing.
Description of Change
Applied the EmptyView’s Margin explicitly to the _emptyView.Marging to ensure consistent layout rendering on Windows.
Validated the behaviour in the following platforms
Issues Fixed:
Fixes #8494
Screenshots