From e5138c8c61526e4946431e643b0a12c98896cf4a Mon Sep 17 00:00:00 2001 From: Rui Marinho Date: Thu, 3 Dec 2020 12:54:14 +0000 Subject: [PATCH] [iOS] Fix when removing current last Item on CarouselView (#12837) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [iOS] Fix when removing current last Item on CarouselView * [Uitests] Try fix test for 12574 * [iOS] Fix scroll when removing item on CarouselView * [iOS] Use CollectionView.PerformBatchUpdates to scroll to item after CollectionView reloads it's items * [iOS] Set Reload next to Scroll to better understand the logic between the 2 * [iOS] Fix for reload items and wait for iOS12 and iOS11 * Provide pre and post update events for CarouselView to do its bookkeeping * Remove animation after item removal * [Controls] Fix sample 12574 * [UITests] Enable test * Remove performBatchUpdates calls to prevent data disparity; * Remove async stuff from ObservableGroupedSource * Fix reload race condition with deletion/insertion of groups * Reset current item and position when ItemsSource resets * Update ObservableSource count before invoking Carousel's changed handler; Reset position/current item on ItemsSource update Don't update position until drag is released * Clean up unnecessary ReloadRequired stuff * Handle UICollectionView internal accounting edge cases Co-authored-by: E.Z. Hart --- .../Issue10300.cs | 5 +- .../Issue12574.cs | 57 +++++-- .../Tests/CarouselViewUITests.cs | 147 +++++++++-------- .../ObservableItemsSourceTests.cs | 62 ++++++++ ...amarin.Forms.Platform.iOS.UnitTests.csproj | 3 +- .../CollectionView/CarouselViewController.cs | 67 +++++--- .../CollectionView/IndexPathHelpers.cs | 36 +++++ .../CollectionView/ItemsViewController.cs | 28 +++- .../CollectionView/ItemsViewLayout.cs | 9 +- .../LoopObservableItemsSource.cs | 20 +-- .../CollectionView/ObservableGroupedSource.cs | 66 +++----- .../CollectionView/ObservableItemsSource.cs | 148 +++++++----------- .../Xamarin.Forms.Platform.iOS.csproj | 1 + 13 files changed, 388 insertions(+), 261 deletions(-) create mode 100644 Xamarin.Forms.Platform.iOS.UnitTests/ObservableItemsSourceTests.cs create mode 100644 Xamarin.Forms.Platform.iOS/CollectionView/IndexPathHelpers.cs diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue10300.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue10300.cs index 7ff7b61cfb0..11fc5506b1d 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue10300.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue10300.cs @@ -103,7 +103,7 @@ protected override void Init() Grid.SetRow(btn, 1); Grid.SetColumn(btn, 0); - btnAdd.Clicked += OnAddlicked; + btnAdd.Clicked += OnAddClicked; Grid.SetRow(btnAdd, 1); Grid.SetColumn(btnAdd, 1); @@ -129,6 +129,7 @@ void Callback(Page page) var index = Items.IndexOf(carousel.CurrentItem as ModelIssue10300); System.Diagnostics.Debug.WriteLine($"Delete {index}"); Items.RemoveAt(index); + MessagingCenter.Instance.Unsubscribe(this, "Delete"); } public ObservableCollection Items { get; set; } @@ -139,7 +140,7 @@ async void OnDeleteClicked(object sender, EventArgs e) await Navigation.PushModalAsync(new ModalPage()); } - void OnAddlicked(object sender, EventArgs e) + void OnAddClicked(object sender, EventArgs e) { Items.Insert(0, new ModelIssue10300("0", Color.PaleGreen)); } diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue12574.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue12574.cs index 491ae99acd7..62cf075f523 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue12574.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue12574.cs @@ -25,8 +25,10 @@ public class Issue12574 : TestContentPage ViewModelIssue12574 viewModel; CarouselView _carouselView; Button _btn; + Button _btn2; string carouselAutomationId = "carouselView"; string btnRemoveAutomationId = "btnRemove"; + string btnRemoveAllAutomationId = "btnRemoveAll"; protected override void Init() { @@ -35,8 +37,15 @@ protected override void Init() Text = "Remove Last", AutomationId = btnRemoveAutomationId }; - _btn.SetBinding(Button.CommandProperty, "RemoveItemsCommand"); - // Initialize ui here instead of ctor + _btn.SetBinding(Button.CommandProperty, "RemoveLastItemCommand"); + + _btn2 = new Button + { + Text = "Remove All", + AutomationId = btnRemoveAllAutomationId + }; + _btn2.SetBinding(Button.CommandProperty, "RemoveAllItemsCommand"); + _carouselView = new CarouselView { AutomationId = carouselAutomationId, @@ -64,9 +73,12 @@ protected override void Init() var layout = new Grid(); layout.RowDefinitions.Add(new RowDefinition { Height = 100 }); + layout.RowDefinitions.Add(new RowDefinition { Height = 100 }); layout.RowDefinitions.Add(new RowDefinition()); - Grid.SetRow(_carouselView, 1); + Grid.SetRow(_btn2, 1); + Grid.SetRow(_carouselView, 2); layout.Children.Add(_btn); + layout.Children.Add(_btn2); layout.Children.Add(_carouselView); BindingContext = viewModel = new ViewModelIssue12574(); @@ -81,7 +93,6 @@ protected override void OnAppearing() #if UITEST [Test] - [Ignore("Ignore while fix is not ready")] public void Issue12574Test() { RunningApp.WaitForElement("0 item"); @@ -102,31 +113,58 @@ public void Issue12574Test() RunningApp.WaitForElement("1 item"); rightX = rect.X + rect.Width - 1; - RunningApp.DragCoordinates(centerX, rect.CenterY, rightX, rect.CenterY); + RunningApp.DragCoordinates(rect.X, rect.CenterY, rightX, rect.CenterY); RunningApp.WaitForElement("0 item"); } + + [Test] + public void RemoveItemsQuickly() + { + RunningApp.WaitForElement("0 item"); + RunningApp.Tap(btnRemoveAllAutomationId); + + // If we haven't crashed, then the other button should be here + RunningApp.WaitForElement(btnRemoveAutomationId); + } #endif } - [Preserve(AllMembers = true)] class ViewModelIssue12574 : BaseViewModel1 { public ObservableCollection Items { get; set; } public Command LoadItemsCommand { get; set; } - public Command RemoveItemsCommand { get; set; } + public Command RemoveAllItemsCommand { get; set; } + public Command RemoveLastItemCommand { get; set; } public ViewModelIssue12574() { Title = "CarouselView Looping"; Items = new ObservableCollection(); LoadItemsCommand = new Command(() => ExecuteLoadItemsCommand()); - RemoveItemsCommand = new Command(() => ExecuteRemoveItemsCommand()); + RemoveAllItemsCommand = new Command(() => ExecuteRemoveItemsCommand(), () => Items.Count > 0); + RemoveLastItemCommand = new Command(() => ExecuteRemoveLastItemCommand(), () => Items.Count > 0); } + void ExecuteRemoveItemsCommand() + { + while (Items.Count > 0) + { + Items.Remove(Items.Last()); + Items.Remove(Items.Last()); + Items.Remove(Items.Last()); + } + RemoveAllItemsCommand.ChangeCanExecute(); + RemoveLastItemCommand.ChangeCanExecute(); + } + + void ExecuteRemoveLastItemCommand() { Items.Remove(Items.Last()); + RemoveAllItemsCommand.ChangeCanExecute(); + RemoveLastItemCommand.ChangeCanExecute(); } + void ExecuteLoadItemsCommand() { IsBusy = true; @@ -146,6 +184,8 @@ void ExecuteLoadItemsCommand() finally { IsBusy = false; + RemoveAllItemsCommand.ChangeCanExecute(); + RemoveLastItemCommand.ChangeCanExecute(); } } @@ -156,7 +196,6 @@ public void OnAppearing() } } - [Preserve(AllMembers = true)] class ModelIssue12574 { public string Id { get; set; } diff --git a/Xamarin.Forms.Core.UITests.Shared/Tests/CarouselViewUITests.cs b/Xamarin.Forms.Core.UITests.Shared/Tests/CarouselViewUITests.cs index ab6fcc2c02e..876584791d0 100644 --- a/Xamarin.Forms.Core.UITests.Shared/Tests/CarouselViewUITests.cs +++ b/Xamarin.Forms.Core.UITests.Shared/Tests/CarouselViewUITests.cs @@ -13,47 +13,57 @@ protected override void NavigateToGallery() App.NavigateToGallery(GalleryQueries.CarouselViewGallery); } + void SwipeRightToLeft(int swipes = 1) + { + var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; + var fromX = rect.CenterX + 40; + var toX = rect.X - 5; + var fromY = rect.CenterY; + var toY = fromY; + + for (int n = 0; n < swipes; n++) + { + App.DragCoordinates(fromX, fromY, toX, toY); + } + } + [TestCase("CarouselView (XAML, Horizontal)")] [TestCase("CarouselView (XAML, Horizontal, Loop)")] public void CarouselViewRemoveAndUpdateCurrentItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); - var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; - var centerX = rect.CenterX; - var rightX = rect.X - 5; - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + SwipeRightToLeft(); - CheckPositionValue("lblPosition", "1"); - CheckPositionValue("lblCurrentItem", "1"); - CheckPositionValue("lblSelected", "1"); + CheckLabelValue("lblPosition", "1"); + CheckLabelValue("lblCurrentItem", "1"); + CheckLabelValue("lblSelected", "1"); App.Tap(x => x.Marked("btnRemove")); - CheckPositionValue("lblPosition", "1"); - CheckPositionValue("lblCurrentItem", "2"); - CheckPositionValue("lblSelected", "2"); + CheckLabelValue("lblPosition", "1"); + CheckLabelValue("lblCurrentItem", "2"); + CheckLabelValue("lblSelected", "2"); App.Back(); } - [TestCase("CarouselView (XAML, Horizontal)")] [TestCase("CarouselView (XAML, Horizontal, Loop)")] public void CarouselViewRemoveFirstCurrentItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); App.Tap(x => x.Marked("btnRemove")); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "1"); - CheckPositionValue("lblSelected", "1"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "1"); + CheckLabelValue("lblSelected", "1"); App.Back(); } @@ -64,48 +74,41 @@ public void CarouselViewGoToNextCurrentItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); App.Tap(x => x.Marked("btnNext")); - CheckPositionValue("lblPosition", "1"); - CheckPositionValue("lblCurrentItem", "1"); - CheckPositionValue("lblSelected", "1"); + CheckLabelValue("lblPosition", "1"); + CheckLabelValue("lblCurrentItem", "1"); + CheckLabelValue("lblSelected", "1"); App.Tap(x => x.Marked("btnPrev")); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); App.Back(); } - [TestCase("CarouselView (XAML, Horizontal)")] [TestCase("CarouselView (XAML, Horizontal, Loop)")] public void CarouselViewRemoveLastCurrentItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); - var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; - var centerX = rect.CenterX; - var rightX = rect.X - 5; - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + SwipeRightToLeft(4); - CheckPositionValue("lblPosition", "4"); - CheckPositionValue("lblCurrentItem", "4"); - CheckPositionValue("lblSelected", "4"); + CheckLabelValue("lblPosition", "4"); + CheckLabelValue("lblCurrentItem", "4"); + CheckLabelValue("lblSelected", "4"); App.Tap(x => x.Marked("btnRemove")); - CheckPositionValue("lblPosition", "3"); - CheckPositionValue("lblCurrentItem", "3"); - CheckPositionValue("lblSelected", "3"); + CheckLabelValue("lblPosition", "3"); + CheckLabelValue("lblCurrentItem", "3"); + CheckLabelValue("lblSelected", "3"); App.Back(); } @@ -115,22 +118,15 @@ public void CarouselViewLoopAfterLastItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); - var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; - var centerX = rect.CenterX; - var rightX = rect.X - 5; - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + SwipeRightToLeft(5); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); App.Back(); } @@ -140,18 +136,18 @@ public void CarouselViewLoopBeforeFirstItem(string subgallery) { VisitSubGallery(subgallery); - CheckPositionValue("lblPosition", "0"); - CheckPositionValue("lblCurrentItem", "0"); - CheckPositionValue("lblSelected", "0"); + CheckLabelValue("lblPosition", "0"); + CheckLabelValue("lblCurrentItem", "0"); + CheckLabelValue("lblSelected", "0"); var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; var centerX = rect.CenterX; var rightX = rect.X - 5; App.DragCoordinates(centerX - 50, rect.CenterY, centerX + rect.Width / 2 - 10, rect.CenterY); - CheckPositionValue("lblPosition", "4"); - CheckPositionValue("lblCurrentItem", "4"); - CheckPositionValue("lblSelected", "4"); + CheckLabelValue("lblPosition", "4"); + CheckLabelValue("lblCurrentItem", "4"); + CheckLabelValue("lblSelected", "4"); App.Back(); } @@ -274,16 +270,18 @@ public void CarouselViewObservableCollection(string subgallery) Assert.AreEqual("0", App.Query(c => c.Marked("lblPosition")).First().Text); App.Tap("btnNewObservable"); Assert.AreEqual("0", App.Query(c => c.Marked("lblPosition")).First().Text); - var rect = App.Query(c => c.Marked("TheCarouselView")).First().Rect; - var centerX = rect.CenterX; - var rightX = rect.X - 5; - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + + SwipeRightToLeft(); + App.Tap("btnAddObservable"); Assert.AreEqual("0", App.Query(c => c.Marked("lblPosition")).First().Text); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + + SwipeRightToLeft(); Assert.AreEqual("1", App.Query(c => c.Marked("lblPosition")).First().Text); - App.DragCoordinates(centerX + 40, rect.CenterY, rightX, rect.CenterY); + + SwipeRightToLeft(); Assert.AreEqual("2", App.Query(c => c.Marked("lblPosition")).First().Text); + App.Back(); } @@ -306,17 +304,16 @@ void VisitSubGallery(string galleryName, bool enableIndicator = false) App.Tap(t => t.Marked(galleryName)); } - static void CheckPositionValue(string marked, string value) + static void CheckLabelValue(string marked, string value) { var positionAfter = App.QueryUntilPresent(() => { - var positionLabel = App.WaitForElement(x => x.Marked(marked)); - if (positionLabel.First().Text == value) - return positionLabel; + var label = App.WaitForElement(x => x.Marked(marked)); + if (label.First().Text == value) + return label; return null; }, delayInMs: 1000); Assert.IsTrue(positionAfter[0].Text == value); } - } } \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS.UnitTests/ObservableItemsSourceTests.cs b/Xamarin.Forms.Platform.iOS.UnitTests/ObservableItemsSourceTests.cs new file mode 100644 index 00000000000..502157b3c45 --- /dev/null +++ b/Xamarin.Forms.Platform.iOS.UnitTests/ObservableItemsSourceTests.cs @@ -0,0 +1,62 @@ +using NUnit.Framework; + +namespace Xamarin.Forms.Platform.iOS.UnitTests +{ + [TestFixture(Category = "CollectionView")] + public class IndexPathTests + { + [Test] + public void GenerateIndexPathRange() + { + var result = IndexPathHelpers.GenerateIndexPathRange(0, 0, 5); + + Assert.That(result.Length, Is.EqualTo(5)); + Assert.That(result[0].Section, Is.EqualTo(0)); + Assert.That((int)result[0].Item, Is.EqualTo(0)); + + Assert.That(result[4].Section, Is.EqualTo(0)); + Assert.That((int)result[4].Item, Is.EqualTo(4)); + } + + [Test] + public void GenerateIndexPathRangeForLoop() + { + // Section 0 + // 5 items, looped 3 times + // Looking for all the items corresponding to indexes 2, 3, and 4 + + var result = IndexPathHelpers.GenerateLoopedIndexPathRange(0, 15, 3, 2, 3); + + // Source: + // 0, 1, 2, 3, 4, 0, 1, 2, 3, 4, 0, 1, 2, 3, 4 + // 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14 + + + // Result: + // 2, 3, 4, 2, 3, 4, 2, 3, 4 + // 2, 3, 4, 7, 8, 9, 12, 13, 14 + + Assert.That(result.Length, Is.EqualTo(9)); + + Assert.That(result[0].Section, Is.EqualTo(0)); + Assert.That(result[1].Section, Is.EqualTo(0)); + Assert.That(result[2].Section, Is.EqualTo(0)); + Assert.That(result[3].Section, Is.EqualTo(0)); + Assert.That(result[4].Section, Is.EqualTo(0)); + Assert.That(result[5].Section, Is.EqualTo(0)); + Assert.That(result[6].Section, Is.EqualTo(0)); + Assert.That(result[7].Section, Is.EqualTo(0)); + Assert.That(result[8].Section, Is.EqualTo(0)); + + Assert.That((int)result[0].Item, Is.EqualTo(2)); + Assert.That((int)result[1].Item, Is.EqualTo(3)); + Assert.That((int)result[2].Item, Is.EqualTo(4)); + Assert.That((int)result[3].Item, Is.EqualTo(7)); + Assert.That((int)result[4].Item, Is.EqualTo(8)); + Assert.That((int)result[5].Item, Is.EqualTo(9)); + Assert.That((int)result[6].Item, Is.EqualTo(12)); + Assert.That((int)result[7].Item, Is.EqualTo(13)); + Assert.That((int)result[8].Item, Is.EqualTo(14)); + } + } +} \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj b/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj index d39f4a95da8..42ae38bc4a1 100644 --- a/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj +++ b/Xamarin.Forms.Platform.iOS.UnitTests/Xamarin.Forms.Platform.iOS.UnitTests.csproj @@ -58,6 +58,7 @@ + @@ -85,4 +86,4 @@ - + \ No newline at end of file diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs index f60a2edc3a8..da6c0021193 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/CarouselViewController.cs @@ -1,9 +1,6 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; -using System.Linq; -using System.Threading.Tasks; using CoreGraphics; using Foundation; using UIKit; @@ -21,6 +18,7 @@ public class CarouselViewController : ItemsViewController int _gotoPosition = -1; CGSize _size; ILoopItemsViewSource LoopItemsSource => ItemsSource as ILoopItemsViewSource; + bool _isDragging; public CarouselViewController(CarouselView itemsView, ItemsViewLayout layout) : base(itemsView, layout) { @@ -86,13 +84,15 @@ public override void ViewDidLayoutSubviews() _updatingScrollOffset = false; } - UpdateInitialPosition(); - if (CollectionView.Bounds.Size != _size) { _size = CollectionView.Bounds.Size; BoundsSizeChanged(); } + else + { + UpdateInitialPosition(); + } } void BoundsSizeChanged() @@ -103,12 +103,14 @@ void BoundsSizeChanged() public override void DraggingStarted(UIScrollView scrollView) { + _isDragging = true; Carousel.SetIsDragging(true); } public override void DraggingEnded(UIScrollView scrollView, bool willDecelerate) { Carousel.SetIsDragging(false); + _isDragging = false; } public override void UpdateItemsSource() @@ -117,6 +119,13 @@ public override void UpdateItemsSource() base.UpdateItemsSource(); //we don't need to Subscribe because base calls CreateItemsViewSource _carouselViewLoopManager?.SetItemsSource(LoopItemsSource); + + if (_initialPositionSet) + { + Carousel.SetValueFromRenderer(CarouselView.CurrentItemProperty, null); + Carousel.SetValueFromRenderer(CarouselView.PositionProperty, 0); + } + _initialPositionSet = false; UpdateInitialPosition(); } @@ -179,34 +188,49 @@ void CarouselViewScrolled(object sender, ItemsViewScrolledEventArgs e) if (_updatingScrollOffset) return; + if (_isDragging) + { + return; + } + SetPosition(e.CenterItemIndex); UpdateVisualStates(); } - void CollectionItemsSourceChanged(object sender, NotifyCollectionChangedEventArgs e) + int _positionAfterUpdate = -1; + + void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e) { int carouselPosition = Carousel.Position; - int newPosition = carouselPosition; + _positionAfterUpdate = carouselPosition; var currentItemPosition = ItemsSource.GetIndexForItem(Carousel.CurrentItem).Row; var count = ItemsSource.ItemCount; if (e.Action == NotifyCollectionChangedAction.Remove) - newPosition = GetPositionWhenRemovingItems(e.OldStartingIndex, carouselPosition, currentItemPosition, count); + _positionAfterUpdate = GetPositionWhenRemovingItems(e.OldStartingIndex, carouselPosition, currentItemPosition, count); if (e.Action == NotifyCollectionChangedAction.Reset) - newPosition = GetPositionWhenResetItems(); + _positionAfterUpdate = GetPositionWhenResetItems(); if (e.Action == NotifyCollectionChangedAction.Add) - newPosition = GetPositionWhenAddingItems(carouselPosition, currentItemPosition); + _positionAfterUpdate = GetPositionWhenAddingItems(carouselPosition, currentItemPosition); + } + + void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e) + { + if (_positionAfterUpdate == -1) + { + return; + } _gotoPosition = -1; - if (count > 0) - ScrollToPosition(newPosition, carouselPosition, false); + var targetPosition = _positionAfterUpdate; + _positionAfterUpdate = -1; - SetCurrentItem(newPosition); - SetPosition(newPosition); + SetPosition(targetPosition); + SetCurrentItem(targetPosition); } int GetPositionWhenAddingItems(int carouselPosition, int currentItemPosition) @@ -241,22 +265,25 @@ int GetPositionWhenRemovingItems(int oldStartingIndex, int carouselPosition, int carouselPosition = currentItemPosition; } - if (removingCurrentElement) - CollectionView.ReloadItems(CollectionView.IndexPathsForVisibleItems); - return carouselPosition; } void SubscribeCollectionItemsSourceChanged(IItemsViewSource itemsSource) { if (itemsSource is ObservableItemsSource newItemsSource) - newItemsSource.CollectionItemsSourceChanged += CollectionItemsSourceChanged; + { + newItemsSource.CollectionViewUpdating += CollectionViewUpdating; + newItemsSource.CollectionViewUpdated += CollectionViewUpdated; + } } void UnsubscribeCollectionItemsSourceChanged(IItemsViewSource oldItemsSource) { if (oldItemsSource is ObservableItemsSource oldObservableItemsSource) - oldObservableItemsSource.CollectionItemsSourceChanged -= CollectionItemsSourceChanged; + { + oldObservableItemsSource.CollectionViewUpdating -= CollectionViewUpdating; + oldObservableItemsSource.CollectionViewUpdated -= CollectionViewUpdated; + } } void CarouselViewPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs changedProperty) @@ -325,7 +352,7 @@ void SetCurrentItem(int carouselPosition) void UpdateFromCurrentItem() { - if (Carousel?.CurrentItem == null || ItemsSource?.ItemCount == 0) + if (Carousel?.CurrentItem == null || ItemsSource == null || ItemsSource.ItemCount == 0) return; var currentItemPosition = GetIndexForItem(Carousel.CurrentItem).Row; diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/IndexPathHelpers.cs b/Xamarin.Forms.Platform.iOS/CollectionView/IndexPathHelpers.cs new file mode 100644 index 00000000000..abee80ce155 --- /dev/null +++ b/Xamarin.Forms.Platform.iOS/CollectionView/IndexPathHelpers.cs @@ -0,0 +1,36 @@ +using Foundation; + +namespace Xamarin.Forms.Platform.iOS +{ + public static class IndexPathHelpers + { + public static NSIndexPath[] GenerateIndexPathRange(int section, int startIndex, int count) + { + var result = new NSIndexPath[count]; + + for (int n = 0; n < count; n++) + { + result[n] = NSIndexPath.Create(section, startIndex + n); + } + + return result; + } + + public static NSIndexPath[] GenerateLoopedIndexPathRange(int section, int sectionCount, int iterations, int startIndex, int count) + { + var result = new NSIndexPath[iterations * count]; + var step = sectionCount / iterations; + + for (int r = 0; r < iterations; r++) + { + for (int n = 0; n < count; n++) + { + var index = startIndex + (r * step) + n; + result[(r * count) + n] = NSIndexPath.Create(section, index); + } + } + + return result; + } + } +} diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs index dc266f9dff5..ce946b5cc8e 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewController.cs @@ -305,7 +305,7 @@ UICollectionViewCell GetPrototype() var indexPath = NSIndexPath.Create(group, 0); - return GetCell(CollectionView, indexPath); + return CreateMeasurementCell(indexPath); } protected virtual void RegisterViewTypes() @@ -444,5 +444,31 @@ void UpdateEmptyViewVisibility(bool isEmpty) _emptyViewDisplayed = false; } } + + TemplatedCell CreateAppropriateCellForLayout() + { + var frame = new CGRect(0, 0, ItemsViewLayout.EstimatedItemSize.Width, ItemsViewLayout.EstimatedItemSize.Height); + + if (ItemsViewLayout.ScrollDirection == UICollectionViewScrollDirection.Horizontal) + { + return new HorizontalCell(frame); + } + + return new VerticalCell(frame); + } + + public TemplatedCell CreateMeasurementCell(NSIndexPath indexPath) + { + if (ItemsView.ItemTemplate == null) + { + return null; + } + + TemplatedCell templatedCell = CreateAppropriateCellForLayout(); + + UpdateTemplatedCell(templatedCell, indexPath); + + return templatedCell; + } } } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs index 9085c91aa39..517172b5d66 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs @@ -527,7 +527,14 @@ public override bool ShouldInvalidateLayoutForBoundsChange(CGRect newBounds) return base.ShouldInvalidateLayoutForBoundsChange(newBounds); } - UpdateConstraints(CollectionView.AdjustedContentInset.InsetRect(newBounds).Size); + if (Forms.IsiOS11OrNewer) + { + UpdateConstraints(CollectionView.AdjustedContentInset.InsetRect(newBounds).Size); + } + else + { + UpdateConstraints(CollectionView.Bounds.Size); + } return true; } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/LoopObservableItemsSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/LoopObservableItemsSource.cs index 543427448c8..02cb0ae9d58 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/LoopObservableItemsSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/LoopObservableItemsSource.cs @@ -1,5 +1,4 @@ -using System; -using System.Collections; +using System.Collections; using Foundation; using UIKit; @@ -20,22 +19,13 @@ public LoopObservableItemsSource(IEnumerable itemSource, UICollectionViewControl protected override NSIndexPath[] CreateIndexesFrom(int startIndex, int count) { - if (Loop) - count *= LoopBy; - - var result = new NSIndexPath[count]; - - for (int n = 0; n < count; n++) + if (!Loop) { - var index = startIndex + n; - if (Loop) - { - index = startIndex + n * Count; - } - result[n] = NSIndexPath.Create(Section, index); + return base.CreateIndexesFrom(startIndex, count); } - return result; + return IndexPathHelpers.GenerateLoopedIndexPathRange(Section, + (int)CollectionView.NumberOfItemsInSection(Section), LoopBy, startIndex, count); } } } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs index 4ebb7956607..69054577fdb 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs @@ -2,8 +2,6 @@ using System.Collections; using System.Collections.Generic; using System.Collections.Specialized; -using System.Threading; -using System.Threading.Tasks; using Foundation; using UIKit; @@ -15,7 +13,6 @@ internal class ObservableGroupedSource : IItemsViewSource readonly UICollectionViewController _collectionViewController; readonly IList _groupSource; bool _disposed; - SemaphoreSlim _batchUpdating = new SemaphoreSlim(1, 1); List _groups = new List(); public ObservableGroupedSource(IEnumerable groupSource, UICollectionViewController collectionViewController) @@ -130,53 +127,49 @@ void ResetGroupTracking() } } - async void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) + void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) { if (Device.IsInvokeRequired) { - await Device.InvokeOnMainThreadAsync(async () => await CollectionChanged(args)); + Device.BeginInvokeOnMainThread(() => CollectionChanged(args)); } else { - await CollectionChanged(args); + CollectionChanged(args); } } - async Task CollectionChanged(NotifyCollectionChangedEventArgs args) + void CollectionChanged(NotifyCollectionChangedEventArgs args) { switch (args.Action) { case NotifyCollectionChangedAction.Add: - await Add(args); + Add(args); break; case NotifyCollectionChangedAction.Remove: - await Remove(args); + Remove(args); break; case NotifyCollectionChangedAction.Replace: - await Replace(args); + Replace(args); break; case NotifyCollectionChangedAction.Move: Move(args); break; case NotifyCollectionChangedAction.Reset: - await Reload(); + Reload(); break; default: throw new ArgumentOutOfRangeException(); } } - async Task Reload() + void Reload() { ResetGroupTracking(); - await _batchUpdating.WaitAsync(); - _collectionView.ReloadData(); _collectionView.CollectionViewLayout.InvalidateLayout(); - - _batchUpdating.Release(); } NSIndexSet CreateIndexSetFrom(int startIndex, int count) @@ -191,11 +184,11 @@ bool NotLoadedYet() return !_collectionViewController.IsViewLoaded || _collectionViewController.View.Window == null; } - async Task Add(NotifyCollectionChangedEventArgs args) + void Add(NotifyCollectionChangedEventArgs args) { if (ReloadRequired()) { - await Reload(); + Reload(); return; } @@ -207,10 +200,10 @@ async Task Add(NotifyCollectionChangedEventArgs args) ResetGroupTracking(); // Queue up the updates to the UICollectionView - BatchUpdate(() => _collectionView.InsertSections(CreateIndexSetFrom(startIndex, count))); + _collectionView.InsertSections(CreateIndexSetFrom(startIndex, count)); } - async Task Remove(NotifyCollectionChangedEventArgs args) + void Remove(NotifyCollectionChangedEventArgs args) { var startIndex = args.OldStartingIndex; @@ -218,13 +211,13 @@ async Task Remove(NotifyCollectionChangedEventArgs args) { // INCC implementation isn't giving us enough information to know where the removed items were in the // collection. So the best we can do is a complete reload - await Reload(); + Reload(); return; } if (ReloadRequired()) { - await Reload(); + Reload(); return; } @@ -236,10 +229,10 @@ async Task Remove(NotifyCollectionChangedEventArgs args) var count = args.OldItems.Count; // Queue up the updates to the UICollectionView - BatchUpdate(() => _collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count))); + _collectionView.DeleteSections(CreateIndexSetFrom(startIndex, count)); } - async Task Replace(NotifyCollectionChangedEventArgs args) + void Replace(NotifyCollectionChangedEventArgs args) { var newCount = args.NewItems.Count; @@ -256,7 +249,7 @@ async Task Replace(NotifyCollectionChangedEventArgs args) // The original and replacement sets are of unequal size; this means that everything currently in view will // have to be updated. So we just have to use ReloadData and let the UICollectionView update everything - await Reload(); + Reload(); } void Move(NotifyCollectionChangedEventArgs args) @@ -349,28 +342,7 @@ bool ReloadRequired() // hard. We'll need to reload the data instead. return NotLoadedYet() - || _collectionView.NumberOfSections() == 0 - || _collectionView.VisibleCells.Length == 0; - } - - void BatchUpdate(Action update) - { - _collectionView.PerformBatchUpdates(() => - { - if (_batchUpdating.CurrentCount > 0) - { - _batchUpdating.Wait(); - } - - update(); - }, - (_) => - { - if (_batchUpdating.CurrentCount == 0) - { - _batchUpdating.Release(); - } - }); + || _collectionView.NumberOfSections() == 0; } } } diff --git a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs index 6b4674e0747..10333b0414f 100644 --- a/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs +++ b/Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs @@ -1,8 +1,6 @@ using System; using System.Collections; using System.Collections.Specialized; -using System.Threading; -using System.Threading.Tasks; using Foundation; using UIKit; @@ -11,17 +9,16 @@ namespace Xamarin.Forms.Platform.iOS internal class ObservableItemsSource : IItemsViewSource { readonly UICollectionViewController _collectionViewController; - readonly UICollectionView _collectionView; + protected readonly UICollectionView CollectionView; readonly bool _grouped; readonly int _section; readonly IEnumerable _itemsSource; bool _disposed; - SemaphoreSlim _batchUpdating = new SemaphoreSlim(1, 1); public ObservableItemsSource(IEnumerable itemSource, UICollectionViewController collectionViewController, int group = -1) { _collectionViewController = collectionViewController; - _collectionView = _collectionViewController.CollectionView; + CollectionView = _collectionViewController.CollectionView; _section = group < 0 ? 0 : group; _grouped = group >= 0; @@ -33,7 +30,8 @@ public ObservableItemsSource(IEnumerable itemSource, UICollectionViewController ((INotifyCollectionChanged)itemSource).CollectionChanged += CollectionChanged; } - internal event NotifyCollectionChangedEventHandler CollectionItemsSourceChanged; + internal event NotifyCollectionChangedEventHandler CollectionViewUpdating; + internal event NotifyCollectionChangedEventHandler CollectionViewUpdated; public int Count { get; private set; } @@ -99,84 +97,81 @@ public object this[NSIndexPath indexPath] } } - async void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) + void CollectionChanged(object sender, NotifyCollectionChangedEventArgs args) { if (Device.IsInvokeRequired) { - await Device.InvokeOnMainThreadAsync(async () => await CollectionChanged(args)); + Device.BeginInvokeOnMainThread(() => CollectionChanged(args)); } else { - await CollectionChanged(args); + CollectionChanged(args); } } - async Task CollectionChanged(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); + switch (args.Action) { case NotifyCollectionChangedAction.Add: - await Add(args); + Add(args); break; case NotifyCollectionChangedAction.Remove: - await Remove(args); + Remove(args); break; case NotifyCollectionChangedAction.Replace: - await Replace(args); + Replace(args); break; case NotifyCollectionChangedAction.Move: Move(args); break; case NotifyCollectionChangedAction.Reset: - await Reload(); + Reload(); break; default: throw new ArgumentOutOfRangeException(); } - - CollectionItemsSourceChanged?.Invoke(this, args); } - async Task Reload() + void Reload() { - await _batchUpdating.WaitAsync(); + var args = new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset); - _collectionView.ReloadData(); - _collectionView.CollectionViewLayout.InvalidateLayout(); Count = ItemsCount(); - _batchUpdating.Release(); + OnCollectionViewUpdating(args); + + CollectionView.ReloadData(); + CollectionView.CollectionViewLayout.InvalidateLayout(); + + OnCollectionViewUpdated(args); } protected virtual NSIndexPath[] CreateIndexesFrom(int startIndex, int count) { - var result = new NSIndexPath[count]; - - for (int n = 0; n < count; n++) - { - result[n] = NSIndexPath.Create(_section, startIndex + n); - } - - return result; + return IndexPathHelpers.GenerateIndexPathRange(_section, startIndex, count); } - async Task Add(NotifyCollectionChangedEventArgs args) + void Add(NotifyCollectionChangedEventArgs args) { - if (ReloadRequired()) - { - await Reload(); - return; - } - var count = args.NewItems.Count; Count += count; var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]); // Queue up the updates to the UICollectionView - BatchUpdate(() => _collectionView.InsertItems(CreateIndexesFrom(startIndex, count))); + Update(() => CollectionView.InsertItems(CreateIndexesFrom(startIndex, count)), args); } - async Task Remove(NotifyCollectionChangedEventArgs args) + void Remove(NotifyCollectionChangedEventArgs args) { var startIndex = args.OldStartingIndex; @@ -184,25 +179,18 @@ async Task Remove(NotifyCollectionChangedEventArgs args) { // INCC implementation isn't giving us enough information to know where the removed items were in the // collection. So the best we can do is a ReloadData() - await Reload(); - return; - } - - if (ReloadRequired()) - { - await Reload(); + Reload(); return; } // If we have a start index, we can be more clever about removing the item(s) (and get the nifty animations) - var count = args.OldItems.Count; + var count = args.OldItems.Count; Count -= count; - // Queue up the updates to the UICollectionView - BatchUpdate(() => _collectionView.DeleteItems(CreateIndexesFrom(startIndex, count))); + Update(() => CollectionView.DeleteItems(CreateIndexesFrom(startIndex, count)), args); } - async Task Replace(NotifyCollectionChangedEventArgs args) + void Replace(NotifyCollectionChangedEventArgs args) { var newCount = args.NewItems.Count; @@ -211,13 +199,15 @@ async Task 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 - _collectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount)); + OnCollectionViewUpdating(args); + CollectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount)); + OnCollectionViewUpdated(args); return; } // The original and replacement sets are of unequal size; this means that everything currently in view will // have to be updated. So we just have to use ReloadData and let the UICollectionView update everything - await Reload(); + Reload(); } void Move(NotifyCollectionChangedEventArgs args) @@ -230,13 +220,18 @@ void Move(NotifyCollectionChangedEventArgs args) var oldPath = NSIndexPath.Create(_section, args.OldStartingIndex); var newPath = NSIndexPath.Create(_section, args.NewStartingIndex); - _collectionView.MoveItem(oldPath, newPath); + OnCollectionViewUpdating(args); + CollectionView.MoveItem(oldPath, newPath); + OnCollectionViewUpdated(args); return; } var start = Math.Min(args.OldStartingIndex, args.NewStartingIndex); var end = Math.Max(args.OldStartingIndex, args.NewStartingIndex) + count; - _collectionView.ReloadItems(CreateIndexesFrom(start, end)); + OnCollectionViewUpdating(args); + CollectionView.ReloadItems(CreateIndexesFrom(start, end)); + + OnCollectionViewUpdated(args); } internal int ItemsCount() @@ -282,51 +277,24 @@ internal int IndexOf(object item) return -1; } - bool NotLoadedYet() + void Update(Action update, NotifyCollectionChangedEventArgs args) { - // If the UICollectionView hasn't actually been loaded, then calling InsertItems or DeleteItems is - // going to crash or get in an unusable state; instead, ReloadData should be used - return !_collectionViewController.IsViewLoaded || _collectionViewController.View.Window == null; + OnCollectionViewUpdating(args); + update(); + OnCollectionViewUpdated(args); } - bool ReloadRequired() + void OnCollectionViewUpdating(NotifyCollectionChangedEventArgs args) { - if (NotLoadedYet()) - { - return true; - } - - // UICollectionView doesn't like when we insert items into a completely empty un-grouped CV, - // and it doesn't like when we insert items into a grouped CV with no actual cells (just empty groups) - // In those circumstances, we just need to ask it to reload the data so it can get its internal - // accounting in order - - if (!_grouped && _collectionView.NumberOfItemsInSection(_section) == 0) - { - return true; - } - - return _collectionView.VisibleCells.Length == 0; + CollectionViewUpdating?.Invoke(this, args); } - void BatchUpdate(Action update) + void OnCollectionViewUpdated(NotifyCollectionChangedEventArgs args) { - _collectionView.PerformBatchUpdates(() => + Device.BeginInvokeOnMainThread(() => { - if (_batchUpdating.CurrentCount > 0) - { - _batchUpdating.Wait(); - } - - update(); - }, - (_) => - { - if (_batchUpdating.CurrentCount == 0) - { - _batchUpdating.Release(); - } - }); + CollectionViewUpdated?.Invoke(this, args); + }); } } } diff --git a/Xamarin.Forms.Platform.iOS/Xamarin.Forms.Platform.iOS.csproj b/Xamarin.Forms.Platform.iOS/Xamarin.Forms.Platform.iOS.csproj index 62538e422db..da22cfe685e 100644 --- a/Xamarin.Forms.Platform.iOS/Xamarin.Forms.Platform.iOS.csproj +++ b/Xamarin.Forms.Platform.iOS/Xamarin.Forms.Platform.iOS.csproj @@ -127,6 +127,7 @@ +