Skip to content

Commit b0877fe

Browse files
Fix SelectionChanged not raised on collection Reset (#20942)
* fix: raise SelectionChanged event on collection Reset when items are deselected When a collection bound to a SelectingItemsControl (e.g. ListBox) was cleared via NotifyCollectionChangedAction.Reset, the SelectionChanged event was not raised despite the selection being lost. The root cause was in InternalSelectionModel.OnSourceReset: the base SelectionModel.OnSourceReset() directly reset _selectedIndex to -1 before any Operation could capture the old selection state. The subsequent SyncFromSelectedItems created an Operation that saw no change (old and new both -1), so CommitOperation never fired SelectionChanged. The fix snapshots _writableSelectedItems before sync, diffs against the post-sync state to find items that were actually lost (not merely re-selected at a new index after reorder), and injects them as DeselectedItems on the pending Operation — following the same pattern used by OnSelectionRemoved for individual item removals. Fixes #20897 * fix: use multiset diff to report lost duplicate selections on Reset The Reset diff in InternalSelectionModel used a HashSet to detect which previously-selected items were still present after sync. Selection allows duplicates (same instance or equal items at multiple indices), so set semantics collapsed duplicates into one entry and under-reported deselections when only some occurrences were lost. Track counts per item plus a null counter and decrement per match, so RemovedItems reflects the actual number of lost selections. Adds a duplicate-items Reset test covering the regression. * fix: raise SelectionChanged for reset-lost selection ListBox and other SelectingItemsControl callers did not receive SelectionChanged when a Reset cleared the selected items. The selection model reports this path through LostSelection, but the control only used that callback for AlwaysSelected recovery. Track the last selected items at the control boundary, capture that snapshot for Reset notifications, and raise the routed SelectionChanged event when LostSelection commits during that reset. This avoids diffing reset contents while preserving the removed-items payload for clear/reset-to-empty cases. * fix: address review feedback on SelectionChanged Reset snapshot - Replace per-change ToArray() snapshot with persistent List<object?> to avoid allocations on every selection change (review: MrJul). - Read snapshot in PreCollectionChanged instead of Selection.SelectedItems because the source is already empty by the time Reset fires. - Align LostSelection event-raising with SelectionChanged path: use BuildEventRoute + HasHandlers guard to avoid allocating args when no handlers are attached (review: copilot). - Harden existing Reset tests to Assert.Single to catch double-fire. * fix: consolidate SelectionChanged raising and defend against stale snapshot - Extract RaiseSelectionChanged helper so both the normal (SelectionChanged) and reset (LostSelection) paths share the same BuildEventRoute/HasHandlers guard and SelectionChangedEventArgs construction (review: copilot). - Move _selectedItemsBeforeReset clear outside the conditional in OnSelectionModelLostSelection so the field is always nulled after LostSelection, preventing accidental reuse (review: copilot). - Add comment documenting the snapshot lifecycle in OnItemsViewPreCollectionChanged. * perf: defer SelectionChangedEventArgs allocations until handlers are confirmed SelectionChangedEventArgs materialized arrays via ToArray() at the call site before checking whether the routed event had any registered handlers. This allocated needlessly in the common case of no external subscribers. The event items (IReadOnlyList<object?>) already implement IList via ReadOnlySelectionListBase. RaiseSelectionChanged now accepts IReadOnlyList<object?> and casts to IList inside the HasHandlers gate, falling back to ToArray() only for non-IList enumerables.
1 parent 498c183 commit b0877fe

2 files changed

Lines changed: 140 additions & 10 deletions

File tree

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

Lines changed: 45 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections;
3+
using System.Collections.Generic;
34
using System.Collections.Specialized;
45
using System.ComponentModel;
56
using System.Diagnostics.CodeAnalysis;
@@ -145,6 +146,8 @@ public class SelectingItemsControl : ItemsControl
145146
private int _oldSelectedIndex;
146147
private WeakReference _oldSelectedItem = new(null);
147148
private WeakReference<IList?> _oldSelectedItems = new(null);
149+
private readonly List<object?> _selectedItemsSnapshot = new();
150+
private object?[]? _selectedItemsBeforeReset;
148151
private bool _ignoreContainerSelectionChanged;
149152
private UpdateState? _updateState;
150153
private bool _hasScrolledToSelectedItem;
@@ -153,7 +156,9 @@ public class SelectingItemsControl : ItemsControl
153156

154157
public SelectingItemsControl()
155158
{
156-
((ItemCollection)ItemsView).SourceChanged += OnItemsViewSourceChanged;
159+
var items = (ItemCollection)ItemsView;
160+
items.SourceChanged += OnItemsViewSourceChanged;
161+
items.PreCollectionChanged += OnItemsViewPreCollectionChanged;
157162
}
158163

159164
/// <summary>
@@ -465,6 +470,14 @@ private protected override void OnItemsViewCollectionChanged(object? sender, Not
465470
}
466471
}
467472

