Skip to content

Fix CollectionView group header not stretching to full width on Windows#34710

Open
rmarinho wants to merge 8 commits into
net11.0from
fix/collectionview-group-header-width-23038
Open

Fix CollectionView group header not stretching to full width on Windows#34710
rmarinho wants to merge 8 commits into
net11.0from
fix/collectionview-group-header-width-23038

Conversation

@rmarinho
Copy link
Copy Markdown
Member

Description of Change

The ListViewHeaderItem and GridViewHeaderItem styles in ItemsViewStyles.xaml had HorizontalContentAlignment set to Left and Padding of 12,8,12,0 which prevented group headers from stretching to match item width.

Changed to HorizontalContentAlignment=Stretch with zero padding to match how regular item containers are configured. Preserved the 4px bottom margin for visual separation between headers and items.

As this is a breaking change i think we might want to net11.0 ?

Issues Fixed

Fixes #23038
Fixes #16824

Copilot AI review requested due to automatic review settings March 28, 2026 01:46
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Mar 28, 2026

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34710

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34710"

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a Windows-specific CollectionView grouped header layout issue by updating the default WinUI header container styles to allow header content to stretch full width, and adds a new UI regression test (HostApp page + Appium/NUnit test) for issue #23038.

Changes:

  • Updated WinUI ListViewHeaderItem/GridViewHeaderItem styles to use HorizontalContentAlignment="Stretch" and remove default padding.
  • Added a HostApp reproduction page for a grouped CollectionView with header/footer templates.
  • Added an Appium-based UI test validating group header width matches the item/footer width.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
src/Controls/src/Core/Platform/Windows/CollectionView/ItemsViewStyles.xaml Adjusts Windows header container defaults to allow group header templates to stretch full width.
src/Controls/tests/TestCases.HostApp/Issues/Issue23038.cs Adds a grouped CollectionView scenario with header/footer templates and AutomationIds for testing.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23038.cs Adds an Appium UI test asserting header width stretches to match the CollectionView/item/footer width.

Comment thread src/Controls/src/Core/Platform/Windows/CollectionView/ItemsViewStyles.xaml Outdated
Comment thread src/Controls/src/Core/Platform/Windows/CollectionView/ItemsViewStyles.xaml Outdated
Comment thread src/Controls/tests/TestCases.HostApp/Issues/Issue23038.cs Outdated
Comment thread src/Controls/tests/TestCases.HostApp/Issues/Issue23038.cs
Comment thread src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue23038.cs Outdated
@github-actions
Copy link
Copy Markdown
Contributor

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The UI test covers the primary fix scenario (list-based grouped CollectionView header stretching) but misses coverage for the grid-based layout path that the fix also modifies.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34710 — Fix CollectionView group header not stretching to full width on Windows
Test files evaluated: 2 (1 NUnit test + 1 HostApp page)
Fix files: 1


Overall Verdict

⚠️ Tests need improvement

The test validates the primary fix (header stretching to full width in list-based CollectionView), but the fix also modifies GridViewHeaderItem styles (for grid-based layouts), and that code path is not covered by any test.


1. Fix Coverage — ✅

The fix changes ListViewHeaderItem and GridViewHeaderItem styles in ItemsViewStyles.xaml:

  • Padding: 12,8,12,00,0,0,0
  • HorizontalContentAlignment: LeftStretch
  • MinHeight: {ThemeResource ...}0

The test creates a grouped CollectionView, measures the rendered width of a group header, and compares it against the group footer width (which already stretched correctly). This directly exercises the HorizontalContentAlignment=Stretch fix for ListViewHeaderItem. The test would fail if the fix were reverted.

Minor gap: the Padding and MinHeight changes are not explicitly verified, but they are secondary effects supporting the primary width fix.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Default list-based grouped CollectionView (ItemsLayout defaulting to LinearItemsLayout)
  • Multiple groups present (Team A, Team B) — only Team A measured, but reasonable
  • Header vs footer width comparison provides a good behavioral baseline

Missing:

  • Grid-based layout: The fix also changes GridViewHeaderItem (used when CollectionView.ItemsLayout is a GridItemsLayout). No test exercises this code path. A test with ItemsLayout = new GridItemsLayout(2, ItemsLayoutOrientation.Vertical) would be needed to cover this.
  • Direct width assertion: The test compares headerRect.Width ≈ footerRect.Width rather than headerRect.Width ≈ collectionViewRect.Width. If both footer and header were equally narrow (e.g., wrong size for a different reason), the test would still pass. The 20px footer-vs-CV tolerance provides a reasonable baseline check, but a tighter direct assertion would be stronger.

3. Test Type Appropriateness — ✅

Current: UI Test (Appium)
Recommendation: Same — UI test is appropriate here

The fix is in Windows XAML styles, and the bug is a visual layout issue requiring actual WinUI rendering to measure element widths. A unit test or XAML test cannot verify rendered pixel widths. A device test could theoretically measure layout rects, but Appium-based UI measurement via GetRect() is the established pattern for this type of layout verification in MAUI.


4. Convention Compliance — ✅

No convention issues detected by the automated script. Verified manually:

  • ✅ File naming: Issue23038.cs
  • [Issue(IssueTracker.Github, 23038, "...", PlatformAffected.UWP)] on HostApp page
  • [Category(UITestCategories.CollectionView)] — exactly one category attribute
  • ✅ Inherits from _IssuesUITest
  • WaitForElement before all GetRect() calls
  • ✅ No Task.Delay / Thread.Sleep
  • ✅ No inline #if ANDROID/#if IOS directives
  • ✅ No obsolete APIs (Application.MainPage, Frame, etc.)

5. Flakiness Risk — ⚠️ Medium

The test relies on GetRect() width measurements with fixed tolerances:

  • footerRect.Width == collectionViewRect.Width ± 20px — generous tolerance, low risk
  • headerRect.Width == footerRect.Width ± 2px — tight tolerance, slightly higher risk if there's sub-pixel rendering variance on Windows

There is no VerifyScreenshot() so cursor blink is not a concern. The tight 2px tolerance between header and footer could be fragile if WinUI renders these with any fractional-pixel difference. Consider increasing the tolerance slightly or using Within(5) as a safer margin.


6. Duplicate Coverage — ✅ No duplicates

The script matched tests containing "Platform" in the keyword but none are related to this issue. No existing test covers grouped CollectionView group header width on Windows.


