From 0ec2206f972f089a2b25a4769bb9d21ce87d8c76 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Fri, 5 Feb 2021 15:55:10 -0700 Subject: [PATCH 1/3] Allow item source updates while CollectionView is hidden; prevent CollectionView updates while CollectionView is hidden; fixes #13126 --- .../Issue13126.cs | 129 ++++++++++++++++++ ...rin.Forms.Controls.Issues.Shared.projitems | 1 + .../CollectionView/CarouselViewController.cs | 12 ++ .../CollectionView/ItemsViewController.cs | 37 ++--- .../CollectionView/ItemsViewRenderer.cs | 10 ++ .../CollectionView/ObservableGroupedSource.cs | 26 +++- .../CollectionView/ObservableItemsSource.cs | 30 ++-- 7 files changed, 196 insertions(+), 49 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs new file mode 100644 index 00000000000..5f0796922fb --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs @@ -0,0 +1,129 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.ComponentModel; +using System.Globalization; +using System.Text; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.Forms.Core.UITests; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Issue(IssueTracker.Github, 13126, "[Bug] Regression: 5.0.0-pre5 often fails to draw dynamically loaded collection view content", PlatformAffected.iOS)] +#if UITEST + [NUnit.Framework.Category(UITestCategories.CollectionView)] +#endif + public class Issue13126 : TestContentPage + { + _13126VM _vm; + const string Success = "Success"; + + protected override void Init() + { + _vm = new _13126VM(); + BindingContext = _vm; + + var collectionView = BindingWithConverter(); + + var grid = new Grid + { + RowDefinitions = new RowDefinitionCollection + { + new RowDefinition() { Height = GridLength.Star }, + } + }; + + grid.Children.Add(collectionView); + + Content = grid; + + _vm.IsBusy = true; + + Device.StartTimer(TimeSpan.FromMilliseconds(300), () => + { + Device.BeginInvokeOnMainThread(() => + { + _vm.Data.Add(Success); + _vm.IsBusy = false; + }); + + return false; + }); + } + + CollectionView BindingWithConverter() + { + var cv = new CollectionView + { + IsVisible = true, + + ItemTemplate = new DataTemplate(() => + { + var label = new Label(); + label.SetBinding(Label.TextProperty, new Binding(".")); + return label; + }) + }; + + cv.SetBinding(CollectionView.ItemsSourceProperty, new Binding("Data")); + cv.SetBinding(VisualElement.IsVisibleProperty, new Binding("IsBusy", converter: new BoolInverter())); + + return cv; + } + + class BoolInverter : IValueConverter + { + public object Convert(object value, Type targetType, object parameter, CultureInfo culture) + { + return !((bool)value); + } + + public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture) + { + throw new NotImplementedException(); + } + } + + class _13126VM : INotifyPropertyChanged + { + private bool _isBusy; + + public bool IsBusy + { + get + { + return _isBusy; + } + + set + { + _isBusy = value; + OnPropertyChanged(nameof(IsBusy)); + } + } + + public ObservableCollection Data { get; } = new ObservableCollection(); + + public event PropertyChangedEventHandler PropertyChanged; + + void OnPropertyChanged(string name) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name)); + } + } + +#if UITEST + [Test] + public void CollectionViewShouldSourceShouldUpdateWhileInvisible() + { + RunningApp.WaitForElement(Success); + } +#endif + } +} diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 84a65ef8817..bdf7a9dc31f 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -12,6 +12,7 @@ + diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs index df300a111b0..a8b34c3b68d 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs @@ -469,6 +469,18 @@ void UpdateVisualStates() _oldViews = newViews; } + + protected internal override void UpdateVisibility() + { + if (ItemsView.IsVisible) + { + CollectionView.Hidden = false; + } + else + { + CollectionView.Hidden = true; + } + } } class CarouselViewLoopManager : IDisposable diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs index 995957b72bd..6226bd615c2 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs @@ -31,8 +31,6 @@ protected ItemsViewController(TItemsView itemsView, ItemsViewLayout layout) : ba { ItemsView = itemsView; ItemsViewLayout = layout; - - ItemsView.PropertyChanged += ItemsViewPropertyChanged; } public void UpdateLayout(ItemsViewLayout newLayout) @@ -62,8 +60,6 @@ protected override void Dispose(bool disposing) if (disposing) { - ItemsView.PropertyChanged -= ItemsViewPropertyChanged; - ItemsSource?.Dispose(); CollectionView.Delegate = null; @@ -100,11 +96,6 @@ public override UICollectionViewCell GetCell(UICollectionView collectionView, NS public override nint GetItemsCount(UICollectionView collectionView, nint section) { - if (!_initialized) - { - return 0; - } - CheckForEmptySource(); return ItemsSource.ItemCountInGroup(section); @@ -172,13 +163,6 @@ void EnsureLayoutInitialized() return; } - if (!ItemsView.IsVisible) - { - // If the CollectionView starts out invisible, we'll get a layout pass with a size of 1,1 and everything will - // go pear-shaped. So until the first time this CollectionView is visible, we do nothing. - return; - } - _initialized = true; ItemsViewLayout.GetPrototype = GetPrototype; @@ -225,11 +209,6 @@ public virtual void UpdateFlowDirection() public override nint NumberOfSections(UICollectionView collectionView) { - if(!_initialized) - { - return 0; - } - CheckForEmptySource(); return ItemsSource.GroupCount; } @@ -610,15 +589,17 @@ internal CGSize GetSizeForItem(NSIndexPath indexPath) return ItemsViewLayout.EstimatedItemSize; } - void ItemsViewPropertyChanged(object sender, PropertyChangedEventArgs changedProperty) + internal protected virtual void UpdateVisibility() { - if (changedProperty.Is(VisualElement.IsVisibleProperty)) + if (ItemsView.IsVisible) { - if (ItemsView.IsVisible) - { - Layout.InvalidateLayout(); - CollectionView.LayoutIfNeeded(); - } + CollectionView.Hidden = false; + Layout.InvalidateLayout(); + CollectionView.LayoutIfNeeded(); + } + else + { + CollectionView.Hidden = true; } } } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewRenderer.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewRenderer.cs index fd740c03e6c..d91585bd871 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewRenderer.cs @@ -71,6 +71,10 @@ protected override void OnElementPropertyChanged(object sender, PropertyChangedE { UpdateFlowDirection(); } + else if (changedProperty.Is(VisualElement.IsVisibleProperty)) + { + UpdateVisibility(); + } } protected abstract ItemsViewLayout SelectLayout(); @@ -101,6 +105,7 @@ protected virtual void SetUpNewElement(TItemsView newElement) UpdateVerticalScrollBarVisibility(); UpdateItemsUpdatingScrollMode(); UpdateFlowDirection(); + UpdateVisibility(); // Listen for ScrollTo requests newElement.ScrollToRequested += ScrollToRequested; @@ -142,6 +147,11 @@ protected virtual void UpdateItemsSource() Controller.UpdateItemsSource(); } + protected virtual void UpdateVisibility() + { + Controller?.UpdateVisibility(); + } + protected abstract TViewController CreateController(TItemsView newElement, ItemsViewLayout layout); NSIndexPath DetermineIndex(ScrollToRequestEventArgs args) diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs index 5ab2bdc6563..534edea04f8 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs @@ -142,7 +142,6 @@ void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) void CollectionChanged(NotifyCollectionChangedEventArgs args) { switch (args.Action) - { case NotifyCollectionChangedAction.Add: Add(args); @@ -168,6 +167,11 @@ void Reload() { ResetGroupTracking(); + if (_collectionView.Hidden) + { + return; + } + _collectionView.ReloadData(); _collectionView.CollectionViewLayout.InvalidateLayout(); } @@ -200,7 +204,7 @@ void Add(NotifyCollectionChangedEventArgs args) ResetGroupTracking(); // Queue up the updates to the UICollectionView - _collectionView.InsertSections(CreateIndexSetFrom(startIndex, count)); + Update(() => _collectionView.InsertSections(CreateIndexSetFrom(startIndex, count))); } void Remove(NotifyCollectionChangedEventArgs args) @@ -229,7 +233,7 @@ void Remove(NotifyCollectionChangedEventArgs args) var count = args.OldItems.Count; // Queue up the updates to the UICollectionView - _collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count)); + Update(() => _collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count))); } void Replace(NotifyCollectionChangedEventArgs args) @@ -243,7 +247,7 @@ void Replace(NotifyCollectionChangedEventArgs args) var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _groupSource.IndexOf(args.NewItems[0]); // We are replacing one set of items with a set of equal size; we can do a simple item range update - _collectionView.ReloadSections(CreateIndexSetFrom(startIndex, newCount)); + Update(() => _collectionView.ReloadSections(CreateIndexSetFrom(startIndex, newCount))); return; } @@ -261,14 +265,14 @@ void Move(NotifyCollectionChangedEventArgs args) if (count == 1) { // For a single item, we can use MoveSection and get the animation - _collectionView.MoveSection(args.OldStartingIndex, args.NewStartingIndex); + Update(() => _collectionView.MoveSection(args.OldStartingIndex, args.NewStartingIndex)); return; } var start = Math.Min(args.OldStartingIndex, args.NewStartingIndex); var end = Math.Max(args.OldStartingIndex, args.NewStartingIndex) + count; - _collectionView.ReloadSections(CreateIndexSetFrom(start, end)); + Update(() => _collectionView.ReloadSections(CreateIndexSetFrom(start, end))); } int GetGroupCount(int groupIndex) @@ -343,5 +347,15 @@ bool ReloadRequired() return NotLoadedYet() || _collectionView.NumberOfSections() == 0; } + + void Update(Action update) + { + if (_collectionView.Hidden) + { + return; + } + + update(); + } } } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs index 596d93a4ea6..fc84c2a2961 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs @@ -111,12 +111,6 @@ void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) void CollectionChanged(NotifyCollectionChangedEventArgs args) { - if (CollectionView.NumberOfSections() == 0) - { - // The CollectionView isn't fully initialized yet - return; - } - // Force UICollectionView to get the internal accounting straight CollectionView.NumberOfItemsInSection(_section); @@ -144,6 +138,11 @@ void CollectionChanged(NotifyCollectionChangedEventArgs args) void Reload() { + if (CollectionView.Hidden) + { + return; + } + var args = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset); Count = ItemsCount(); @@ -199,9 +198,8 @@ void Replace(NotifyCollectionChangedEventArgs args) var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]); // We are replacing one set of items with a set of equal size; we can do a simple item range update - OnCollectionViewUpdating(args); - CollectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount)); - OnCollectionViewUpdated(args); + + Update(() => CollectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount)), args); return; } @@ -220,18 +218,14 @@ void Move(NotifyCollectionChangedEventArgs args) var oldPath = NSIndexPath.Create(_section, args.OldStartingIndex); var newPath = NSIndexPath.Create(_section, args.NewStartingIndex); - OnCollectionViewUpdating(args); - CollectionView.MoveItem(oldPath, newPath); - OnCollectionViewUpdated(args); + Update(() => CollectionView.MoveItem(oldPath, newPath), args); return; } var start = Math.Min(args.OldStartingIndex, args.NewStartingIndex); var end = Math.Max(args.OldStartingIndex, args.NewStartingIndex) + count; - OnCollectionViewUpdating(args); - CollectionView.ReloadItems(CreateIndexesFrom(start, end)); - OnCollectionViewUpdated(args); + Update(() => CollectionView.ReloadItems(CreateIndexesFrom(start, end)), args); } internal int ItemsCount() @@ -276,8 +270,14 @@ internal int IndexOf(object item) return -1; } + void Update(Action update, NotifyCollectionChangedEventArgs args) { + if (CollectionView.Hidden) + { + return; + } + OnCollectionViewUpdating(args); update(); OnCollectionViewUpdated(args); From 9a8849b7583831fe6cf556efd99e73cb8d863ce7 Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Sat, 6 Feb 2021 17:37:31 -0700 Subject: [PATCH 2/3] Fix bug where measurement cell content is applied to the wrong cell Only force layout on transition from invisible to visible Move cell size cache to layout and clear cache on size change (e.g., rotation) Use ItemsView size for constraints when possible; --- .../CollectionView/ItemsViewController.cs | 64 +++++++++++++------ .../CollectionView/ItemsViewLayout.cs | 27 ++++++++ 2 files changed, 72 insertions(+), 19 deletions(-) diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs index 6226bd615c2..076b29176b7 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs @@ -22,8 +22,7 @@ public abstract class ItemsViewController : UICollectionViewControll UIView _emptyUIView; VisualElement _emptyViewFormsElement; - Dictionary _measurementCells = new Dictionary(); - Dictionary _cellSizeCache = new Dictionary(); + Dictionary _measurementCells = new Dictionary(); protected UICollectionViewDelegateFlowLayout Delegator { get; set; } @@ -110,7 +109,7 @@ void CheckForEmptySource() if (_isEmpty) { _measurementCells.Clear(); - _cellSizeCache.Clear(); + ItemsViewLayout?.ClearCellSizeCache(); } if (wasEmpty != _isEmpty) @@ -145,15 +144,35 @@ public override void ViewDidLoad() } RegisterViewTypes(); + + EnsureLayoutInitialized(); + } + + public override void ViewWillAppear(bool animated) + { + base.ViewWillAppear(animated); + ConstrainToItemsView(); } public override void ViewWillLayoutSubviews() { + ConstrainToItemsView(); base.ViewWillLayoutSubviews(); + LayoutEmptyView(); + } - EnsureLayoutInitialized(); + void ConstrainToItemsView() + { + var itemsViewWidth = ItemsView.Width; + var itemsViewHeight = ItemsView.Height; - LayoutEmptyView(); + if (itemsViewHeight < 0 || itemsViewWidth < 0) + { + ItemsViewLayout.UpdateConstraints(CollectionView.Bounds.Size); + return; + } + + ItemsViewLayout.UpdateConstraints(new CGSize(itemsViewWidth, itemsViewHeight)); } void EnsureLayoutInitialized() @@ -189,7 +208,7 @@ protected virtual IItemsViewSource CreateItemsViewSource() public virtual void UpdateItemsSource() { _measurementCells.Clear(); - _cellSizeCache.Clear(); + ItemsViewLayout?.ClearCellSizeCache(); ItemsSource = CreateItemsViewSource(); CollectionView.ReloadData(); CollectionView.CollectionViewLayout.InvalidateLayout(); @@ -228,10 +247,12 @@ protected virtual void UpdateTemplatedCell(TemplatedCell cell, NSIndexPath index cell.ContentSizeChanged -= CellContentSizeChanged; cell.LayoutAttributesChanged -= CellLayoutAttributesChanged; + var bindingContext = ItemsSource[indexPath]; + // If we've already created a cell for this index path (for measurement), re-use the content - if (_measurementCells.TryGetValue(indexPath, out TemplatedCell measurementCell)) + if (_measurementCells.TryGetValue(bindingContext, out TemplatedCell measurementCell)) { - _measurementCells.Remove(indexPath); + _measurementCells.Remove(bindingContext); measurementCell.ContentSizeChanged -= CellContentSizeChanged; measurementCell.LayoutAttributesChanged -= CellLayoutAttributesChanged; cell.UseContent(measurementCell); @@ -295,7 +316,7 @@ protected virtual void CacheCellAttributes(NSIndexPath indexPath, CGSize size) var item = ItemsSource[indexPath]; if (item != null) { - _cellSizeCache[item] = size; + ItemsViewLayout.CacheCellSize(item, size); } } @@ -547,14 +568,16 @@ public UICollectionViewCell CreateMeasurementCell(NSIndexPath indexPath) { var frame = new CGRect(0, 0, ItemsViewLayout.EstimatedItemSize.Width, ItemsViewLayout.EstimatedItemSize.Height); + DefaultCell cell; if (ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Horizontal) { - var cell1 = new HorizontalDefaultCell(frame); - UpdateDefaultCell(cell1, indexPath); - return cell1; + cell = new HorizontalDefaultCell(frame); } - - var cell = new VerticalDefaultCell(frame); + else + { + cell = new VerticalDefaultCell(frame); + } + UpdateDefaultCell(cell, indexPath); return cell; } @@ -564,7 +587,7 @@ public UICollectionViewCell CreateMeasurementCell(NSIndexPath indexPath) UpdateTemplatedCell(templatedCell, indexPath); // Keep this cell around, we can transfer the contents to the actual cell when the UICollectionView creates it - _measurementCells[indexPath] = templatedCell; + _measurementCells[ItemsSource[indexPath]] = templatedCell; return templatedCell; } @@ -580,7 +603,7 @@ internal CGSize GetSizeForItem(NSIndexPath indexPath) { var item = ItemsSource[indexPath]; - if (item != null && _cellSizeCache.TryGetValue(item, out CGSize size)) + if (item != null && ItemsViewLayout.TryGetCachedCellSize(item, out CGSize size)) { return size; } @@ -593,9 +616,12 @@ internal protected virtual void UpdateVisibility() { if (ItemsView.IsVisible) { - CollectionView.Hidden = false; - Layout.InvalidateLayout(); - CollectionView.LayoutIfNeeded(); + if (CollectionView.Hidden) + { + CollectionView.Hidden = false; + Layout.InvalidateLayout(); + CollectionView.LayoutIfNeeded(); + } } else { diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs index 9dd62c5bcbd..ecac7073591 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using CoreGraphics; using Foundation; @@ -16,6 +17,8 @@ public abstract class ItemsViewLayout : UICollectionViewFlowLayout CGSize _adjustmentSize1; CGSize _currentSize; + Dictionary _cellSizeCache = new Dictionary(); + public ItemsUpdatingScrollMode ItemsUpdatingScrollMode { get; set; } public nfloat ConstrainedDimension { get; set; } @@ -90,6 +93,8 @@ internal virtual void UpdateConstraints(CGSize size) return; } + ClearCellSizeCache(); + _currentSize = size; var newSize = new CGSize(Math.Floor(size.Width), Math.Floor(size.Height)); @@ -561,5 +566,27 @@ public override bool ShouldInvalidateLayoutForBoundsChange(CGRect newBounds) return true; } + + internal bool TryGetCachedCellSize(object item, out CGSize size) + { + if (_cellSizeCache.TryGetValue(item, out CGSize internalSize)) + { + size = internalSize; + return true; + } + + size = CGSize.Empty; + return false; + } + + internal void CacheCellSize(object item, CGSize size) + { + _cellSizeCache[item] = size; + } + + internal void ClearCellSizeCache() + { + _cellSizeCache.Clear(); + } } } From c9fb5ce6e0e02e6595d6c3b8ce0f78c098a3fffb Mon Sep 17 00:00:00 2001 From: "E.Z. Hart" Date: Mon, 8 Feb 2021 10:15:40 -0700 Subject: [PATCH 3/3] Add test for Reset situation; fix bug with Reset before CollectionView is visible; --- .../Issue13126.cs | 82 +++++++++++++++---- .../Issue13126_2.cs | 74 +++++++++++++++++ ...rin.Forms.Controls.Issues.Shared.projitems | 1 + .../CollectionView/ObservableGroupedSource.cs | 5 -- .../CollectionView/ObservableItemsSource.cs | 5 -- 5 files changed, 142 insertions(+), 25 deletions(-) create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126_2.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs index 5f0796922fb..efb0cbaab8b 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; +using System.Collections.Specialized; using System.ComponentModel; using System.Globalization; using System.Text; +using System.Threading.Tasks; using Xamarin.Forms.CustomAttributes; using Xamarin.Forms.Internals; @@ -26,9 +28,6 @@ public class Issue13126 : TestContentPage protected override void Init() { - _vm = new _13126VM(); - BindingContext = _vm; - var collectionView = BindingWithConverter(); var grid = new Grid @@ -43,21 +42,23 @@ protected override void Init() Content = grid; + _vm = new _13126VM(); + BindingContext = _vm; + } + + protected async override void OnParentSet() + { + base.OnParentSet(); _vm.IsBusy = true; - Device.StartTimer(TimeSpan.FromMilliseconds(300), () => - { - Device.BeginInvokeOnMainThread(() => - { - _vm.Data.Add(Success); - _vm.IsBusy = false; - }); + await Task.Delay(1000); - return false; - }); + _vm.Data.Add(Success); + + _vm.IsBusy = false; } - CollectionView BindingWithConverter() + internal static CollectionView BindingWithConverter() { var cv = new CollectionView { @@ -71,6 +72,8 @@ CollectionView BindingWithConverter() }) }; + cv.EmptyView = new Label { Text = "Should not see me" }; + cv.SetBinding(CollectionView.ItemsSourceProperty, new Binding("Data")); cv.SetBinding(VisualElement.IsVisibleProperty, new Binding("IsBusy", converter: new BoolInverter())); @@ -90,7 +93,7 @@ public object ConvertBack(object value, Type targetType, object parameter, Cultu } } - class _13126VM : INotifyPropertyChanged + internal class _13126VM : INotifyPropertyChanged { private bool _isBusy; @@ -108,7 +111,7 @@ public bool IsBusy } } - public ObservableCollection Data { get; } = new ObservableCollection(); + public OptimizedObservableCollection Data { get; } = new OptimizedObservableCollection(); public event PropertyChangedEventHandler PropertyChanged; @@ -118,6 +121,55 @@ void OnPropertyChanged(string name) } } + internal class OptimizedObservableCollection : ObservableCollection + { + bool _shouldRaiseNotifications = true; + + public OptimizedObservableCollection() + { + } + + public OptimizedObservableCollection(IEnumerable collection) + : base(collection) + { + } + + public IDisposable BeginMassUpdate() + { + return new MassUpdater(this); + } + + protected override void OnCollectionChanged(NotifyCollectionChangedEventArgs e) + { + if (_shouldRaiseNotifications) + base.OnCollectionChanged(e); + } + + protected override void OnPropertyChanged(PropertyChangedEventArgs e) + { + if (_shouldRaiseNotifications) + base.OnPropertyChanged(e); + } + + class MassUpdater : IDisposable + { + readonly OptimizedObservableCollection parent; + public MassUpdater(OptimizedObservableCollection parent) + { + this.parent = parent; + parent._shouldRaiseNotifications = false; + } + + public void Dispose() + { + parent._shouldRaiseNotifications = true; + parent.OnPropertyChanged(new PropertyChangedEventArgs("Count")); + parent.OnPropertyChanged(new PropertyChangedEventArgs("Item[]")); + parent.OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)); + } + } + } + #if UITEST [Test] public void CollectionViewShouldSourceShouldUpdateWhileInvisible() diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126_2.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126_2.cs new file mode 100644 index 00000000000..263d40cdbad --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13126_2.cs @@ -0,0 +1,74 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Collections.Specialized; +using System.ComponentModel; +using System.Globalization; +using System.Text; +using System.Threading.Tasks; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; +using static Xamarin.Forms.Controls.Issues.Issue13126; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +using Xamarin.Forms.Core.UITests; +#endif + +namespace Xamarin.Forms.Controls.Issues +{ + [Issue(IssueTracker.Github, 13126, "[Bug] Regression: 5.0.0-pre5 often fails to draw dynamically loaded collection view content", + PlatformAffected.iOS, issueTestNumber:1)] +#if UITEST + [NUnit.Framework.Category(UITestCategories.CollectionView)] +#endif + public class Issue13126_2 : TestContentPage + { + _13126VM _vm; + const string Success = "Success"; + + protected override void Init() + { + var collectionView = BindingWithConverter(); + + var grid = new Grid + { + RowDefinitions = new RowDefinitionCollection + { + new RowDefinition() { Height = GridLength.Star }, + } + }; + + grid.Children.Add(collectionView); + + Content = grid; + + _vm = new _13126VM(); + BindingContext = _vm; + } + + protected async override void OnParentSet() + { + base.OnParentSet(); + _vm.IsBusy = true; + + await Task.Delay(1000); + + using (_vm.Data.BeginMassUpdate()) + { + _vm.Data.Add(Success); + } + + _vm.IsBusy = false; + } + +#if UITEST + [Test] + public void CollectionViewShouldSourceShouldResetWhileInvisible() + { + RunningApp.WaitForElement(Success); + } +#endif + } +} diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index bdf7a9dc31f..c3239ce5963 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -13,6 +13,7 @@ + diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs index 534edea04f8..0358e6b2671 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs @@ -167,11 +167,6 @@ void Reload() { ResetGroupTracking(); - if (_collectionView.Hidden) - { - return; - } - _collectionView.ReloadData(); _collectionView.CollectionViewLayout.InvalidateLayout(); } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs index fc84c2a2961..bcf85a9a912 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs @@ -138,11 +138,6 @@ void CollectionChanged(NotifyCollectionChangedEventArgs args) void Reload() { - if (CollectionView.Hidden) - { - return; - } - var args = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset); Count = ItemsCount();