[Bug] [iOS] CollectionView - Invalid Height Measure when using different datatemplates

[BrayanKhosravian]

Description

When we define two datatemplates with a different height in a datatemplateSelector and use that selector for the collectionview, then some heights of the cells are invalid.

• Check the gif atttached.

More related: #8583 #9520 Possibly Related: #5455 #7788 #10625

Steps to Reproduce

1. use different datatemplates, where both templates differ in the heightrequest!
2. write a datatemplateselector
3. add different templates to the collection

Expected Behavior

• the height of the already added items should not change
• the new added item shouldn use the height of the cell located above

Actual Behavior

• It seems like that the new item added gets the height of the item located above it.
• All items located below the new added item get the same invalid height like the new added one
• this visual layout issue is only visible for a second

Basic Information

• Version with issue: 4.4 - master
• Last known good version: -
• IDE: VS 2019
• Platform Target Frameworks:
  • iOS: -
• Nuget Packages: none
• Affected Devices: general ios issue

Screenshots

Reproduction Link

Just create a new ContentPage and copy and paste the content into the view and the c# code into the code behind.

View:

<ContentPage xmlns="http://xamarin.com/schemas/2014/forms"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
             xmlns:d="http://xamarin.com/schemas/2014/forms/design"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             xmlns:collectionViewGalleries="clr-namespace:Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries;assembly=Xamarin.Forms.Controls"
             mc:Ignorable="d"
             x:Class="Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.VaryingDataTempleteSelectorGallery">
    <ContentPage.Resources>
        <ResourceDictionary>
            <DataTemplate x:Key="CarTemplate">
                <Frame BorderColor="Red" BackgroundColor="Yellow">
                    <StackLayout HeightRequest="100">
                        <Label Text="{Binding Name}" />
                    </StackLayout>
                </Frame>
            </DataTemplate>

            <DataTemplate x:Key="PlaneTemplate">
                <Frame BorderColor="Red" BackgroundColor="Aqua">
                    <StackLayout HeightRequest="50">
                        <Label Text="{Binding Name}" />
                    </StackLayout>
                </Frame>
            </DataTemplate>

            <collectionViewGalleries:VehicleTemplateSelector x:Key="VehicleTemplateSelector"
                                                             CareTemplate="{StaticResource CarTemplate}"
                                                             PlaneTemplate="{StaticResource PlaneTemplate}"/>
        </ResourceDictionary>
    </ContentPage.Resources>

    <ContentPage.Content>
        <Grid>
            <Grid.RowDefinitions>
                <RowDefinition Height="*"/>
                <RowDefinition Height="Auto"/>
            </Grid.RowDefinitions>

            <CollectionView ItemsSource="{Binding Items}" 
                            ItemTemplate="{StaticResource VehicleTemplateSelector}"
                            VerticalOptions="FillAndExpand">

            </CollectionView>

            <StackLayout Grid.Row="1">
                <StackLayout Orientation="Horizontal">
                    <Label Text="Index"/>
                    <Entry Text="{Binding Index}"/>
                </StackLayout>
                <Button Text="InsertCar" Clicked="InsertCar_OnClicked"/>
                <Button Text="InsertPlane" Clicked="InsertPlane_OnClicked"/>
            </StackLayout>
        </Grid>
    </ContentPage.Content>
</ContentPage>

the code behind:

public partial class VaryingDataTempleteSelectorGallery : ContentPage, INotifyPropertyChanged
	{
		string _index;

		public VaryingDataTempleteSelectorGallery()
		{
			InitializeComponent();
			BindingContext = this;

			foreach (var vehicle in CreateDefaultVehicles())
			{
				Items.Add(vehicle);
			}

			IEnumerable<VehicleBase> CreateDefaultVehicles()
			{
				yield return new Plane("Plane1");
				yield return new Plane("Plane2");
				yield return new Car("Car1");
				yield return new Plane("Plane3");
				yield return new Car("Car2");
				yield return new Plane("Plane4");
			}
		}

		public ObservableCollection<VehicleBase> Items { get; set; } =
			new ObservableCollection<VehicleBase>();

		public string Index
		{
			get => _index;
			set => SetValue(ref _index, value);
		}

		void InsertCar_OnClicked(object sender, EventArgs e)
		{
			if (string.IsNullOrWhiteSpace(Index)) return;
			if (!int.TryParse(Index, out var index)) return;
			if (index > Items.Count || index < 0) return;

			Items.Insert(index, new Car("rCar " + new Random().Next(0,1000).ToString()));
		}

		void InsertPlane_OnClicked(object sender, EventArgs e)
		{
			if (string.IsNullOrWhiteSpace(Index)) return;
			if (!int.TryParse(Index, out var index)) return;
			if (index > Items.Count || index < 0) return;

			Items.Insert(index, new Plane("rPlane " + new Random().Next(0,1000).ToString()));
		}

		void SetValue<T>(ref T backingField, in T value, [CallerMemberName] string callerName = null)
		{
			if (Equals(backingField, value)) return;
			OnPropertyChanging(callerName);
			backingField = value;
			OnPropertyChanged(callerName);
		}
	}

	class VehicleTemplateSelector : DataTemplateSelector
	{
		public DataTemplate CareTemplate { get; set; }
		public DataTemplate PlaneTemplate { get; set; }
		protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
		{
			if (CareTemplate == null || PlaneTemplate == null) throw new ArgumentNullException();

			if (item is Car car) return CareTemplate;
			if (item is Plane plane) return PlaneTemplate;

			throw new ArgumentOutOfRangeException();
		}
	}

	public abstract class VehicleBase
	{
		protected VehicleBase(string name) => Name = name;

		public string Name { get; set; }
	}

	class Car : VehicleBase
	{
		public Car(string name) : base(name) { }
	}

	class Plane : VehicleBase
	{
		public Plane(string name) : base(name) { }
	}

Workaround

none

[BrayanKhosravian]

@gentledepp @bruzkovsky fyi

[TopSkillDeveloper]

Confirm. If change the height of one item, the height of all items changes. At the same time, everything becomes normal during scrolling (as it should-1 item changes). Only on iOS. The latest stable version of xamarin

[liveinvarun]

This issue has already been reported for forms collectionview
#8583
#9520

[BrayanKhosravian]

@liveinvarun thanks for mentioning the issues. ill add them to my issue report

[BrayanKhosravian]

@liveinvarun wait did you actually say UICollectionView?
The one located in the namespace UIKit ?
If yes could you post a link to the report? i wasnt able to see such a report in the issues u posted in before (#8583 #9520).

[liveinvarun]

@BrayanKhosravian we already have the height issue raised as bug [Xamarin Forms CollectionView ]-
Dynamic resizing of items not working - There are samples attached to the bug. Its easily reproducible. Both are bugs are duplicate items i believe, but its a high prority issue as collection views are preffered over listview especially considering the performance factor. The current bug also looks like the same issue. Whenever the redraw or reload happens the cells are not rendering properly.

[BrayanKhosravian]

Ok i spend a few days to investigate the issue and and i want to share what i found out.
Maybe this information could help to fix this issue more quickly or the responses to this comment helps to fix this issue.

It seems like that the auto-sizing does not work properly when using templates with different heights/widths.
I tested this with: vertical-listview, horizontal-listview, vertical gridview

In order to understand how that sizing works "natively" on iOS i made some research about the UICollectionView and stepped through the XF source code.

We can apply two different layouts to the CollectionView.

• UICollectionViewFlowLayout
• UICollectionViewLayout

The UICollectionViewFlowLayout inherits from the UICollectionViewLayout and we use the UICollectionViewFlowLayout for the Xamarin forms collectionview.

However, the flowlayout is used to automatically layout the items in the collection.
That automatically layouting is also known as "AutoLayout". In order to make this work we have to assign constrains to the items.

Another way to fix this issue is to use the UICollectionViewLayout instead of the FlowLayout and do all the measurement manually.

But tbh, im not sure if this will fix this issue completely, as i have seen that we register 4 templates for the collectionview and there is always 1 template reused, based on the layout we use, also when we use a custom templateselector.

So there could be maybe two issues.
1.) dynamic sizing
2.) when changing the height/width of an item, the changes get reflected to other items as well as there is always one template reused. (i could be wrong with this. This is just an assumption)
(Update: the second assumption is wrong as we use a single template for the listview and the uitableview as well)

Ill add the links which i saved during research.
https://forums.developer.apple.com/thread/71685

https://docs.microsoft.com/en-us/xamarin/ios/user-interface/controls/uicollectionview
https://www.rightpoint.com/rplabs/items-in-a-uicollectionview-animation
https://stackoverflow.com/questions/25895311/uicollectionview-self-sizing-cells-with-auto-layout
https://stackoverflow.com/questions/44187881/uicollectionview-full-width-cells-allow-autolayout-dynamic-height/44352072
https://engineering.shopspring.com/dynamic-cell-sizing-in-uicollectionview-fd95f614ef80

[BrayanKhosravian]

OK it seems like that i found a way to improve the layout/measure issue a little bit.
It is still not completely resolved.