7. Platform Scope — ✅

The fix is Windows-only (Platform/Windows/CollectionView/ItemsViewStyles.xaml), and the HostApp page correctly declares PlatformAffected.UWP. The test will only run on Windows. This is appropriate.


8. Assertion Quality — ⚠️

The assertions are reasonably specific:

  • ✅ Compares actual rendered widths, not just presence/null checks
  • ✅ Two-step comparison (footer proves baseline, header must match footer) is logical
  • ⚠️ Comparing headerRect.Width ≈ footerRect.Width is indirect — if both were wrong together, the test would still pass
  • ⚠️ 2px tolerance for the header-footer comparison may be too tight for CI stability on different Windows DPI settings

A stronger assertion would be: Assert.That(headerRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20)) directly, rather than routing through the footer as a proxy.


9. Fix-Test Alignment — ⚠️

Fix touches: ListViewHeaderItem style AND GridViewHeaderItem style
Test exercises: Only list-based CollectionView (uses ListViewHeaderItem)

The GridViewHeaderItem code path is untested. This is the main alignment gap.


Recommendations

  1. Add a grid-layout test — Add a second test method (or a second HostApp page) with a grid-based grouped CollectionView using ItemsLayout = new GridItemsLayout(2, ItemsLayoutOrientation.Vertical) to verify the GridViewHeaderItem fix works as well.

  2. Strengthen the width assertion — Consider asserting headerRect.Width ≈ collectionViewRect.Width directly (within 20px) rather than only comparing header to footer. This makes the test self-sufficient and doesn't rely on the footer being correct as a proxy.

  3. Slightly relax the header-footer tolerance — The current Within(2) for the header-footer comparison is very tight. Consider Within(5) to reduce flakiness risk from DPI-related rendering differences across Windows CI machines.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@MauiBot MauiBot added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 2, 2026

🧪 PR Test Evaluation

Overall Verdict: ✅ Tests are adequate

The test directly validates the fix — it verifies that group headers stretch to the full CollectionView width on Windows, which is exactly what the XAML style change achieves.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34710 — [Windows] CollectionView group header not stretching to full width
Test files evaluated: 2 (HostApp + NUnit)
Fix files: 1 (ItemsViewStyles.xaml)


Overall Verdict

Tests are adequate

The test exercises the exact behavior fixed by the PR: it measures group header and footer widths to confirm the header now stretches to full CollectionView width, which directly reflects the HorizontalContentAlignment: Left → Stretch style change.


1. Fix Coverage — ✅

The fix changes HorizontalContentAlignment from Left to Stretch (and zeroes Padding from 12,8,12,0) on both ListViewHeaderItem and GridViewHeaderItem styles in Windows XAML.

The test:

  • Measures the GroupedCollectionView width as a reference
  • Measures the HeaderTeamA and FooterTeamA widths
  • Asserts that the footer reaches the full CollectionView width (within 20px for scrollbar)
  • Asserts that the header matches the footer width (within 2px)

The test would fail without the fix (header stays Left-aligned, narrower than footer) and passes with it. ✅


2. Edge Cases & Gaps — ⚠️

Covered:

  • Happy path: first group header and footer stretch to full width

Missing:

  • Second group (TeamB): Only HeaderTeamA/FooterTeamA are measured; HeaderTeamB/FooterTeamB are not checked. Not critical, but would reinforce correctness.
  • GridView layout type: The fix also patches GridViewHeaderItem, but the test uses a CollectionView with default (list) layout. A grouped Grid layout variant is untested.
  • No GroupFooterTemplate case: The fix targets headers specifically; behavior when only a GroupHeaderTemplate is set (no footer for comparison) isn't tested.
  • Scrolled state: If the CollectionView has many items and is scrolled, does the header still stretch correctly? Not tested.

3. Test Type Appropriateness — ✅

Current: UI Test (Appium, NUnit)
Recommendation: UI test is appropriate here.

The bug is purely a visual layout issue — the header renders narrower than the list items on Windows. This requires actual Windows rendering to validate. Element width measurements via Appium (GetRect()) are the right tool. A device test could theoretically measure widths via the platform handler, but a UI test is a natural fit and consistent with other CollectionView issue tests in this repo.


4. Convention Compliance — ✅

The script reported 2 warnings for HeaderTeamA and FooterTeamA not found as literal strings in the HostApp — these are false positives. The AutomationIds are set via data binding:

label.SetBinding(Label.AutomationIdProperty, "Key", stringFormat: "Header{0}");
// Key = "TeamA" → AutomationId = "HeaderTeamA" at runtime ✅

All other conventions pass:

  • ✅ File naming: Issue23038.cs
  • [Issue(IssueTracker.Github, 23038, ..., PlatformAffected.UWP)] attribute present
  • ✅ Single [Category(UITestCategories.CollectionView)]
  • ✅ Inherits from _IssuesUITest
  • App.WaitForElement used before GetRect()
  • ✅ No Task.Delay / Thread.Sleep
  • ✅ No inline #if platform directives
  • ✅ No obsolete APIs

5. Flakiness Risk — ✅ Low

  • Within(20) tolerance for the footer-vs-CollectionView width comparison is reasonable (accounts for potential scrollbar width).
  • Within(2) for the header-vs-footer comparison is tight but appropriate (both are rendered in the same frame).
  • No screenshot assertions (no cursor-blink risk).
  • No animations or async timing issues — just layout measurement after WaitForElement.

Minor note: If the CollectionView hasn't fully laid out when GetRect() is called (possible on slow CI machines), there could be occasional flakiness. Adding WaitForElement for all three elements before any measurement (which the test already does) mitigates this.


6. Duplicate Coverage — ✅ No duplicates

No existing test covers this specific scenario (grouped CollectionView header width on Windows). Existing similar tests cover unrelated Windows platform behaviors.


7. Platform Scope — ✅

The fix is Windows-only (ItemsViewStyles.xaml in Platform/Windows/). The HostApp page uses PlatformAffected.UWP. The test is scoped correctly — no cross-platform concern here.


8. Assertion Quality — ✅

The assertion design is clever and avoids magic numbers:

  • Uses the footer as a baseline ("full-width") reference rather than hardcoding an expected pixel value.
  • The Within(20) tolerance for scrollbar is pragmatic.
  • The Within(2) for header-vs-footer comparison is specific enough to catch the bug.

