Skip to content

Fix SelectionChanged not raised on collection Reset#20942

Open
NathanDrake2406 wants to merge 4 commits intoAvaloniaUI:masterfrom
NathanDrake2406:fix/listbox-selectionchanged-on-reset
Open

Fix SelectionChanged not raised on collection Reset#20942
NathanDrake2406 wants to merge 4 commits intoAvaloniaUI:masterfrom
NathanDrake2406:fix/listbox-selectionchanged-on-reset

Conversation

@NathanDrake2406
Copy link
Copy Markdown
Contributor

@NathanDrake2406 NathanDrake2406 commented Mar 19, 2026

What does the pull request do?

Fixes SelectingItemsControl.SelectionChanged not being raised when an items collection sends a Reset notification that clears the current selection. This covers controls such as ListBox bound to an ObservableCollection that is cleared.

What is the current behavior?

When a selected item is removed by a Reset, SelectionModel reports the loss through LostSelection. SelectingItemsControl only uses that callback to recover AlwaysSelected selection, so no public routed SelectionChanged event is raised for reset-to-empty cases.

What is the updated/expected behavior with this PR?

SelectionChanged is raised when a reset causes the control to lose its selected items. The event reports the previously selected items in RemovedItems and does not report removals when a reset preserves the selection.

Validated with:

  • dotnet run --project tests/Avalonia.Controls.UnitTests/Avalonia.Controls.UnitTests.csproj -- --filter-class "Avalonia.Controls.UnitTests.Primitives.SelectingItemsControlTests"
  • dotnet run --project tests/Avalonia.Controls.UnitTests/Avalonia.Controls.UnitTests.csproj -- --filter-class "Avalonia.Controls.UnitTests.Selection.InternalSelectionModelTests"
  • dotnet run --project tests/Avalonia.Controls.UnitTests/Avalonia.Controls.UnitTests.csproj -- --filter-namespace "Avalonia.Controls.UnitTests.Selection"

How was the solution implemented (if it's not obvious)?

SelectingItemsControl now keeps a snapshot of the last selected items at the control boundary. When an items Reset starts, the control captures that existing snapshot. If SelectionModel.LostSelection is committed for that reset, the control raises the routed SelectionChanged event using the captured items as RemovedItems.

This keeps SelectionModel reset semantics intact and avoids diffing the reset collection contents.

Checklist

Breaking changes

None.

Obsoletions / Deprecations

None.

Fixed issues

Fixes #20897

@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.0.999-cibuild0063669-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added bug backport-candidate-11.3.x Consider this PR for backporting to 11.3 branch labels Mar 19, 2026
@NathanDrake2406 NathanDrake2406 marked this pull request as draft April 14, 2026 08:10
@MrJul
Copy link
Copy Markdown
Member

MrJul commented Apr 29, 2026

@NathanDrake2406 Is there something missing that you converted this PR to a draft?

@NathanDrake2406 NathanDrake2406 marked this pull request as ready for review April 29, 2026 17:03
Copilot AI review requested due to automatic review settings April 29, 2026 17:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0065147-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@NathanDrake2406 NathanDrake2406 force-pushed the fix/listbox-selectionchanged-on-reset branch from ecab00d to f2e8388 Compare April 30, 2026 09:24
…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 AvaloniaUI#20897
@NathanDrake2406 NathanDrake2406 force-pushed the fix/listbox-selectionchanged-on-reset branch from 68eb1d7 to 7db50f8 Compare April 30, 2026 09:32
@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0065164-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread src/Avalonia.Controls/Selection/InternalSelectionModel.cs Outdated
@MrJul
Copy link
Copy Markdown
Member

MrJul commented Apr 30, 2026

I had a deeper look at this.

From my understanding, the current behavior is expected from the SelectionModel side: the SelectionModel.LostSelection event is raised instead of SelectionModel.SelectionChanged. Imo, we shouldn't try to diff after a reset as this might be costly: after all, there's a reason a Reset was received.

However, the SelectionModel.LostSelection event doesn't trigger result in any event in the SelectingItemsControl, which is the original issue, #20897.

An alternative would be to add a flag or operation type to SelectionChangedEventArgs, informing the user that it's a complete reset.

cc @grokys for additional thoughts.

@NathanDrake2406
Copy link
Copy Markdown
Contributor Author

@codex review the implementation and what @MrJul said.

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.
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.
@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.1.999-cibuild0065178-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-candidate-11.3.x Consider this PR for backporting to 11.3 branch bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ListBox does not raise SelectionChanged event when clearing items with NotifyCollectionChangedAction.Reset

4 participants