From 241f2a96396e650ece44aa55485e4ad3e71ae827 Mon Sep 17 00:00:00 2001 From: itsjoshpark Date: Sat, 26 Sep 2026 01:18:48 -0400 Subject: [PATCH 1/4] feat: add move to trash to the file menu 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. --- .sparkle/notes/next.md | 4 +- .../Models/PresentationModelTests.swift | 16 ++ .../ControlAndMenuStateUITests.swift | 61 +++++++ Front Row/ContentView.swift | 1 + Front Row/Files/FileRemovalAlert.swift | 151 ++++++++++++++++++ Front Row/Main Menu/FileCommands.swift | 30 ++++ Front Row/Models/PresentationModel.swift | 23 ++- Front Row/Resources/Localizable.xcstrings | 24 ++- 8 files changed, 302 insertions(+), 8 deletions(-) create mode 100644 Front Row/Files/FileRemovalAlert.swift diff --git a/.sparkle/notes/next.md b/.sparkle/notes/next.md index a1dab14..3fe0575 100644 --- a/.sparkle/notes/next.md +++ b/.sparkle/notes/next.md @@ -1,3 +1 @@ -- New: -- Changed: -- Fixed: +- New: File ▸ Move to Trash moves the playing file to the Trash and returns to the welcome window. Hold Option for Delete Immediately…, which deletes it outright after asking diff --git a/Front Row Tests/Models/PresentationModelTests.swift b/Front Row Tests/Models/PresentationModelTests.swift index f83aa88..983b40f 100644 --- a/Front Row Tests/Models/PresentationModelTests.swift +++ b/Front Row Tests/Models/PresentationModelTests.swift @@ -128,6 +128,22 @@ struct PresentationModelTests { /// Playback commands stay disabled while any question is up, which is the one thing outside /// this file that reads the slot. + /// Confirming a deletion shares the slot with the other questions, and gives it back. + @Test + func aFileRemovalAlertHoldsTheSlotUntilDismissed() { + let model = PresentationModel() + #expect(model.raise(.confirmDeletion(film))) + + #expect(!model.raise(problem(film)), "A conversion talked over a deletion") + #expect(!model.raise(unopenable(film)), "A recent-file alert talked over a deletion") + #expect(!model.raise(.confirmDeletion(film)), "A second deletion replaced the first") + + model.dismissFileRemovalAlert() + #expect(!model.isAskingAboutAFile) + #expect(model.raise(problem(film))) + #expect(!model.raise(.confirmDeletion(film)), "A deletion talked over a conversion") + } + @Test func aQuestionCountsAsPresenting() { let model = PresentationModel() diff --git a/Front Row UI Tests/ControlAndMenuStateUITests.swift b/Front Row UI Tests/ControlAndMenuStateUITests.swift index e16f7c4..b0bbdd6 100644 --- a/Front Row UI Tests/ControlAndMenuStateUITests.swift +++ b/Front Row UI Tests/ControlAndMenuStateUITests.swift @@ -34,6 +34,10 @@ final class ControlAndMenuStateUITests: FrontRowUITestCase { menus.states(of: "File", for: ["Show in Finder"])["Show in Finder"], false, "File ▸ Show in Finder" ) + assertEqual( + menus.states(of: "File", for: ["Move to Trash"])["Move to Trash"], false, + "File ▸ Move to Trash" + ) assertEqual( menus.states(of: "Window", for: ["Natural Size"])["Natural Size"], false, "Window ▸ Natural Size" @@ -54,6 +58,63 @@ final class ControlAndMenuStateUITests: FrontRowUITestCase { ) } + func testMoveToTrashTrashesTheFileAndReturnsToTheWelcomeWindow() async throws { + let movie = try await MediaFixtures.makeMovie( + size: CGSize(width: 640, height: 360), named: "trash", in: fixtures) + try openInFinder(movie) + let player = try playerWindow(for: movie) + waitForSizeToSettle(player) + + menus.click("Move to Trash", in: "File") + + XCTAssertTrue( + app.windows["welcome"].waitForExistence(timeout: 10), + "The welcome window did not come back" + ) + XCTAssertTrue( + player.waitForNonExistence(timeout: 10), "The player window is still open") + XCTAssertFalse( + FileManager.default.fileExists(atPath: movie.path(percentEncoded: false)), + "The file is still where it was" + ) + XCTAssertEqual(app.state, .runningForeground, "The app quit") + } + + func testDeleteImmediatelyAsksFirstAndReturnsToTheWelcomeWindow() async throws { + let movie = try await MediaFixtures.makeMovie( + size: CGSize(width: 640, height: 360), named: "delete", in: fixtures) + try openInFinder(movie) + let player = try playerWindow(for: movie) + waitForSizeToSettle(player) + let path = movie.path(percentEncoded: false) + + let confirmation = player.sheets.firstMatch + XCUIElement.perform(withKeyModifiers: .option) { + menus.click("Delete Immediately...", in: "File") + } + XCTAssertTrue(confirmation.waitForExistence(timeout: 10), "No confirmation was asked for") + confirmation.buttons["Cancel"].click() + XCTAssertTrue( + confirmation.waitForNonExistence(timeout: 10), "The confirmation stayed up") + XCTAssertTrue(FileManager.default.fileExists(atPath: path), "Cancel deleted the file") + XCTAssertTrue(player.exists, "Cancel closed the player window") + + XCUIElement.perform(withKeyModifiers: .option) { + menus.click("Delete Immediately...", in: "File") + } + XCTAssertTrue(confirmation.waitForExistence(timeout: 10), "No confirmation was asked for") + confirmation.buttons["Delete"].click() + + XCTAssertTrue( + app.windows["welcome"].waitForExistence(timeout: 10), + "The welcome window did not come back" + ) + XCTAssertTrue( + player.waitForNonExistence(timeout: 10), "The player window is still open") + XCTAssertFalse(FileManager.default.fileExists(atPath: path), "The file is still there") + XCTAssertEqual(app.state, .runningForeground, "The app quit") + } + /// The item is titled for what it will do, so it reads "Pause" only while something is playing. func testPlayPauseItemFollowsWhetherAnythingIsPlaying() async throws { try await openFixture(named: "clip") diff --git a/Front Row/ContentView.swift b/Front Row/ContentView.swift index a46f191..236702d 100644 --- a/Front Row/ContentView.swift +++ b/Front Row/ContentView.swift @@ -41,6 +41,7 @@ struct ContentView: View { } .unopenableRecentFileAlert(in: .player) .remuxAlert(in: .player) + .fileRemovalAlert() .onAppear { chrome.mouseMoved() } diff --git a/Front Row/Files/FileRemovalAlert.swift b/Front Row/Files/FileRemovalAlert.swift new file mode 100644 index 0000000..8846961 --- /dev/null +++ b/Front Row/Files/FileRemovalAlert.swift @@ -0,0 +1,151 @@ +// +// FileRemovalAlert.swift +// Front Row +// +// Created by Joshua Park on 9/26/26. +// + +import SwiftUI + +/// How the playing file is taken away. +enum FileRemoval: Equatable { + case trash + case delete +} + +/// A file that couldn't be moved to the Trash or deleted, and what went wrong. +struct FileRemovalFailure { + var url: URL + var removal: FileRemoval + var reason: String +} + +/// What removing the playing file is asking the user. Only the player window presents it, since +/// only a file playing there can be removed. +enum FileRemovalAlert { + case confirmDeletion(URL) + case failure(FileRemovalFailure) +} + +/// Moves the playing file to the Trash or deletes it, and drops its recents entry, which would +/// otherwise point at a file that's gone. A failure is raised as an alert instead. +/// +/// Runs while the engine still holds the file's security-scoped access. +/// - Returns: Whether the file is gone. +@MainActor +@discardableResult +func removeCurrentFile(_ removal: FileRemoval) -> Bool { + let playEngine = PlayEngine.shared + guard playEngine.isLocalFile, let url = playEngine.fileURL else { return false } + do { + switch removal { + case .trash: try FileManager.default.trashItem(at: url, resultingItemURL: nil) + case .delete: try FileManager.default.removeItem(at: url) + } + } catch { + PresentationModel.shared.raise( + .failure( + FileRemovalFailure( + url: url, removal: removal, reason: error.localizedDescription))) + return false + } + RecentDocumentsStore.shared.removeRecentDocument(url) + return true +} + +extension View { + /// Presents the file removal alert. Applied to the player window only. + /// + /// Once the file is gone, the welcome window is opened before the player closes, since closing + /// the last window quits the app. + func fileRemovalAlert() -> some View { + modifier(FileRemovalAlertModifier()) + } +} + +private struct FileRemovalAlertModifier: ViewModifier { + @Environment(PresentationModel.self) private var presentationModel: PresentationModel + @Environment(\.openWindow) private var openWindow + @Environment(\.dismissWindow) private var dismissWindow + + private var isPresented: Binding { + Binding( + get: { presentationModel.fileRemovalAlert != nil }, + set: { isPresented in + if !isPresented { presentationModel.dismissFileRemovalAlert() } + } + ) + } + + func body(content: Content) -> some View { + content.alert( + FileRemovalAlertTitle.text(for: presentationModel.fileRemovalAlert), + isPresented: isPresented, + presenting: presentationModel.fileRemovalAlert + ) { alert in + switch alert { + case .confirmDeletion: + Button(role: .destructive) { + // After the confirmation has gone: a failure raised from here would find + // the slot still taken. + Task { + guard removeCurrentFile(.delete) else { return } + openWindow(id: WindowID.welcome) + dismissWindow(id: WindowID.main) + } + } label: { + Text("Delete", comment: "Alert button that confirms Delete Immediately") + } + Button(role: .cancel) { + } label: { + Text("Cancel", comment: "Alert button that cancels Delete Immediately") + } + case .failure: + Button { + } label: { + Text( + "OK", + comment: "Dismisses the alert shown when a file couldn’t be removed" + ) + } + } + } message: { alert in + switch alert { + case .confirmDeletion: + Text( + "This item will be deleted immediately. You can’t undo this action.", + comment: "Message of the alert confirming Delete Immediately" + ) + case .failure(let failure): + Text(failure.reason) + } + } + .onDisappear { presentationModel.dismissFileRemovalAlert() } + } +} + +/// An alert title has to be a `Text`, so this is a function rather than a view. +private enum FileRemovalAlertTitle { + static func text(for alert: FileRemovalAlert?) -> Text { + switch alert { + case .confirmDeletion(let url): + Text( + "Are you sure you want to delete “\(url.lastPathComponent)”?", + comment: + "Title of the alert confirming Delete Immediately; the argument is a file name" + ) + case .failure(let failure) where failure.removal == .trash: + Text( + "Couldn’t Move “\(failure.url.lastPathComponent)” to the Trash", + comment: "Title of the alert shown when a file couldn’t be moved to the Trash" + ) + case .failure(let failure): + Text( + "Couldn’t Delete “\(failure.url.lastPathComponent)”", + comment: "Title of the alert shown when a file couldn’t be deleted" + ) + case nil: + Text(verbatim: "") + } + } +} diff --git a/Front Row/Main Menu/FileCommands.swift b/Front Row/Main Menu/FileCommands.swift index 5723115..7e6d356 100644 --- a/Front Row/Main Menu/FileCommands.swift +++ b/Front Row/Main Menu/FileCommands.swift @@ -9,6 +9,9 @@ import AVKit import SwiftUI struct FileCommands: Commands { + @Environment(\.openWindow) private var openWindow + @Environment(\.dismissWindow) private var dismissWindow + private let playEngine = PlayEngine.shared private let presentationModel = PresentationModel.shared private let recentDocumentsStore = RecentDocumentsStore.shared @@ -86,6 +89,33 @@ struct FileCommands: Commands { ) } .disabled(!playEngine.isLocalFile) + + Divider() + + Button { + guard removeCurrentFile(.trash) else { return } + // Opened first: closing the last window quits the app. + openWindow(id: WindowID.welcome) + dismissWindow(id: WindowID.main) + } label: { + Text( + "Move to Trash", + comment: "Move the currently playing file to the Trash" + ) + } + .modifierKeyAlternate(.option) { + Button { + guard let url = playEngine.fileURL else { return } + presentationModel.raise(.confirmDeletion(url)) + } label: { + Text( + "Delete Immediately...", + comment: "Delete the currently playing file without moving it to the Trash" + ) + } + .disabled(!playEngine.isLocalFile || presentationModel.isPresenting) + } + .disabled(!playEngine.isLocalFile || presentationModel.isPresenting) } } } diff --git a/Front Row/Models/PresentationModel.swift b/Front Row/Models/PresentationModel.swift index 4000014..ed56d42 100644 --- a/Front Row/Models/PresentationModel.swift +++ b/Front Row/Models/PresentationModel.swift @@ -26,6 +26,11 @@ import SwiftUI /// Raised through `raise(_:)` and cleared through `dismissRemuxAlert()`. private(set) var remuxAlert: RemuxAlert? + /// A question or failure about removing the playing file. + /// + /// Raised through `raise(_:)` and cleared through `dismissFileRemovalAlert()`. + private(set) var fileRemovalAlert: FileRemovalAlert? + /// Whether a file is being checked over before anything is offered about it. /// /// Holds the slot below for the same reason a running conversion does: the check puts a sheet @@ -40,12 +45,13 @@ import SwiftUI /// Whether the app is already occupied with a file. /// - /// Both alerts are stacked on the same view in each scene, so between them they have one place + /// The alerts are stacked on the same view in each scene, so between them they have one place /// to appear, and a conversion's sheet is sitting in it while one runs. A question raised into /// any of that is not a second alert - it is one alert wearing another's buttons, or one that /// is dropped and never answered. var isAskingAboutAFile: Bool { - remuxAlert != nil || unopenableRecentFile != nil || isConverting || isCheckingFile + remuxAlert != nil || unopenableRecentFile != nil || fileRemovalAlert != nil + || isConverting || isCheckingFile } var isPresenting: Bool { @@ -73,6 +79,14 @@ import SwiftUI return true } + /// The same for removing the playing file. + @discardableResult + func raise(_ alert: FileRemovalAlert) -> Bool { + guard !isAskingAboutAFile else { return false } + fileRemovalAlert = alert + return true + } + /// Takes the conversion question down, if `scene` is the one holding it. /// /// Each scene applies its own modifier to the same app-wide value, so both are told when either @@ -90,6 +104,11 @@ import SwiftUI unopenableRecentFile = nil } + /// The same for the file removal alert, which only the player scene presents. + func dismissFileRemovalAlert() { + fileRemovalAlert = nil + } + /// Marks a file as being checked over, which holds the slot until the check is done. func checkBegan() { isCheckingFile = true diff --git a/Front Row/Resources/Localizable.xcstrings b/Front Row/Resources/Localizable.xcstrings index 3257e62..d3fdb22 100644 --- a/Front Row/Resources/Localizable.xcstrings +++ b/Front Row/Resources/Localizable.xcstrings @@ -2467,6 +2467,9 @@ } } }, + "Are you sure you want to delete “%@”?" : { + "comment" : "Title of the alert confirming Delete Immediately; the argument is a file name" + }, "Artist" : { "comment" : "Media metadata field", "localizations" : { @@ -3287,7 +3290,7 @@ } }, "Cancel" : { - "comment" : "Alert button that declines converting a Matroska file\nButton that stops a conversion that is running\nButton that stops a file being checked\nDismisses the alert shown when ffmpeg isn't installed", + "comment" : "Alert button that cancels Delete Immediately\nAlert button that declines converting a Matroska file\nButton that stops a conversion that is running\nButton that stops a file being checked\nDismisses the alert shown when ffmpeg isn't installed", "localizations" : { "ar" : { "stringUnit" : { @@ -5337,6 +5340,12 @@ } } }, + "Couldn’t Delete “%@”" : { + "comment" : "Title of the alert shown when a file couldn’t be deleted" + }, + "Couldn’t Move “%@” to the Trash" : { + "comment" : "Title of the alert shown when a file couldn’t be moved to the Trash" + }, "Couldn’t Open File" : { "comment" : "Alert title shown when a file can't be opened\nAlert title shown when a file can't be played or converted\nTitle of the alert shown when a recent file could not be opened", "localizations" : { @@ -5884,6 +5893,12 @@ } } }, + "Delete" : { + "comment" : "Alert button that confirms Delete Immediately" + }, + "Delete Immediately..." : { + "comment" : "Delete the currently playing file without moving it to the Trash" + }, "Description" : { "comment" : "Media metadata field", "localizations" : { @@ -11619,7 +11634,7 @@ } }, "Move to Trash" : { - "comment" : "Alert button that trashes the original file", + "comment" : "Alert button that trashes the original file\nMove the currently playing file to the Trash", "localizations" : { "ar" : { "stringUnit" : { @@ -13122,7 +13137,7 @@ } }, "OK" : { - "comment" : "Dismisses the alert shown when a file couldn't be converted\nDismisses the alert shown when a recent file couldn't be opened", + "comment" : "Dismisses the alert shown when a file couldn't be converted\nDismisses the alert shown when a file couldn’t be removed\nDismisses the alert shown when a recent file couldn't be opened", "localizations" : { "ar" : { "stringUnit" : { @@ -17632,6 +17647,9 @@ } } }, + "This item will be deleted immediately. You can’t undo this action." : { + "comment" : "Message of the alert confirming Delete Immediately" + }, "Title" : { "comment" : "Media metadata field", "localizations" : { From eaa05e9c67b458bdf66b96dbb8e91cb97143d6a2 Mon Sep 17 00:00:00 2001 From: itsjoshpark Date: Sat, 26 Sep 2026 01:20:37 -0400 Subject: [PATCH 2/4] feat: translate delete immediately and the file removal alerts 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. --- Front Row/Resources/Localizable.xcstrings | 564 +++++++++++++++++++++- 1 file changed, 558 insertions(+), 6 deletions(-) diff --git a/Front Row/Resources/Localizable.xcstrings b/Front Row/Resources/Localizable.xcstrings index d3fdb22..ef93529 100644 --- a/Front Row/Resources/Localizable.xcstrings +++ b/Front Row/Resources/Localizable.xcstrings @@ -2468,7 +2468,99 @@ } }, "Are you sure you want to delete “%@”?" : { - "comment" : "Title of the alert confirming Delete Immediately; the argument is a file name" + "comment" : "Title of the alert confirming Delete Immediately; the argument is a file name", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "Opravdu chcete smazat „%@“?" + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Er du sikker på, at du vil slette “%@”?" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Möchtest du „%@“ wirklich löschen?" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "¿Seguro que quieres eliminar “%@”?" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Voulez-vous vraiment supprimer « %@ » ?" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Vuoi davvero eliminare “%@”?" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%@” を削除してもよろしいですか？" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "‘%@’을(를) 삭제하시겠습니까?" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Weet je zeker dat je ‘%@’ wilt verwijderen?" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Czy na pewno chcesz usunąć „%@”?" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Вы действительно хотите удалить «%@»?" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Vill du verkligen radera ”%@”?" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ви дійсно хочете видалити «%@»?" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "确定要删除“%@”吗？" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "確定要刪除「%@」嗎？" + } + } + } }, "Artist" : { "comment" : "Media metadata field", @@ -5341,10 +5433,194 @@ } }, "Couldn’t Delete “%@”" : { - "comment" : "Title of the alert shown when a file couldn’t be deleted" + "comment" : "Title of the alert shown when a file couldn’t be deleted", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "„%@“ nelze smazat" + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%@” kunne ikke slettes" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "„%@“ konnte nicht gelöscht werden" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "No se pudo eliminar “%@”" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Impossible de supprimer « %@ »" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Impossibile eliminare “%@”" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%@” を削除できませんでした" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "‘%@’을(를) 삭제할 수 없습니다" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kan ‘%@’ niet verwijderen" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nie można usunąć „%@”" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не удалось удалить «%@»" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kunde inte radera ”%@”" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не вдалося видалити «%@»" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "无法删除“%@”" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "無法刪除「%@」" + } + } + } }, "Couldn’t Move “%@” to the Trash" : { - "comment" : "Title of the alert shown when a file couldn’t be moved to the Trash" + "comment" : "Title of the alert shown when a file couldn’t be moved to the Trash", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "„%@“ nelze přesunout do koše" + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%@” kunne ikke flyttes til papirkurven" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "„%@“ konnte nicht in den Papierkorb gelegt werden" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "No se pudo mover “%@” a la papelera" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Impossible de placer « %@ » dans la corbeille" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Impossibile spostare “%@” nel Cestino" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%@” をゴミ箱に入れられませんでした" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "‘%@’을(를) 휴지통으로 이동할 수 없습니다" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kan ‘%@’ niet naar prullenmand verplaatsen" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Nie można przenieść „%@” do Kosza" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не удалось переместить «%@» в Корзину" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Kunde inte flytta ”%@” till papperskorgen" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Не вдалося перемістити «%@» у кошик" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "无法将“%@”移到废纸篓" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "無法將「%@」移到垃圾桶" + } + } + } }, "Couldn’t Open File" : { "comment" : "Alert title shown when a file can't be opened\nAlert title shown when a file can't be played or converted\nTitle of the alert shown when a recent file could not be opened", @@ -5894,10 +6170,194 @@ } }, "Delete" : { - "comment" : "Alert button that confirms Delete Immediately" + "comment" : "Alert button that confirms Delete Immediately", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "Smazat" + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Slet" + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Löschen" + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Eliminar" + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Supprimer" + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elimina" + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "削除" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "삭제" + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Verwijder" + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Usuń" + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Удалить" + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radera" + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Видалити" + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "删除" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "刪除" + } + } + } }, "Delete Immediately..." : { - "comment" : "Delete the currently playing file without moving it to the Trash" + "comment" : "Delete the currently playing file without moving it to the Trash", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "Okamžitě smazat..." + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Slet med det samme..." + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Sofort löschen..." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Eliminar inmediatamente..." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Supprimer immédiatement..." + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Elimina immediatamente..." + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "すぐに削除..." + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "즉시 삭제..." + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Verwijder direct..." + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Usuń natychmiast..." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Удалить немедленно..." + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Radera direkt..." + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Видалити негайно..." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "立即删除..." + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "立即刪除⋯" + } + } + } }, "Description" : { "comment" : "Media metadata field", @@ -17648,7 +18108,99 @@ } }, "This item will be deleted immediately. You can’t undo this action." : { - "comment" : "Message of the alert confirming Delete Immediately" + "comment" : "Message of the alert confirming Delete Immediately", + "localizations" : { + "cs" : { + "stringUnit" : { + "state" : "translated", + "value" : "Tato položka bude okamžitě smazána. Tuto akci nelze vrátit zpět." + } + }, + "da" : { + "stringUnit" : { + "state" : "translated", + "value" : "Emnet slettes med det samme. Du kan ikke fortryde denne handling." + } + }, + "de" : { + "stringUnit" : { + "state" : "translated", + "value" : "Dieses Objekt wird sofort gelöscht. Du kannst diese Aktion nicht widerrufen." + } + }, + "es" : { + "stringUnit" : { + "state" : "translated", + "value" : "Este elemento se eliminará inmediatamente. No puedes deshacer esta acción." + } + }, + "fr" : { + "stringUnit" : { + "state" : "translated", + "value" : "Cet élément sera supprimé immédiatement. Vous ne pouvez pas annuler cette action." + } + }, + "it" : { + "stringUnit" : { + "state" : "translated", + "value" : "Questo elemento verrà eliminato immediatamente. Non puoi annullare questa azione." + } + }, + "ja" : { + "stringUnit" : { + "state" : "translated", + "value" : "この項目はすぐに削除されます。この操作は取り消せません。" + } + }, + "ko" : { + "stringUnit" : { + "state" : "translated", + "value" : "이 항목이 즉시 삭제됩니다. 이 작업은 실행 취소할 수 없습니다." + } + }, + "nl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Dit onderdeel wordt direct verwijderd. Je kunt deze handeling niet ongedaan maken." + } + }, + "pl" : { + "stringUnit" : { + "state" : "translated", + "value" : "Ten element zostanie natychmiast usunięty. Tej czynności nie można cofnąć." + } + }, + "ru" : { + "stringUnit" : { + "state" : "translated", + "value" : "Этот объект будет удален немедленно. Это действие нельзя отменить." + } + }, + "sv" : { + "stringUnit" : { + "state" : "translated", + "value" : "Det här objektet raderas direkt. Du kan inte ångra den här åtgärden." + } + }, + "uk" : { + "stringUnit" : { + "state" : "translated", + "value" : "Цей елемент буде видалено негайно. Цю дію не можна скасувати." + } + }, + "zh-Hans" : { + "stringUnit" : { + "state" : "translated", + "value" : "此项目将被立即删除。此操作无法撤销。" + } + }, + "zh-Hant" : { + "stringUnit" : { + "state" : "translated", + "value" : "此項目將立即刪除。此動作無法還原。" + } + } + } }, "Title" : { "comment" : "Media metadata field", From 05d0e60667fe79a0e6488959cbdaf1ffde953c21 Mon Sep 17 00:00:00 2001 From: itsjoshpark Date: Sat, 26 Sep 2026 01:25:39 -0400 Subject: [PATCH 3/4] fix: delete only the file the confirmation named 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. --- Front Row Tests/Models/PresentationModelTests.swift | 4 ++-- Front Row/Files/FileRemovalAlert.swift | 12 +++++++----- Front Row/Main Menu/FileCommands.swift | 4 +++- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/Front Row Tests/Models/PresentationModelTests.swift b/Front Row Tests/Models/PresentationModelTests.swift index 983b40f..c2d34e6 100644 --- a/Front Row Tests/Models/PresentationModelTests.swift +++ b/Front Row Tests/Models/PresentationModelTests.swift @@ -126,8 +126,6 @@ struct PresentationModelTests { #expect(model.raise(problem(film)), "The slot stayed shut after the conversion ended") } - /// Playback commands stay disabled while any question is up, which is the one thing outside - /// this file that reads the slot. /// Confirming a deletion shares the slot with the other questions, and gives it back. @Test func aFileRemovalAlertHoldsTheSlotUntilDismissed() { @@ -144,6 +142,8 @@ struct PresentationModelTests { #expect(!model.raise(.confirmDeletion(film)), "A deletion talked over a conversion") } + /// Playback commands stay disabled while any question is up, which is the one thing outside + /// this file that reads the slot. @Test func aQuestionCountsAsPresenting() { let model = PresentationModel() diff --git a/Front Row/Files/FileRemovalAlert.swift b/Front Row/Files/FileRemovalAlert.swift index 8846961..4395609 100644 --- a/Front Row/Files/FileRemovalAlert.swift +++ b/Front Row/Files/FileRemovalAlert.swift @@ -30,13 +30,15 @@ enum FileRemovalAlert { /// Moves the playing file to the Trash or deletes it, and drops its recents entry, which would /// otherwise point at a file that's gone. A failure is raised as an alert instead. /// -/// Runs while the engine still holds the file's security-scoped access. +/// Refuses a `url` that is no longer the one playing: a file opened while the confirmation was up +/// would otherwise be deleted in place of the one it named. Runs while the engine still holds the +/// file's security-scoped access. /// - Returns: Whether the file is gone. @MainActor @discardableResult -func removeCurrentFile(_ removal: FileRemoval) -> Bool { +func removeCurrentFile(at url: URL, _ removal: FileRemoval) -> Bool { let playEngine = PlayEngine.shared - guard playEngine.isLocalFile, let url = playEngine.fileURL else { return false } + guard playEngine.isLocalFile, playEngine.fileURL == url else { return false } do { switch removal { case .trash: try FileManager.default.trashItem(at: url, resultingItemURL: nil) @@ -84,12 +86,12 @@ private struct FileRemovalAlertModifier: ViewModifier { presenting: presentationModel.fileRemovalAlert ) { alert in switch alert { - case .confirmDeletion: + case .confirmDeletion(let url): Button(role: .destructive) { // After the confirmation has gone: a failure raised from here would find // the slot still taken. Task { - guard removeCurrentFile(.delete) else { return } + guard removeCurrentFile(at: url, .delete) else { return } openWindow(id: WindowID.welcome) dismissWindow(id: WindowID.main) } diff --git a/Front Row/Main Menu/FileCommands.swift b/Front Row/Main Menu/FileCommands.swift index 7e6d356..dc0c02b 100644 --- a/Front Row/Main Menu/FileCommands.swift +++ b/Front Row/Main Menu/FileCommands.swift @@ -93,7 +93,9 @@ struct FileCommands: Commands { Divider() Button { - guard removeCurrentFile(.trash) else { return } + guard let url = playEngine.fileURL, removeCurrentFile(at: url, .trash) else { + return + } // Opened first: closing the last window quits the app. openWindow(id: WindowID.welcome) dismissWindow(id: WindowID.main) From 278679dc0a259b26bd846ee6901007896cff1b6e Mon Sep 17 00:00:00 2001 From: itsjoshpark Date: Sat, 26 Sep 2026 01:30:36 -0400 Subject: [PATCH 4/4] refactor: move file removal into a testable FileRemover 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. --- Front Row Tests/Files/FileRemoverTests.swift | 100 +++++++++++++++++++ Front Row/Files/FileRemovalAlert.swift | 42 +------- Front Row/Files/FileRemover.swift | 72 +++++++++++++ Front Row/Main Menu/FileCommands.swift | 9 +- 4 files changed, 177 insertions(+), 46 deletions(-) create mode 100644 Front Row Tests/Files/FileRemoverTests.swift create mode 100644 Front Row/Files/FileRemover.swift diff --git a/Front Row Tests/Files/FileRemoverTests.swift b/Front Row Tests/Files/FileRemoverTests.swift new file mode 100644 index 0000000..0ade310 --- /dev/null +++ b/Front Row Tests/Files/FileRemoverTests.swift @@ -0,0 +1,100 @@ +// +// FileRemoverTests.swift +// Front Row Tests +// + +import Foundation +import Testing + +@testable import Front_Row + +/// Removing the playing file acts only on the file that was named, and cleans up after itself only +/// once the file is actually gone. +/// +/// Deletes rather than trashes wherever a removal succeeds, so a test run leaves nothing in the +/// user's Trash. +@MainActor +struct FileRemoverTests { + + private let presentationModel = PresentationModel() + private let recents: RecentDocumentsStore + + init() { + let suite = "FileRemoverTests-\(UUID().uuidString)" + let defaults = UserDefaults(suiteName: suite)! + defaults.removePersistentDomain(forName: suite) + recents = RecentDocumentsStore( + defaults: defaults, bookmarkProvider: FakeBookmarkProvider(), + mountedVolumes: FakeMountedVolumesProvider()) + } + + private func makeFile() throws -> URL { + let file = URL.temporaryDirectory.appending(path: "\(UUID().uuidString).mov") + try Data().write(to: file) + return file + } + + private func remover(playing file: URL?) -> FileRemover { + FileRemover( + playingFile: { file }, recentDocuments: recents, presentationModel: presentationModel) + } + + @Test + func removesThePlayingFileAndItsRecentsEntry() throws { + let file = try makeFile() + recents.noteRecentDocument(file) + + #expect(remover(playing: file).remove(file, .delete)) + + #expect(!FileManager.default.fileExists(atPath: file.path(percentEncoded: false))) + #expect(!recents.recentURLs.contains(file), "The entry outlived its file") + #expect(presentationModel.fileRemovalAlert == nil) + } + + /// The confirmation names one file, and something else can be opened while it's up. Removing + /// whatever is playing by then would delete a file nobody was asked about. + @Test + func refusesAFileThatIsNoLongerPlaying() throws { + let named = try makeFile() + let nowPlaying = try makeFile() + defer { + try? FileManager.default.removeItem(at: named) + try? FileManager.default.removeItem(at: nowPlaying) + } + + #expect(!remover(playing: nowPlaying).remove(named, .delete)) + + #expect(FileManager.default.fileExists(atPath: named.path(percentEncoded: false))) + #expect(FileManager.default.fileExists(atPath: nowPlaying.path(percentEncoded: false))) + #expect(presentationModel.fileRemovalAlert == nil) + } + + /// A stream, or nothing at all, has no file behind it to remove. + @Test + func refusesWhenNoLocalFileIsPlaying() throws { + let file = try makeFile() + defer { try? FileManager.default.removeItem(at: file) } + + #expect(!remover(playing: nil).remove(file, .delete)) + + #expect(FileManager.default.fileExists(atPath: file.path(percentEncoded: false))) + } + + /// A file that couldn't be removed is still there, so its recents entry stays too. + @Test + func aFailureRaisesAnAlertAndKeepsTheRecentsEntry() throws { + let file = try makeFile() + recents.noteRecentDocument(file) + try FileManager.default.removeItem(at: file) + + #expect(!remover(playing: file).remove(file, .trash)) + + guard case .failure(let failure) = presentationModel.fileRemovalAlert else { + Issue.record("No failure was raised") + return + } + #expect(failure.url == file) + #expect(failure.removal == .trash) + #expect(recents.recentURLs.contains(file), "A failed removal dropped the entry") + } +} diff --git a/Front Row/Files/FileRemovalAlert.swift b/Front Row/Files/FileRemovalAlert.swift index 4395609..f7a5452 100644 --- a/Front Row/Files/FileRemovalAlert.swift +++ b/Front Row/Files/FileRemovalAlert.swift @@ -7,12 +7,6 @@ import SwiftUI -/// How the playing file is taken away. -enum FileRemoval: Equatable { - case trash - case delete -} - /// A file that couldn't be moved to the Trash or deleted, and what went wrong. struct FileRemovalFailure { var url: URL @@ -27,39 +21,8 @@ enum FileRemovalAlert { case failure(FileRemovalFailure) } -/// Moves the playing file to the Trash or deletes it, and drops its recents entry, which would -/// otherwise point at a file that's gone. A failure is raised as an alert instead. -/// -/// Refuses a `url` that is no longer the one playing: a file opened while the confirmation was up -/// would otherwise be deleted in place of the one it named. Runs while the engine still holds the -/// file's security-scoped access. -/// - Returns: Whether the file is gone. -@MainActor -@discardableResult -func removeCurrentFile(at url: URL, _ removal: FileRemoval) -> Bool { - let playEngine = PlayEngine.shared - guard playEngine.isLocalFile, playEngine.fileURL == url else { return false } - do { - switch removal { - case .trash: try FileManager.default.trashItem(at: url, resultingItemURL: nil) - case .delete: try FileManager.default.removeItem(at: url) - } - } catch { - PresentationModel.shared.raise( - .failure( - FileRemovalFailure( - url: url, removal: removal, reason: error.localizedDescription))) - return false - } - RecentDocumentsStore.shared.removeRecentDocument(url) - return true -} - extension View { /// Presents the file removal alert. Applied to the player window only. - /// - /// Once the file is gone, the welcome window is opened before the player closes, since closing - /// the last window quits the app. func fileRemovalAlert() -> some View { modifier(FileRemovalAlertModifier()) } @@ -91,9 +54,8 @@ private struct FileRemovalAlertModifier: ViewModifier { // After the confirmation has gone: a failure raised from here would find // the slot still taken. Task { - guard removeCurrentFile(at: url, .delete) else { return } - openWindow(id: WindowID.welcome) - dismissWindow(id: WindowID.main) + FileRemover().remove( + url, .delete, openWindow: openWindow, dismissWindow: dismissWindow) } } label: { Text("Delete", comment: "Alert button that confirms Delete Immediately") diff --git a/Front Row/Files/FileRemover.swift b/Front Row/Files/FileRemover.swift new file mode 100644 index 0000000..b57a332 --- /dev/null +++ b/Front Row/Files/FileRemover.swift @@ -0,0 +1,72 @@ +// +// FileRemover.swift +// Front Row +// +// Created by Joshua Park on 9/26/26. +// + +import SwiftUI + +/// How the playing file is taken away. +enum FileRemoval { + case trash + case delete +} + +/// Moves the playing file to the Trash or deletes it, on the user's say-so. +@MainActor +struct FileRemover { + /// The local file the player is showing, if any. + var playingFile: () -> URL? + var recentDocuments: RecentDocumentsStore + var presentationModel: PresentationModel + + init( + playingFile: @escaping () -> URL? = { + PlayEngine.shared.isLocalFile ? PlayEngine.shared.fileURL : nil + }, + recentDocuments: RecentDocumentsStore = .shared, + presentationModel: PresentationModel = .shared + ) { + self.playingFile = playingFile + self.recentDocuments = recentDocuments + self.presentationModel = presentationModel + } + + /// Removes `url` and drops its recents entry, which would otherwise point at a file that's + /// gone. A failure is raised as an alert instead. + /// + /// Refuses a `url` that is no longer the one playing: a file opened while the confirmation was + /// up would otherwise be deleted in place of the one it named. Runs while the engine still + /// holds the file's security-scoped access. + /// - Returns: Whether the file is gone. + @discardableResult + func remove(_ url: URL, _ removal: FileRemoval) -> Bool { + guard playingFile() == url else { return false } + do { + switch removal { + case .trash: try FileManager.default.trashItem(at: url, resultingItemURL: nil) + case .delete: try FileManager.default.removeItem(at: url) + } + } catch { + presentationModel.raise( + .failure( + FileRemovalFailure( + url: url, removal: removal, reason: error.localizedDescription))) + return false + } + recentDocuments.removeRecentDocument(url) + return true + } + + /// Removes `url` and, once it's gone, puts the welcome window in the player's place. The + /// welcome window opens first, since closing the last window quits the app. + func remove( + _ url: URL, _ removal: FileRemoval, openWindow: OpenWindowAction, + dismissWindow: DismissWindowAction + ) { + guard remove(url, removal) else { return } + openWindow(id: WindowID.welcome) + dismissWindow(id: WindowID.main) + } +} diff --git a/Front Row/Main Menu/FileCommands.swift b/Front Row/Main Menu/FileCommands.swift index dc0c02b..34013b2 100644 --- a/Front Row/Main Menu/FileCommands.swift +++ b/Front Row/Main Menu/FileCommands.swift @@ -93,12 +93,9 @@ struct FileCommands: Commands { Divider() Button { - guard let url = playEngine.fileURL, removeCurrentFile(at: url, .trash) else { - return - } - // Opened first: closing the last window quits the app. - openWindow(id: WindowID.welcome) - dismissWindow(id: WindowID.main) + guard let url = playEngine.fileURL else { return } + FileRemover().remove( + url, .trash, openWindow: openWindow, dismissWindow: dismissWindow) } label: { Text( "Move to Trash",