This approach is resilient to different window sizes and DPI settings.


9. Fix-Test Alignment — ✅

Fix Test
Control ListViewHeaderItem / GridViewHeaderItem (Windows XAML) Grouped CollectionView header on Windows
Property HorizontalContentAlignment: Left → Stretch Header width == full CollectionView width
Would fail without fix? ✅ Yes — header stays narrow ✅ Yes — headerRect.Width != footerRect.Width

Recommendations

  1. (Optional) Add a second assertion for HeaderTeamB/FooterTeamB to verify all groups are affected, not just the first.
  2. (Optional) Consider a brief comment in the test explaining why the footer is used as the "full width" baseline — it's a non-obvious but correct approach.
  3. (Non-blocking) The GridViewHeaderItem style fix is untested (no test uses ItemsLayout = GridItemsLayout). Not strictly required since the root cause is the same, but worth noting.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 6, 2026

🧪 PR Test Evaluation

Overall Verdict: ⚠️ Tests need improvement

The test correctly validates the primary fix (list-layout group header stretching), but the fix also changes GridViewHeaderItem (used by GridItemsLayout) with no corresponding test coverage for that path.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34710 — Fix CollectionView group header not stretching to full width on Windows
Test files evaluated: 1 UI test (Issue23038.cs + HostApp Issue23038.cs)
Fix files: 1 (ItemsViewStyles.xaml)


Overall Verdict

⚠️ Tests need improvement

The test adequately covers the default linear-layout case but the fix also changes GridViewHeaderItem (for GridItemsLayout), which is not tested at all.


1. Fix Coverage — ✅

The test directly measures headerRect.Width vs footerRect.Width (within 2px) and footerRect.Width vs collectionViewRect.Width (within 20px for scrollbar). If the HorizontalContentAlignment were reverted from Stretch back to Left, the header would be narrower than the footer and the assertion would fail. The test would catch a regression.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Default vertical list layout (CollectionView with default ItemsLayout) — ListViewHeaderItem style path
  • First group only (TeamA)

Missing:

  • GridItemsLayout path — The fix changes both ListViewHeaderItem (list layout) and GridViewHeaderItem (grid layout, GridItemsLayout). When a grouped CollectionView uses GridItemsLayout, the GridViewHeaderItem style is applied. This code path is not tested. A simple second test method or a [TestCase] variation with a 2-column GridItemsLayout would close this gap.
  • Multiple group headers — Only TeamA header is measured. Testing TeamB is redundant, but acceptable to skip.
  • Second group after scrolling — Not critical for this fix.

3. Test Type Appropriateness — ✅

Current: UI Test (Appium + GetRect())
Recommendation: Same — this is appropriate.

The fix is a Windows-specific visual layout change (WinUI item container styles). Validating actual rendered widths requires a real platform context. A device test could theoretically measure layout too, but UI tests with GetRect() are the standard pattern for this kind of check in the MAUI repo, and no lighter alternative exists here.


4. Convention Compliance — ✅

  • ✅ File naming: Issue23038.cs — correct
  • [Issue(IssueTracker.Github, 23038, ..., PlatformAffected.UWP)] on HostApp
  • [Category(UITestCategories.CollectionView)] — exactly one
  • ✅ Inherits _IssuesUITest
  • WaitForElement before each GetRect() call
  • ✅ No Task.Delay / Thread.Sleep
  • ✅ No inline #if directives
  • ✅ No obsolete APIs

Note on script warnings: The automated script flagged HeaderTeamA and FooterTeamA as AutomationIds not found in the HostApp. These are false positives — the IDs are set via bindings (label.SetBinding(Label.AutomationIdProperty, nameof(Issue23038Team.Key), stringFormat: "Header{0}")), yielding "HeaderTeamA" at runtime. The test and HostApp are in sync.


5. Flakiness Risk — ✅ Low

  • WaitForElement is called before every GetRect()
  • Tolerances are justified: Within(20) accounts for vertical scrollbar width; Within(2) allows for 1-pixel rendering rounding ✅
  • No arbitrary delays, no screenshot comparison, no external URLs ✅

6. Duplicate Coverage — ✅ No duplicates

No existing tests cover grouped CollectionView header width on Windows.


7. Platform Scope — ✅

Fix is in src/Controls/src/Core/Platform/Windows/ — Windows-only. The HostApp uses PlatformAffected.UWP. The test will only execute on Windows in CI. This is correct.


8. Assertion Quality — ✅

  • Assert.That(footerRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20)) — specific, justified tolerance, includes message
  • Assert.That(headerRect.Width, Is.EqualTo(footerRect.Width).Within(2)) — directly targets the bug (header narrower than content), tight tolerance, includes message

Using footerRect as the "full width" baseline is sound: the GroupFooterTemplate container was not broken and uses the default WinUI style, so it stretches correctly and serves as a reliable reference.


9. Fix-Test Alignment — ✅

The fix sets HorizontalContentAlignment="Stretch" on ListViewHeaderItem and GridViewHeaderItem. The test verifies that the rendered header width matches the full CollectionView width. Direct cause-and-effect alignment.


Recommendations

  1. Add a test case for GridItemsLayout — The GridViewHeaderItem style was also changed but is untested. Add a second [Test] method (or a [TestCase] variation) that creates a grouped CollectionView with a 2-column GridItemsLayout and verifies that the group header stretches to the CollectionView's full width. The HostApp page could expose both layouts, or a second HostApp page could be added.

  2. No action needed on AutomationId script warnings — These are false positives from dynamic binding; the test and HostApp are correctly matched.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 2 items

Integrity filtering activated and filtered the following items during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

🧪 PR Test Evaluation

Overall Verdict: ✅ Tests are adequate

The tests directly validate the fix by measuring element widths, covering both list and grid layout modes. Minor naming and platform-scope concerns don't block the fix.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34710 — [Windows] GroupHeaderTemplate width smaller than ItemTemplate
Test files evaluated: 2 UI test pairs (HostApp + NUnit)
Fix files: 1 (ItemsViewStyles.xaml)


Overall Verdict

Tests are adequate

Both tests directly measure group header width against item/footer width, which is precisely what the fix changes (HorizontalContentAlignment: Left → Stretch, Padding: 12,8,12,0 → 0,0,0,0 for both ListViewHeaderItem and GridViewHeaderItem styles).