473+
private void OnItemsViewPreCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
474+
{
475+
if (e.Action == NotifyCollectionChangedAction.Reset && _selectedItemsSnapshot.Count > 0)
476+
{
477+
_selectedItemsBeforeReset = _selectedItemsSnapshot.ToArray();
478+
}
479+
}
480+
468481
/// <inheritdoc />
469482
protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
470483
{
@@ -1013,16 +1026,11 @@ void Mark(int index, bool selected)
10131026
UpdateSelectedValueFromItem();
10141027
}
10151028

1016-
var route = BuildEventRoute(SelectionChangedEvent);
1029+
_selectedItemsSnapshot.Clear();
1030+
_selectedItemsSnapshot.AddRange(Selection.SelectedItems);
1031+
_selectedItemsBeforeReset = null;
10171032

1018-
if (route.HasHandlers)
1019-
{
1020-
var ev = new SelectionChangedEventArgs(
1021-
SelectionChangedEvent,
1022-
e.DeselectedItems.ToArray(),
1023-
e.SelectedItems.ToArray());
1024-
RaiseEvent(ev);
1025-
}
1033+
RaiseSelectionChanged(e.DeselectedItems, e.SelectedItems);
10261034
}
10271035

10281036
/// <summary>
@@ -1033,12 +1041,37 @@ void Mark(int index, bool selected)
10331041
/// <param name="e">The event args.</param>
10341042
private void OnSelectionModelLostSelection(object? sender, EventArgs e)
10351043
{
1044+
if (_selectedItemsBeforeReset?.Length > 0)
1045+
{
1046+
RaiseSelectionChanged(_selectedItemsBeforeReset, Array.Empty<object?>());
1047+
}
1048+
1049+
_selectedItemsBeforeReset = null;
1050+
10361051
if (AlwaysSelected && ItemsView.Count > 0)
10371052
{
10381053
SelectedIndex = 0;
10391054
}
10401055
}
10411056

