Skip to content

Reorder the tree rows when the assembly list is sorted - #4138

Merged
siegfriedpammer merged 1 commit into
masterfrom
fix/assembly-list-sort-move
Sep 14, 2026
Merged

siegfriedpammer merged 1 commit into
masterfrom
fix/assembly-list-sort-move

Conversation

@siegfriedpammer

Copy link
Copy Markdown
Member

Sorting the assembly list reorders it in place and reports each step as a
Move. Nothing downstream could act on that: AssemblyListTreeNode's handler
had cases for Add, Remove and Reset only, and neither
SharpTreeNodeCollection nor TreeFlattener had a move at all, so the rows kept
their pre-sort order until something else forced a rebuild.

This adds the move to all three layers:

  • SharpTreeNodeCollection.Move(oldIndex, newIndex) reorders the backing list
    and raises a single NotifyCollectionChangedAction.Move.
  • SharpTreeNode.OnChildrenChanged handles it without the attach/detach work the
    other actions need - the node keeps its parent and its visibility - and moves
    the node's run in the flat list in one step.
  • TreeFlattener.NodesMoved raises one ranged Move towards the ItemsSource.
  • AssemblyListTreeNode reorders its children instead of ignoring the
    notification.

Moving the node rather than removing and re-inserting it keeps its identity, so
an expanded subtree stays expanded and its row is not rebuilt.

The one thing worth a second opinion

A run longer than one row has to be reported the way the consumer reads it:
Avalonia's VirtualizingStackPanel applies a ranged move by removing
OldItems.Count rows and re-inserting them at NewStartingIndex - (Count - 1).
A run reported by its final start index therefore lands one run-length short -
the headless test in this PR renders A, C, B, C instead of A, C, B, B1 if
NodesMoved reports the start. It now reports the row the run ends on for a
forward move, which is documented at the call site. For a single row - a
collapsed node, and every move a sort makes - the two readings coincide.

Also worth knowing: a Move does not carry the selection.
SelectionNodeBase.OnSourceCollectionChanged in Avalonia handles Move as
remove-then-add and drops the selection of the moved items. Restoring it would be
a TreeSelectionBinder change and is not part of this PR.

Tests

Five model tests in FlatListTreeNodeTests (event shape, no-op move, flattened
order, expanded-run move, ranged-event indices) and one headless rendering test in
SharpTreeViewTests. Full ILSpy.Tests run: 1283 tests, 0 failed, 3 skipped.

Opened by an AI agent (Claude) on Siegfried's behalf.

Sorting reorders the assembly list in place and reports it as a Move, which
nothing downstream could act on: the tree node's handler had cases for Add,
Remove and Reset only, and neither the child collection nor the flattener had a
move at all. The rows therefore kept their pre-sort order until something else
forced a rebuild. Moving the node rather than removing and re-inserting it keeps
its identity, so an expanded subtree stays expanded and its row is not rebuilt.

A run longer than one row has to be reported the way the consumer reads it:
Avalonia's VirtualizingStackPanel applies a ranged move by removing OldItems.Count
rows and re-inserting them at NewStartingIndex - (Count - 1), so a run reported by
its final start index lands short by its own length. For a single row - a
collapsed node, and every move a sort makes - the two readings coincide.

Assisted-by: Claude:claude-opus-5[1m]:Claude Code
@siegfriedpammer
siegfriedpammer merged commit c9f9008 into master Sep 14, 2026
17 checks passed
@siegfriedpammer
siegfriedpammer deleted the fix/assembly-list-sort-move branch September 14, 2026 04:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant