-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix CV1 GridItemsLayout centering single item AND Fix Empty view not resizing when bounds change #29639
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
Fix CV1 GridItemsLayout centering single item AND Fix Empty view not resizing when bounds change #29639
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 |
|---|---|---|
|
|
@@ -207,23 +207,22 @@ void InvalidateLayoutIfItemsMeasureChanged() | |
| { | ||
| var collectionView = CollectionView; | ||
| var visibleCells = collectionView.VisibleCells; | ||
| List<NSIndexPath> invalidatedPaths = null; | ||
| List<TemplatedCell2> invalidatedCells = null; | ||
|
Member
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. Why did this have to change? I see before we get the index and store it here, then invalidate. Now you get the cell, store it, and then later get the index. Does this different path do different things? Looks like in the end you still use the same cell index in the invalidate.
Contributor
Author
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. I wanted to avoid a reference to NS objects when possible. |
||
|
|
||
| var visibleCellsLength = visibleCells.Length; | ||
| for (int n = 0; n < visibleCellsLength; n++) | ||
| { | ||
| if (visibleCells[n] is TemplatedCell2 { MeasureInvalidated: true } cell) | ||
| { | ||
| invalidatedPaths ??= new List<NSIndexPath>(visibleCellsLength); | ||
| var path = collectionView.IndexPathForCell(cell); | ||
| invalidatedPaths.Add(path); | ||
| invalidatedCells ??= []; | ||
| invalidatedCells.Add(cell); | ||
| } | ||
| } | ||
|
|
||
| if (invalidatedPaths != null) | ||
| if (invalidatedCells is not null) | ||
| { | ||
| var layoutInvalidationContext = new UICollectionViewLayoutInvalidationContext(); | ||
| layoutInvalidationContext.InvalidateItems(invalidatedPaths.ToArray()); | ||
| layoutInvalidationContext.InvalidateItems(invalidatedCells.Select(CollectionView.IndexPathForCell).ToArray()); | ||
| collectionView.CollectionViewLayout.InvalidateLayout(layoutInvalidationContext); | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| using System.Collections.ObjectModel; | ||
| using Microsoft.Maui.Controls.Shapes; | ||
|
|
||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 29595, "iOS CV: GridItemsLayout not left-aligning a single item", PlatformAffected.iOS)] | ||
| public class Issue29595 : ContentPage | ||
| { | ||
| readonly ObservableCollection<string> _items = []; | ||
|
|
||
| public Issue29595() | ||
| { | ||
| var grid = new Grid(); | ||
|
|
||
| var cv = new CollectionView | ||
| { | ||
| Margin = 10, | ||
| VerticalOptions = LayoutOptions.Fill, | ||
| ItemsLayout = new GridItemsLayout(3, ItemsLayoutOrientation.Vertical) | ||
| { | ||
| HorizontalItemSpacing = 8, | ||
| VerticalItemSpacing = 8 | ||
| }, | ||
| ItemTemplate = GetItemTemplate(), | ||
| ItemsSource = _items | ||
| }; | ||
|
|
||
| grid.Add(cv); | ||
|
|
||
| Content = grid; | ||
| } | ||
|
|
||
| static DataTemplate GetItemTemplate(double fontSize = 14) | ||
| { | ||
| return new DataTemplate(() => | ||
| { | ||
| var border = new Border | ||
| { | ||
| StrokeThickness = 0, | ||
| StrokeShape = new RoundRectangle { CornerRadius = 32 } | ||
| }; | ||
|
|
||
| var innerGrid = new Grid | ||
| { | ||
| BackgroundColor = Colors.WhiteSmoke, | ||
| RowDefinitions = | ||
| { | ||
| new RowDefinition { Height = new GridLength(1, GridUnitType.Star) }, | ||
| new RowDefinition { Height = GridLength.Auto } | ||
| } | ||
| }; | ||
|
|
||
| var image = new FFImageLoadingStubImage | ||
| { | ||
| Aspect = Aspect.AspectFill, | ||
| Source = "dotnet_bot.png" | ||
| }; | ||
| Grid.SetRow(image, 0); | ||
|
|
||
| var label = new Label | ||
| { | ||
| Text = "Test", | ||
| AutomationId = "StubLabel", | ||
| FontSize = fontSize, | ||
| FontFamily = "OpenSansRegular", | ||
| TextColor = Colors.Black, | ||
| HorizontalOptions = LayoutOptions.Center, | ||
| VerticalOptions = LayoutOptions.Center | ||
| }; | ||
| Grid.SetRow(label, 1); | ||
|
|
||
| innerGrid.Add(image); | ||
| innerGrid.Add(label); | ||
|
|
||
| border.Content = innerGrid; | ||
| return border; | ||
| }); | ||
| } | ||
|
|
||
| protected override async void OnAppearing() | ||
| { | ||
| base.OnAppearing(); | ||
| await Task.Delay(300); | ||
| _items.Add("item1"); | ||
| } | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// This is a normal image which simulates FFImageLoading loading behavior which may trigger an additional measure pass | ||
| /// once the image is loaded, and the new measure could be different from the previous one. | ||
| /// </summary> | ||
| file class FFImageLoadingStubImage : Image | ||
| { | ||
| int counter; | ||
|
|
||
| protected async override void OnPropertyChanged(string propertyName = null) | ||
| { | ||
| base.OnPropertyChanged(propertyName); | ||
|
|
||
| if (propertyName == SourceProperty.PropertyName) | ||
| { | ||
| ++counter; | ||
| await Task.Delay(100); | ||
| InvalidateMeasure(); | ||
| } | ||
| } | ||
|
|
||
| protected override SizeRequest OnMeasure(double widthConstraint, double heightConstraint) | ||
| { | ||
| var desiredSize = base.OnMeasure(double.PositiveInfinity, double.PositiveInfinity); | ||
| var desiredWidth = double.IsNaN(desiredSize.Request.Width) ? 0 : desiredSize.Request.Width + counter; | ||
| var desiredHeight = double.IsNaN(desiredSize.Request.Height) ? 0 : desiredSize.Request.Height; | ||
|
|
||
| if (double.IsNaN(widthConstraint)) | ||
| widthConstraint = 0; | ||
| if (double.IsNaN(heightConstraint)) | ||
| heightConstraint = 0; | ||
|
|
||
| if (Math.Abs(desiredWidth) < double.Epsilon || Math.Abs(desiredHeight) < double.Epsilon) | ||
| return new SizeRequest(new Size(0, 0)); | ||
|
|
||
| if (double.IsPositiveInfinity(widthConstraint) && double.IsPositiveInfinity(heightConstraint)) | ||
| { | ||
| return new SizeRequest(new Size(desiredWidth, desiredHeight)); | ||
| } | ||
|
|
||
| if (double.IsPositiveInfinity(widthConstraint)) | ||
| { | ||
| var factor = heightConstraint / desiredHeight; | ||
| return new SizeRequest(new Size(desiredWidth * factor, desiredHeight * factor)); | ||
| } | ||
|
|
||
| if (double.IsPositiveInfinity(heightConstraint)) | ||
| { | ||
| var factor = widthConstraint / desiredWidth; | ||
| return new SizeRequest(new Size(desiredWidth * factor, desiredHeight * factor)); | ||
| } | ||
|
|
||
| var fitsWidthRatio = widthConstraint / desiredWidth; | ||
| var fitsHeightRatio = heightConstraint / desiredHeight; | ||
|
|
||
| if (double.IsNaN(fitsWidthRatio)) | ||
| fitsWidthRatio = 0; | ||
| if (double.IsNaN(fitsHeightRatio)) | ||
| fitsHeightRatio = 0; | ||
|
|
||
| if (Math.Abs(fitsWidthRatio) < double.Epsilon && Math.Abs(fitsHeightRatio) < double.Epsilon) | ||
| return new SizeRequest(new Size(0, 0)); | ||
|
|
||
| if (Math.Abs(fitsWidthRatio) < double.Epsilon) | ||
| return new SizeRequest(new Size(desiredWidth * fitsHeightRatio, desiredHeight * fitsHeightRatio)); | ||
|
|
||
| if (Math.Abs(fitsHeightRatio) < double.Epsilon) | ||
| return new SizeRequest(new Size(desiredWidth * fitsWidthRatio, desiredHeight * fitsWidthRatio)); | ||
|
|
||
| var ratioFactor = Math.Min(fitsWidthRatio, fitsHeightRatio); | ||
|
|
||
| return new SizeRequest(new Size(desiredWidth * ratioFactor, desiredHeight * ratioFactor)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 29634, "iOS CV: Empty view not resizing when bounds change", PlatformAffected.iOS)] | ||
| public class Issue29634 : ContentPage | ||
|
PureWeen marked this conversation as resolved.
|
||
| { | ||
| CollectionView _collectionView; | ||
| public Issue29634() | ||
| { | ||
| Grid grid = null; | ||
| var button = new Button | ||
| { | ||
| Margin = new Thickness(0, 0, 0, 5), | ||
| BackgroundColor = Colors.LightSeaGreen, | ||
| FontSize = 12, | ||
| TextColor = Colors.DarkSlateGray, | ||
| Text = "Button text", | ||
| Command = new Command(() => grid.WidthRequest = 200), | ||
| AutomationId = "RunTest" | ||
| }; | ||
|
|
||
| button.SizeChanged += async (sender, e) => | ||
| { | ||
| await Task.Yield(); // Ensure the layout pass is complete before checking size | ||
| if (sender is Button b && Content is VerticalStackLayout l && grid.WidthRequest == 200) | ||
| { | ||
| if (l.Children.Count > 1) | ||
| l.Children.RemoveAt(1); // Remove the previous label if it exists | ||
|
|
||
| if (b.Width == 200) | ||
| l.Add(new Label() { Text = "Button Successfully resized", AutomationId = "SuccessLabel" }); | ||
| else | ||
| l.Add(new Label() { Text = $"Button Failed To Resize to 200: {b.Width}x{b.Height}", AutomationId = "FailLabel" }); | ||
| } | ||
| }; | ||
|
|
||
| _collectionView = new CollectionView | ||
| { | ||
| ItemsLayout = new LinearItemsLayout(ItemsLayoutOrientation.Horizontal) { ItemSpacing = 10 }, | ||
| ItemTemplate = new DataTemplate(), | ||
| EmptyView = button | ||
| }; | ||
|
|
||
| grid = new Grid | ||
| { | ||
| WidthRequest = 400, | ||
| HeightRequest = 200, | ||
| ColumnDefinitions = | ||
| [ | ||
| new ColumnDefinition(GridLength.Auto), | ||
| new ColumnDefinition(GridLength.Star) | ||
| ], | ||
| Children = { _collectionView } | ||
| }; | ||
|
|
||
| Grid.SetColumn(_collectionView, 1); | ||
|
|
||
| Content = new VerticalStackLayout | ||
| { | ||
| Children = | ||
| { | ||
| grid | ||
| } | ||
| }; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| #if IOSUITEST | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
| public class Issue29595 : _IssuesUITest | ||
| { | ||
| public override string Issue => "iOS CV: GridItemsLayout not left-aligning a single item"; | ||
|
|
||
| public Issue29595(TestDevice device) | ||
| : base(device) | ||
| { } | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.CollectionView)] | ||
| public void VerifyGridItemsLayoutLeftAlignsSingleItem() | ||
| { | ||
| App.WaitForElement("StubLabel"); | ||
| VerifyScreenshot(); | ||
| } | ||
| } | ||
| #endif |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| #if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_WINDOWS | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue29634 : _IssuesUITest | ||
| { | ||
| public override string Issue => "iOS CV: Empty view not resizing when bounds change"; | ||
|
|
||
| public Issue29634(TestDevice device) | ||
| : base(device) | ||
| { } | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.CollectionView)] | ||
| public void VerifyEmptyViewResizesWhenBoundsChange() | ||
| { | ||
| App.WaitForElement("RunTest"); | ||
| App.Tap("RunTest"); | ||
| App.WaitForElement("SuccessLabel"); | ||
| } | ||
| } | ||
| #endif |
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.
Is this extra code path not needed in CV2?