Well i finally found the root cause of the bug and i will try to explain it as good as possible.
At the end of this comment ill explain how i tried to "fix" this issue.

Information needed before i explain the issue:

• Inside the ItemsViewController there is the overridden method GetCell. This method is called inside the UIKit.CollectionViewController whenever we add an item to the view.
• The cells which we use to render VerticalCell and HorizontalCell inherit from the UICollectionViewCell. This cells have a custom declared method called Measure.
  There is also an overridden Method called PreferredLayoutAttributesFittingAttributes. This method actually gets called after the ViewCell was added to the visual tree.
  Inside this method we call that Measure method and return a PreferredLayoutAttribute.
• Inside the ItemsViewLayout there is a property called EstimatedSize which we set to a specific value when we init the collectionview or when we add the first item to the view.
  This EstimatedSize is used for autolayouting. This means that the collectionview will try to layout all items automatically.
  There is also an overridden Method called ShouldInvalidateLayout. This method gets called after the ViewCell.PreferredLayoutAttributesFittingAttributes method was called. And again at this point the viewcell is already visible!

Explaining the issue:

• Whenever we add a new cell to the collection the GetCell gets called and the height/width is always the Estimated size which gets also displayed with that height/width value. This means that the cell is already visible in the view with a wrong value for the size.
• Then the PreferredLayoutAttributesFittingAttributes method gets called the the Measure method which apply correct height/size to the layoutattribute and return that attribute.
• then the ItemsViewlayout.ShouldInvalidateLayout method gets called and we check if the preferedLAyoutattributes height/width is equal to the origin layoutattribute which is not true because at this point there is already an invalid viewcell visible! So this method returns false.
• After that method returned false the viewcell gets relayouted/rerendered and this is the reason why the cell looks small at first (its small because the EstimatedSize is applied to the heigh in a verticallcollectionview) and then gets bigger.

I hope that makes sense and my explanation is clear enough. Else just write a feedback or ask what is unclear so i can explain that more clearly.

After knowing whats wrong:

• So this means that we have to measure the viewcells before they get added to the visual tree.
• But we have to measure the items afterwards as well, because some content of the views may change which should update the viewcell as well.

My "fix" which improved the behavior:
This fix is not the final fix tbh, it just improved the visual appearance.

I override a method called GetSizeForItem which is located in the ItemsViewDelegator.
This method gets called after the item gets added to the internal children of the collectionview but before the viewcell gets rendered/visible.
This method return the EstimatedSize per default when its not overridden. (Thats the reason why the cells always had that estimatedsize applied)
The method return a CGSize. So i try to get the viewcell at the indexPath, call Measure on it and return the value of the Measure method.
The correct size gets applied to the viewcell then the temsViewlayout.ShouldInvalidateLayout return false and dosnt remeasure and relayout the viewcell.
I dont call this method from anywhere, it just gets called internally from the base class!

The issue which is still present with my "fix":

• i already told that i try to get the viewcell with the indexpath. I do this by calling the method collectionView.CellForItem(indexPath). The issue with this method is it returns null when a viewcell at an indexpath is not visible.
  At this point i again use the EstimatedSize as fallback.
  And this is the reason why it dosnt work completely yet.

The code i added for the fix:

This is located in Xamarin.Forms.Platform.iOS.ItemsViewDelegator:

/// <summary>
/// Per default this method returns the Estimated size when its not overriden.
/// This method is called before the rendering process and sizes the cell correctly before it is displayed in the collectionview
/// Calling the base implementation of this method will throw an exception when overriding the method
/// </summary>
/// <returns></returns>
public override CGSize GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout, NSIndexPath indexPath)
{
	if (ItemsViewLayout.ItemSizingStrategy == ItemSizingStrategy.MeasureFirstItem)
		return ItemsViewLayout.EstimatedItemSize;

	// ".CellForItem" is not reliable here because when the cell at "indexpath" is not visible it will return null
	var cell = collectionView.CellForItem(indexPath); 
	if (cell is ItemsViewCell itemsViewCell)
	{
		var size = itemsViewCell.Measure();
		return size;
	}
	else return ItemsViewLayout.EstimatedItemSize; // This is basically a Fallback when ".CellForItem" returns null
}

Gif which shows the fix:

[stefanbogaard86]

Hi @BrayanKhosravian, is it possible to apply this improvement in my production code or do I need to import the entire xamarin forms project into my solution in order to apply it?

[BrayanKhosravian]

