From cf23f66dc7a2439a8b9ec6a2190fc82803e8ecd0 Mon Sep 17 00:00:00 2001 From: niksedk Date: Wed, 5 Aug 2026 12:00:40 +0200 Subject: [PATCH] Sync TableView SelectedItems with the always-selected row 0 SelectionMode.AlwaysSelected picks row 0 the moment ItemsSource is assigned to a populated collection, but that pick only reaches the internal selection model: the row is drawn highlighted and SelectedItem/SelectedIndex point at it, while the SelectedItems collection stays empty and no SelectionChanged is raised. Neither a layout pass nor a Selection.Clear()/Select(0) repairs it - only moving the selection to another row does. Subtitle Edit reads every selection through SelectedItems, so the grid showed the first row highlighted while the application saw nothing selected (#13230): - the edit box, Show and Duration went blank a few seconds after start-up, when the open-last-file restore reached SetRecentFileProperties -> UpdateVideoOffsetStatus -> SubtitleGridSelectionChanged, which read the empty SelectedItems and cleared SelectedSubtitle - a shift-selection starting at row 1 left row 1 out of the selection, so italic, copy, delete and friends silently skipped the first line Repair the collection from the selection model on every ItemsSource change, in MakeTableView so every grid in the application is covered rather than the main subtitle grid alone. Co-Authored-By: Claude Opus 5 --- src/ui/Logic/TableViewExtras.cs | 50 ++++++ tests/UI/Logic/TableViewSelectionSyncTests.cs | 146 ++++++++++++++++++ 2 files changed, 196 insertions(+) create mode 100644 tests/UI/Logic/TableViewSelectionSyncTests.cs diff --git a/src/ui/Logic/TableViewExtras.cs b/src/ui/Logic/TableViewExtras.cs index b146b899ee..62f03cf07a 100644 --- a/src/ui/Logic/TableViewExtras.cs +++ b/src/ui/Logic/TableViewExtras.cs @@ -273,6 +273,18 @@ public static TableView MakeTableView(bool alwaysSelected = true, bool multiSele UiUtil.ApplyTableViewRowStyle(tableView); + // SelectionMode.AlwaysSelected picks row 0 the moment ItemsSource is assigned, but that + // pick only reaches the internal selection model (and SelectedItem/SelectedIndex) - the + // SelectedItems collection stays empty and no SelectionChanged is raised. Repair it + // (#13230), see SyncSelectedItemsWithSelection. + tableView.PropertyChanged += (_, e) => + { + if (e.Property == ItemsControl.ItemsSourceProperty) + { + SyncSelectedItemsWithSelection(tableView); + } + }; + // Home/End/PageUp/PageDown (and Ctrl+Home/End) list navigation in every grid (#13194). // Handlers attached earlier in the routing path (window-level tunnel handlers, shortcut // dispatch) still win; this is the fallback when nothing else handled the key. @@ -281,6 +293,44 @@ public static TableView MakeTableView(bool alwaysSelected = true, bool multiSele return tableView; } + /// + /// Puts the rows the control considers selected into its + /// collection when that collection is empty but the selection model is not. + /// + /// selects row 0 as soon as ItemsSource is assigned + /// to a populated collection. That selection reaches the selection model - the row is drawn + /// highlighted and SelectedItem/SelectedIndex point at it - but SelectedItems is left empty and + /// no SelectionChanged is raised, and neither a layout pass nor a Selection.Clear()/Select(0) + /// repairs it; only moving the selection to a different row does. Everything in Subtitle Edit + /// reads the selection through SelectedItems, so the grid showed row 1 highlighted while the + /// app saw nothing selected: the edit box, Show and Duration went blank the moment anything + /// re-read the selection, and a shift-selection starting at row 1 silently left row 1 out of + /// every operation (issue #13230). + /// + /// + public static void SyncSelectedItemsWithSelection(TableView tableView) + { + var selectedItems = tableView.SelectedItems; + if (selectedItems == null || selectedItems.Count > 0) + { + return; + } + + var selection = tableView.Selection; + if (selection.Count == 0) + { + return; + } + + foreach (var item in selection.SelectedItems) + { + if (item != null) + { + selectedItems.Add(item); + } + } + } + /// /// A read-only text cell whose flow direction follows its own content, the way the /// main subtitle grid's text cells do. Use it for every column showing subtitle text diff --git a/tests/UI/Logic/TableViewSelectionSyncTests.cs b/tests/UI/Logic/TableViewSelectionSyncTests.cs new file mode 100644 index 0000000000..e44bfafa8f --- /dev/null +++ b/tests/UI/Logic/TableViewSelectionSyncTests.cs @@ -0,0 +1,146 @@ +using Avalonia.Controls; +using Avalonia.Data; +using Avalonia.Headless.XUnit; +using Nikse.SubtitleEdit.Logic; +using System.Collections.ObjectModel; +using System.ComponentModel; + +namespace UITests.Logic; + +/// +/// SelectionMode.AlwaysSelected picks row 0 when ItemsSource is assigned without ever filling the +/// control's SelectedItems collection. Subtitle Edit reads every selection through SelectedItems, +/// so the grid showed the first row highlighted while the app saw nothing selected - the edit box +/// and time codes went blank and a selection starting at row 1 left row 1 out (issue #13230). +/// repairs the collection on every ItemsSource change. +/// +public class TableViewSelectionSyncTests +{ + private sealed class Row + { + public string Text { get; set; } = string.Empty; + } + + private sealed class Vm : INotifyPropertyChanged + { + private object? _selected; + private int? _selectedIndex; + + public object? Selected + { + get => _selected; + set + { + _selected = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Selected))); + } + } + + public int? SelectedIndex + { + get => _selectedIndex; + set + { + _selectedIndex = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(SelectedIndex))); + } + } + + public event PropertyChangedEventHandler? PropertyChanged; + } + + /// + /// The main subtitle grid: multi-select, always selected, with the view model's current row and + /// index bound two-way - and filled the way SetSubtitles fills it (detach, fill, re-attach). + /// + private static (TableView Grid, Vm Vm, ObservableCollection Items) MakeGridWithRows(int rowCount) + { + var items = new ObservableCollection(); + var vm = new Vm(); + + var grid = TableViewExtras.MakeTableView(); + grid.Columns.Add(new TableViewColumn { Header = "Text", Binding = new Binding(nameof(Row.Text)) }); + grid.ItemsSource = items; + grid[!TableView.SelectedItemProperty] = new Binding(nameof(vm.Selected)) { Mode = BindingMode.TwoWay, Source = vm }; + grid[!TableView.SelectedIndexProperty] = new Binding(nameof(vm.SelectedIndex)) { Mode = BindingMode.TwoWay, Source = vm }; + + new Window { Width = 400, Height = 300, Content = grid }.Show(); + + grid.ItemsSource = null; + for (var i = 1; i <= rowCount; i++) + { + items.Add(new Row { Text = "line " + i }); + } + + grid.ItemsSource = items; + + return (grid, vm, items); + } + + [AvaloniaFact] + public void FirstRowIsInSelectedItemsAfterFillingTheGrid() + { + var (grid, vm, items) = MakeGridWithRows(20); + + Assert.Same(items[0], grid.SelectedItem); + Assert.Same(items[0], vm.Selected); + Assert.Equal(1, grid.Selection.Count); + + // Without the repair this is empty: the row is highlighted but nothing is selected as far + // as the rest of the application can tell. + Assert.Single(grid.SelectedItems!); + Assert.Same(items[0], grid.SelectedItems![0]); + } + + [AvaloniaFact] + public void SelectingARangeFromTheFirstRowKeepsTheFirstRow() + { + var (grid, _, items) = MakeGridWithRows(20); + + // How the main grid applies a shift-selection (SelectGridRange). + grid.Selection.BeginBatchUpdate(); + grid.Selection.Clear(); + grid.Selection.Select(4); + grid.Selection.SelectRange(0, 4); + grid.Selection.EndBatchUpdate(); + + Assert.Equal(5, grid.Selection.Count); + + // Without the repair row 0 is missing here, so italic/copy/delete skipped the first line. + var selected = grid.SelectedItems!.Cast().Select(r => r.Text).Order().ToArray(); + Assert.Equal(new[] { "line 1", "line 2", "line 3", "line 4", "line 5" }, selected); + Assert.Contains(items[0], grid.SelectedItems!.Cast()); + } + + [AvaloniaFact] + public void EmptyGridSelectsNothing() + { + var (grid, vm, _) = MakeGridWithRows(0); + + Assert.Equal(0, grid.Selection.Count); + Assert.Empty(grid.SelectedItems!); + Assert.Null(grid.SelectedItem); + Assert.Null(vm.Selected); + } + + [AvaloniaFact] + public void RefillingTheGridKeepsSelectedItemsInSync() + { + var (grid, vm, _) = MakeGridWithRows(20); + + // A later reload (format change, undo, import, ...) goes through the same detach/fill/attach. + var newItems = new ObservableCollection(); + for (var i = 1; i <= 5; i++) + { + newItems.Add(new Row { Text = "new " + i }); + } + + grid.ItemsSource = null; + grid.ItemsSource = newItems; + + Assert.Same(newItems[0], grid.SelectedItem); + Assert.Same(newItems[0], vm.Selected); + Assert.Single(grid.SelectedItems!); + Assert.Same(newItems[0], grid.SelectedItems![0]); + } +}