Skip to content

Commit

Permalink
Updating Picker remove index calculation logic to make it easier to r…
Browse files Browse the repository at this point in the history 
…ead. Added unit tests for Picker to check SelectedIndex and SelectedItem when adding/removing single and multiple items.
  • Loading branch information
@BurningLights @PureWeen
BurningLights authored and PureWeen committed Jul 5, 2024
1 parent cb3a348 commit 5ad8da5
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 65 deletions.
19 changes: 17 additions & 2 deletions src/Controls/src/Core/Picker/Picker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,23 @@ void AddItems(NotifyCollectionChangedEventArgs e)

void RemoveItems(NotifyCollectionChangedEventArgs e)
{
int removeStart = e.OldStartingIndex < Items.Count ? e.OldStartingIndex : Items.Count - e.OldItems.Count;
int index = removeStart + e.OldItems.Count - 1;
int removeStart;
// Items are removed in reverse order, so index starts at the index of the last item to remove
int index;

if (e.OldStartingIndex < Items.Count)
{
// Remove e.OldItems.Count items starting at e.OldStartingIndex
removeStart = e.OldStartingIndex;
index = e.OldStartingIndex + e.OldItems.Count - 1;
}
else
{
// Remove e.OldItems.Count items at the end when e.OldStartingIndex is past the end of the Items collection
removeStart = Items.Count - e.OldItems.Count;
index = Items.Count - 1;
}

foreach (object _ in e.OldItems)
((LockableObservableListWrapper)Items).InternalRemoveAt(index--);
if (removeStart <= SelectedIndex)
Expand Down
137 changes: 74 additions & 63 deletions src/Controls/tests/Core.UnitTests/PickerTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Collections.Specialized;
using System.ComponentModel;
using System.Globalization;
using System.Linq;
using Microsoft.Maui.Graphics;
using Xunit;

Expand Down Expand Up @@ -57,6 +61,42 @@ public object ConvertBack(object value, Type targetType, object parameter, Cultu
}
}

class ObservableRangeCollection<T> : ObservableCollection<T>
{
static readonly PropertyChangedEventArgs CountChangedArgs = new(nameof(Count));
static readonly PropertyChangedEventArgs IndexerChangedArgs = new("Item[]");

public void InsertRange(int index, IEnumerable<T> items)
{
CheckReentrancy();
int currIndex = index;
foreach (T item in items)
{
Items.Insert(currIndex++, item);
}

OnPropertyChanged(CountChangedArgs);
OnPropertyChanged(IndexerChangedArgs);
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, items.ToList(), index));
}

public void RemoveRange(int index, int count)
{
CheckReentrancy();
T[] removeItems = new T[count];
for (int i = 0; i < count; i++)
{
// Always remove at index, since removing each item at index shifts the next item to that spot
removeItems[i] = Items[index];
Items.RemoveAt(index);
}

OnPropertyChanged(CountChangedArgs);
OnPropertyChanged(IndexerChangedArgs);
OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, removeItems, index));
}
}

[Fact]
public void TestSetSelectedIndexOnNullRows()
{
Expand Down Expand Up @@ -339,11 +379,17 @@ public void TestItemsSourceCollectionChangedRemove()
}