1057+
/// <summary>
1058+
/// Raises the <see cref="SelectionChangedEvent"/> if there are registered handlers.
1059+
/// </summary>
1060+
/// <param name="removedItems">The items removed from the selection.</param>
1061+
/// <param name="addedItems">The items added to the selection.</param>
1062+
private void RaiseSelectionChanged(IReadOnlyList<object?> removedItems, IReadOnlyList<object?> addedItems)
1063+
{
1064+
var route = BuildEventRoute(SelectionChangedEvent);
1065+
1066+
if (route.HasHandlers)
1067+
{
1068+
RaiseEvent(new SelectionChangedEventArgs(
1069+
SelectionChangedEvent,
1070+
removedItems as IList ?? removedItems.ToArray(),
1071+
addedItems as IList ?? addedItems.ToArray()));
1072+
}
1073+
}
1074+
10421075
private void SelectItemWithValue(object? value)
10431076
{
10441077
if (ItemCount == 0 || _isSelectionChangeActive)
@@ -1248,6 +1281,8 @@ private void InitializeSelectionModel(ISelectionModel model)
12481281

12491282
_oldSelectedIndex = model.SelectedIndex;
12501283
_oldSelectedItem.Target = model.SelectedItem;
1284+
_selectedItemsSnapshot.Clear();
1285+
_selectedItemsSnapshot.AddRange(model.SelectedItems);
12511286

12521287
if (_updateState is null && AlwaysSelected && model.Count == 0)
12531288
{

tests/Avalonia.Controls.UnitTests/Primitives/SelectingItemsControlTests.cs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,6 +856,101 @@ public void Resetting_Items_Collection_Should_Clear_Selection()
856856
Assert.Equal(-1, target.SelectedIndex);
857857
}
858858

859+
[Fact]
860+
public void Resetting_Items_Collection_Should_Raise_SelectionChanged()
861+
{
862+
var items = new ObservableCollection<Item>
863+
{
864+
new Item(),
865+
new Item(),
866+
new Item(),
867+
};
868+
869+
var target = new SelectingItemsControl
870+
{
871+
ItemsSource = items,
872+
Template = Template(),
873+
};
874+
875+
Prepare(target);
876+
target.SelectedIndex = 1;
877+
878+
var selectedItem = items[1];
879+
880+
var receivedArgs = new List<SelectionChangedEventArgs>();
881+
target.SelectionChanged += (_, args) => receivedArgs.Add(args);
882+
883+
items.Clear();
884+
885+
Assert.Null(target.SelectedItem);
886+
Assert.Equal(-1, target.SelectedIndex);
887+
Assert.Single(receivedArgs);
888+
Assert.Empty(receivedArgs[0].AddedItems);
889+
Assert.Equal(new[] { selectedItem }, receivedArgs[0].RemovedItems);
890+
}
891+
892+
[Fact]
893+
public void Resetting_Items_To_Empty_With_Multiple_Selection_Should_Raise_SelectionChanged()
894+
{
895+
var items = new ObservableCollection<Item>
896+
{
897+
new Item(),
898+
new Item(),
899+
new Item(),
900+
};
901+
902+
var target = new TestSelector
903+
{
904+
ItemsSource = items,
905+
Template = Template(),
906+
SelectionMode = SelectionMode.Multiple,
907+
};
908+
909+
Prepare(target);
910+
target.SelectedIndex = 0;
911+
target.Selection.Select(2);
912+
913+
var selected0 = items[0];
914+
var selected2 = items[2];
915+
916+
var receivedArgs = new List<SelectionChangedEventArgs>();
917+
target.SelectionChanged += (_, args) => receivedArgs.Add(args);
918+
919+
items.Clear();
920+
921+
Assert.Null(target.SelectedItem);
922+
Assert.Equal(-1, target.SelectedIndex);
923+
Assert.Single(receivedArgs);
924+
Assert.Empty(receivedArgs[0].AddedItems);
925+
Assert.Equal(2, receivedArgs[0].RemovedItems.Count);
926+
Assert.Contains(selected0, receivedArgs[0].RemovedItems.Cast<object>());
927+
Assert.Contains(selected2, receivedArgs[0].RemovedItems.Cast<object>());
928+
}
929+
930+
[Fact]
931+
public void Resetting_Items_With_Preserved_Selection_Should_Not_Report_Deselection()
932+
{
933+
var items = new ResettingCollection(3);
934+
935+
var target = new SelectingItemsControl
936+
{
937+
ItemsSource = items,
938+
Template = Template(),
939+
};
940+
941+
target.ApplyTemplate();
942+
target.SelectedIndex = 1;
943+
944+
var receivedArgs = new List<SelectionChangedEventArgs>();
945+
target.SelectionChanged += (_, args) => receivedArgs.Add(args);
946+
947+
items.Reset(new[] { "Item2", "Item0", "Item1" });
948+
949+
Assert.Equal("Item1", target.SelectedItem);
950+
Assert.Single(receivedArgs);
951+
Assert.Empty(receivedArgs[0].RemovedItems);
952+
}
953+
859954
[Fact]
860955
public void Raising_IsSelectedChanged_On_Item_Should_Update_Selection()
861956
{

0 commit comments

Comments
 (0)