Fix: Picker selects incorrect item after item removal (#29313)

* Fixed the selected item changed

* fix for adding items

* Added the test cases

* Modified the logics

* Modified the test case

* Corrected the code

* Modified the logics

* Modified the changes

* Add unit tests for Picker selection preservation

- Add PickerPreservesSelectedItemAfterRemovingItemBeforeSelection test
- Add PickerPreservesSelectedItemAfterInsertingItemBeforeSelection test
- Update existing tests to verify the new correct behavior

* Remove UI test, keep only unit test

Unit tests are sufficient to validate this fix.

---------

Co-authored-by: Stephane Delcroix <[email protected]>

Author: Shalini-Ashokan

src/Controls/src/Core/Picker/Picker.cs

@@ -275,7 +275,8 @@ void OnItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
 			if (((LockableObservableListWrapper)Items).IsLocked)
 				return;
 
-			ClampSelectedIndex();
+			int index = GetSelectedIndex();
+			ClampSelectedIndex(index);
 			Handler?.UpdateValue(nameof(IPicker.Items));
 		}
 
@@ -370,8 +371,13 @@ void AddItems(NotifyCollectionChangedEventArgs e)
 			int index = insertIndex;
 			foreach (object newItem in e.NewItems)
 				((LockableObservableListWrapper)Items).InternalInsert(index++, GetDisplayMember(newItem));
-			if (insertIndex <= SelectedIndex)
-				UpdateSelectedItem(SelectedIndex);
+
+			index = GetSelectedIndex();
+			if (insertIndex <= index)
+			{
+				// When an item is inserted before the current selection, the selected item changes because the selected index is not properly updated.
+				ClampSelectedIndex(index);
+			}
 		}
 
 		void RemoveItems(NotifyCollectionChangedEventArgs e)
@@ -395,10 +401,23 @@ void RemoveItems(NotifyCollectionChangedEventArgs e)
 
 			foreach (object _ in e.OldItems)
 				((LockableObservableListWrapper)Items).InternalRemoveAt(index--);
-			if (removeStart <= SelectedIndex)
+
+			index = GetSelectedIndex();
+			if (removeStart <= index)
+			{
+				ClampSelectedIndex(index);
+			}
+		}
+
+		int GetSelectedIndex()
+		{
+			if (SelectedItem is null)
 			{
-				ClampSelectedIndex();
+				return SelectedIndex;
 			}
+
+			int newIndex = ItemsSource?.IndexOf(SelectedItem) ?? Items?.IndexOf(SelectedItem) ?? -1;
+			return newIndex >= 0 ? newIndex : SelectedIndex;
 		}
 
 		void ResetItems()
@@ -410,7 +429,7 @@ void ResetItems()
 				((LockableObservableListWrapper)Items).InternalAdd(GetDisplayMember(item));
 			Handler?.UpdateValue(nameof(IPicker.Items));
 
-			ClampSelectedIndex();
+			ClampSelectedIndex(SelectedIndex);
 		}
 
 		static void OnSelectedIndexChanged(object bindable, object oldValue, object newValue)
@@ -426,10 +445,10 @@ static void OnSelectedItemChanged(BindableObject bindable, object oldValue, obje
 			picker.UpdateSelectedIndex(newValue);
 		}
 
-		void ClampSelectedIndex()
+		void ClampSelectedIndex(int selectedIndex)
 		{
-			var oldIndex = SelectedIndex;
-			var newIndex = SelectedIndex.Clamp(-1, Items.Count - 1);
+			var oldIndex = selectedIndex;
+			var newIndex = selectedIndex.Clamp(-1, Items.Count - 1);
 			//FIXME use the specificity of the caller
 			SetValue(SelectedIndexProperty, newIndex, SetterSpecificity.FromHandler);
 			// If the index has not changed, still need to change the selected item

src/Controls/tests/Core.UnitTests/PickerTests.cs

@@ -401,20 +401,24 @@ public void TestItemsSourceCollectionChangedInsertBeforeSelected(int insertionIn
 				ItemsSource = items,
 				SelectedIndex = 1
 			};
+			var originalSelectedItem = picker.SelectedItem;
 			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);
