[iOS] Fixes crash in CollectionViews when using DataTemplateSelector as a DataTemplat...
authorMario van Zeist <m.vanzeist@home.nl>
Wed, 4 Dec 2019 12:52:43 +0000 (13:52 +0100)
committerStephane Delcroix <stephane@delcroix.org>
Wed, 4 Dec 2019 12:52:43 +0000 (13:52 +0100)
- fixes #8647

* Added repro and partial fix

* Removed some code in Templated cell to prevent an extension method being called twice.
Modified ItemsViewController to call different bind function. (Removed older one)
Added checks for Header/Footer in groupableItemsViewController.
Some code cleanup

* Removed unittest as I could not get it to run Due to missing Provisioningprofile

* Revert unintended change

* Cleanup some code

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8647.cs [new file with mode: 0644]
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
Xamarin.Forms.Platform.iOS/CollectionView/GroupableItemsViewController.cs
Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs
Xamarin.Forms.Platform.iOS/CollectionView/TemplatedCell.cs

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8647.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8647.cs
new file mode 100644 (file)
index 0000000..757108a
--- /dev/null
@@ -0,0 +1,74 @@
+using System;
+using System.Collections.Generic;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+
+#if UITEST
+using Xamarin.Forms.Core.UITests;
+using Xamarin.UITest;
+using NUnit.Framework;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+
+#if UITEST
+       [Category(UITestCategories.CollectionView)]
+#endif
+
+       [Preserve(AllMembers = true)]
+       [Issue(IssueTracker.Github, 8647, "Crash on iOS when using DataTemplateSelector as a GroupingHeaderTemplate in a CollectionView", PlatformAffected.iOS)]
+       public class Issue8647 : TestContentPage
+       {
+               const string ButtonId = "ButtonId";
+               CollectionView _view;
+
+               protected override void Init()
+               {
+                       var layout = new StackLayout { Margin = 50 };
+                       layout.Children.Add(new Button { HorizontalOptions = LayoutOptions.Center, Text = "Fill Data", AutomationId = ButtonId, Command = new Command(_ => FillData()) });
+                       _view = new CollectionView
+                       {
+                               IsGrouped = true,
+                               GroupHeaderTemplate = new DummySelector(),
+                               ItemTemplate = new DataTemplate(() =>
+                               {
+                                       var item = new Label();
+                                       item.SetBinding(Label.TextProperty, nameof(Item.Value));
+                                       return item;
+                               })
+                       };
+                       layout.Children.Add(_view);
+                       Content = layout;
+               }
+
+               void FillData()
+               {
+                       if (_view.ItemsSource == null)
+                       {
+                               _view.ItemsSource = new List<List<Item>>
+                               {
+                                       new List<Item> { "1", "2", "3" },
+                                       new List<Item> { "4", "5" }
+                               };
+                       }
+               }
+
+               class DummySelector : DataTemplateSelector
+               {
+                       protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
+                       {
+
+                               return new DataTemplate(() => new Label { Text = "GroupHeader" });
+                       }
+               }
+
+               class Item
+               {
+                       public string Value { get; set; }
+
+                       public static implicit operator Item(string value) => new Item { Value = value };
+               }
+       }
+}
index ed771f2..83e27ab 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)Issue7249.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8200.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8417.xaml.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue8647.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue7510.cs" />
   </ItemGroup>
   <ItemGroup>
index ae416e5..3fc91e7 100644 (file)
@@ -110,12 +110,9 @@ namespace Xamarin.Forms.Platform.iOS
 
                string DetermineViewReuseId(NSString elementKind)
                {
-                       if (elementKind == UICollectionElementKindSectionKey.Header)
-                       {
-                               return DetermineViewReuseId(ItemsView.GroupHeaderTemplate);
-                       }
-
-                       return DetermineViewReuseId(ItemsView.GroupFooterTemplate);
+                       return DetermineViewReuseId(elementKind == UICollectionElementKindSectionKey.Header 
+                               ? ItemsView.GroupHeaderTemplate 
+                               : ItemsView.GroupFooterTemplate);
                }
 
                string DetermineViewReuseId(DataTemplate template)