@stefanbogaard86 this is actually not rly a fix.
However, when u want to test it with that code i posted ull have to clone the xamarin forms repository, select the master branch, sync the branch, add my change, pack a nuget and finally use that nuget for ur production product.

[BrayanKhosravian]

Ok it seems like that i fixed the layouting issue.
But i have found some other issues. At first i taught that these are caused by my fix but i could verify the issues also on the master branch.
The issues i found: (will create separate issues)

• app breaks when adding/inserting/removing items to the collectionview when using .AddRange()

I have sadly no time for the PR as i have to refactor, clean stuff up and im working on other tasks as well. Ill start the PR on Wednesday somewhen this week hopefully.

some preview:

[daltonks]

Running into this problem as well. CollectionView is unusable with ScrollTo when you have differently-sized elements. Please take a look at his PR.

[shults-s]

@BrayanKhosravian
You cannot imagine how much time you saved for me. Thanks a lot!

Your solution (comment from June, 5) overlaps (except for minor nuances) at least 10 more issues: #5455, #8583, #9200, #9365, #9520, #10288, #10625, #10891, #10993, #11511.

For those who need to fix the problem before Brayan's edits get into the Xamarin.Forms release: all you need to do is create 3 classes in your iOS project. One for the renderer and two others for the UICollectionViewController and UICollectionViewDelegateFlowLayout.

CustomCollectionViewRenderer.cs:

[assembly: ExportRenderer(typeof(CollectionView), typeof(CustomCollectionViewRenderer))]

...

internal sealed class CustomCollectionViewRenderer : GroupableItemsViewRenderer<GroupableItemsView,
    CustomItemsViewController>
{
    protected override CustomItemsViewController CreateController(
        GroupableItemsView itemsView,
        ItemsViewLayout itemsLayout
    )
    {
        return new CustomItemsViewController(itemsView, itemsLayout);
    }

    // protected override ItemsViewLayout SelectLayout()
    // {
    //     LinearItemsLayout itemsLayout = new LinearItemsLayout(ItemsLayoutOrientation.Vertical);
    //     return new ListViewLayout(itemsLayout, ItemSizingStrategy.MeasureAllItems);
    // }
}

CustomItemsViewController.cs:

internal sealed class CustomItemsViewController : GroupableItemsViewController<GroupableItemsView>
{
    // protected override Boolean IsHorizontal => false;

    public CustomItemsViewController(GroupableItemsView itemsView, ItemsViewLayout itemsLayout)
        : base(itemsView, itemsLayout)
    {
    }

    protected override UICollectionViewDelegateFlowLayout CreateDelegator()
    {
        return new CustomItemsViewDelegator(ItemsViewLayout, this as CustomItemsViewController);
    }
}

CustomItemsViewDelegator.cs:

internal sealed class CustomItemsViewDelegator : ItemsViewDelegator<GroupableItemsView, CustomItemsViewController>
{
    public CustomItemsViewDelegator(ItemsViewLayout itemsLayout, CustomItemsViewController itemsController)
        : base(itemsLayout, itemsController)
    {
    }

    /// <summary>
    /// Per default this method returns the Estimated size when its not overriden. This method is called before
    /// the rendering process and sizes the cell correctly before it is displayed in the CollectionView.
    /// Calling the base implementation of this method will throw an exception when overriding the method.
    /// </summary>
    public override CGSize GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout,
        NSIndexPath indexPath)
    {
        // CellForItem() is not reliable here because when the cell at indexPath is not visible it will return null.
        UICollectionViewCell cell = collectionView.CellForItem(indexPath);
        if (cell is ItemsViewCell itemsViewCell)
        {
            return itemsViewCell.Measure(); // Get the real cell size.
        }
        else
        {
            return ItemsViewLayout.EstimatedItemSize; // This is basically a fallback when CellForItem() returns null.
        }
    }
}

[libin85]

@BrayanKhosravian @shults-s : I cant thank you guys enough. This issue has been driving me nuts for the past few days. You guys really made my day :)

[FIELDPOINT]

@shults-s @BrayanKhosravian Thank you so much for sharing the fix. Resolved my issue as well.

[tcerdaj]

@BrayanKhosravian
Thank you for your contribution!

[tcerdaj]

[Bug] After @BrayanKhosravian code implementation I got the follow bug: SelectionChanged, and SelectionChangedCommand are not hitting in the backend. Look the events are not implemented in the delegated.

[tcerdaj]

*Chaanging ItemsViewDelegator by SelectableItemsViewDelegator.
*Changing ItemsViewLayout.EstimatedItemSize by ItemsViewLayout.ItemSize .
Solve the issue.

[Reveon]

