From c5f95cc35374682b9969a4226d3f9cc2bb16b698 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Fri, 15 Mar 2019 05:32:17 -0600 Subject: [PATCH] [iOS] Fix crash when going from empty to having items (#5430) * Fix crash on iOS when going from empty to having items * Temporarily patching EditorRenderer so iOS tests can run --- .../EmptyViewLoadSimulateGallery.xaml.cs | 5 +- .../CollectionViewGalleries/ExampleTemplates.cs | 3 +- .../Tests/CollectionViewUITests.cs | 13 ++++- .../CollectionView/ItemsViewController.cs | 56 +++++++++++++++++++++- .../Renderers/EditorRenderer.cs | 2 +- 5 files changed, 74 insertions(+), 5 deletions(-) diff --git a/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/EmptyViewGalleries/EmptyViewLoadSimulateGallery.xaml.cs b/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/EmptyViewGalleries/EmptyViewLoadSimulateGallery.xaml.cs index 066abd6..839cc8c 100644 --- a/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/EmptyViewGalleries/EmptyViewLoadSimulateGallery.xaml.cs +++ b/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/EmptyViewGalleries/EmptyViewLoadSimulateGallery.xaml.cs @@ -1,4 +1,5 @@ -using System.Threading; +using System.Collections.Generic; +using System.Threading; using Xamarin.Forms.Xaml; namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.EmptyViewGalleries @@ -17,6 +18,8 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries.EmptyViewG new Thread(() => { Thread.Sleep(1000); + Device.BeginInvokeOnMainThread(() => CollectionView.ItemsSource = new List()); + Thread.Sleep(1000); Device.BeginInvokeOnMainThread(() => CollectionView.ItemsSource = _demoFilteredItemSource.Items); }).Start(); } diff --git a/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/ExampleTemplates.cs b/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/ExampleTemplates.cs index e884f5c..6707c68 100644 --- a/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/ExampleTemplates.cs +++ b/Xamarin.Forms.Controls/GalleryPages/CollectionViewGalleries/ExampleTemplates.cs @@ -20,7 +20,8 @@ namespace Xamarin.Forms.Controls.GalleryPages.CollectionViewGalleries { HeightRequest = 100, WidthRequest = 100, HorizontalOptions = LayoutOptions.Center, - VerticalOptions = LayoutOptions.Center + VerticalOptions = LayoutOptions.Center, + AutomationId = "photo" }; image.SetBinding(Image.SourceProperty, new Binding("Image")); diff --git a/Xamarin.Forms.Core.UITests.Shared/Tests/CollectionViewUITests.cs b/Xamarin.Forms.Core.UITests.Shared/Tests/CollectionViewUITests.cs index 31df17e..2edb46c 100644 --- a/Xamarin.Forms.Core.UITests.Shared/Tests/CollectionViewUITests.cs +++ b/Xamarin.Forms.Core.UITests.Shared/Tests/CollectionViewUITests.cs @@ -279,5 +279,16 @@ namespace Xamarin.Forms.Core.UITests } } } - } + + [TestCase("EmptyView", "EmptyView (load simulation)", "photo")] + public void VisitAndCheckItem(string collectionTestName, string subgallery, string item) + { + VisitInitialGallery(collectionTestName); + + App.WaitForElement(t => t.Marked(subgallery)); + App.Tap(t => t.Marked(subgallery)); + + App.WaitForElement(t => t.Marked(item)); + } + } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs index 8b52736..7b9b502 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Foundation; using UIKit; @@ -12,6 +13,7 @@ namespace Xamarin.Forms.Platform.iOS readonly ItemsView _itemsView; ItemsViewLayout _layout; bool _initialConstraintsSet; + bool _safeForReload; bool _wasEmpty; bool _currentBackgroundIsEmptyView; @@ -25,6 +27,10 @@ namespace Xamarin.Forms.Platform.iOS { _itemsView = itemsView; _itemsSource = ItemsSourceFactory.Create(_itemsView.ItemsSource, CollectionView); + + // If we already have data, the UICollectionView will have items and we'll be safe to call + // ReloadData if the ItemsSource changes in the future (see UpdateItemsSource for more). + _safeForReload = _itemsSource?.Count > 0; UpdateLayout(layout); } @@ -113,11 +119,59 @@ namespace Xamarin.Forms.Platform.iOS public virtual void UpdateItemsSource() { - _itemsSource = ItemsSourceFactory.Create(_itemsView.ItemsSource, CollectionView); + if (_safeForReload) + { + UpdateItemsSourceAndReload(); + } + else + { + // Okay, thus far this UICollectionView has never had any items in it. At this point, if + // we set the ItemsSource and try to call ReloadData(), it'll crash. AFAICT this is a bug, but + // until it's fixed (or we can figure out another way to go from empty -> having items), we'll + // have to use this crazy workaround + EmptyCollectionViewReloadWorkaround(); + } + } + + void UpdateItemsSourceAndReload() + { + _itemsSource = ItemsSourceFactory.Create(_itemsView.ItemsSource, CollectionView); CollectionView.ReloadData(); CollectionView.CollectionViewLayout.InvalidateLayout(); } + void EmptyCollectionViewReloadWorkaround() + { + var enumerator = _itemsView.ItemsSource.GetEnumerator(); + + if (!enumerator.MoveNext()) + { + // The source we're updating to is empty, so we can just update as normal; it won't crash + UpdateItemsSourceAndReload(); + } + else + { + // Grab the first item from the new ItemsSource and create a usable source for the UICollectionView + // from that + var firstItem = new List { enumerator.Current }; + _itemsSource = ItemsSourceFactory.Create(firstItem, CollectionView); + + // Insert that item into the UICollectionView + // TODO ezhart When we implement grouping, this will need to be the index of the first actual item + // Which might not be zero,zero if we have empty groups + var indexesToInsert = new NSIndexPath[1] { NSIndexPath.Create(0, 0) }; + + UIView.PerformWithoutAnimation(() => + { + CollectionView.InsertItems(indexesToInsert); + }); + + // Okay, from now on we can just call ReloadData and things will work fine + _safeForReload = true; + UpdateItemsSource(); + } + } + protected virtual void UpdateDefaultCell(DefaultCell cell, NSIndexPath indexPath) { cell.Label.Text = _itemsSource[indexPath.Row].ToString(); diff --git a/Xamarin.Forms.Platform.iOS/Renderers/EditorRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/EditorRenderer.cs index e14d888..5433328 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/EditorRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/EditorRenderer.cs @@ -60,7 +60,7 @@ namespace Xamarin.Forms.Platform.iOS { _placeholderLabel.Text = Element.Placeholder; } - + protected internal override void UpdatePlaceholderColor() { if (Element.PlaceholderColor == Color.Default) -- 2.7.4