1. Fix Coverage — ✅

The fix changes two WinUI styles in ItemsViewStyles.xaml:

  • ListViewHeaderItem: covered by Issue23038 — checks that headerRect.Width == footerRect.Width (within 2px)
  • GridViewHeaderItem: covered by Issue23038_GridLayout — same assertion for grid mode

Both tests would fail if the fix were reverted (header would be narrower than footer due to HorizontalContentAlignment="Left"). The fix-to-test mapping is 1:1 and explicit.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Standard list layout (linear CollectionView with groups)
  • Grid layout (GridItemsLayout(2, Vertical))
  • Two groups present in the data (TeamA, TeamB)

Missing:

  • Only TeamA is asserted on — TeamB headers/footers are in the data but not measured. This is acceptable given the root cause is a style-level fix (all groups are affected equally), but asserting TeamB would strengthen the test.
  • Horizontal scrolling — the HorizontalListItemsPanel template exists; a horizontal grouped list isn't tested. Low priority.
  • GroupFooterTemplateGroupFooterTemplate is tested indirectly (footer width used as the reference), but there's no explicit test that the footer itself stretches. This is by design (the footer uses ListViewItem style, which already stretched).

3. Test Type Appropriateness — ✅

Current: UI Test (Appium, measuring element dimensions)

Recommendation: Same. The fix is a visual rendering/layout issue on Windows requiring measurement of native WinUI element dimensions (GetRect()). This cannot be validated with a unit or device test — the layout is computed by the WinUI ListViewHeaderItem control template, which only produces meaningful dimensions at runtime on a real platform. UI test with GetRect() is the correct choice.


4. Convention Compliance — ⚠️

Automated checks flagged 5 issues; analysis below:

Finding Assessment
Issue23038_GridLayout does not follow IssueXXXXX naming Real concern — convention is IssueXXXXX only. Consider renaming to a separate issue number or using Issue23038 for both and splitting with test method names. The _GridLayout suffix is non-standard.
HeaderTeamA, FooterTeamA AutomationIds not found in HostApp False positive — these are set via SetBinding(Label.AutomationIdProperty, "Key", stringFormat: "Header{0}") which produces "HeaderTeamA" at runtime. The script can't detect dynamic bindings.
GridHeaderTeamA, GridFooterTeamA AutomationIds not found in HostApp False positive — same reason, dynamically bound.

Genuine issues:

  • Issue23038_GridLayout naming violates IssueXXXXX convention (the guidelines state names should follow this pattern).

Looks good:

  • [Issue()] attribute with PlatformAffected.UWP on both HostApp pages
  • [Category(UITestCategories.CollectionView)] — exactly one per test
  • ✅ Inherits _IssuesUITest
  • WaitForElement used before all interactions
  • ✅ No Task.Delay/Thread.Sleep
  • ✅ No inline #if platform directives
  • ✅ No obsolete APIs

5. Flakiness Risk — ✅ Low

  • WaitForElement used for all elements before GetRect() — correct
  • Within(20) tolerance for CollectionView vs footer width (accounts for scrollbar) — appropriate
  • Within(2) tolerance for header vs footer (sub-pixel precision) — appropriate
  • No screenshot comparisons, no animation timing concerns
  • No Task.Delay/Thread.Sleep anti-patterns

6. Duplicate Coverage — ✅ No duplicates

The similar tests surfaced by the script (Issue12060, Issue13123, etc.) match on generic "Platform" keywords and are unrelated to CollectionView grouped layout width. No existing test covers GroupHeaderTemplate width stretching on Windows.


7. Platform Scope — ⚠️

The fix is Windows-only (WinUI ListViewHeaderItem/GridViewHeaderItem styles). The tests have no platform restriction — they will run on Android, iOS, and MacCatalyst as well.

Impact: On non-Windows platforms, the test should still pass (group headers already stretch to full width there), so this won't cause failures. However, it means unnecessary CI time on other platforms.

Recommendation: Consider adding #if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_IOS && TEST_FAILS_ON_MACCATALYST or a similar guard (following the pattern used in Issue16973.cs). Not blocking, but worth noting.


8. Assertion Quality — ✅

Assert.That(footerRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20));
Assert.That(headerRect.Width, Is.EqualTo(footerRect.Width).Within(2));
  • First assertion validates footer (a ListViewItem) stretches to the CollectionView width — establishes a reliable baseline
  • Second assertion validates header equals footer — directly tests that the fix's HorizontalContentAlignment="Stretch" is working
  • Tolerances are justified and documented in comments
  • Not brittle (no hardcoded pixel values)

9. Fix-Test Alignment — ✅

Fix Change Test Coverage
ListViewHeaderItemHorizontalContentAlignment: Stretch Issue23038 measures headerRect.Width == footerRect.Width
GridViewHeaderItemHorizontalContentAlignment: Stretch Issue23038_GridLayout measures same for grid mode
ListViewHeaderItemPadding: 0,0,0,0 Covered (padding affects effective width)
GridViewHeaderItemPadding: 0,0,0,0 Covered

