feat: add move to trash to the file menu - #148
Merged
Merged
Conversation
Finishing a video and wanting it gone meant switching to the Finder to find it. File > Move to Trash now trashes the playing file, closes the player window and returns to the welcome window. Holding Option turns the item into Delete Immediately..., which asks first, the way the Finder does, and then deletes the file without going through the Trash. Either way the file's recents entry goes too, so the welcome window doesn't list a file that is no longer there. The welcome window is opened before the player closes, since closing the last window quits the app. A file that can't be removed, such as one on a volume with no Trash, leaves the player as it was and says why. The confirmation and the failure are SwiftUI alerts held by PresentationModel, so they share the one alert slot with the conversion and recent-file alerts rather than appearing over them.
The six strings added with Move to Trash shipped untranslated. Fifteen languages get them, worded after the Finder's own Delete Immediately confirmation and matching the catalog's existing form of address and quotation marks in each. Arabic, Greek, Finnish, Hungarian, Romanian, Turkish and Vietnamese are left for translators.
The Delete button removed whatever was playing when it was clicked. A file opened from the Finder while the confirmation was up replaced the playing one, so Delete removed that file, unconfirmed and unrecoverably, and left the one the alert named. Removal now takes the file it is meant for and does nothing if that is no longer the one playing. Also put a test's doc comment back above the test it describes.
Move to Trash and the Delete confirmation each removed the file and then switched windows themselves, and only one of them explained why the welcome window has to open before the player closes. Both now go through FileRemover, which holds that order in one place. FileRemover also takes the playing file, the recents store and the presentation model as dependencies rather than reaching for the shared instances. That puts the check against deleting a file other than the one the confirmation named under unit tests, along with the recents cleanup and the failure alert.
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.
Description:
Finishing a video and wanting it gone meant switching to the Finder to find it. File > Move to Trash now trashes the playing file, closes the player window and returns to the welcome window. There is no keyboard shortcut.
Holding Option turns the item into Delete Immediately.... It asks first, the way the Finder does (“Are you sure you want to delete “file.mkv”?” / “This item will be deleted immediately. You can’t undo this action.”), and then deletes the file without going through the Trash.
PresentationModel, so they share the one alert slot with the conversion and recent-file alerts. Both items are disabled while any sheet or alert is up, as the Playback items are.Testing
PresentationModelTestscase for the removal alert's slot.