Skip to content

Commit

Permalink
[iOS] Fix when removing current last Item on CarouselView (xamarin#12837
Browse files Browse the repository at this point in the history
)

* [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 <hartez@gmail.com>
  • Loading branch information
2 people authored and pictos committed Dec 29, 2020
1 parent 9c459f9 commit e5138c8
Show file tree
Hide file tree
Showing 13 changed files with 388 additions and 261 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 All @@ -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<Page>(this, "Delete");
}

public ObservableCollection<ModelIssue10300> Items { get; set; }
Expand All @@ -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));
}
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 All @@ -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");
Expand All @@ -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<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());
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;
Expand All @@ -146,6 +184,8 @@ void ExecuteLoadItemsCommand()
finally
{
IsBusy = false;
RemoveAllItemsCommand.ChangeCanExecute();
RemoveLastItemCommand.ChangeCanExecute();
}
}

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

[Preserve(AllMembers = true)]
class ModelIssue12574
{
public string Id { get; set; }
Expand Down
Loading

0 comments on commit e5138c8

Please sign in to comment.