Recommendations

  1. Rename Issue23038_GridLayout to follow IssueXXXXX naming convention. Since this is the same issue, the test could be renamed (e.g., keep both as separate test classes in one file, or open a second tracking issue). This is the only real convention violation.

  2. Consider adding platform guards to restrict these tests to Windows only (e.g., using #if preprocessor guards following Issue16973.cs pattern). The fix is Windows-only and running on other platforms adds unnecessary CI overhead.

  3. Optionally assert TeamB headers as well to strengthen confidence that the fix applies to all groups (low priority — the fix is style-level and applies universally).

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@rmarinho
Copy link
Copy Markdown
Member Author

rmarinho commented Apr 8, 2026

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 8, 2026

🧪 PR Test Evaluation

Overall Verdict: ✅ Tests are adequate

The two UI tests directly verify the core fix effect (group headers stretching to full CollectionView width on Windows), covering both the standard list layout and the grid layout scenarios. Script-reported AutomationId warnings are false positives caused by dynamic data binding.

👍 / 👎 — Was this evaluation helpful? React to let us know!

📊 Expand Full Evaluation

PR Test Evaluation Report

PR: #34710 — Fix CollectionView group header not stretching to full width on Windows
Test files evaluated: 4 (2 HostApp + 2 NUnit)
Fix files: 1 (ItemsViewStyles.xaml)


Overall Verdict

Tests are adequate

Both UI tests directly verify the visual effect of the fix (header stretching to full width) and are appropriate in type for a Windows-specific layout fix requiring pixel-width measurement.


1. Fix Coverage — ✅

The fix changes HorizontalContentAlignment from LeftStretch and Padding from 12,8,12,00,0,0,0 on both ListViewHeaderItem and GridViewHeaderItem styles.

  • Issue23038 verifies that headerRect.Width == footerRect.Width (±2px) — if HorizontalContentAlignment were reverted to Left, the header would be narrower than the footer, failing the assertion. ✅
  • Issue23038_GridLayout verifies that headerRect.Width ≈ collectionViewRect.Width (±20px) — same revert would cause failure. ✅

The tests would fail if the fix were reverted.

ℹ️ Note on padding change: The fix also zeroes the padding (12,8,12,00,0,0,0). This aspect is not explicitly asserted but would affect the header's visual spacing. A test checking that the header's X position is 0 (not offset by 12px) would add extra coverage, though it's not critical.


2. Edge Cases & Gaps — ⚠️

Covered:

  • Standard list CollectionView with group headers and footers
  • Grid layout CollectionView (2-column) with group headers
  • Multiple groups defined (TeamA, TeamB) — only TeamA asserted (acceptable)

Missing / Minor Gaps:

  • Padding offset not tested: The fix changes padding from 12,8,12,0 to 0,0,0,0. The tests verify width but not that the header starts at X=0 (not indented 12px). A low-priority gap.
  • Grid footer: Issue23038_GridLayout intentionally skips footer comparison (comment notes footers only span one grid column — correct reasoning). The grid footer AutomationId GridFooter{key} is set in HostApp but not used in the test. This is by design and correct.
  • Horizontal CollectionView: Not tested; the fix is for ListViewHeaderItem/GridViewHeaderItem vertical layout only — acceptable scope.

3. Test Type Appropriateness — ✅

Current: UI Test (Appium)
Recommendation: Appropriate — no lighter alternative

The tests compare actual rendered pixel widths (GetRect()) of visual elements on screen. This requires a running app and cannot be done with unit or device tests. UI tests are the right choice here.


4. Convention Compliance — ⚠️

Script warnings (4 total):

Warning Assessment
AutomationId 'HeaderTeamA' not found in HostApp False positive — Set via SetBinding(Label.AutomationIdProperty, nameof(Key), stringFormat: "Header{0}") → resolves to "HeaderTeamA" at runtime
AutomationId 'FooterTeamA' not found in HostApp False positive — Same dynamic binding pattern
AutomationId 'GridHeaderTeamA' not found in HostApp False positive — Same dynamic binding pattern
Issue23038_GridLayout name doesn't follow IssueXXXXX pattern ⚠️ Real issue — See below

Naming convention (Issue23038_GridLayout):
The _GridLayout suffix on both HostApp and test files is non-standard. However, the HostApp class correctly uses [Issue(..., issueTestNumber: 1)] which is the proper mechanism for multiple test pages per issue. The naming is consistent between HostApp and NUnit test. This is a minor deviation — the suffix is descriptive but doesn't follow the convention strictly.

Everything else is compliant:

  • _IssuesUITest base class
  • [Issue()] attribute with correct PlatformAffected.UWP
  • ✅ Exactly one [Category(UITestCategories.CollectionView)] per test
  • WaitForElement before all GetRect() calls
  • ✅ No Task.Delay / Thread.Sleep
  • ✅ No inline #if directives
  • ✅ No obsolete APIs

5. Flakiness Risk — ✅ Low

  • Tests use GetRect() for width comparison — not screenshot-based, so no cursor-blink risk
  • WaitForElement is used before every element access
  • Tolerances are justified: Within(20) accounts for Windows scrollbar width; Within(2) for header vs footer comparison is tight but reasonable
  • Assertions are relative (element width vs other element width), not absolute pixel values — more robust across display scaling

6. Duplicate Coverage — ✅ No duplicates

No existing tests cover CollectionView grouped layout header width on Windows. The script results showing Issue12060, Issue13123, etc., are unrelated (matched on Platform keyword, not CollectionView grouped headers).


7. Platform Scope — ✅

The fix is in Platform/Windows/CollectionView/ItemsViewStyles.xaml — Windows-only. Both HostApp tests correctly set PlatformAffected.UWP. Tests will be skipped on Android/iOS/Mac automatically.


8. Assertion Quality — ✅

Assertion Quality
footerRect.Width ≈ collectionViewRect.Width (±20) ✅ Specific, justified tolerance (scrollbar)
headerRect.Width ≈ footerRect.Width (±2) ✅ Tight, specific, meaningful
headerRect.Width ≈ collectionViewRect.Width (±20) ✅ Specific, same justified tolerance

All assertions would catch a regression if HorizontalContentAlignment were reverted to Left.


9. Fix-Test Alignment — ✅

  • Fix: ListViewHeaderItemHorizontalContentAlignment=Stretch + zero padding
  • Test 1 (Issue23038): Standard list CollectionView → verifies header width matches footer (full-width stretch) ✅
  • Fix: GridViewHeaderItem → same changes
  • Test 2 (Issue23038_GridLayout): Grid CollectionView → verifies header spans full width ✅

Both test/fix pairs are aligned.


Recommendations

  1. Minor: Rename Issue23038_GridLayout files to a convention-compliant name if the team's naming standards require strict IssueXXXXX format. The issueTestNumber: 1 attribute is correct; the file name deviation is cosmetic.
  2. Optional: Add an assertion that headerRect.X == 0 (or very close to 0) to verify the padding change (left indent removed). This would catch if the padding were reverted independently of the alignment.
  3. Note: The script's AutomationId warnings (HeaderTeamA, FooterTeamA, GridHeaderTeamA) are false positives — all three are set via dynamic SetBinding and will resolve correctly at runtime.

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

Note

🔒 Integrity filtering filtered 1 item

Integrity filtering activated and filtered the following item during workflow execution.
This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

🧪 Test evaluation by Evaluate PR Tests

@MauiBot MauiBot added s/agent-approved AI agent recommends approval - PR fix is correct and optimal s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates and removed s/agent-changes-requested AI agent recommends changes - found a better alternative or issues labels Apr 8, 2026
@rmarinho
Copy link
Copy Markdown
Member Author

rmarinho commented Apr 8, 2026

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@rmarinho
Copy link
Copy Markdown
Member Author

rmarinho commented Apr 8, 2026

🔍 Multi-Reviewer Code Review — PR #34710 Fix CollectionView group header not stretching to full width on Windows CI Status: ✅ All 35 checks passing Prior Review Comments: None (bot comments only) --- ### 🔴 CRITICAL 1. Breaking change ships without a compatibility guard (3/3 reviewers agreed) - File: src/Controls/src/Core/Platform/Windows/CollectionView/ItemsViewStyles.xaml (lines 209–211, 237–239) - What: Padding changed from 12,8,12,0 → 0,0,0,0 and HorizontalContentAlignment from Left → Stretch on both ListViewHeaderItem and GridViewHeaderItem. This applies unconditionally to all .NET versions. - Why it matters: Every existing Windows app using grouped CollectionView will silently get different header layout on upgrade — headers that relied on the default 12px side padding and 8px top padding will now render flush edge-to-edge. The PR author correctly identifies this as a breaking change and asks "should we net11.0 this?", but no AppContext switch, TFM guard, or opt-in mechanism is implemented. Apps that intentionally designed their header templates around the implicit container padding will break with no compile-time warning and no diagnostic. - Ask: Add a compatibility switch (e.g., AppContext flag) or confirm this is gated to a major version boundary with release notes. --- ### 🟡 MODERATE 2. UI tests lack a Windows-only platform guard (2/3 reviewers flagged) - Files: Tests/Issues/Issue23038.cs, Tests/Issues/Issue23038_GridLayout.cs - What: Neither test class carries a [Platform] attribute or equivalent category to restrict execution to Windows. The [Issue(..., PlatformAffected.UWP)] on the host app is metadata only — it does not gate test execution. - Why it matters: On non-Windows CI agents, WaitForElement("HeaderTeamA") will time out waiting for elements that don't exist, potentially burning CI time or masking real failures. Please verify that the _IssuesUITest base class handles platform routing automatically; if not, add explicit platform gating. 3. Footer-proxy assertion is logically fragile — compare header to CollectionView directly (3/3 reviewers agreed) - File: Tests/Issues/Issue23038.cs (lines 21–28) - What: The test uses footerRect.Width as the ground truth to verify headerRect.Width. If footers ever have their own width bug, both would be equally wrong and the Within(2) assertion would still pass. The effective error budget transitively is 22px (20 + 2), which nearly matches the removed padding (12 + 12 = 24px). - Why it matters: The grid layout test already correctly asserts header vs. CollectionView width directly. The list layout test should do the same for consistency and correctness. - Ask: Add Assert.That(headerRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20)) as the primary assertion in Issue23038.GroupHeaderShouldStretchToFullWidth. 4. No test validates vertical spacing after top padding removal (2/3 reviewers flagged) - What: The old padding included 8px top padding. Removing it changes vertical spacing for any header template that doesn't define its own padding. No test verifies the header's Y-position, height, or vertical spacing is still correct. - Why it matters: If a header template doesn't set its own padding, content will render flush against the top of the container. A simple positional assertion (e.g., headerRect.Y >= expectedMinY) or documentation that templates now own all spacing would address this. 5. Within(2) tolerance may be fragile at non-100% DPI (2/3 reviewers flagged; 1 reviewer disagreed, arguing same-container comparisons and logical pixel reporting make DPI a non-issue) - File: Tests/Issues/Issue23038.cs (line 27) - What: Header-to-footer width is compared with 2px tolerance. WinUI layout under DPI scaling involves device-pixel rounding that can produce 1–3px natural differences. - Counterpoint: One reviewer noted that both elements stretch to the same parent container width, and Appium reports logical pixels, making DPI rounding unlikely to cause differential width. CI agents also run at 100% DPI. - Ask: Consider widening to Within(4) or Within(5) as a safety margin, or document that CI agents are guaranteed 100% DPI. --- ### 🟢 MINOR 6. Only TeamA header validated — TeamB never asserted (3/3 reviewers noted) - Files: Both test files - What: Both tests create two groups (TeamA, TeamB) but only assert on TeamA's header. A container-recycling or virtualization issue manifesting on the second group would go undetected. 7. Inconsistent nameof() vs magic strings between host app files (2/3 reviewers flagged) - File: Issue23038_GridLayout.cs uses raw strings ("Name", "Key") where Issue23038.cs uses nameof(Issue23038Team.Name), nameof(Issue23038Team.Key) - Why it matters: If model properties are renamed, the grid layout bindings silently break while the original page gets a compile-time error. 8. Public model classes with implicit cross-file dependency (2/3 reviewers flagged) - File: Issue23038.cs (lines 63–79) - What: Issue23038Team and Issue23038Member are defined in Issue23038.cs but consumed by Issue23038_GridLayout.cs with no formal shared location. If the first file is removed, the second silently breaks. 9. GridLayout test doesn't validate footer width (2/3 reviewers flagged) - File: Tests/Issues/Issue23038_GridLayout.cs - What: The host app defines a GroupFooterTemplate with GridFooterTeamA AutomationId, but the test never retrieves or asserts on it. 10. No RTL (right-to-left) coverage (2/3 reviewers flagged) - What: HorizontalContentAlignment=Stretch is symmetric, but no test validates grouped CollectionView header rendering under RTL FlowDirection. The original Left alignment mapped to Leading in RTL layouts. 11. No test for empty groups (2/3 reviewers flagged) - What: Both host apps create groups with members. An empty group would test headers still render and stretch correctly with no items. 12. Non-grouped CollectionView regression untested (2/3 reviewers flagged; 1 reviewer disagreed with strong technical argument) - What: One reviewer raised concern that ListViewHeaderItem/GridViewHeaderItem styles apply globally. However, another reviewer provided a strong counterpoint: these WinUI container types are only instantiated when CollectionViewSource.IsSourceGrouped is true — non-grouped CollectionViews never create these containers. This is likely a non-issue but noted for transparency. --- ### ℹ️ Discarded Findings (did not survive consensus) - Padding="0,0,0,0" vs Padding="0" verbosity — only 1 reviewer flagged; cosmetic only --- ### 📊 Test Coverage Assessment - ✅ Core fix is tested for both list and grid layouts - ✅ Tests use correct approach (Appium UI tests with pixel-width measurements) - ⚠️ Assertions could be strengthened (direct CollectionView comparison, vertical spacing) - ⚠️ Edge cases not covered (empty groups, second group, RTL) --- ### ✅ What This PR Gets Right - The fix is correct and well-targeted — Stretch + zero padding is the right approach for letting MAUI templates control their own spacing - Good test coverage for the primary scenarios (list + grid layouts) - Clear, descriptive PR description and issue linking - Updated XAML comments explaining the style's purpose --- ### Recommended Action: ⚠️ Request Changes Blocking: 1. Add a compatibility switch or confirm version-gating strategy for this breaking change (Finding #1) Strongly Recommended: 2. Verify platform gating on tests or add explicit Windows-only attribute (Finding #2) 3. Assert header width against CollectionView width directly in the list layout test (Finding #3) Nice to Have: 4. Address minor findings (#6–#12) to leave the test suite stronger


🔧 Fix Pushed (commit 075b56cc71)

Finding #2 (UI tests lack Windows-only platform guard) has been addressed:

  • Added [FailsOnAndroidWhenRunningOnXamarinUITest], [FailsOnIOSWhenRunningOnXamarinUITest], and [FailsOnMacWhenRunningOnXamarinUITest] attributes to both GroupHeaderShouldStretchToFullWidth() and GridGroupHeaderShouldStretchToFullWidth() test methods.
  • This follows the established repo pattern (see UITestIgnoreAttributes.cs) — on non-Windows platforms, these attributes compile to IgnoreAttribute, causing NUnit to skip the tests.

Remaining items still require PR author action:

@rmarinho
Copy link
Copy Markdown
Member Author

rmarinho commented Apr 8, 2026

⚠️ Breaking Change Assessment

This PR changes the default ListViewHeaderItem and GridViewHeaderItem WinUI styles in ItemsViewStyles.xaml:

Property Before After
Padding 12,8,12,0 0,0,0,0
HorizontalContentAlignment Left Stretch

This IS a visual breaking change for existing Windows apps

Who is affected:

  • Apps using grouped CollectionView on Windows that rely on the default container padding for group headers. These apps will see headers shift from having 12px side padding + 8px top padding with left-alignment to zero padding with full-width stretch.

Who is NOT affected:

  • Apps with custom GroupHeaderTemplate that set their own Padding and HorizontalOptions on the template root element — these already control their own spacing and will be unaffected.
  • Apps on Android, iOS, and macOS — this change is Windows-only (ListViewHeaderItem/GridViewHeaderItem are WinUI container types).

Question for MAUI team

Does this need a compatibility switch (AppContext flag) or should it be gated to net11.0 TFM? The PR author raised this question in the description — would appreciate guidance on the preferred approach.


Also pushed commit 075b56cc71 adding FailsOnAndroid/iOS/Mac platform guards to both UI test methods so they only run on Windows.

@kubaflo kubaflo dismissed their stale review April 24, 2026 09:49

The base branch was changed.

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 24, 2026

@rmarinho the change looks good! I've changed the base to net11 - I agree that it is a breaking change, so it will be safer to include it in net11

kubaflo
kubaflo previously approved these changes Apr 24, 2026
@dotnet dotnet deleted a comment from MauiBot Apr 24, 2026
@rmarinho rmarinho dismissed kubaflo’s stale review April 27, 2026 11:55

The merge-base changed after approval.

PureWeen added a commit that referenced this pull request Apr 28, 2026
- Community PR: replaced PR #35144 (got closed during eval) with
  PR #34710 (older, stable state). Reworded rubric to not assume
  'community PR' — tests the general 'no builds' detection.
- Removed non-activation scenarios: 'general query' baseline was
  already 5/5 (no room to improve), 'code review' timed out at
  120s. These dragged the improvement score negative. The skill's
  boundary is CI-vs-not-CI which is better tested by prompt routing
  than eval scenarios.
- Kept 3 positive scenarios that showed real improvement (+1.0 and
  +2.3 in the first eval run).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho
Copy link
Copy Markdown
Member Author

@copilot can you rebase this on top of net11.0 branch please

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 29, 2026

@copilot can you rebase this on top of net11.0 branch please

Done — merged net11.0 into the branch (commit 6481b22). The branch is now up to date with the latest net11.0.

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 29, 2026

/review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

Expert Code Review completed successfully!

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expert Code Review — PR #34710

Methodology: 3 independent reviewers with adversarial consensus (findings only included when 2+ reviewers agree on the same root cause).


Findings Summary

# Severity Finding Consensus
1 ⚠️ MODERATE Breaking visual change — branch targeting unresolved 3/3 reviewers
2 ⚠️ MODERATE Test coverage gap for non-group header scenarios 3/3 reviewers
3 💡 MINOR New tests should include VerifyScreenshot() 3/3 reviewers (after follow-up)
4 💡 MINOR MinHeight + zero padding interaction worth documenting 2/3 reviewers

Key Decision Point

The primary open question is branch targeting. The author correctly identifies this as a potential breaking change. All 3 reviewers independently flagged this as the most important unresolved item:

  • Removing implicit 12,8,12,0 container padding changes visual appearance for every Windows app using grouped CollectionView without explicit padding in GroupHeaderTemplate
  • The style is implicit (no x:Key), so it applies globally to all ListViewHeaderItem/GridViewHeaderItem instances

A maintainer decision is needed: Is the old Left alignment a bug (fix on main/net11.0) or is the new Stretch behavior a deliberate breaking change (requiring release notes)?


Discarded Findings (single reviewer, not confirmed)

  • Snapshot baselines need cross-platform verification — Both follow-up reviewers noted the change is Windows-only; cross-platform drift is not mechanically possible
  • No gallery sample added — Discarded (single reviewer, lowest severity)

CI & Test Coverage Assessment

The PR includes 2 new UI test pages and 2 corresponding NUnit test files covering both linear and grid layout scenarios. 9 existing Windows snapshot baselines were updated to reflect the new visual appearance. The tests validate the fix target (grouped headers stretching) but lack coverage for adjacent scenarios (non-grouped headers, virtualized containers).


Verdict: NEEDS_DISCUSSION

Confidence: High

The XAML fix is mechanically correct and well-tested for its stated purpose. The outstanding blocker is the unresolved breaking-change question. Once a maintainer confirms the branch target, the secondary items (test coverage gaps, VerifyScreenshot) should be straightforward to address.

Generated by Expert Code Review for issue #34710 · ● 10.6M

Comment on lines +212 to +213
<Setter Property="Padding" Value="0,0,0,0" />
<Setter Property="HorizontalContentAlignment" Value="Stretch" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Breaking visual change — branch targeting unresolved (3/3 reviewers)

Removing the 12,8,12,0 padding and switching to HorizontalContentAlignment="Stretch" is a silent visual regression for existing Windows apps that rely on the implicit container padding. Group header labels that previously had 12px of breathing room will now abut the container edge.

The author correctly flags this as a breaking change, but the PR currently targets net11.0. Per MAUI branching conventions, main is for non-breaking fixes and the highest netN.0 branch is for behavioral changes. The question is whether the old Left alignment is treated as a bug (fixable on main) or the new behavior is a deliberate breaking change (requiring a major release).

Recommendation: A maintainer should explicitly decide the branch target and, if this ships as-is, add a BREAKING_CHANGE marker to the release notes so adopters are aware their headers will lose the built-in padding.

Comment on lines +14 to +18
[Category(UITestCategories.CollectionView)]
[FailsOnAndroidWhenRunningOnXamarinUITest("Windows-only test: validates WinUI CollectionView group header styles")]
[FailsOnIOSWhenRunningOnXamarinUITest("Windows-only test: validates WinUI CollectionView group header styles")]
[FailsOnMacWhenRunningOnXamarinUITest("Windows-only test: validates WinUI CollectionView group header styles")]
public void GroupHeaderShouldStretchToFullWidth()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Test coverage gap for non-group header scenarios (3/3 reviewers)

The ListViewHeaderItem/GridViewHeaderItem styles are implicit (no x:Key), so the padding and alignment changes affect all headers using those container types — not only group headers. This includes the top-level CollectionView.Header on Windows.

The tests validate grouped-header stretching (which is the fix target), but there is no regression guard for:

  1. Regular (non-grouped) CollectionView.Header behavior — does it still render correctly with zero container padding?
  2. Virtualized group headers that enter the viewport after scrolling (container recycling edge case)

Recommendation: Add a companion test (or assertion within an existing test) that verifies a non-grouped CollectionView with a Header property still renders correctly on Windows after this change.

"Group header should render at or below the CollectionView top edge");
Assert.That(headerARect.Y, Is.LessThan(footerARect.Y),
"Group header should render above its group footer");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Consider adding VerifyScreenshot() for visual regression protection (3/3 reviewers after follow-up)

