Skip to content

Commit

Permalink
[iOS] Figure a better EstimatedItemSize for HorizontalList (#20022)
Browse files Browse the repository at this point in the history
* [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
  • Loading branch information
rmarinho committed Feb 22, 2024
1 parent f0491b9 commit 6ce9b50
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?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.Issues15815"
Title="Issues15815">
<Grid RowDefinitions="Auto,Auto, Auto" >
<Label Grid.Row="0" Text="The last cell (index 2) is not visible with MeasureAllItems if 1st cell is 50px width" />
<CollectionView x:Name="col" BackgroundColor="Pink" Grid.Row="1" ItemsLayout="HorizontalList"
ItemSizingStrategy="MeasureAllItems" HeightRequest="200">
<CollectionView.ItemTemplate>
<DataTemplate>
<Grid BackgroundColor="{Binding Color}" WidthRequest="{Binding Width}" HeightRequest="200">
<Label AutomationId="{Binding Id}" Text="{Binding Name}" FontSize="Header" TextColor="Black"/>
</Grid>
</DataTemplate>
</CollectionView.ItemTemplate>
</CollectionView>
<Label Grid.Row="2" Text="Last Cell Blue should be visible" />
</Grid>

</ContentPage>
Original file line number Diff line number Diff line change
@@ -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<ItemViewModel>
{
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}";
}
}
}
12 changes: 11 additions & 1 deletion src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ public abstract class ItemsViewController<TItemsView> : UICollectionViewControll

[UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
Func<UICollectionViewCell> _getPrototype;

[UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
Func<NSIndexPath, UICollectionViewCell> _getPrototypeForIndexPath;
CGSize _previousContentSize = CGSize.Empty;

[UnconditionalSuppressMessage("Memory", "MEM0002", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
Expand Down Expand Up @@ -236,7 +239,6 @@ void InvalidateMeasureIfContentSizeChanged()
(ItemsView as IView)?.InvalidateMeasure();
}
}

_previousContentSize = contentSize.Value;
}

Expand Down Expand Up @@ -269,6 +271,9 @@ void EnsureLayoutInitialized()
_getPrototype ??= GetPrototype;
ItemsViewLayout.GetPrototype = _getPrototype;

_getPrototypeForIndexPath ??= GetPrototypeForIndexPath;
ItemsViewLayout.GetPrototypeForIndexPath = _getPrototypeForIndexPath;

Delegator = CreateDelegator();
CollectionView.Delegate = Delegator;

Expand Down Expand Up @@ -476,6 +481,11 @@ UICollectionViewCell GetPrototype()

var indexPath = NSIndexPath.Create(group, 0);

return GetPrototypeForIndexPath(indexPath);
}

internal UICollectionViewCell GetPrototypeForIndexPath(NSIndexPath indexPath)
{
return CreateMeasurementCell(indexPath);
}

Expand Down
14 changes: 0 additions & 14 deletions src/Controls/src/Core/Handlers/Items/iOS/ItemsViewDelegator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
55 changes: 55 additions & 0 deletions src/Controls/src/Core/Handlers/Items/iOS/ItemsViewLayout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public abstract class ItemsViewLayout : UICollectionViewFlowLayout
CGSize _currentSize;
WeakReference<Func<UICollectionViewCell>> _getPrototype;

WeakReference<Func<NSIndexPath, UICollectionViewCell>> _getPrototypeForIndexPath;

readonly Dictionary<object, CGSize> _cellSizeCache = new();

public ItemsUpdatingScrollMode ItemsUpdatingScrollMode { get; set; }
Expand All @@ -31,6 +33,12 @@ public Func<UICollectionViewCell> GetPrototype
set => _getPrototype = new(value);
}

internal Func<NSIndexPath, UICollectionViewCell> 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)
Expand Down Expand Up @@ -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;
}
}
Expand Down Expand Up @@ -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;
}
}
}
21 changes: 21 additions & 0 deletions src/Controls/tests/UITests/Tests/Issues/Issue15815.cs
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit 6ce9b50

Please sign in to comment.