From f179ab72f1e891cf63bd5e811883c465b117e79e Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Wed, 19 Aug 2026 15:17:23 +0000 Subject: [PATCH] fix: refresh the Trash view in place after restore or permanent delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #62. The Trash list did not update when a snip was restored or permanently deleted; navigating away from Trash and back was needed to see the change. The shell rebuilt the view by assigning a replacement TrashViewModel to CurrentContent. That works when the content *type* changes, because the shell's ContentControl then resolves a different DataTemplate and rebuilds its tree, but a TrashViewModel -> TrashViewModel swap keeps the same template and leaves the one-time compiled bindings pointing at the old list. TrashViewModel now owns a persistent Snips collection that Load() repopulates in place, raising notifications for the computed HasSnips / IsEmpty, and the shell reuses the live instance when Trash is the current view. The ItemsControl sees collection-change notifications, so the list tracks the document. This is the pattern HistoryViewModel already uses — the one content view that refreshes in place, and the only one whose template was already bound Mode=OneWay. The empty-state placeholder had the same root cause and needed Mode=OneWay to appear when the last trashed snip goes, which is the "clearing the trash" half of the report. Covered by three shell tests that fail against the old assign-a-replacement implementation, plus direct TrashViewModel tests for Load. 253 Core tests pass and the full solution builds clean on the Windows head. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 11 +-- src/Snipdeck.App/Views/ShellPage.xaml | 2 +- .../ViewModels/ShellViewModel.cs | 12 ++- .../ViewModels/TrashViewModel.cs | 31 +++++-- .../ViewModels/ShellViewModelCommandsTests.cs | 83 +++++++++++++++++++ .../ViewModels/TrashViewModelTests.cs | 67 +++++++++++++++ 6 files changed, 192 insertions(+), 14 deletions(-) create mode 100644 tests/Snipdeck.Core.Tests/ViewModels/TrashViewModelTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 36751ab..837846a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,16 +10,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - Migrated the test suites to xUnit v3 on Microsoft Testing Platform, replacing the VSTest stack. Development-only; the shipped app is unaffected. +- Refreshed dependencies to their latest stable releases, including the Windows + App SDK (2.4.0), WebView2, the Windows SDK build tools, SQLite, the dependency + injection container, Spectre.Console and the test tooling. ### Fixed +- The Trash view now updates as soon as a snip is restored or permanently + deleted, and shows its empty-state message once the last item goes. Previously + the list only refreshed after navigating away from Trash and back. - CI now runs the `Snipdeck.Execution` test suite, on both Ubuntu and Windows. It was built but never executed, so 61 tests were only ever run locally. -### Changed -- Refreshed dependencies to their latest stable releases, including the Windows - App SDK (2.4.0), WebView2, the Windows SDK build tools, SQLite, the dependency - injection container, Spectre.Console and the test tooling. - ## [1.1.0] - 2026-07-01 ### Added diff --git a/src/Snipdeck.App/Views/ShellPage.xaml b/src/Snipdeck.App/Views/ShellPage.xaml index 15ed934..ec63d70 100644 --- a/src/Snipdeck.App/Views/ShellPage.xaml +++ b/src/Snipdeck.App/Views/ShellPage.xaml @@ -453,7 +453,7 @@ + Visibility="{x:Bind IsEmpty, Mode=OneWay, Converter={StaticResource BoolToVisibility}}" /> diff --git a/src/Snipdeck.Core/ViewModels/ShellViewModel.cs b/src/Snipdeck.Core/ViewModels/ShellViewModel.cs index 9f4c8ea..d60b499 100644 --- a/src/Snipdeck.Core/ViewModels/ShellViewModel.cs +++ b/src/Snipdeck.Core/ViewModels/ShellViewModel.cs @@ -893,7 +893,17 @@ private async Task SaveAndRefreshTrashAsync() _suppressShellRefresh = false; } - CurrentContent = BuildTrashViewModel(); + // Refresh the Trash list in place when it is the live view, rather than + // swapping in a replacement: the content area binds a template per + // view-model type, so a same-type swap leaves the old list on screen. + if (CurrentContent is TrashViewModel trash) + { + trash.Load(_document.Snips.Where(s => s.IsTrash)); + } + else + { + CurrentContent = BuildTrashViewModel(); + } } private async Task SaveAndRefreshAsync() diff --git a/src/Snipdeck.Core/ViewModels/TrashViewModel.cs b/src/Snipdeck.Core/ViewModels/TrashViewModel.cs index e7d78c6..09fba1c 100644 --- a/src/Snipdeck.Core/ViewModels/TrashViewModel.cs +++ b/src/Snipdeck.Core/ViewModels/TrashViewModel.cs @@ -16,18 +16,35 @@ public sealed partial class TrashViewModel : ObservableObject { public TrashViewModel(IEnumerable trashedSnips) { - ArgumentNullException.ThrowIfNull(trashedSnips); - - Snips = new ObservableCollection( - trashedSnips - .OrderBy(s => s.Title, StringComparer.OrdinalIgnoreCase) - .Select(s => new SnipCardViewModel(s))); + Load(trashedSnips); } - public ObservableCollection Snips { get; } + /// The trashed snips currently shown, ordered by title. + public ObservableCollection Snips { get; } = []; public bool HasSnips => Snips.Count > 0; public bool IsEmpty => Snips.Count == 0; + + /// + /// Repopulates the list in place. The shell reuses the live instance after a + /// restore or a permanent delete rather than building a replacement, so the + /// content area sees collection-change notifications: its bindings resolve + /// once per template instantiation, and swapping in a new view model of the + /// same type leaves the stale list on screen until the user navigates away. + /// + public void Load(IEnumerable trashedSnips) + { + ArgumentNullException.ThrowIfNull(trashedSnips); + + Snips.Clear(); + foreach (var snip in trashedSnips.OrderBy(s => s.Title, StringComparer.OrdinalIgnoreCase)) + { + Snips.Add(new SnipCardViewModel(snip)); + } + + OnPropertyChanged(nameof(HasSnips)); + OnPropertyChanged(nameof(IsEmpty)); + } } } diff --git a/tests/Snipdeck.Core.Tests/ViewModels/ShellViewModelCommandsTests.cs b/tests/Snipdeck.Core.Tests/ViewModels/ShellViewModelCommandsTests.cs index 3743ece..e27143a 100644 --- a/tests/Snipdeck.Core.Tests/ViewModels/ShellViewModelCommandsTests.cs +++ b/tests/Snipdeck.Core.Tests/ViewModels/ShellViewModelCommandsTests.cs @@ -647,6 +647,89 @@ public async Task DeleteForever_only_removes_the_snip_when_confirmed() Assert.Empty(trash.Snips); } + [Fact] + public async Task DeleteForever_refreshes_the_live_trash_view_in_place() + { + // Regression: the shell used to swap in a replacement TrashViewModel, which + // the content area does not re-read for a same-type change — the deleted + // snip stayed on screen until the user navigated away and back. + Cli cli = null!; + var (vm, _, _, ix, _) = await BuildAsync(d => + { + cli = new Cli { Name = "pl-app" }; + d.Clis.Add(cli); + d.Snips.Add(new Snip { CliId = cli.Id, Title = "Keep", CommandTemplate = "k", IsTrash = true }); + d.Snips.Add(new Snip { CliId = cli.Id, Title = "Purge", CommandTemplate = "p", IsTrash = true }); + }); + + vm.OpenTrash(); + var trash = Assert.IsType(vm.CurrentContent); + var snips = trash.Snips; + var card = trash.Snips.Single(c => c.Title == "Purge"); + + ix.NextConfirmResult = true; + await vm.DeleteForeverCommand.ExecuteAsync(card); + + // Same view model and same collection instance, so the bound list updates. + Assert.Same(trash, vm.CurrentContent); + Assert.Same(snips, trash.Snips); + var remaining = Assert.Single(trash.Snips); + Assert.Equal("Keep", remaining.Title); + } + + [Fact] + public async Task Deleting_the_last_trashed_snip_announces_the_empty_state() + { + Cli cli = null!; + var (vm, _, _, ix, _) = await BuildAsync(d => + { + cli = new Cli { Name = "pl-app" }; + d.Clis.Add(cli); + d.Snips.Add(new Snip { CliId = cli.Id, Title = "Last", CommandTemplate = "l", IsTrash = true }); + }); + + vm.OpenTrash(); + var trash = Assert.IsType(vm.CurrentContent); + Assert.False(trash.IsEmpty); + + var changed = new List(); + trash.PropertyChanged += (_, e) => changed.Add(e.PropertyName!); + + ix.NextConfirmResult = true; + await vm.DeleteForeverCommand.ExecuteAsync(trash.Snips[0]); + + Assert.Empty(trash.Snips); + Assert.True(trash.IsEmpty); + Assert.False(trash.HasSnips); + // Without these the "Trash is empty" placeholder never appears. + Assert.Contains(nameof(TrashViewModel.IsEmpty), changed); + Assert.Contains(nameof(TrashViewModel.HasSnips), changed); + } + + [Fact] + public async Task RestoreSnip_refreshes_the_live_trash_view_in_place() + { + Cli cli = null!; + var (vm, _, _, _, _) = await BuildAsync(d => + { + cli = new Cli { Name = "pl-app" }; + d.Clis.Add(cli); + d.Snips.Add(new Snip { CliId = cli.Id, Title = "Keep", CommandTemplate = "k", IsTrash = true }); + d.Snips.Add(new Snip { CliId = cli.Id, Title = "Restore me", CommandTemplate = "r", IsTrash = true }); + }); + + vm.OpenTrash(); + var trash = Assert.IsType(vm.CurrentContent); + var snips = trash.Snips; + + await vm.RestoreSnipCommand.ExecuteAsync(trash.Snips.Single(c => c.Title == "Restore me")); + + Assert.Same(trash, vm.CurrentContent); + Assert.Same(snips, trash.Snips); + var remaining = Assert.Single(trash.Snips); + Assert.Equal("Keep", remaining.Title); + } + [Fact] public async Task NewCli_adds_the_cli_and_writes_icon_bytes_when_provided() { diff --git a/tests/Snipdeck.Core.Tests/ViewModels/TrashViewModelTests.cs b/tests/Snipdeck.Core.Tests/ViewModels/TrashViewModelTests.cs new file mode 100644 index 0000000..8d905d9 --- /dev/null +++ b/tests/Snipdeck.Core.Tests/ViewModels/TrashViewModelTests.cs @@ -0,0 +1,67 @@ +using Snipdeck.Core.Models; +using Snipdeck.Core.ViewModels; + +namespace Snipdeck.Core.Tests.ViewModels +{ + public class TrashViewModelTests + { + private static Snip Trashed(string title) + { + return new Snip { CliId = Guid.NewGuid(), Title = title, CommandTemplate = "x", IsTrash = true }; + } + + [Fact] + public void Constructor_orders_snips_by_title_case_insensitively() + { + var vm = new TrashViewModel([Trashed("beta"), Trashed("Alpha"), Trashed("gamma")]); + + Assert.Equal(["Alpha", "beta", "gamma"], vm.Snips.Select(s => s.Title)); + } + + [Fact] + public void Empty_trash_reports_the_empty_state() + { + var vm = new TrashViewModel([]); + + Assert.True(vm.IsEmpty); + Assert.False(vm.HasSnips); + } + + [Fact] + public void Load_repopulates_the_same_collection_instance() + { + // The shell relies on this: the bound list only updates if the collection + // instance survives, because the content area resolves its bindings once. + var vm = new TrashViewModel([Trashed("first")]); + var collection = vm.Snips; + + vm.Load([Trashed("second"), Trashed("third")]); + + Assert.Same(collection, vm.Snips); + Assert.Equal(["second", "third"], vm.Snips.Select(s => s.Title)); + } + + [Fact] + public void Load_raises_change_notifications_for_the_computed_state() + { + var vm = new TrashViewModel([Trashed("only")]); + var changed = new List(); + vm.PropertyChanged += (_, e) => changed.Add(e.PropertyName!); + + vm.Load([]); + + Assert.True(vm.IsEmpty); + Assert.False(vm.HasSnips); + Assert.Contains(nameof(TrashViewModel.IsEmpty), changed); + Assert.Contains(nameof(TrashViewModel.HasSnips), changed); + } + + [Fact] + public void Load_rejects_a_null_sequence() + { + var vm = new TrashViewModel([]); + + _ = Assert.Throws(() => vm.Load(null!)); + } + } +}