From c459b0d76d09ec3165596049c891908db95ef809 Mon Sep 17 00:00:00 2001 From: Jonathan Dick Date: Mon, 15 May 2023 15:54:25 -0400 Subject: [PATCH] CollectionView - Recycle DataTemplates when using template selector (#12011) * [Android] Use different view types per item template Previously we were returning a single view type id for all 'items' in collectionview, which meant if you were using a template selector, and had a variety of returned different DataTemplates, each would share the same recycled item. When a recycled item is reused for a different template, it causes the view for that template to be recreated to be correct for the template selected. This change returns a unique item id per DataTemplate.Type, so that recycled items should always have the correct DataTemplate.Type and not need to be recreated with a different view hierarchy. * [iOS] Distinct Cell reuse id's for different DataTemplates Like on Android, we were using a single cell reuse identifier for iOS "items" regardless of if they had different DataTemplates. If you were using a data template selector, and returned different data templates, whenever a recycled cell's data template didn't match the data template it needed to display for the new/recycled context, the whole view tree of that template would be recreated. This change ensures we have a unique cell reuse id for every different DataTemplate.Type that might be returned by the template selector so that when a cell is recycled it should always be reused for the same Data Template it was created for, and so the view hierarchy won't be recreated. This does require the DetermineCellReuseId to know the indexpath we're getting it for, so a new method was introduced, internal for NET7, protected for NET8+, and in NET8+ the old method is obsoleted. * Switch up some xaml controls in the sample * Expose Id internally to use for android view types * Cache datatemplates for unique item view types * Api changes (overrides) * Improve sample * Remove api txt change that shouldn't have been added * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: E.Z. Hart * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho * Update src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs Co-authored-by: Rui Marinho * Use DataTemplate Id for cellReuseId * Calculate correct position for carousel adapter The `CarouselViewAdapter` loops by basically returning a very large value for item count of its adapter/source which means the recyclerview is going to ask for view types with a position value that does not actually exist. Previously this was ok because we returned only a single item view type ever, however now that we want to return a different type for template selector scenarios where there are potentially multiple template types, we need to override also the `GetItemViewType` and pass in the calculated position in the list properly (as we do in `OnBindViewHolder` here already) since the base `ItemsViewAdapter` class now calls `ItemsSource.GetItem(position)` in its `GetItemViewType()` call. --------- Co-authored-by: Rui Marinho Co-authored-by: E.Z. Hart Co-authored-by: Shane Neuville --- ...VariedSizeDataTemplateSelectorGallery.xaml | 35 +++++++---------- .../DataTemplateSelectorGallery.xaml | 32 +++++---------- .../DataTemplateSelectorGallery.xaml.cs | 2 +- src/Controls/src/Core/DataTemplate.cs | 2 + .../Android/Adapters/CarouselViewAdapter.cs | 32 +++++++++++---- .../Android/Adapters/ItemsViewAdapter.cs | 29 +++++++++++++- .../Items/iOS/CarouselViewController.cs | 5 ++- .../Handlers/Items/iOS/ItemsViewController.cs | 39 ++++++++++++++++++- .../net-android/PublicAPI.Unshipped.txt | 2 + 9 files changed, 125 insertions(+), 53 deletions(-) diff --git a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGalleries/VariedSizeDataTemplateSelectorGallery.xaml b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGalleries/VariedSizeDataTemplateSelectorGallery.xaml index e7db0274bd79..29725db62d20 100644 --- a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGalleries/VariedSizeDataTemplateSelectorGallery.xaml +++ b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGalleries/VariedSizeDataTemplateSelectorGallery.xaml @@ -6,35 +6,35 @@ - - + + - + + - - + + - + + - - + + - + + + - - - - - + diff --git a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml index 533dadb3fe31..d688afea9ef9 100644 --- a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml +++ b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml @@ -7,24 +7,16 @@ - - - - - - + + - - - - - - + + @@ -32,15 +24,11 @@ - - - + - - - + - + - + - - + \ No newline at end of file diff --git a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml.cs b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml.cs index 3eb7438d85ba..129a963f1cde 100644 --- a/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml.cs +++ b/src/Controls/samples/Controls.Sample/Pages/Controls/CollectionViewGalleries/DataTemplateSelectorGallery.xaml.cs @@ -18,7 +18,7 @@ public DataTemplateSelectorGallery() { InitializeComponent(); - _demoFilteredItemSource = new DemoFilteredItemSource(filter: ItemMatches); + _demoFilteredItemSource = new DemoFilteredItemSource(count: 200, filter: ItemMatches); CollectionView.ItemsSource = _demoFilteredItemSource.Items; diff --git a/src/Controls/src/Core/DataTemplate.cs b/src/Controls/src/Core/DataTemplate.cs index 0198b5da3967..de82e4868108 100644 --- a/src/Controls/src/Core/DataTemplate.cs +++ b/src/Controls/src/Core/DataTemplate.cs @@ -45,6 +45,8 @@ public DataTemplate(Func loadTemplate) : base(loadTemplate) string IDataTemplateController.IdString => _idString; + internal int Id => _id; + int IDataTemplateController.Id => _id; /// diff --git a/src/Controls/src/Core/Handlers/Items/Android/Adapters/CarouselViewAdapter.cs b/src/Controls/src/Core/Handlers/Items/Android/Adapters/CarouselViewAdapter.cs index 2c8026d9ab66..96b2c5eef98b 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/Adapters/CarouselViewAdapter.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/Adapters/CarouselViewAdapter.cs @@ -18,17 +18,22 @@ internal CarouselViewAdapter(CarouselView itemsView, Func CarouselView.Loop && !(ItemsSource is EmptySource) && ItemsSource.Count > 0 ? CarouselViewLoopManager.LoopScale : ItemsSource.Count; - public override void OnBindViewHolder(RecyclerView.ViewHolder holder, int position) + public override int GetItemViewType(int position) { - if (CarouselView == null || ItemsSource == null) - return; + int positionInList = GetPositionInList(position); - bool hasItems = ItemsSource != null && ItemsSource.Count > 0; + if (positionInList < 0) + return ItemViewType.TextItem; - if (!hasItems) - return; + return base.GetItemViewType(positionInList); + } + + public override void OnBindViewHolder(RecyclerView.ViewHolder holder, int position) + { + int positionInList = GetPositionInList(position); - int positionInList = (CarouselView.Loop && hasItems) ? position % ItemsSource.Count : position; + if (positionInList < 0) + return; switch (holder) { @@ -40,5 +45,18 @@ public override void OnBindViewHolder(RecyclerView.ViewHolder holder, int positi break; } } + + int GetPositionInList(int position) + { + if (CarouselView == null || ItemsSource == null) + return -1; + + bool hasItems = ItemsSource != null && ItemsSource.Count > 0; + + if (!hasItems) + return -1; + + return (CarouselView.Loop && hasItems) ? position % ItemsSource.Count : position; + } } } diff --git a/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs b/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs index 478e00f44acb..34dfa5981b4c 100644 --- a/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs +++ b/src/Controls/src/Core/Handlers/Items/Android/Adapters/ItemsViewAdapter.cs @@ -18,6 +18,7 @@ public class ItemsViewAdapter : RecyclerView.Adapt bool _disposed; bool _usingItemTemplate = false; + DataTemplateSelector _itemTemplateSelector = null; protected internal ItemsViewAdapter(TItemsView itemsView, Func createItemContentView = null) { @@ -86,16 +87,36 @@ public override RecyclerView.ViewHolder OnCreateViewHolder(ViewGroup parent, int var itemContentView = _createItemContentView.Invoke(ItemsView, context); + // See if our cached templates have a match + if (_viewTypeDataTemplates.TryGetValue(viewType, out var dataTemplate)) + { + return new TemplatedItemViewHolder(itemContentView, dataTemplate, IsSelectionEnabled(parent, viewType)); + } + return new TemplatedItemViewHolder(itemContentView, ItemsView.ItemTemplate, IsSelectionEnabled(parent, viewType)); } public override int ItemCount => ItemsSource.Count; + System.Collections.Generic.Dictionary _viewTypeDataTemplates = new(); + public override int GetItemViewType(int position) { if (_usingItemTemplate) { - return ItemViewType.TemplatedItem; + if (_itemTemplateSelector is null) + return ItemViewType.TemplatedItem; + + var item = ItemsSource?.GetItem(position); + + var template = _itemTemplateSelector?.SelectTemplate(item, ItemsView); + var id = template?.Id ?? ItemViewType.TemplatedItem; + + // Cache the data template for future use + if (!_viewTypeDataTemplates.ContainsKey(id)) + _viewTypeDataTemplates.Add(id, template); + + return id; } // No template, just use the Text view @@ -118,6 +139,11 @@ protected override void Dispose(bool disposing) } } + public override long GetItemId(int position) + { + return position; + } + public virtual int GetPositionForItem(object item) { return ItemsSource.GetPosition(item); @@ -131,6 +157,7 @@ protected virtual void BindTemplatedItemViewHolder(TemplatedItemViewHolder templ void UpdateUsingItemTemplate() { _usingItemTemplate = ItemsView.ItemTemplate != null; + _itemTemplateSelector = ItemsView.ItemTemplate as DataTemplateSelector; } } } diff --git a/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs index 41efcdc56dc8..45e7a7f0b69b 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs @@ -37,7 +37,7 @@ public override UICollectionViewCell GetCell(UICollectionView collectionView, NS if (Carousel?.Loop == true && _carouselViewLoopManager != null) { - var cellAndCorrectedIndex = _carouselViewLoopManager.GetCellAndCorrectIndex(collectionView, indexPath, DetermineCellReuseId()); + var cellAndCorrectedIndex = _carouselViewLoopManager.GetCellAndCorrectIndex(collectionView, indexPath, DetermineCellReuseId(indexPath)); cell = cellAndCorrectedIndex.cell; var correctedIndexPath = NSIndexPath.FromRowSection(cellAndCorrectedIndex.correctedIndex, 0); @@ -135,6 +135,9 @@ public override void UpdateItemsSource() protected override UICollectionViewDelegateFlowLayout CreateDelegator() => new CarouselViewDelegator(ItemsViewLayout, this); +#if NET8_0_OR_GREATER + [Obsolete("Use DetermineCellReuseId(NSIndexPath indexPath) instead.")] +#endif protected override string DetermineCellReuseId() { if (Carousel.ItemTemplate != null) diff --git a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs index 73d34a37c26d..803b3b131ff7 100644 --- a/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs +++ b/src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs @@ -4,6 +4,7 @@ using System.ComponentModel; using CoreGraphics; using Foundation; +using Microsoft.Maui.Controls.Internals; using Microsoft.Maui.Graphics; using ObjCRuntime; using UIKit; @@ -34,6 +35,7 @@ public abstract class ItemsViewController : UICollectionViewControll UIView _emptyUIView; VisualElement _emptyViewFormsElement; Dictionary _measurementCells = new Dictionary(); + List _cellReuseIds = new List(); protected UICollectionViewDelegateFlowLayout Delegator { get; set; } @@ -89,7 +91,7 @@ protected override void Dispose(bool disposing) public override UICollectionViewCell GetCell(UICollectionView collectionView, NSIndexPath indexPath) { - var cell = collectionView.DequeueReusableCell(DetermineCellReuseId(), indexPath) as UICollectionViewCell; + var cell = collectionView.DequeueReusableCell(DetermineCellReuseId(indexPath), indexPath) as UICollectionViewCell; switch (cell) { @@ -339,6 +341,41 @@ protected virtual void CacheCellAttributes(NSIndexPath indexPath, CGSize size) } } +#if NET8_0_OR_GREATER + protected +#else + internal +#endif + virtual string DetermineCellReuseId(NSIndexPath indexPath) + { + if (ItemsView.ItemTemplate != null) + { + var item = ItemsSource[indexPath]; + + var dataTemplate = ItemsView.ItemTemplate.SelectDataTemplate(item, ItemsView); + + var cellOrientation = ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Vertical ? "v" : "h"; + var cellType = ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Vertical ? typeof(VerticalCell) : typeof(HorizontalCell); + + var reuseId = $"_maui_{cellOrientation}_{dataTemplate.Id}"; + + if (!_cellReuseIds.Contains(reuseId)) + { + CollectionView.RegisterClassForCell(cellType, new NSString(reuseId)); + _cellReuseIds.Add(reuseId); + } + + return reuseId; + } + + return ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Horizontal + ? HorizontalDefaultCell.ReuseId + : VerticalDefaultCell.ReuseId; + } + +#if NET8_0_OR_GREATER + [Obsolete("Use DetermineCellReuseId(NSIndexPath indexPath) instead.")] +#endif protected virtual string DetermineCellReuseId() { if (ItemsView.ItemTemplate != null) diff --git a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt index 71ec8e8a20a5..69f909650cd5 100644 --- a/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt +++ b/src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt @@ -3,6 +3,8 @@ Microsoft.Maui.Controls.Border.~Border() -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper) -> void Microsoft.Maui.Controls.Handlers.Compatibility.FrameRenderer.FrameRenderer(Android.Content.Context! context, Microsoft.Maui.IPropertyMapper! mapper, Microsoft.Maui.CommandMapper! commandMapper) -> void +override Microsoft.Maui.Controls.Handlers.Items.CarouselViewAdapter.GetItemViewType(int position) -> int +override Microsoft.Maui.Controls.Handlers.Items.ItemsViewAdapter.GetItemId(int position) -> long Microsoft.Maui.Controls.Handlers.Items.MauiRecyclerView.~MauiRecyclerView() -> void override Microsoft.Maui.Controls.Handlers.Items.ItemsViewHandler.GetDesiredSize(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size override Microsoft.Maui.Controls.Handlers.Items.ItemsViewHandler.PlatformArrange(Microsoft.Maui.Graphics.Rect frame) -> void