[Theory]
[InlineData(0)]
[InlineData(1)]
public void TestItemsSourceCollectionChangedInsertBeforeSelected(int insertionIndex)
{
var items = new ObservableCollection<object>
[InlineData(0, new string[] { "George" })]
[InlineData(1, new string[] { "George" })]
[InlineData(2, new string[] { "George" })]
[InlineData(3, new string[] { "George" })]
[InlineData(0, new string[] { "George", "Pete" })]
[InlineData(1, new string[] { "George", "Pete" })]
[InlineData(2, new string[] { "George", "Pete" })]
[InlineData(3, new string[] { "George", "Pete" })]
public void TestItemsSourceCollectionChangedInsertBeforeSelected(int insertionIndex, string[] insertNames)
{
var items = new ObservableRangeCollection<object>
{
new { Name = "John" },
new { Name = "Paul" },
Expand All @@ -355,39 +401,22 @@ public void TestItemsSourceCollectionChangedInsertBeforeSelected(int insertionIn
ItemsSource = items,
SelectedIndex = 1
};
items.Insert(insertionIndex, new { Name = "George" });
items.InsertRange(insertionIndex, insertNames.Select(name => new { Name = name }));
Assert.Equal(3 + insertNames.Length, picker.Items.Count);
Assert.Equal(1, picker.SelectedIndex);
Assert.Equal(items[1], picker.SelectedItem);
}

[Theory]
[InlineData(2)]
[InlineData(3)]
public void TestItemsSourceCollectionChangedInsertAfterSelected(int insertionIndex)
[InlineData(0, 1)]
[InlineData(1, 1)]
[InlineData(2, 1)]
[InlineData(0, 2)]
[InlineData(1, 2)]
[InlineData(2, 2)]
public void TestItemsSourceCollectionChangedRemoveBeforeSelected(int removeIndex, int removeCount)
{
var items = new ObservableCollection<object>
{
new { Name = "John" },
new { Name = "Paul" },
new { Name = "Ringo" }
};
var picker = new Picker
{
ItemDisplayBinding = new Binding("Name"),
ItemsSource = items,
SelectedIndex = 1
};
items.Insert(insertionIndex, new { Name = "George" });
Assert.Equal(1, picker.SelectedIndex);
Assert.Equal(items[1], picker.SelectedItem);
}

[Theory]
[InlineData(0)]
[InlineData(1)]
public void TestItemsSourceCollectionChangedRemoveBeforeSelected(int removeIndex)
{
var items = new ObservableCollection<object>
var items = new ObservableRangeCollection<object>
{
new { Name = "John" },
new { Name = "Paul" },
Expand All @@ -400,15 +429,19 @@ public void TestItemsSourceCollectionChangedRemoveBeforeSelected(int removeIndex
ItemsSource = items,
SelectedIndex = 1
};
items.RemoveAt(removeIndex);
items.RemoveRange(removeIndex, removeCount);

Assert.Equal(4 - removeCount, picker.Items.Count);
Assert.Equal(1, picker.SelectedIndex);
Assert.Equal(items[1], picker.SelectedItem);
}

[Fact]
public void TestItemsSourceCollectionChangedRemoveAtEndSelected()
[Theory]
[InlineData(1)]
[InlineData(2)]
public void TestItemsSourceCollectionChangedRemoveAtEndSelected(int removeCount)
{
var items = new ObservableCollection<object>
var items = new ObservableRangeCollection<object>
{
new { Name = "John" },
new { Name = "Paul" },
Expand All @@ -419,37 +452,15 @@ public void TestItemsSourceCollectionChangedRemoveAtEndSelected()
{
ItemDisplayBinding = new Binding("Name"),
ItemsSource = items,
SelectedIndex = 3
SelectedIndex = 4 - removeCount
};
items.RemoveAt(3);
Assert.Equal(2, picker.SelectedIndex);
Assert.Equal(items[2], picker.SelectedItem);
}
items.RemoveRange(4 - removeCount, removeCount);

[Fact]
public void TestItemsSourceCollectionChangedRemoveAfterSelected()
{
var items = new ObservableCollection<object>
{
new { Name = "John" },
new { Name = "Paul" },
new { Name = "Ringo" },
new { Name = "George" }
};
var picker = new Picker
{
ItemDisplayBinding = new Binding("Name"),
ItemsSource = items,
SelectedIndex = 1
};
items.RemoveAt(2);
Assert.Equal(1, picker.SelectedIndex);
Assert.Equal(items[1], picker.SelectedItem);
Assert.Equal(4 - removeCount, picker.Items.Count);
Assert.Equal(items.Count - 1, picker.SelectedIndex);
Assert.Equal(items[^1], picker.SelectedItem);
}


// TODO: Multi-item add and remove

[Fact]
public void TestSelectedIndexAssignedItemsSourceCollectionChangedAppend()
{
Expand Down

0 comments on commit 5ad8da5

Please sign in to comment.