-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix CollectionView group header not stretching to full width on Windows #34710
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: net11.0
Are you sure you want to change the base?
Changes from all commits
3df707a
53da900
f54ccbc
0e60973
f7556f3
e1a00e2
964f565
31ced80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,14 +203,14 @@ | |
| </Setter> | ||
| </Style> | ||
|
|
||
| <!-- Custom version of the style which removes the horizontal rule below the header --> | ||
| <!-- Custom style: removes horizontal rule, stretches content to full width, and zeroes container padding so MAUI header templates control their own spacing --> | ||
| <Style TargetType="ListViewHeaderItem"> | ||
| <Setter Property="FontFamily" Value="{ThemeResource ContentControlThemeFontFamily}" /> | ||
| <Setter Property="FontSize" Value="{ThemeResource ListViewHeaderItemThemeFontSize}" /> | ||
| <Setter Property="Background" Value="{ThemeResource ListViewHeaderItemBackground}" /> | ||
| <Setter Property="Margin" Value="0,0,0,4" /> | ||
| <Setter Property="Padding" Value="12,8,12,0" /> | ||
| <Setter Property="HorizontalContentAlignment" Value="Left" /> | ||
| <Setter Property="Padding" Value="0,0,0,0" /> | ||
| <Setter Property="HorizontalContentAlignment" Value="Stretch" /> | ||
| <Setter Property="VerticalContentAlignment" Value="Top" /> | ||
| <Setter Property="MinHeight" Value="{ThemeResource ListViewHeaderItemMinHeight}" /> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 MinHeight + zero padding interaction (2/3 reviewers)
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. |
||
| <Setter Property="UseSystemFocusVisuals" Value="True" /> | ||
|
|
@@ -231,14 +231,14 @@ | |
| </Setter> | ||
| </Style> | ||
|
|
||
| <!-- Custom version of the style which removes the horizontal rule below the header --> | ||
| <!-- Custom style: removes horizontal rule, stretches content to full width, and zeroes container padding so MAUI header templates control their own spacing --> | ||
| <Style TargetType="GridViewHeaderItem"> | ||
| <Setter Property="FontFamily" Value="{ThemeResource ContentControlThemeFontFamily}" /> | ||
| <Setter Property="FontSize" Value="{ThemeResource GridViewHeaderItemThemeFontSize}" /> | ||
| <Setter Property="Background" Value="{ThemeResource GridViewHeaderItemBackground}" /> | ||
| <Setter Property="Margin" Value="0,0,0,4" /> | ||
| <Setter Property="Padding" Value="12,8,12,0" /> | ||
| <Setter Property="HorizontalContentAlignment" Value="Left" /> | ||
| <Setter Property="Padding" Value="0,0,0,0" /> | ||
| <Setter Property="HorizontalContentAlignment" Value="Stretch" /> | ||
| <Setter Property="VerticalContentAlignment" Value="Top" /> | ||
| <Setter Property="MinHeight" Value="{ThemeResource GridViewHeaderItemMinHeight}" /> | ||
| <Setter Property="UseSystemFocusVisuals" Value="True" /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 23038, "[Windows] GroupHeaderTemplate width smaller than ItemTemplate", PlatformAffected.UWP)] | ||
| public class Issue23038 : ContentPage | ||
| { | ||
| public Issue23038() | ||
| { | ||
| var teams = new List<Issue23038Team> | ||
| { | ||
| new("TeamA", "Team A", new List<Issue23038Member> | ||
| { | ||
| new("Member 1"), new("Member 2"), new("Member 3"), | ||
| }), | ||
| new("TeamB", "Team B", new List<Issue23038Member> | ||
| { | ||
| new("Member 4"), new("Member 5"), | ||
| }), | ||
| }; | ||
|
|
||
| var collectionView = new CollectionView | ||
| { | ||
| AutomationId = "GroupedCollectionView", | ||
| IsGrouped = true, | ||
| ItemsSource = teams, | ||
| GroupHeaderTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| BackgroundColor = Colors.LightGreen, | ||
| FontAttributes = FontAttributes.Bold, | ||
| Padding = new Thickness(5), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, nameof(Issue23038Team.Name)); | ||
| label.SetBinding(Label.AutomationIdProperty, nameof(Issue23038Team.Key), stringFormat: "Header{0}"); | ||
| return label; | ||
| }), | ||
| GroupFooterTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| BackgroundColor = Colors.Orange, | ||
| Padding = new Thickness(5), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, "Count", stringFormat: "Total: {0}"); | ||
| label.SetBinding(Label.AutomationIdProperty, nameof(Issue23038Team.Key), stringFormat: "Footer{0}"); | ||
| return label; | ||
|
rmarinho marked this conversation as resolved.
|
||
| }), | ||
| ItemTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| Padding = new Thickness(5, 2), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, nameof(Issue23038Member.Name)); | ||
| return label; | ||
| }), | ||
| }; | ||
|
|
||
| Content = collectionView; | ||
| } | ||
| } | ||
|
|
||
| public class Issue23038Team : List<Issue23038Member> | ||
| { | ||
| public Issue23038Team(string key, string name, List<Issue23038Member> members) : base(members) | ||
| { | ||
| Key = key; | ||
| Name = name; | ||
| } | ||
|
|
||
| public string Key { get; set; } | ||
| public string Name { get; set; } | ||
| } | ||
|
|
||
| public class Issue23038Member | ||
| { | ||
| public Issue23038Member(string name) => Name = name; | ||
| public string Name { get; set; } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 23038, "[Windows] GroupHeaderTemplate width smaller than ItemTemplate (GridLayout)", PlatformAffected.UWP, issueTestNumber: 1)] | ||
| public class Issue23038_GridLayout : ContentPage | ||
| { | ||
| public Issue23038_GridLayout() | ||
| { | ||
| var teams = new List<Issue23038Team> | ||
| { | ||
| new("TeamA", "Team A", new List<Issue23038Member> | ||
| { | ||
| new("Member 1"), new("Member 2"), new("Member 3"), new("Member 4"), | ||
| }), | ||
| new("TeamB", "Team B", new List<Issue23038Member> | ||
| { | ||
| new("Member 5"), new("Member 6"), | ||
| }), | ||
| }; | ||
|
|
||
| var collectionView = new CollectionView | ||
| { | ||
| AutomationId = "GroupedGridCollectionView", | ||
| IsGrouped = true, | ||
| ItemsSource = teams, | ||
| ItemsLayout = new GridItemsLayout(2, ItemsLayoutOrientation.Vertical), | ||
| GroupHeaderTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| BackgroundColor = Colors.LightBlue, | ||
| FontAttributes = FontAttributes.Bold, | ||
| Padding = new Thickness(5), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, nameof(Issue23038Team.Name)); | ||
| label.SetBinding(Label.AutomationIdProperty, nameof(Issue23038Team.Key), stringFormat: "GridHeader{0}"); | ||
| return label; | ||
| }), | ||
| GroupFooterTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| BackgroundColor = Colors.LightCoral, | ||
| Padding = new Thickness(5), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, "Count", stringFormat: "Total: {0}"); | ||
| label.SetBinding(Label.AutomationIdProperty, nameof(Issue23038Team.Key), stringFormat: "GridFooter{0}"); | ||
| return label; | ||
| }), | ||
| ItemTemplate = new DataTemplate(() => | ||
| { | ||
| var label = new Label | ||
| { | ||
| Padding = new Thickness(5, 2), | ||
| }; | ||
| label.SetBinding(Label.TextProperty, nameof(Issue23038Member.Name)); | ||
| return label; | ||
| }), | ||
| }; | ||
|
|
||
| Content = collectionView; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue23038 : _IssuesUITest | ||
| { | ||
| public override string Issue => "[Windows] GroupHeaderTemplate width smaller than ItemTemplate"; | ||
|
|
||
| public Issue23038(TestDevice device) : base(device) { } | ||
|
|
||
| [Test] | ||
| [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() | ||
|
Comment on lines
+14
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The The tests validate grouped-header stretching (which is the fix target), but there is no regression guard for:
Recommendation: Add a companion test (or assertion within an existing test) that verifies a non-grouped |
||
| { | ||
| var collectionViewRect = App.WaitForElement("GroupedCollectionView").GetRect(); | ||
| var headerARect = App.WaitForElement("HeaderTeamA").GetRect(); | ||
| var footerARect = App.WaitForElement("FooterTeamA").GetRect(); | ||
| var headerBRect = App.WaitForElement("HeaderTeamB").GetRect(); | ||
| var footerBRect = App.WaitForElement("FooterTeamB").GetRect(); | ||
|
|
||
| // Primary assertion: header must stretch to full CollectionView width. | ||
| // Tolerance of 20px accounts for the possible scrollbar width on Windows. | ||
| Assert.That(headerARect.Width, Is.EqualTo(collectionViewRect.Width).Within(20), | ||
| "Group header (TeamA) should stretch to full CollectionView width"); | ||
|
|
||
| // Footer (regular list item) should also stretch to CollectionView width. | ||
| Assert.That(footerARect.Width, Is.EqualTo(collectionViewRect.Width).Within(20), | ||
| "Group footer (TeamA) should stretch to full CollectionView width"); | ||
|
|
||
| // Header must match footer width — both stretch to full width. | ||
| // Tolerance of 5px accounts for device-pixel rounding under DPI scaling. | ||
| Assert.That(headerARect.Width, Is.EqualTo(footerARect.Width).Within(5), | ||
| "Group header (TeamA) width should match group footer width"); | ||
|
|
||
| // Second group must also stretch — protects against virtualization/container-recycling regressions. | ||
| Assert.That(headerBRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20), | ||
| "Group header (TeamB) should stretch to full CollectionView width"); | ||
| Assert.That(headerBRect.Width, Is.EqualTo(footerBRect.Width).Within(5), | ||
| "Group header (TeamB) width should match group footer width"); | ||
|
|
||
| // Vertical spacing sanity: header sits within the CollectionView and above its footer. | ||
| Assert.That(headerARect.Y, Is.GreaterThanOrEqualTo(collectionViewRect.Y), | ||
| "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"); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Consider adding
VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2)); |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue23038_GridLayout : _IssuesUITest | ||
| { | ||
| public override string Issue => "[Windows] GroupHeaderTemplate width smaller than ItemTemplate (GridLayout)"; | ||
|
|
||
| public Issue23038_GridLayout(TestDevice device) : base(device) { } | ||
|
|
||
| [Test] | ||
| [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 GridGroupHeaderShouldStretchToFullWidth() | ||
| { | ||
| var collectionViewRect = App.WaitForElement("GroupedGridCollectionView").GetRect(); | ||
| var headerARect = App.WaitForElement("GridHeaderTeamA").GetRect(); | ||
| var headerBRect = App.WaitForElement("GridHeaderTeamB").GetRect(); | ||
|
|
||
| // In a GridItemsLayout, group headers use GridViewHeaderItem and should span the full width. | ||
| // (Group footers are regular grid items that only span one column, so we compare header to CollectionView.) | ||
| // Tolerance of 20px accounts for the possible scrollbar width on Windows. | ||
| Assert.That(headerARect.Width, Is.EqualTo(collectionViewRect.Width).Within(20), | ||
| "Grid group header (TeamA) should stretch to full CollectionView width"); | ||
|
|
||
| // Second group must also stretch — protects against virtualization/container-recycling regressions. | ||
| Assert.That(headerBRect.Width, Is.EqualTo(collectionViewRect.Width).Within(20), | ||
| "Grid group header (TeamB) should stretch to full CollectionView width"); | ||
|
|
||
| // Vertical sanity: TeamA header sits within the CollectionView and above TeamB's header. | ||
| Assert.That(headerARect.Y, Is.GreaterThanOrEqualTo(collectionViewRect.Y), | ||
| "Grid group header should render at or below the CollectionView top edge"); | ||
| Assert.That(headerARect.Y, Is.LessThan(headerBRect.Y), | ||
| "First group header should render above the second group header"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing the
12,8,12,0padding and switching toHorizontalContentAlignment="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,mainis for non-breaking fixes and the highestnetN.0branch is for behavioral changes. The question is whether the oldLeftalignment 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_CHANGEmarker to the release notes so adopters are aware their headers will lose the built-in padding.