-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Bug] [iOS] CollectionView - Invalid Height Measure when using different datatemplates #10842
Comments
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 thanks for mentioning the issues. ill add them to my issue report |
@liveinvarun wait did you actually say |
@BrayanKhosravian we already have the height issue raised as bug [Xamarin Forms CollectionView ]- |
Ok i spend a few days to investigate the issue and and i want to share what i found out. It seems like that the auto-sizing does not work properly when using templates with different heights/widths. 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. 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. 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 Ill add the links which i saved during research. https://docs.microsoft.com/en-us/xamarin/ios/user-interface/controls/uicollectionview |
OK it seems like that i found a way to improve the layout/measure issue a little bit. Well i finally found the root cause of the bug and i will try to explain it as good as possible. Information needed before i explain the issue:
Explaining the issue:
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:
My "fix" which improved the behavior: I override a method called The issue which is still present with my "fix":
The code i added for the fix: This is located in /// <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
} |
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? |
@stefanbogaard86 this is actually not rly a fix. |
Ok it seems like that i fixed the layouting issue.
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 |
Running into this problem as well. CollectionView is unusable with ScrollTo when you have differently-sized elements. Please take a look at his PR. |
@BrayanKhosravian 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 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.
}
}
} |
@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 :) |
@shults-s @BrayanKhosravian Thank you so much for sharing the fix. Resolved my issue as well. |
@BrayanKhosravian |
[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. |
*Chaanging ItemsViewDelegator by SelectableItemsViewDelegator. |
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. |
#10625 * Cache template sizes for use as estimated item sizes; fixes #10842 * Restore test items * Add clarifying comments * Cache cell measurements * Correct caching index for Carousel * Fix caching exception during item removal; Fix empty view switching overlap; * Update size when reusing Forms element on dequeued cell
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) |
@BrayanKhosravian You saved my day! internal sealed class CustomItemsViewDelegator : ItemsViewDelegator<GroupableItemsView, CustomItemsViewController> to internal sealed class CustomItemsViewDelegator : GroupableItemsViewDelegator<GroupableItemsView, CustomItemsViewController> |
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.
More related: #8583 #9520 Possibly Related: #5455 #7788 #10625
Steps to Reproduce
Expected Behavior
Actual Behavior
Basic Information
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:
the code behind:
Workaround
none
The text was updated successfully, but these errors were encountered: