From 6ce9b50fe5ad3ab838bc1b250edf2bb09431b59c Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Mon, 19 Feb 2024 14:19:07 +0000 Subject: [PATCH] =?UTF-8?q?[iOS]=C2=A0Figure=20a=20better=20EstimatedItemS?= =?UTF-8?q?ize=20for=20HorizontalList=20(#20022)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Samples] Add repro case for issue #15815 * [iOS] Remove dead code * [iOS] Try find a better EstimatedItemSize for horizontal list * [uitest] Add Uitest for CollectionView * Fix test * Pink color --- .../Issues/Issues15815.xaml | 21 +++++++ .../Issues/Issues15815.xaml.cs | 34 ++++++++++++ .../Handlers/Items/iOS/ItemsViewController.cs | 12 +++- .../Handlers/Items/iOS/ItemsViewDelegator.cs | 14 ----- .../Handlers/Items/iOS/ItemsViewLayout.cs | 55 +++++++++++++++++++ .../tests/UITests/Tests/Issues/Issue15815.cs | 21 +++++++ 6 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml create mode 100644 src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml.cs create mode 100644 src/Controls/tests/UITests/Tests/Issues/Issue15815.cs diff --git a/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml b/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml new file mode 100644 index 000000000000..c6c614f7ce58 --- /dev/null +++ b/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml @@ -0,0 +1,21 @@ + + + + + + \ No newline at end of file diff --git a/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml.cs b/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml.cs new file mode 100644 index 000000000000..9c72998d012c --- /dev/null +++ b/src/Controls/samples/Controls.Sample.UITests/Issues/Issues15815.xaml.cs @@ -0,0 +1,34 @@ +using System; +using System.Collections.ObjectModel; +using Microsoft.Maui.Controls; +using Microsoft.Maui.Controls.Xaml; +using Microsoft.Maui.Graphics; + +namespace Maui.Controls.Sample.Issues +{ + + [XamlCompilation(XamlCompilationOptions.Compile)] + [Issue(IssueTracker.Github, 15815, "Horizontal CollectionView does not show the last element under some condition", PlatformAffected.iOS)] + public partial class Issues15815 : ContentPage + { + public Issues15815() + { + InitializeComponent(); + col.ItemsSource = new ObservableCollection + { + new ItemViewModel { Index = 0, Color = Colors.Red, Width = 50 }, + new ItemViewModel { Index = 1, Color = Colors.Green, Width = 100 }, + new ItemViewModel { Index = 2, Color = Colors.Blue, Width = 100 }, + }; + } + + class ItemViewModel + { + public Color Color { get; set; } + public int Width { get; set; } + public int Index { get; set; } + public string Id => $"id-{Index}"; + public string Name => $"Item {Index}"; + } + } +} \ No newline at end of file diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs index 43fc156726d1..420baed2a5b5 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs @@ -35,6 +35,9 @@ public abstract class ItemsViewController : UICollectionViewControll [UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")] Func _getPrototype; + + [UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")] + Func _getPrototypeForIndexPath; CGSize _previousContentSize = CGSize.Empty; [UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")] @@ -236,7 +239,6 @@ void InvalidateMeasureIfContentSizeChanged() (ItemsView as IView)?.InvalidateMeasure(); } } - _previousContentSize = contentSize.Value; } @@ -269,6 +271,9 @@ void EnsureLayoutInitialized() _getPrototype ??= GetPrototype; ItemsViewLayout.GetPrototype = _getPrototype; + _getPrototypeForIndexPath ??= GetPrototypeForIndexPath; + ItemsViewLayout.GetPrototypeForIndexPath = _getPrototypeForIndexPath; + Delegator = CreateDelegator(); CollectionView.Delegate = Delegator; @@ -476,6 +481,11 @@ UICollectionViewCell GetPrototype() var indexPath = NSIndexPath.Create(group, 0); + return GetPrototypeForIndexPath(indexPath); + } + + internal UICollectionViewCell GetPrototypeForIndexPath(NSIndexPath indexPath) + { return CreateMeasurementCell(indexPath); } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs index 3716b1b87dd8..49d0124ea87e 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs @@ -123,20 +123,6 @@ public override void CellDisplayingEnded(UICollectionView collectionView, UIColl templatedCell.Unbind(); } } - - if (ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Horizontal) - { - var actualWidth = collectionView.ContentSize.Width - collectionView.Bounds.Size.Width; - if (collectionView.ContentOffset.X >= actualWidth || collectionView.ContentOffset.X < 0) - return; - } - else - { - var actualHeight = collectionView.ContentSize.Height - collectionView.Bounds.Size.Height; - - if (collectionView.ContentOffset.Y >= actualHeight || collectionView.ContentOffset.Y < 0) - return; - } } protected virtual (bool VisibleItems, NSIndexPath First, NSIndexPath Center, NSIndexPath Last) GetVisibleItemsIndexPath() diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs index cf7af6be1963..b8275140b167 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs @@ -19,6 +19,8 @@ public abstract class ItemsViewLayout : UICollectionViewFlowLayout CGSize _currentSize; WeakReference> _getPrototype; + WeakReference> _getPrototypeForIndexPath; + readonly Dictionary _cellSizeCache = new(); public ItemsUpdatingScrollMode ItemsUpdatingScrollMode { get; set; } @@ -31,6 +33,12 @@ public Func GetPrototype set => _getPrototype = new(value); } + internal Func GetPrototypeForIndexPath + { + get => _getPrototypeForIndexPath is not null && _getPrototypeForIndexPath.TryGetTarget(out var func) ? func : null; + set => _getPrototypeForIndexPath = new(value); + } + internal ItemSizingStrategy ItemSizingStrategy { get; private set; } protected ItemsViewLayout(ItemsLayout itemsLayout, ItemSizingStrategy itemSizingStrategy = ItemSizingStrategy.MeasureFirstItem) @@ -243,6 +251,7 @@ protected void DetermineCellSize() else { // Autolayout is now enabled, and this is the size used to guess scrollbar size and progress + measure = TryFindEstimatedSize(measure); EstimatedItemSize = measure; } } @@ -595,5 +604,51 @@ internal void ClearCellSizeCache() { _cellSizeCache.Clear(); } + + CGSize TryFindEstimatedSize(CGSize existingMeasurement) + { + if (CollectionView == null || GetPrototypeForIndexPath == null) + return existingMeasurement; + + //Since this issue only seems to be reproducible on Horizontal scrolling, we only check for that + if (ScrollDirection == UICollectionViewScrollDirection.Horizontal) + { + return FindEstimatedSizeUsingWidth(existingMeasurement); + } + + return existingMeasurement; + } + + CGSize FindEstimatedSizeUsingWidth(CGSize existingMeasurement) + { + // TODO: Handle grouping + var group = 0; + var collectionViewWidth = CollectionView.Bounds.Width; + var numberOfItemsInGroup = CollectionView.NumberOfItemsInSection(group); + + // Calculate the number of cells that can fit in the viewport + var numberOfCellsToCheck = Math.Min((int)(collectionViewWidth / existingMeasurement.Width) + 1, numberOfItemsInGroup); + + // Iterate through the cells and find the one with a wider width + for (int i = 1; i < numberOfCellsToCheck; i++) + { + var indexPath = NSIndexPath.Create(group, i); + if (GetPrototypeForIndexPath(indexPath) is ItemsViewCell cellAtIndex) + { + cellAtIndex.ConstrainTo(ConstrainedDimension); + var measureCellAtIndex = cellAtIndex.Measure(); + + // Check if the cell has a wider width + if (measureCellAtIndex.Width > existingMeasurement.Width) + { + existingMeasurement = measureCellAtIndex; + } + + // TODO: Cache this cell size + } + } + + return existingMeasurement; + } } } diff --git a/src/Controls/tests/UITests/Tests/Issues/Issue15815.cs b/src/Controls/tests/UITests/Tests/Issues/Issue15815.cs new file mode 100644 index 000000000000..ddea6591b420 --- /dev/null +++ b/src/Controls/tests/UITests/Tests/Issues/Issue15815.cs @@ -0,0 +1,21 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.AppiumTests.Issues; + +public class Issue15815 : _IssuesUITest +{ + public Issue15815(TestDevice device) + : base(device) + { } + + public override string Issue => "Horizontal CollectionView does not show the last element under some condition"; + + [Test] + public void LastItemIsVisilbe() + { + var lastItem = App.WaitForElement("id-2"); + Assert.AreEqual("Item 2", lastItem.GetText()); + } +}