fix: refresh the Trash view in place after restore or permanent delete - #70
Merged
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #62.
The bug
The Trash list did not update when a snip was restored or permanently deleted — the removed card stayed on screen until the user navigated away from Trash and back.
Cause
SaveAndRefreshTrashAsyncrebuilt the view by assigning a replacementTrashViewModeltoCurrentContent. That works when the content type changes: the shell'sContentControlresolves a differentDataTemplateviaShellContentTemplateSelectorand rebuilds its visual tree. But aTrashViewModel→TrashViewModelswap keeps the same template, so the trash template's compiled bindings —ItemsSource="{x:Bind Snips}"andVisibility="{x:Bind IsEmpty, ...}", both one-time — keep pointing at the discarded view model's list.That also explains the reported workaround exactly: navigating away swaps in a different content type, which forces the rebuild.
Fix
TrashViewModelnow owns a persistentSnipscollection that a newLoad()repopulates in place, raising change notifications for the computedHasSnips/IsEmpty. The shell reuses the live instance when Trash is the current view:The
ItemsControlnow seesINotifyCollectionChangednotifications from a collection instance that survives the refresh, so the list tracks the document regardless of binding mode.This is the pattern
HistoryViewModelalready uses — the one content view that refreshes in place, and, tellingly, the only one whose template was already boundMode=OneWay. Trash is now consistent with it.The empty-state placeholder ("Trash is empty.") had the same root cause and additionally needed
Mode=OneWayto appear once the last trashed snip goes — that is the "clearing the trash" half of the report.I deliberately left the Home and CLI content paths alone: they assign replacement view models too, but their refresh is always driven by a flow that works today, and reworking them is out of scope for this fix.
Verification
DeleteForever_refreshes_the_live_trash_view_in_place,Deleting_the_last_trashed_snip_announces_the_empty_state,RestoreSnip_refreshes_the_live_trash_view_in_place) — I confirmed all three fail against the old implementation and pass with the fix, so they genuinely guard the regression.TrashViewModelTestscoveringLoadordering, collection-instance reuse, notifications and null rejection.Needs your eyes: I can compile the WinUI head but not run it, so the on-screen behaviour is unverified — worth a quick manual check that restore, delete-permanently and emptying the last item all update the list live.
One question
There is no "empty trash" / "clear all" command anywhere in the codebase —
RestoreSnipandDeleteForeverare the only trash actions. I read the "clearing the trash" half of the issue as deleting the last remaining item, which this fixes. If you meant a bulk Empty trash button, that is a new feature rather than this bug — say the word and I will raise it separately.🤖 Generated with Claude Code