@@ -135,34 +132,31 @@ namespace Xamarin.Forms.Platform.iOS
 
                internal CGSize GetReferenceSizeForHeader(UICollectionView collectionView, UICollectionViewLayout layout, nint section)
                {
-                       if (!_isGrouped)
-                       {
-                               return CGSize.Empty;
-                       }
-
                        // Currently we explicitly measure all of the headers/footers 
                        // Long-term, we might want to look at performance hints (similar to ItemSizingStrategy) for 
                        // headers/footers (if the dev knows for sure they'll all the be the same size)
-
-                       var cell = GetViewForSupplementaryElement(collectionView, UICollectionElementKindSectionKey.Header, 
-                               NSIndexPath.FromItemSection(0, section)) as ItemsViewCell;
-
-                       return cell.Measure();
+                       return GetReferenceSizeForheaderOrFooter(collectionView, ItemsView.GroupHeaderTemplate, UICollectionElementKindSectionKey.Header, section);
                }
 
                internal CGSize GetReferenceSizeForFooter(UICollectionView collectionView, UICollectionViewLayout layout, nint section)
                {
-                       if (!_isGrouped)
+                       return GetReferenceSizeForheaderOrFooter(collectionView, ItemsView.GroupFooterTemplate, UICollectionElementKindSectionKey.Footer, section);
+               }
+
+               internal CGSize GetReferenceSizeForheaderOrFooter(UICollectionView collectionView,DataTemplate template, NSString elementKind, nint section)
+               {
+                       if (!_isGrouped || template == null)
                        {
                                return CGSize.Empty;
                        }
 
-                       var cell = GetViewForSupplementaryElement(collectionView, UICollectionElementKindSectionKey.Footer, 
+                       var cell = GetViewForSupplementaryElement(collectionView, elementKind,
                                NSIndexPath.FromItemSection(0, section)) as ItemsViewCell;
 
                        return cell.Measure();
                }
 
+
                internal void SetScrollAnimationEndedCallback(Action callback)
                {
                        _scrollAnimationEndedCallback = callback;
index 7f3d398..7e4fd6c 100644 (file)
@@ -196,7 +196,7 @@ namespace Xamarin.Forms.Platform.iOS
                {
                        cell.ContentSizeChanged -= CellContentSizeChanged;
 
-                       cell.Bind(ItemsView, ItemsSource[indexPath]);
+                       cell.Bind(ItemsView.ItemTemplate, ItemsSource[indexPath], ItemsView);
 
                        cell.ContentSizeChanged += CellContentSizeChanged;
 
index f215e60..35065f2 100644 (file)
@@ -64,21 +64,14 @@ namespace Xamarin.Forms.Platform.iOS
                        return preferredAttributes;
                }
 
-               public void Bind(ItemsView itemsView, object bindingContext)
-               {
-                       var template = itemsView.ItemTemplate;
-
-                       // Run this through the extension method in case it's really a DataTemplateSelector
-                       template = template.SelectDataTemplate(bindingContext, itemsView);
-
-                       Bind(template, bindingContext, itemsView);
-               }
-
                public void Bind(DataTemplate template, object bindingContext, ItemsView itemsView)
                {
                        var oldElement = VisualElementRenderer?.Element;
 
-                       if (template != _currentTemplate)
+                       // Run this through the extension method in case it's really a DataTemplateSelector
+                       var itemTemplate = template.SelectDataTemplate(bindingContext, itemsView);
+
+                       if (itemTemplate != _currentTemplate)
                        {
                                // Remove the old view, if it exists
                                if (oldElement != null)
@@ -90,7 +83,7 @@ namespace Xamarin.Forms.Platform.iOS
                                }
 
                                // Create the content and renderer for the view 
-                               var view = template.CreateContent() as View;
+                               var view = itemTemplate.CreateContent() as View;
                                var renderer = TemplateHelpers.CreateRenderer(view);
                                SetRenderer(renderer);
                        }
@@ -101,7 +94,7 @@ namespace Xamarin.Forms.Platform.iOS
                        if (currentElement != null)
                                currentElement.BindingContext = bindingContext;
 
-                       if (template != _currentTemplate)
+                       if (itemTemplate != _currentTemplate)
                        {
                                // And make the Element a "child" of the ItemsView
                                // We deliberately do this _after_ setting the binding context for the new element;
@@ -110,7 +103,7 @@ namespace Xamarin.Forms.Platform.iOS
                                itemsView.AddLogicalChild(currentElement);
                        }
 
-                       _currentTemplate = template;
+                       _currentTemplate = itemTemplate;
                }
 
                void SetRenderer(IVisualElementRenderer renderer)