-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS, macOS] Fixed CollectionView group header size changes with ItemSizingStrategy #33161
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
Changes from all commits
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 |
|---|---|---|
|
|
@@ -40,6 +40,8 @@ public event EventHandler<LayoutAttributesChangedEventArgs2> LayoutAttributesCha | |
| Size _measuredSize; | ||
| Size _cachedConstraints; | ||
|
|
||
| // Indicates the cell is being used as a supplementary view (group header/footer) | ||
| internal bool isSupplementaryView = false; | ||
|
||
| internal bool MeasureInvalidated => _measureInvalidated; | ||
|
|
||
| // Flags changes confined to the header/footer, preventing unnecessary recycling and revalidation of templated cells. | ||
|
|
@@ -107,20 +109,28 @@ public override UICollectionViewLayoutAttributes PreferredLayoutAttributesFittin | |
|
|
||
| if (_measureInvalidated || _cachedConstraints != constraints) | ||
| { | ||
| // Check if we should use the cached first item size for MeasureFirstItem optimization | ||
| var cachedSize = GetCachedFirstItemSizeFromHandler(); | ||
| if (cachedSize != CGSize.Empty) | ||
| // Only use the cached first-item measurement for actual item cells (not headers/footers) | ||
| if (!isSupplementaryView) | ||
| { | ||
| _measuredSize = cachedSize.ToSize(); | ||
| // Even when we have a cached measurement, we still need to call Measure | ||
| // to update the virtual view's internal state and bookkeeping | ||
| virtualView.Measure(constraints.Width, _measuredSize.Height); | ||
| var cachedSize = GetCachedFirstItemSizeFromHandler(); | ||
| if (cachedSize != CGSize.Empty) | ||
| { | ||
| _measuredSize = cachedSize.ToSize(); | ||
| // Even when we have a cached measurement, we still need to call Measure | ||
| // to update the virtual view's internal state and bookkeeping | ||
| virtualView.Measure(constraints.Width, _measuredSize.Height); | ||
| } | ||
| else | ||
| { | ||
| _measuredSize = virtualView.Measure(constraints.Width, constraints.Height); | ||
| // If this is the first item being measured, cache it for MeasureFirstItem strategy | ||
| SetCachedFirstItemSizeToHandler(_measuredSize.ToCGSize()); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| // For headers/footers, always measure directly without using or updating the first-item cache | ||
| _measuredSize = virtualView.Measure(constraints.Width, constraints.Height); | ||
| // If this is the first item being measured, cache it for MeasureFirstItem strategy | ||
| SetCachedFirstItemSizeToHandler(_measuredSize.ToCGSize()); | ||
| } | ||
| _cachedConstraints = constraints; | ||
| _needsArrange = true; | ||
|
|
@@ -194,6 +204,7 @@ public override void LayoutSubviews() | |
| public override void PrepareForReuse() | ||
| { | ||
| //Unbind(); | ||
| isSupplementaryView = false; | ||
| base.PrepareForReuse(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| <?xml version="1.0" encoding="utf-8" ?> | ||
| <ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui" | ||
| xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml" | ||
| x:Class="Maui.Controls.Sample.Issues.Issue33130" | ||
| Title="Issue33130"> | ||
| <Grid Margin="20" | ||
| RowDefinitions="Auto, *"> | ||
| <StackLayout Grid.Row="0" | ||
| Spacing="10" | ||
| Margin="0,10"> | ||
| <Button Text="Switch to MeasureFirstItem" | ||
| Clicked="OnSwitchToMeasureFirstItem" | ||
| AutomationId="SwitchStrategyButton"/> | ||
| <Label x:Name="StatusLabel" | ||
| AutomationId="StatusLabel" | ||
| Text="ItemSizingStrategy: MeasureAllItems"/> | ||
| </StackLayout> | ||
|
|
||
| <CollectionView Grid.Row="1" | ||
| x:Name="TestCollectionView" | ||
| ItemsSource="{Binding Animals}" | ||
| IsGrouped="true" | ||
| AutomationId="TestCollectionView" | ||
| ItemSizingStrategy="MeasureAllItems"> | ||
| <CollectionView.Header> | ||
| <Label Text="Animals Collection" | ||
| FontSize="24" | ||
| AutomationId="CollectionViewHeader" | ||
| FontAttributes="Bold" | ||
| HorizontalOptions="Center"/> | ||
| </CollectionView.Header> | ||
| <CollectionView.ItemTemplate> | ||
| <DataTemplate> | ||
| <Grid Padding="10"> | ||
| <Grid.RowDefinitions> | ||
| <RowDefinition Height="Auto"/> | ||
| <RowDefinition Height="Auto"/> | ||
| </Grid.RowDefinitions> | ||
| <Grid.ColumnDefinitions> | ||
| <ColumnDefinition Width="Auto"/> | ||
| <ColumnDefinition Width="*"/> | ||
| </Grid.ColumnDefinitions> | ||
| <Image Grid.RowSpan="2" | ||
| Source="{Binding ImageUrl}" | ||
| Aspect="AspectFill" | ||
| WidthRequest="80" | ||
| HeightRequest="80"/> | ||
| <Label Grid.Column="1" | ||
| Text="{Binding Name}" | ||
| FontAttributes="Bold"/> | ||
| <Label Grid.Row="1" | ||
| Grid.Column="1" | ||
| Text="{Binding Location}" | ||
| FontAttributes="Italic" | ||
| VerticalOptions="End"/> | ||
| </Grid> | ||
| </DataTemplate> | ||
| </CollectionView.ItemTemplate> | ||
| <CollectionView.GroupHeaderTemplate> | ||
| <DataTemplate> | ||
| <Label x:Name="GroupHeaderLabel" | ||
| Text="{Binding Name}" | ||
| BackgroundColor="LightGray" | ||
| FontSize="20" | ||
| FontAttributes="Bold" | ||
| AutomationId="GroupHeader"/> | ||
| </DataTemplate> | ||
| </CollectionView.GroupHeaderTemplate> | ||
| <CollectionView.GroupFooterTemplate> | ||
| <DataTemplate> | ||
| <Label Text="{Binding Count, StringFormat='Total animals: {0:D}'}" | ||
| Margin="0,0,0,10"/> | ||
| </DataTemplate> | ||
| </CollectionView.GroupFooterTemplate> | ||
| </CollectionView> | ||
| </Grid> | ||
| </ContentPage> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| using System; | ||
| using System.Collections.ObjectModel; | ||
| using Microsoft.Maui.Controls; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 33130, "CollectionView group header size changes with ItemSizingStrategy", PlatformAffected.iOS | PlatformAffected.macOS)] | ||
| public partial class Issue33130 : ContentPage | ||
| { | ||
| public Issue33130() | ||
| { | ||
| InitializeComponent(); | ||
| BindingContext = new Issue33130ViewModel(); | ||
| } | ||
|
|
||
| private void OnSwitchToMeasureFirstItem(object sender, EventArgs e) | ||
| { | ||
| TestCollectionView.ItemSizingStrategy = ItemSizingStrategy.MeasureFirstItem; | ||
| StatusLabel.Text = $"ItemSizingStrategy: {TestCollectionView.ItemSizingStrategy}"; | ||
| } | ||
| } | ||
|
|
||
| public class Issue33130ViewModel | ||
| { | ||
| public ObservableCollection<Issue33130AnimalGroup> Animals { get; set; } | ||
|
|
||
| public Issue33130ViewModel() | ||
| { | ||
| Animals = new ObservableCollection<Issue33130AnimalGroup> | ||
| { | ||
| new Issue33130AnimalGroup("Bears") | ||
| { | ||
| new Issue33130Animal { Name = "Grizzly Bear", Location = "North America", ImageUrl = "bear.jpg" }, | ||
| new Issue33130Animal { Name = "Polar Bear", Location = "Arctic", ImageUrl = "bear.jpg" }, | ||
| }, | ||
| new Issue33130AnimalGroup("Monkeys") | ||
| { | ||
| new Issue33130Animal { Name = "Baboon", Location = "Africa", ImageUrl = "monkey.jpg" }, | ||
| new Issue33130Animal { Name = "Capuchin Monkey", Location = "South America", ImageUrl = "monkey.jpg" }, | ||
| new Issue33130Animal { Name = "Spider Monkey", Location = "Central America", ImageUrl = "monkey.jpg" }, | ||
| }, | ||
| new Issue33130AnimalGroup("Elephants") | ||
| { | ||
| new Issue33130Animal { Name = "African Elephant", Location = "Africa", ImageUrl = "elephant.jpg" }, | ||
| new Issue33130Animal { Name = "Asian Elephant", Location = "Asia", ImageUrl = "elephant.jpg" }, | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| public class Issue33130AnimalGroup : ObservableCollection<Issue33130Animal> | ||
| { | ||
| public string Name { get; set; } | ||
|
|
||
| public Issue33130AnimalGroup(string name) : base() | ||
| { | ||
| Name = name; | ||
| } | ||
| } | ||
|
|
||
| public class Issue33130Animal | ||
| { | ||
| public string Name { get; set; } | ||
| public string Location { get; set; } | ||
| public string ImageUrl { get; set; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue33130 : _IssuesUITest | ||
| { | ||
| public override string Issue => "CollectionView group header size changes with ItemSizingStrategy"; | ||
|
|
||
| public Issue33130(TestDevice device) : base(device) { } | ||
| [Test] | ||
| [Category(UITestCategories.CollectionView)] | ||
| public void GroupHeaderSizeShouldNotChangeWithItemSizingStrategy() | ||
| { | ||
| // Wait for the CollectionView to load | ||
| App.WaitForElement("TestCollectionView"); | ||
| App.WaitForElement("GroupHeader"); | ||
| App.WaitForElement("CollectionViewHeader"); | ||
|
|
||
| // Get the initial header size (before changing ItemSizingStrategy) | ||
| var headerElementBefore = App.FindElement("GroupHeader"); | ||
| var headerRectBefore = headerElementBefore.GetRect(); | ||
|
|
||
| var collectionViewHeaderBefore = App.FindElement("CollectionViewHeader"); | ||
| var collectionViewHeaderRectBefore = collectionViewHeaderBefore.GetRect(); | ||
|
|
||
| Assert.That(headerRectBefore.Height, Is.GreaterThan(0), "Header should have a height before strategy change"); | ||
| Assert.That(collectionViewHeaderRectBefore.Height, Is.GreaterThan(0), "CollectionView header should have a height before strategy change"); | ||
|
|
||
| // Switch ItemSizingStrategy | ||
| App.WaitForElement("SwitchStrategyButton"); | ||
| App.Tap("SwitchStrategyButton"); | ||
|
|
||
| // Get the header size after changing ItemSizingStrategy | ||
| var headerElementAfter = App.FindElement("GroupHeader"); | ||
| var headerRectAfter = headerElementAfter.GetRect(); | ||
| var collectionViewHeaderAfter = App.FindElement("CollectionViewHeader"); | ||
| var collectionViewHeaderRectAfter = collectionViewHeaderAfter.GetRect(); | ||
|
|
||
| Assert.That(headerRectAfter.Height, Is.GreaterThan(0), "Header should have a height after strategy change"); | ||
| Assert.That(collectionViewHeaderRectAfter.Height, Is.GreaterThan(0), "CollectionView header should have a height after strategy change"); | ||
|
|
||
| // The header size should remain the same (within a small tolerance for rendering differences) | ||
| // Allow for small rounding differences but not significant changes | ||
| var groupHeaderHeightDifference = Math.Abs(headerRectBefore.Height - headerRectAfter.Height); | ||
| var collectionViewHeaderHeightDifference = Math.Abs(collectionViewHeaderRectBefore.Height - collectionViewHeaderRectAfter.Height); | ||
|
|
||
| // Assert that the height difference is minimal (less than 5 pixels tolerance) | ||
| Assert.That(groupHeaderHeightDifference, Is.LessThan(5), | ||
| $"Header height should not change significantly. Before: {headerRectBefore.Height}, After: {headerRectAfter.Height}, Difference: {groupHeaderHeightDifference}"); | ||
|
|
||
| Assert.That(collectionViewHeaderHeightDifference, Is.LessThan(5), $"CollectionView header height should not change significantly. Before: {collectionViewHeaderRectBefore.Height}, After: {collectionViewHeaderRectAfter.Height}, Difference: {collectionViewHeaderHeightDifference}"); | ||
| } | ||
| } |
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.
The comment "Ensure this cell is treated as a regular item cell (not a supplementary view)" is helpful, but it would be more informative to explain why this is necessary. Consider expanding it to: "Ensure this cell is treated as a regular item cell (not a supplementary view) so it can use the cached first-item measurement for MeasureFirstItem strategy" to provide better context about the fix's purpose.