Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Commit

Permalink
Remove performBatchUpdates calls to prevent data disparity;
Browse files Browse the repository at this point in the history
  • Loading branch information
hartez committed Nov 25, 2020
1 parent 3783a77 commit 15b0276
Show file tree
Hide file tree
Showing 11 changed files with 219 additions and 124 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -139,7 +139,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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand All @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -105,28 +117,54 @@ public void Issue12574Test()

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<ModelIssue12574> 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<ModelIssue12574>();
LoadItemsCommand = new Command(() => ExecuteLoadItemsCommand());
RemoveItemsCommand = new Command(() => ExecuteRemoveItemsCommand(), () => Items.Count > 0);
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());
RemoveItemsCommand.ChangeCanExecute();
RemoveAllItemsCommand.ChangeCanExecute();
RemoveLastItemCommand.ChangeCanExecute();
}

void ExecuteLoadItemsCommand()
{
IsBusy = true;
Expand All @@ -146,7 +184,8 @@ void ExecuteLoadItemsCommand()
finally
{
IsBusy = false;
RemoveItemsCommand.ChangeCanExecute();
RemoveAllItemsCommand.ChangeCanExecute();
RemoveLastItemCommand.ChangeCanExecute();
}
}

Expand All @@ -157,7 +196,6 @@ public void OnAppearing()
}
}

[Preserve(AllMembers = true)]
class ModelIssue12574
{
public string Id { get; set; }
Expand Down
62 changes: 62 additions & 0 deletions Xamarin.Forms.Platform.iOS.UnitTests/ObservableItemsSourceTests.cs
Original file line number Diff line number Diff line change
@@ -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));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
<Compile Include="IsEnabledTests.cs" />
<Compile Include="IsVisibleTests.cs" />
<Compile Include="NavigationTests.cs" />
<Compile Include="ObservableItemsSourceTests.cs" />
<Compile Include="OpacityTests.cs" />
<Compile Include="PlatformTestFixture.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
Expand Down Expand Up @@ -85,4 +86,4 @@
</PackageReference>
</ItemGroup>
<Import Project="$(MSBuildExtensionsPath)\Xamarin\iOS\Xamarin.iOS.CSharp.targets" />
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,15 @@ public override void ViewDidLayoutSubviews()
_updatingScrollOffset = false;
}

UpdateInitialPosition();

if (CollectionView.Bounds.Size != _size)
{
_size = CollectionView.Bounds.Size;
BoundsSizeChanged();
}
else
{
UpdateInitialPosition();
}
}

void BoundsSizeChanged()
Expand Down Expand Up @@ -188,6 +190,11 @@ void CarouselViewScrolled(object sender, ItemsViewScrolledEventArgs e)

void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e)
{
if (e.Action == NotifyCollectionChangedAction.Reset)
{
return;
}

int carouselPosition = Carousel.Position;
_positionAfterUpdate = carouselPosition;
var currentItemPosition = ItemsSource.GetIndexForItem(Carousel.CurrentItem).Row;
Expand All @@ -205,7 +212,7 @@ void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e)

void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e)
{
if (_positionAfterUpdate == -1)
if (e.Action == NotifyCollectionChangedAction.Reset || _positionAfterUpdate == -1)
{
return;
}
Expand All @@ -215,7 +222,7 @@ void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e)
var targetPosition = _positionAfterUpdate;
_positionAfterUpdate = -1;

ScrollToPosition(targetPosition, Carousel.Position, false);
Carousel.ScrollTo(targetPosition, position: Xamarin.Forms.ScrollToPosition.Center, animate: false);
}

int GetPositionWhenAddingItems(int carouselPosition, int currentItemPosition)
Expand Down Expand Up @@ -337,7 +344,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;
Expand Down
36 changes: 36 additions & 0 deletions Xamarin.Forms.Platform.iOS/CollectionView/IndexPathHelpers.cs
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
9 changes: 8 additions & 1 deletion Xamarin.Forms.Platform.iOS/CollectionView/ItemsViewLayout.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Collections;
using System.Collections;
using Foundation;
using UIKit;

Expand All @@ -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);
}
}
}
Loading

0 comments on commit 15b0276

Please sign in to comment.