GetRect() assertions validate layout metrics but won't catch visual regressions like content clipping at the new zero-padding boundary or rendering artifacts. Since this PR already updates 9 existing Windows snapshot baselines (proving the visual output changed), adding VerifyScreenshot() here would establish a baseline for the new expected appearance.

VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2));

<Setter Property="Padding" Value="0,0,0,0" />
<Setter Property="HorizontalContentAlignment" Value="Stretch" />
<Setter Property="VerticalContentAlignment" Value="Top" />
<Setter Property="MinHeight" Value="{ThemeResource ListViewHeaderItemMinHeight}" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 MinHeight + zero padding interaction (2/3 reviewers)

MinHeight is still bound to {ThemeResource ListViewHeaderItemMinHeight}. With the old 8px top padding, content was offset downward and the effective content area was MinHeight - 8. Now with zero padding and VerticalContentAlignment="Top", a header template smaller than MinHeight will have visible empty space below the content (previously the dead space was above/around it).

This is likely acceptable — the PR's intent is to hand layout control to the template — but worth noting in the PR description so adopters understand the full visual impact.

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented May 24, 2026

/review -b feature/refactor-copilot-yml

rmarinho and others added 8 commits May 26, 2026 14:04
The ListViewHeaderItem and GridViewHeaderItem styles in ItemsViewStyles.xaml
had HorizontalContentAlignment set to Left and Padding of 12,8,12,0 which
prevented group headers from stretching to match item width.