+			// The selected item should remain the same, but the index should update
+			Assert.Equal(originalSelectedItem, picker.SelectedItem);
+			Assert.Equal(items.IndexOf(originalSelectedItem), picker.SelectedIndex);
 		}
 
 		[Theory]
-		[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)
+		// Cases where removed items do NOT include the selected item (Paul at index 1)
+		[InlineData(0, 1, true)]  // Remove John - Paul should be preserved
+		[InlineData(2, 1, true)]  // Remove Ringo - Paul should be preserved
+		[InlineData(2, 2, true)]  // Remove Ringo and George - Paul should be preserved
+		// Cases where removed items include the selected item
+		[InlineData(1, 1, false)] // Remove Paul - selection changes
+		[InlineData(0, 2, false)] // Remove John and Paul - selection changes
+		[InlineData(1, 2, false)] // Remove Paul and Ringo - selection changes
+		public void TestItemsSourceCollectionChangedRemoveBeforeSelected(int removeIndex, int removeCount, bool selectedItemPreserved)
 		{
 			var items = new ObservableRangeCollection<object>
 			{
@@ -429,11 +433,21 @@ public void TestItemsSourceCollectionChangedRemoveBeforeSelected(int removeIndex
 				ItemsSource = items,
 				SelectedIndex = 1
 			};
+			var originalSelectedItem = picker.SelectedItem;
 			items.RemoveRange(removeIndex, removeCount);
 
 			Assert.Equal(4 - removeCount, picker.Items.Count);
-			Assert.Equal(1, picker.SelectedIndex);
-			Assert.Equal(items[1], picker.SelectedItem);
+			if (selectedItemPreserved)
+			{
+				// The selected item should remain the same, but the index should update
+				Assert.Equal(originalSelectedItem, picker.SelectedItem);
+				Assert.Equal(items.IndexOf(originalSelectedItem), picker.SelectedIndex);
+			}
+			else
+			{
+				// The selected item was removed, so a different item is now selected
+				Assert.NotEqual(originalSelectedItem, picker.SelectedItem);
+			}
 		}
 
 		[Theory]
@@ -792,5 +806,51 @@ public void NullItemReturnsEmptyStringFromInterface()
 			var thing = (picker as IPicker).GetItem(0);
 			Assert.NotNull(thing);
 		}
+
+		// https://github.com/dotnet/maui/issues/29235
+		[Fact]
+		public void PickerPreservesSelectedItemAfterRemovingItemBeforeSelection()
+		{
+			// Arrange: Create a picker with items and select "Item2" (index 2)
+			var items = new ObservableCollection<string> { "Item0", "Item1", "Item2" };
+			var picker = new Picker
+			{
+				ItemsSource = items,
+				SelectedIndex = 2 // Select "Item2"
+			};
+
+			Assert.Equal("Item2", picker.SelectedItem);
+			Assert.Equal(2, picker.SelectedIndex);
+
+			// Act: Remove the first item (before the selection)
+			items.RemoveAt(0);
+
+			// Assert: SelectedItem should still be "Item2", but index should now be 1
+			Assert.Equal("Item2", picker.SelectedItem);
+			Assert.Equal(1, picker.SelectedIndex);
+		}
+
+		// https://github.com/dotnet/maui/issues/29235
+		[Fact]
+		public void PickerPreservesSelectedItemAfterInsertingItemBeforeSelection()
+		{
+			// Arrange: Create a picker with items and select "Dog" (index 1)
+			var items = new ObservableCollection<string> { "Cat", "Dog", "Rabbit" };
+			var picker = new Picker
+			{
+				ItemsSource = items,
+				SelectedIndex = 1 // Select "Dog"
+			};
+
+			Assert.Equal("Dog", picker.SelectedItem);
+			Assert.Equal(1, picker.SelectedIndex);
+
+			// Act: Insert an item at the beginning
+			items.Insert(0, "Goat");
+
+			// Assert: SelectedItem should still be "Dog", and index should now be 2
+			Assert.Equal("Dog", picker.SelectedItem);
+			Assert.Equal(2, picker.SelectedIndex);
+		}
 	}
 }