Does anyone have a solution for Grouped CollectionView? Workaround from @BrayanKhosravian works but it removes Group headers from the CollectionView

[Tommigun1980]

Does anyone have a solution for Grouped CollectionView? Workaround from @BrayanKhosravian works but it removes Group headers from the CollectionView

You may want to try using a ListView (or a bindable StackLayout if you don't have a lot of elements), as they seem to work fine. CollectionView just isn't ready for prime time and it's a shame Xamarin has removed the experimental tag from it and deprecated other components that actually work. This issue is an absolute must fix for CollectionView to be usable and I'd urge the team to take a look at this asap. The number of bug reports this spawns is crazy.

Please note that the ListView does leak memory until #12535 makes its way into a release, I truly hope it will happen soon as right now there is not a single usable collection component in Xamarin.

Edit: Accidentally linked to a wrong pull request, fixed.

[cjsharp01]

@BrayanKhosravian You cannot imagine how much time you saved for me. Thanks a lot!

Your solution (comment from June, 5) overlaps (except for minor nuances) at least 10 more issues: #5455, #8583, #9200, #9365, #9520, #10288, #10625, #10891, #10993, #11511.

For those who need to fix the problem before Brayan's edits get into the Xamarin.Forms release: all you need to do is create 3 classes in your iOS project. One for the renderer and two others for the UICollectionViewController and UICollectionViewDelegateFlowLayout.

CustomCollectionViewRenderer.cs:

[assembly: ExportRenderer(typeof(CollectionView), typeof(CustomCollectionViewRenderer))]

...

internal sealed class CustomCollectionViewRenderer : GroupableItemsViewRenderer<GroupableItemsView,
    CustomItemsViewController>
{
    protected override CustomItemsViewController CreateController(
        GroupableItemsView itemsView,
        ItemsViewLayout itemsLayout
    )
    {
        return new CustomItemsViewController(itemsView, itemsLayout);
    }

    // protected override ItemsViewLayout SelectLayout()
    // {
    //     LinearItemsLayout itemsLayout = new LinearItemsLayout(ItemsLayoutOrientation.Vertical);
    //     return new ListViewLayout(itemsLayout, ItemSizingStrategy.MeasureAllItems);
    // }
}

CustomItemsViewController.cs:

internal sealed class CustomItemsViewController : GroupableItemsViewController<GroupableItemsView>
{
    // protected override Boolean IsHorizontal => false;

    public CustomItemsViewController(GroupableItemsView itemsView, ItemsViewLayout itemsLayout)
        : base(itemsView, itemsLayout)
    {
    }

    protected override UICollectionViewDelegateFlowLayout CreateDelegator()
    {
        return new CustomItemsViewDelegator(ItemsViewLayout, this as CustomItemsViewController);
    }
}

CustomItemsViewDelegator.cs:

internal sealed class CustomItemsViewDelegator : ItemsViewDelegator<GroupableItemsView, CustomItemsViewController>
{
    public CustomItemsViewDelegator(ItemsViewLayout itemsLayout, CustomItemsViewController itemsController)
        : base(itemsLayout, itemsController)
    {
    }

    /// <summary>
    /// Per default this method returns the Estimated size when its not overriden. This method is called before
    /// the rendering process and sizes the cell correctly before it is displayed in the CollectionView.
    /// Calling the base implementation of this method will throw an exception when overriding the method.
    /// </summary>
    public override CGSize GetSizeForItem(UICollectionView collectionView, UICollectionViewLayout layout,
        NSIndexPath indexPath)
    {
        // CellForItem() is not reliable here because when the cell at indexPath is not visible it will return null.
        UICollectionViewCell cell = collectionView.CellForItem(indexPath);
        if (cell is ItemsViewCell itemsViewCell)
        {
            return itemsViewCell.Measure(); // Get the real cell size.
        }
        else
        {
            return ItemsViewLayout.EstimatedItemSize; // This is basically a fallback when CellForItem() returns null.
        }
    }
}

Thank you! That allowed me to workaround my issue (see #11011) - except instead of returning EstimatedItemSize, I found that returning ItemSize worked better for me (EstimatedItemSize was just returning 0)

[ghost]

@BrayanKhosravian You saved my day!
Only 1 issue. The CollectionView SelectionChanged event stopped working for me, but I fixed it by changing the following line

internal sealed class CustomItemsViewDelegator : ItemsViewDelegator<GroupableItemsView, CustomItemsViewController>

to

internal sealed class CustomItemsViewDelegator : GroupableItemsViewDelegator<GroupableItemsView, CustomItemsViewController>