Changed to HorizontalContentAlignment=Stretch with zero padding to match
how regular item containers are configured. Preserved the 4px bottom margin
for visual separation between headers and items.

Fixes #23038
Fixes #16824

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…lish

- Revert MinHeight to theme resources for ListViewHeaderItem and
  GridViewHeaderItem (unrelated to the stretching fix)
- Use PascalCase AutomationIds without spaces for group headers/footers
  by adding a Key property to the team model
- Update XAML comments to reflect full scope of style customizations
- Remove redundant WaitForElement call and add tolerance explanation
- Update UI test to reference the sanitized AutomationIds

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Cover the GridViewHeaderItem style change with a test that uses
GridItemsLayout(2, Vertical) and verifies group headers stretch
to full CollectionView width in grid mode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… footers

- Add issueTestNumber:1 to Issue23038_GridLayout to avoid duplicate
  DisplayName conflict with Issue23038 (both had 'G23038')
- Fix grid test assertion: in GridItemsLayout, group footers are regular
  grid items that span one column, not full width. Compare header directly
  to CollectionView width instead.
- Both tests verified passing locally on Windows via Appium UI tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add FailsOn*WhenRunningOnXamarinUITest attributes to both Issue23038
and Issue23038_GridLayout test methods so they are skipped on Android,
iOS, and Mac. These tests validate WinUI-specific CollectionView group
header styles and should only run on Windows.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tyle changes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add direct header-vs-CollectionView width assertion in list test (primary)
- Assert on both TeamA and TeamB groups to catch virtualization regressions
- Widen header-vs-footer tolerance from 2px to 5px for DPI safety
- Add vertical-spacing sanity assertions (header within CV, above footer)
- Fix nameof() inconsistency in Issue23038_GridLayout.cs host app

Addresses findings #8-#12 from PR review.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rmarinho rmarinho force-pushed the fix/collectionview-group-header-width-23038 branch from 6481b22 to 31ced80 Compare May 26, 2026 13:06
@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented May 26, 2026

/review -b feature/refactor-copilot-yml -p windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

copilot platform/windows s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] GroupHeaderTemplate width smaller than ItemTemplate Group in a CollectionView on windows does not span horizontally

5 participants