diff --git a/Radix/App/RadixCommands.swift b/Radix/App/RadixCommands.swift index bd34e0e..5da602a 100644 --- a/Radix/App/RadixCommands.swift +++ b/Radix/App/RadixCommands.swift @@ -12,6 +12,10 @@ struct RadixCommands: Commands { @FocusedValue(\.chartViewportAction) private var chartViewportAction var body: some Commands { + let selectionIsInDiscardPile = appModel.selectionIsInDiscardPile + let discardPileSelectionTitle: LocalizedStringKey = selectionIsInDiscardPile + ? "Review Discard Pile" + : "Add to Discard Pile" let selectedActionAvailability = FileNodeActionAvailability( nodes: navigation.selectedNodes, activeTarget: scanState.selectedTarget, @@ -174,8 +178,8 @@ struct RadixCommands: Commands { Divider() - Button(addSelectionToDiscardPileTitle, systemImage: "checklist") { - if appModel.selectionIncludesHiddenNodes { + Button(discardPileSelectionTitle, systemImage: "checklist") { + if selectionIsInDiscardPile { appModel.presentDiscardPileReview() } else { appModel.addSelectedNodesToDiscardPile() @@ -184,7 +188,7 @@ struct RadixCommands: Commands { .keyboardShortcut("l", modifiers: [.command, .shift]) .disabled( !canUseWorkspaceCommands || - (!appModel.selectionIncludesHiddenNodes + (!selectionIsInDiscardPile && !selectedActionAvailability.canMoveToTrash) ) @@ -268,20 +272,6 @@ struct RadixCommands: Commands { && appModel.presentationCoordinator.activeDestination == nil } - private var addSelectionToDiscardPileTitle: String { - if appModel.selectionIncludesHiddenNodes { - return String( - localized: "Review Discard Pile", - comment: "Command for reviewing the Discard Pile when the selection is already included." - ) - } - let count = navigation.selectedNodeIDs.count - guard count > 1 else { - return String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion.") - } - return String(localized: "Add \(count) Items to Discard Pile", comment: "Action for marking multiple selected items for possible deletion.") - } - private var commandSelectedFileActions: SelectedFileActions { SelectedFileActions( quickLook: { appModel.toggleQuickLookForSelected() }, @@ -307,7 +297,7 @@ struct RadixCommands: Commands { !canUseWorkspaceCommands || !action.isEnabled(in: availability) || (action == .moveToTrash && - (appModel.selectionIncludesHiddenNodes || isFileListSearchActive == true)) + (appModel.directTrashOverlapsProtectedNodes(navigation.selectedNodes) || isFileListSearchActive == true)) ) } } diff --git a/Radix/ContentView.swift b/Radix/ContentView.swift index 72ded81..8a82f8e 100644 --- a/Radix/ContentView.swift +++ b/Radix/ContentView.swift @@ -42,7 +42,6 @@ struct ContentView: View { ), maxRenderedDepth: appModel.maxRenderedDepth, showFreeSpaceInDiskMaps: appModel.showFreeSpaceInDiskMaps, - workspaceHiddenNodeIDs: appModel.workspaceHiddenNodeIDs, discardPileRootNodeIDs: appModel.discardPileRootNodeIDs, movingToTrashRootNodeIDs: appModel.movingToTrashRootNodeIDs, startupDiskTarget: appModel.startupDiskTarget, @@ -172,7 +171,7 @@ struct ContentView: View { GoToPathSheet( resolver: ScanPathResolver( treeStore: snapshot.treeStore, - hiddenNodeIDs: appModel.workspaceHiddenNodeIDs, + hiddenNodeIDs: appModel.movingToTrashRootNodeIDs, baseDirectoryID: request.directoryID ), onCancel: { appModel.dismissGoToPath() }, @@ -660,7 +659,6 @@ private struct WorkspaceDetailView: View { let maxRenderedDepth: Int let showFreeSpaceInDiskMaps: Bool - let workspaceHiddenNodeIDs: Set let discardPileRootNodeIDs: Set let movingToTrashRootNodeIDs: Set let startupDiskTarget: ScanTarget? @@ -686,7 +684,6 @@ private struct WorkspaceDetailView: View { visualizationMode: $visualizationMode, maxRenderedDepth: maxRenderedDepth, showFreeSpaceInDiskMaps: showFreeSpaceInDiskMaps, - workspaceHiddenNodeIDs: workspaceHiddenNodeIDs, discardPileRootNodeIDs: discardPileRootNodeIDs, movingToTrashRootNodeIDs: movingToTrashRootNodeIDs, startupDiskTarget: startupDiskTarget, @@ -814,7 +811,6 @@ private extension ContentView { selectAndFocusNodeAfterViewUpdate: { appModel.selectAndFocusAfterViewUpdate(nodeID: $0) }, expandSummarizedNode: { appModel.expandSummarizedNode($0) {} }, selectedFileActions: primarySelectedFileActions, - addPrimarySelectionToDiscardPile: { appModel.addPrimarySelectionToDiscardPileAfterViewUpdate() }, removeDiscardPileNode: { appModel.removeDiscardPileNode(id: $0) }, presentDiscardPileReview: { appModel.presentDiscardPileReview() }, bulkFileActions: bulkFileActions, @@ -849,6 +845,7 @@ private extension ContentView { revealInFinder: { appModel.revealNodesInFinder($0) }, copyPaths: { appModel.copyPaths(for: $0) }, addToDiscardPile: { appModel.addNodesToDiscardPile($0) }, + directTrashOverlapsProtectedNodes: { appModel.directTrashOverlapsProtectedNodes($0) }, moveToTrash: { appModel.requestMoveNodesToTrash($0) } ) } diff --git a/Radix/Features/FileList/FileBrowserNameCell.swift b/Radix/Features/FileList/FileBrowserNameCell.swift index cd7531b..f87b803 100644 --- a/Radix/Features/FileList/FileBrowserNameCell.swift +++ b/Radix/Features/FileList/FileBrowserNameCell.swift @@ -3,6 +3,7 @@ import SwiftUI struct FileBrowserNameCell: View { let node: FileNodeRecord let subtitleOverride: String? + let isInDiscardPile: Bool let isExpanding: Bool let expandAction: () -> Void @Binding var presentedSharedStorageNodeID: FileNodeRecord.ID? @@ -25,8 +26,15 @@ struct FileBrowserNameCell: View { .foregroundStyle(node.isDirectory || node.isSynthetic ? Color.accentColor : Color.secondary) VStack(alignment: .leading, spacing: 2) { - Text(node.displayName) - .lineLimit(1) + if isInDiscardPile { + ViewThatFits(in: .horizontal) { + nameWithDiscardPileBadge(style: TitleAndIconLabelStyle()) + nameWithDiscardPileBadge(style: IconOnlyLabelStyle()) + } + } else { + Text(node.displayName) + .lineLimit(1) + } if let statusText = subtitleOverride ?? node.secondaryStatusText { Text(statusText) @@ -59,6 +67,19 @@ struct FileBrowserNameCell: View { .accessibilityElement(children: .contain) } + private func nameWithDiscardPileBadge(style: some LabelStyle) -> some View { + HStack(spacing: 6) { + Text(node.displayName) + .lineLimit(1) + Label("In Discard Pile", systemImage: "checklist") + .labelStyle(style) + .font(.caption) + .foregroundStyle(.secondary) + .fixedSize() + .help("In Discard Pile") + } + } + private var fileBrowserSystemImageName: String { node.cloneIdentity == nil ? node.systemImageName : "doc.on.doc" } diff --git a/Radix/Features/FileList/FileBrowserTableView.swift b/Radix/Features/FileList/FileBrowserTableView.swift index 8d42c70..c1f92db 100644 --- a/Radix/Features/FileList/FileBrowserTableView.swift +++ b/Radix/Features/FileList/FileBrowserTableView.swift @@ -22,6 +22,7 @@ struct FileBrowserTableView: View { @ObservedObject var navigation: WorkspaceNavigationModel @FocusState.Binding var focusedWorkspaceTarget: WorkspaceFocusTarget? let hiddenNodeIDs: Set + let discardPileRootNodeIDs: Set let fileDragController: FileDragController let actions: FileBrowserActions @@ -34,6 +35,7 @@ struct FileBrowserTableView: View { navigation: WorkspaceNavigationModel, focusedWorkspaceTarget: FocusState.Binding, hiddenNodeIDs: Set, + discardPileRootNodeIDs: Set, fileDragController: FileDragController, actions: FileBrowserActions, model: @autoclosure @escaping () -> FileBrowserModel = FileBrowserModel() @@ -42,6 +44,7 @@ struct FileBrowserTableView: View { self.navigation = navigation self._focusedWorkspaceTarget = focusedWorkspaceTarget self.hiddenNodeIDs = hiddenNodeIDs + self.discardPileRootNodeIDs = discardPileRootNodeIDs self.fileDragController = fileDragController self.actions = actions _model = StateObject(wrappedValue: model()) @@ -176,12 +179,19 @@ struct FileBrowserTableView: View { let tourFolderID = tourFolder?.id let tour = self.tour let tourPresentation = self.tourPresentation + let tree = scanState.fileTreeStore + let markedNodes = discardPileRootNodeIDs.isEmpty ? nil : tree?.preparedNodeSet(for: discardPileRootNodeIDs) + func isInDiscardPile(_ id: FileNodeRecord.ID) -> Bool { + guard let markedNodes, let tree else { return false } + return tree.isNodeOrDescendant(id, of: markedNodes) + } return Table(of: FileNodeRecord.self, selection: tableSelection, sortOrder: sortOrderBinding) { TableColumn("Name", sortUsing: FileNodeTableComparator(field: .name)) { node in FileBrowserNameCell( node: node, subtitleOverride: subtitle(for: node), + isInDiscardPile: isInDiscardPile(node.id), isExpanding: isExpanding(node), expandAction: { expandSummarizedNode(node) }, presentedSharedStorageNodeID: $presentedSharedStorageNodeID @@ -243,7 +253,7 @@ struct FileBrowserTableView: View { .accessibilityLabel("Contents table") .accessibilityHint("Select a row to inspect it. Double-click a folder to zoom in, or a summarized folder to expand it. Press Space for Quick Look.") .contextMenu(forSelectionType: FileNodeRecord.ID.self) { selectedIDs in - fileContextMenu(for: selectedIDs) + fileContextMenu(for: selectedIDs, isInDiscardPile: isInDiscardPile) } primaryAction: { selectedIDs in performPrimaryAction(for: selectedIDs) } @@ -336,18 +346,26 @@ struct FileBrowserTableView: View { } @ViewBuilder - private func fileContextMenu(for selectedIDs: Set) -> some View { + private func fileContextMenu( + for selectedIDs: Set, + isInDiscardPile: (FileNodeRecord.ID) -> Bool + ) -> some View { if let selection = selectionContext(for: selectedIDs) { + let canAddToDiscardPile = selection.actionAvailability.canMoveToTrash + && !selection.ids.allSatisfy(isInDiscardPile) if selection.nodes.count > 1 { - bulkFileContextMenu(for: selection) + bulkFileContextMenu(for: selection, canAddToDiscardPile: canAddToDiscardPile) } else { - singleFileContextMenu(for: selection) + singleFileContextMenu(for: selection, canAddToDiscardPile: canAddToDiscardPile) } } } @ViewBuilder - private func singleFileContextMenu(for selection: FileBrowserSelectionContext) -> some View { + private func singleFileContextMenu( + for selection: FileBrowserSelectionContext, + canAddToDiscardPile: Bool + ) -> some View { if let node = selection.primaryNode, let id = selection.primaryID { fileActionButton(.quickLook, availability: selection.actionAvailability, selectedID: selection.id) @@ -391,7 +409,7 @@ struct FileBrowserTableView: View { actions.selectNode(id) actions.bulkFileActions.addToDiscardPile([node]) } - .disabled(!selection.actionAvailability.canMoveToTrash) + .disabled(!canAddToDiscardPile) Button( FileNodeAction.moveToTrash.title, @@ -401,7 +419,10 @@ struct FileBrowserTableView: View { actions.selectNode(id) actions.bulkFileActions.moveToTrash([node]) } - .disabled(!FileNodeAction.moveToTrash.isEnabled(in: selection.actionAvailability)) + .disabled( + !selection.actionAvailability.canMoveToTrash + || actions.bulkFileActions.directTrashOverlapsProtectedNodes(selection.nodes) + ) fileActionButton(.copyPath, availability: selection.actionAvailability, selectedID: selection.id) .labelStyle(.titleOnly) @@ -409,7 +430,10 @@ struct FileBrowserTableView: View { } @ViewBuilder - private func bulkFileContextMenu(for selection: FileBrowserSelectionContext) -> some View { + private func bulkFileContextMenu( + for selection: FileBrowserSelectionContext, + canAddToDiscardPile: Bool + ) -> some View { Button(FileNodeAction.quickLook.title, systemImage: FileNodeAction.quickLook.systemImageName) { actions.selectNodes(selection.ids, selection.primaryID) actions.selectedFileActions.quickLook() @@ -430,17 +454,20 @@ struct FileBrowserTableView: View { Divider() - Button("Add \(selection.nodes.count) Items to Discard Pile", systemImage: "checklist") { + Button("Add to Discard Pile", systemImage: "checklist") { actions.selectNodes(selection.ids, selection.primaryID) actions.bulkFileActions.addToDiscardPile(selection.nodes) } - .disabled(!selection.actionAvailability.canMoveToTrash) + .disabled(!canAddToDiscardPile) Button("Move \(selection.nodes.count) Items to Trash", systemImage: FileNodeAction.moveToTrash.systemImageName, role: .destructive) { actions.selectNodes(selection.ids, selection.primaryID) actions.bulkFileActions.moveToTrash(selection.nodes) } - .disabled(!selection.actionAvailability.canMoveToTrash) + .disabled( + !selection.actionAvailability.canMoveToTrash + || actions.bulkFileActions.directTrashOverlapsProtectedNodes(selection.nodes) + ) } private func performPrimaryAction(for selectedIDs: Set) { diff --git a/Radix/Features/Inspector/InspectorMultiSelectionView.swift b/Radix/Features/Inspector/InspectorMultiSelectionView.swift index 8cc7586..eb71257 100644 --- a/Radix/Features/Inspector/InspectorMultiSelectionView.swift +++ b/Radix/Features/Inspector/InspectorMultiSelectionView.swift @@ -4,6 +4,7 @@ struct InspectorMultiSelectionView: View { let summary: InspectorSelectionSummary let percentOfScan: String let availability: FileNodeActionAvailability + let canAddSelectionToDiscardPile: Bool let canMoveSelectionToTrash: Bool let availabilityNotice: InspectorAvailabilityNoticeKind? let warnings: [ScanWarning] @@ -49,15 +50,15 @@ struct InspectorMultiSelectionView: View { .formStyle(.grouped) .contentMargins(.horizontal, 0, for: .scrollContent) - if availability.canRevealInFinder || canMoveSelectionToTrash { + if availability.canRevealInFinder || canAddSelectionToDiscardPile { InspectorActionBar( revealAction: availability.canRevealInFinder ? { actions.revealInFinder(summary.selectedNodes) } : nil, - discardPileAction: canMoveSelectionToTrash + discardPileAction: canAddSelectionToDiscardPile ? { actions.addToDiscardPile(summary.topLevelSelectedNodes) } : nil, - discardPileTitle: addToDiscardPileTitle, + discardPileTitle: String(localized: "Add to Discard Pile"), discardPileSystemImageName: "checklist" ) } @@ -178,13 +179,6 @@ struct InspectorMultiSelectionView: View { ) } - private var addToDiscardPileTitle: String { - String( - localized: "Add \(summary.topLevelSelectedCount) Items to Discard Pile", - comment: "Action for marking one or more effective top-level selected items for possible deletion." - ) - } - private var moveToTrashTitle: String { String( localized: "Move \(summary.topLevelSelectedCount) Items to Trash", diff --git a/Radix/Features/Inspector/SelectionInspectorView.swift b/Radix/Features/Inspector/SelectionInspectorView.swift index b1eca30..9d3f3e1 100644 --- a/Radix/Features/Inspector/SelectionInspectorView.swift +++ b/Radix/Features/Inspector/SelectionInspectorView.swift @@ -5,7 +5,6 @@ struct SelectionInspectorActions { let selectAndFocusNodeAfterViewUpdate: (String) -> Void let expandSummarizedNode: (FileNodeRecord) -> Void let selectedFileActions: SelectedFileActions - let addPrimarySelectionToDiscardPile: () -> Void let removeDiscardPileNode: (FileNodeRecord.ID) -> Void let presentDiscardPileReview: () -> Void let bulkFileActions: BulkFileActions @@ -52,7 +51,7 @@ struct SelectionInspectorView: View { trashSafetyPolicy: scanState.trashSafetyPolicy, snapshotSource: scanState.snapshotSource ) - let canMoveSelectionToTrash = summary.missingSelectedNodeCount == 0 + let canMarkOrMoveSelection = summary.missingSelectedNodeCount == 0 && removalEligibility.canMoveToTrash && !isReadOnlyMode let warnings = relevantWarnings(for: summary.topLevelSelectedNodes) @@ -61,7 +60,10 @@ struct SelectionInspectorView: View { summary: summary, percentOfScan: selectionPercentOfScanText(summary) ?? "—", availability: availability, - canMoveSelectionToTrash: canMoveSelectionToTrash, + canAddSelectionToDiscardPile: canMarkOrMoveSelection + && !summary.topLevelSelectedNodes.allSatisfy { discardPileMembership(for: $0) != nil }, + canMoveSelectionToTrash: canMarkOrMoveSelection + && !actions.bulkFileActions.directTrashOverlapsProtectedNodes(summary.topLevelSelectedNodes), availabilityNotice: multiSelectionAvailabilityNotice( summary: summary, removalIsEligible: removalEligibility.canMoveToTrash @@ -90,7 +92,7 @@ struct SelectionInspectorView: View { InspectorSummarySection( node: node, availability: availability, - allowsMoveToTrash: discardPileMembership == nil, + allowsMoveToTrash: !actions.bulkFileActions.directTrashOverlapsProtectedNodes([node]), actions: actions.selectedFileActions ) @@ -125,7 +127,7 @@ struct SelectionInspectorView: View { InspectorAvailabilityNotice(kind: notice) } - if discardPileMembership == nil, canExpandSummarizedSelection(node) { + if canExpandSummarizedSelection(node) { InspectorSummarizedSection { actions.expandSummarizedNode(node) } @@ -200,7 +202,7 @@ struct SelectionInspectorView: View { } return availability.canMoveToTrash - ? actions.addPrimarySelectionToDiscardPile + ? { actions.bulkFileActions.addToDiscardPile([node]) } : nil } @@ -317,8 +319,7 @@ struct SelectionInspectorView: View { } private func selectLargestChild(_ child: FileNodeRecord) { - guard discardPileMembership(for: child) == nil, - child.isDirectory, + guard child.isDirectory, scanState.fileTreeStore?.containsChildren(id: child.id) == true else { actions.selectNodeAfterViewUpdate(child.id) return diff --git a/Radix/Features/Visualization/SunburstChartView.swift b/Radix/Features/Visualization/SunburstChartView.swift index 11da4f5..1e72d01 100644 --- a/Radix/Features/Visualization/SunburstChartView.swift +++ b/Radix/Features/Visualization/SunburstChartView.swift @@ -435,12 +435,6 @@ struct SunburstChartView: View { return } - if discardPileOverlay.isQueued(nodeID) { - onSegmentClick() - onSelect(nodeID) - return - } - if clickCount >= 2, treeStore.node(id: nodeID)?.isDirectory == true { onSegmentClick() diff --git a/Radix/Features/Visualization/TreemapChartView.swift b/Radix/Features/Visualization/TreemapChartView.swift index edda759..a292835 100644 --- a/Radix/Features/Visualization/TreemapChartView.swift +++ b/Radix/Features/Visualization/TreemapChartView.swift @@ -446,11 +446,6 @@ struct TreemapChartView: View { return } - if discardPileOverlay.isQueued(nodeID) { - onSelect(nodeID) - return - } - if clickCount >= 2, treeStore.node(id: nodeID)?.isDirectory == true { onZoom(nodeID) diff --git a/Radix/Features/Workspace/ActiveWorkspaceView.swift b/Radix/Features/Workspace/ActiveWorkspaceView.swift index 2da0655..52f8f2b 100644 --- a/Radix/Features/Workspace/ActiveWorkspaceView.swift +++ b/Radix/Features/Workspace/ActiveWorkspaceView.swift @@ -10,7 +10,6 @@ struct ActiveWorkspaceView: View { let visualizationMode: ScanVisualizationMode let maxRenderedDepth: Int let showFreeSpaceInDiskMaps: Bool - let workspaceHiddenNodeIDs: Set let discardPileRootNodeIDs: Set let movingToTrashRootNodeIDs: Set let fullDiskAccessStatus: FullDiskAccessStatus @@ -114,7 +113,8 @@ struct ActiveWorkspaceView: View { scanState: scanState, navigation: navigation, focusedWorkspaceTarget: $focusedWorkspaceTarget, - hiddenNodeIDs: workspaceHiddenNodeIDs, + hiddenNodeIDs: movingToTrashRootNodeIDs, + discardPileRootNodeIDs: discardPileRootNodeIDs, fileDragController: actions.fileDragController, actions: fileBrowserActions, model: actions.makeFileBrowserModel() diff --git a/Radix/Features/Workspace/WorkspaceView.swift b/Radix/Features/Workspace/WorkspaceView.swift index 40df4de..dce9880 100644 --- a/Radix/Features/Workspace/WorkspaceView.swift +++ b/Radix/Features/Workspace/WorkspaceView.swift @@ -62,6 +62,7 @@ struct BulkFileActions { let revealInFinder: ([FileNodeRecord]) -> Void let copyPaths: ([FileNodeRecord]) -> Void let addToDiscardPile: ([FileNodeRecord]) -> Void + let directTrashOverlapsProtectedNodes: ([FileNodeRecord]) -> Bool let moveToTrash: ([FileNodeRecord]) -> Void } @@ -76,7 +77,6 @@ struct WorkspaceView: View { let maxRenderedDepth: Int let showFreeSpaceInDiskMaps: Bool - let workspaceHiddenNodeIDs: Set let discardPileRootNodeIDs: Set let movingToTrashRootNodeIDs: Set let startupDiskTarget: ScanTarget? @@ -97,7 +97,6 @@ struct WorkspaceView: View { visualizationMode: visualizationMode, maxRenderedDepth: maxRenderedDepth, showFreeSpaceInDiskMaps: showFreeSpaceInDiskMaps, - workspaceHiddenNodeIDs: workspaceHiddenNodeIDs, discardPileRootNodeIDs: discardPileRootNodeIDs, movingToTrashRootNodeIDs: movingToTrashRootNodeIDs, fullDiskAccessStatus: fullDiskAccessStatus, diff --git a/Radix/Services/DiscardPileVisualizationOverlay.swift b/Radix/Services/DiscardPileVisualizationOverlay.swift index c4b681e..2647d79 100644 --- a/Radix/Services/DiscardPileVisualizationOverlay.swift +++ b/Radix/Services/DiscardPileVisualizationOverlay.swift @@ -166,18 +166,13 @@ nonisolated struct DiscardPileVisualizationOverlay: Equatable, Sendable { role(for: nodeID) ?? role(for: aggregateContainerNodeID) } - func isQueued(_ nodeID: FileNodeRecord.ID?) -> Bool { - guard let nodeID else { return false } - return queuedNodeIDs.contains(nodeID) - } - func isMovingToTrash(_ nodeID: FileNodeRecord.ID?) -> Bool { guard let nodeID else { return false } return movingToTrashNodeIDs.contains(nodeID) } func allowsChartNodeAction(for nodeID: FileNodeRecord.ID?) -> Bool { - !isQueued(nodeID) && !isMovingToTrash(nodeID) + !isMovingToTrash(nodeID) } private static func renderedMemberships( diff --git a/Radix/ViewModels/AppModel.swift b/Radix/ViewModels/AppModel.swift index bbfef0c..f49adb0 100644 --- a/Radix/ViewModels/AppModel.swift +++ b/Radix/ViewModels/AppModel.swift @@ -184,7 +184,6 @@ final class AppModel: ObservableObject { guard canChangeReadOnlyMode, isEnabled != isReadOnlyMode, trashFlow.setReadOnlyMode(isEnabled) else { return } if isEnabled { - cancelDeferredDiscardPileAdd() cancelDiscardPilePreparation() } dependencies.preferences.saveReadOnlyMode(isEnabled) @@ -296,7 +295,6 @@ final class AppModel: ObservableObject { case sidebarSelection case navigationAction case visualizationMode - case discardPileAdd case navigationContext } @@ -418,13 +416,6 @@ final class AppModel: ObservableObject { guard let self else { return } projectedDiscardPileIDs = matchingNodeIDs ?? pile.matchingNodeIDs(in: scanCoordinator.snapshot) } - trashFlow.onDiscardPileHistoryReplay = { [weak self] _ in - guard let self, let fileTreeStore = self.scanCoordinator.fileTreeStore else { return } - self.reconcileNavigationForHiddenNodes( - hiddenNodeIDs: self.workspaceHiddenNodeIDs, - fileTreeStore: fileTreeStore - ) - } comparisonFlow.onChange = { [weak self] in guard let self else { return } self.synchronizeComparisonSetupPresentation() @@ -467,7 +458,6 @@ final class AppModel: ObservableObject { cancelDeferredSidebarSelection() cancelDeferredNavigationAction() cancelDeferredVisualizationModeUpdate() - cancelDeferredDiscardPileAdd() cancelDeferredNavigationContextUpdate() trashFlow.cancelPostTrashSnapshotRemoval() trashFlow.cancelConfirmedTrashMoves() @@ -505,7 +495,6 @@ final class AppModel: ObservableObject { cancelDeferredSidebarSelection() cancelDeferredNavigationAction() cancelDeferredVisualizationModeUpdate() - cancelDeferredDiscardPileAdd() cancelDeferredNavigationContextUpdate() trashFlow.cancelPostTrashSnapshotRemoval() trashFlow.cancelConfirmedTrashMoves() @@ -616,10 +605,6 @@ final class AppModel: ObservableObject { && canUseWorkspaceCommands && !trashFlow.isMovingFiles && !isPreparingDiscardPileTrash } - var workspaceHiddenNodeIDs: Set { - hiddenNodeIDs(for: scanCoordinator.snapshot?.id) - } - var discardPileRootNodeIDs: Set { projectedDiscardPileIDs } @@ -634,10 +619,12 @@ final class AppModel: ObservableObject { return navigationModel.canZoomIntoSelection && isVisibleNavigationNode(selectedNodeID) } - var selectionIncludesHiddenNodes: Bool { + var selectionIsInDiscardPile: Bool { let selectedNodeIDs = navigationModel.selectedNodeIDs - return !selectedNodeIDs.isEmpty - && visibleNavigationNodeIDs(from: selectedNodeIDs) != selectedNodeIDs + guard !selectedNodeIDs.isEmpty, !discardPileRootNodeIDs.isEmpty, + let tree = scanCoordinator.fileTreeStore else { return false } + let markedNodes = tree.preparedNodeSet(for: discardPileRootNodeIDs) + return selectedNodeIDs.allSatisfy { tree.isNodeOrDescendant($0, of: markedNodes) } } var startupDiskTarget: ScanTarget? { @@ -705,7 +692,6 @@ final class AppModel: ObservableObject { cancelDeferredSidebarSelection() cancelDeferredNavigationAction() cancelDeferredNavigationContextUpdate() - cancelDeferredDiscardPileAdd() cancelDeferredVisualizationModeUpdate() sidebarScanCacheController.cancelPendingSidebarTargetRestore() quickLookController.closePreview() @@ -734,7 +720,6 @@ final class AppModel: ObservableObject { cancelDeferredSidebarSelection() cancelDeferredNavigationAction() cancelDeferredNavigationContextUpdate() - cancelDeferredDiscardPileAdd() cancelDeferredVisualizationModeUpdate() trashFlow.cancelConfirmedTrashMoves() trashFlow.cancelPostTrashSnapshotRemoval() @@ -1719,7 +1704,6 @@ final class AppModel: ObservableObject { cancelDeferredScanStart() cancelDeferredSidebarSelection() cancelDeferredNavigationContextUpdate() - cancelDeferredDiscardPileAdd() trashFlow.cancelPostTrashSnapshotRemoval() sidebarScanCacheController.cancelPendingSidebarTargetRestore() @@ -1756,10 +1740,6 @@ final class AppModel: ObservableObject { cancelDeferrals(.visualizationMode) } - private func cancelDeferredDiscardPileAdd() { - cancelDeferrals(.discardPileAdd) - } - private func cancelDeferredNavigationContextUpdate() { cancelDeferrals(.navigationContext) } @@ -1931,7 +1911,6 @@ final class AppModel: ObservableObject { cancelDeferredScanStart() cancelDeferredSidebarSelection() cancelDeferredNavigationAction() - cancelDeferredDiscardPileAdd() cancelDeferredNavigationContextUpdate() trashFlow.cancelPostTrashSnapshotRemoval() clearOptimisticTrashVisibility() @@ -2010,7 +1989,7 @@ final class AppModel: ObservableObject { snapshot.id == request.snapshotID, snapshot.treeStore.contentID == request.treeContentID else { return false } let resolver = ScanPathResolver( treeStore: snapshot.treeStore, - hiddenNodeIDs: workspaceHiddenNodeIDs, + hiddenNodeIDs: movingToTrashRootNodeIDs, baseDirectoryID: request.directoryID ) let resolution = Task.detached(priority: .userInitiated) { @@ -2131,13 +2110,8 @@ final class AppModel: ObservableObject { } private func visibleNavigationNodeIDs(from nodeIDs: Set) -> Set { - guard let snapshotID = scanCoordinator.snapshot?.id, - let fileTreeStore = scanCoordinator.fileTreeStore else { - return nodeIDs - } - - let hiddenIDs = hiddenNodeIDs(for: snapshotID) - guard !hiddenIDs.isEmpty else { return nodeIDs } + let hiddenIDs = movingToTrashRootNodeIDs + guard !hiddenIDs.isEmpty, let fileTreeStore = scanCoordinator.fileTreeStore else { return nodeIDs } let hiddenNodes = fileTreeStore.preparedNodeSet(for: hiddenIDs) return nodeIDs.filter { !fileTreeStore.isNodeOrDescendant($0, of: hiddenNodes) } } @@ -2318,7 +2292,7 @@ final class AppModel: ObservableObject { func requestMoveSelectedToTrash() { do { - try requestTrashMove(for: validatedSelectedNodesForMutation()) + try requestTrashMove(for: validatedNodesForMutation(navigationModel.selectedNodes)) } catch { presentError(error) } @@ -2365,15 +2339,6 @@ final class AppModel: ObservableObject { addNodesToDiscardPile(navigationModel.selectedNodes) } - func addPrimarySelectionToDiscardPileAfterViewUpdate() { - guard let node = navigationModel.selectedNode else { - presentError(FileActionError.noSelection) - return - } - - scheduleDeferredDiscardPileAdd([node]) - } - @discardableResult func addNodeIDsToDiscardPile( _ nodeIDs: [FileNodeRecord.ID], @@ -2410,17 +2375,7 @@ final class AppModel: ObservableObject { @discardableResult func addNodesToDiscardPile(_ nodes: [FileNodeRecord]) -> Bool { do { - try validateDiscardPileAddition(nodes) - let discardNodes = topLevelTrashNodes(from: nodes) - if let cloudImpact = cloudStorageImpact(of: discardNodes.map(\.url)) { - pendingCloudFileAction = PendingCloudFileAction( - kind: .addToDiscardPile, - nodes: discardNodes, - cloudImpact: cloudImpact - ) - return true - } - try commitDiscardPileAddition(discardNodes) + try addDiscardPileNodes(nodes) return true } catch { presentError(error) @@ -2428,16 +2383,6 @@ final class AppModel: ObservableObject { } } - private func scheduleDeferredDiscardPileAdd(_ nodes: [FileNodeRecord]) { - cancelDeferredDiscardPileAdd() - - scheduleDeferredViewUpdate( - .discardPileAdd - ) { model in - model.addNodesToDiscardPile(nodes) - } - } - var discardPileUndoManager: UndoManager { trashFlow.discardPileUndoManager } private var canReplayDiscardPileChange: Bool { @@ -2544,7 +2489,7 @@ final class AppModel: ObservableObject { do { try validateSnapshotAllowsMutation() if selection.discardPileBatch == nil { - try validateMutationDoesNotIncludeHiddenNodes(selection.nodes) + try validateDirectTrash(selection.nodes) } let resolvedURLs = selection.discardPileBatch?.resolvedURLsByOperationID ?? [:] @@ -2572,8 +2517,7 @@ final class AppModel: ObservableObject { do { switch action.kind { case .addToDiscardPile: - try validateDiscardPileAddition(action.nodes) - try commitDiscardPileAddition(topLevelTrashNodes(from: action.nodes)) + try addDiscardPileNodes(action.nodes, cloudWarningAccepted: true) case .moveToTrash: let nodes: [FileNodeRecord] if action.discardPileBatch == nil { @@ -2593,13 +2537,32 @@ final class AppModel: ObservableObject { pendingCloudFileAction = nil } - private func commitDiscardPileAddition(_ nodes: [FileNodeRecord]) throws { - try validateSnapshotAllowsMutation() + private func addDiscardPileNodes( + _ requestedNodes: [FileNodeRecord], + cloudWarningAccepted: Bool = false + ) throws { + try validateDiscardPileAddition(requestedNodes) guard let snapshot = scanCoordinator.snapshot else { throw FileActionError.unsupported } + let nodes = topLevelTrashNodes(from: requestedNodes) + let (updatedPile, matchingNodeIDs, addedNodeIDs) = trashFlow.discardPileAdding(nodes, in: snapshot) + if !cloudWarningAccepted, + let cloudImpact = cloudStorageImpact(of: nodes.filter { addedNodeIDs.contains($0.id) }.map(\.url)) { + pendingCloudFileAction = PendingCloudFileAction( + kind: .addToDiscardPile, + nodes: nodes, + cloudImpact: cloudImpact + ) + return + } + let previousIDs = Set(discardPile.nodeIDs) - addDiscardPileNodes(nodes, snapshot: snapshot) + trashFlow.changeDiscardPile( + updatedPile, + actionName: String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion."), + matchingNodeIDs: matchingNodeIDs + ) workspaceTour.didAddMarks( Set(discardPile.nodeIDs).subtracting(previousIDs), snapshotID: snapshot.id @@ -2656,7 +2619,7 @@ final class AppModel: ObservableObject { ) if didHide, let statsFileTreeStore { reconcileNavigationForHiddenNodes( - hiddenNodeIDs: hiddenNodeIDs(for: originalSnapshotID), + hiddenNodeIDs: movingToTrashRootNodeIDs, fileTreeStore: statsFileTreeStore ) } @@ -2999,26 +2962,17 @@ final class AppModel: ObservableObject { private func validatedSelectionForMutation() throws -> FileNodeRecord { try validateSnapshotAllowsMutation() let node = try validatedSelection(requiresLivePath: true) - try validateMutationDoesNotIncludeHiddenNodes([node]) + try validateDirectTrash([node]) return node } private func validatedNodesForMutation(_ nodes: [FileNodeRecord]) throws -> [FileNodeRecord] { try validateSnapshotAllowsMutation() let nodes = try validatedNodes(nodes, requiresLivePath: true) - try validateMutationDoesNotIncludeHiddenNodes(nodes) + try validateDirectTrash(nodes) return nodes } - private func validatedSelectedNodesForMutation() throws -> [FileNodeRecord] { - try validateSnapshotAllowsMutation() - let visibleNodeIDs = visibleNavigationNodeIDs(from: navigationModel.selectedNodeIDs) - let visibleNodes = navigationModel.selectedNodes.filter { - visibleNodeIDs.contains($0.id) - } - return try validatedNodes(visibleNodes, requiresLivePath: true) - } - private func validateDiscardPileAddition(_ nodes: [FileNodeRecord]) throws { try validateSnapshotAllowsMutation() guard !nodes.isEmpty else { throw FileActionError.noSelection } @@ -3061,28 +3015,20 @@ final class AppModel: ObservableObject { } } - private func validateMutationDoesNotIncludeHiddenNodes(_ nodes: [FileNodeRecord]) throws { - guard let snapshotID = scanCoordinator.snapshot?.id, - let fileTreeStore = scanCoordinator.fileTreeStore else { - return - } - - let hiddenIDs = hiddenNodeIDs(for: snapshotID) - guard !hiddenIDs.isEmpty else { return } + func directTrashOverlapsProtectedNodes(_ nodes: [FileNodeRecord]) -> Bool { + let protectedIDs = discardPileRootNodeIDs.union(movingToTrashRootNodeIDs) + guard !nodes.isEmpty, !protectedIDs.isEmpty, + let tree = scanCoordinator.fileTreeStore else { return false } let requestedIDs = Set(nodes.map(\.id)) - let hiddenNodes = fileTreeStore.preparedNodeSet(for: hiddenIDs) - let requestedNodes = fileTreeStore.preparedNodeSet(for: requestedIDs) - let requestedNodeIsHidden = requestedIDs.contains(where: { requestedID in - fileTreeStore.isNodeOrDescendant(requestedID, of: hiddenNodes) - }) - let requestedNodeContainsHiddenNode = hiddenIDs.contains(where: { hiddenID in - fileTreeStore.isNodeOrDescendant(hiddenID, of: requestedNodes) - }) - - guard !requestedNodeIsHidden && !requestedNodeContainsHiddenNode else { - throw FileActionError.unsupported - } + let protectedNodes = tree.preparedNodeSet(for: protectedIDs) + if requestedIDs.contains(where: { tree.isNodeOrDescendant($0, of: protectedNodes) }) { return true } + let requestedNodes = tree.preparedNodeSet(for: requestedIDs) + return protectedIDs.contains { tree.isNodeOrDescendant($0, of: requestedNodes) } + } + + private func validateDirectTrash(_ nodes: [FileNodeRecord]) throws { + guard !directTrashOverlapsProtectedNodes(nodes) else { throw FileActionError.unsupported } } private func topLevelTrashNodes(from nodes: [FileNodeRecord]) -> [FileNodeRecord] { @@ -3092,40 +3038,9 @@ final class AppModel: ObservableObject { ) } - private func hiddenNodeIDs(for snapshotID: UUID?) -> Set { - guard let snapshotID else { return [] } - - var nodeIDs = Set() - if scanCoordinator.snapshot?.id == snapshotID { - nodeIDs.formUnion(projectedDiscardPileIDs) - } - if optimisticTrashVisibility.snapshotID == snapshotID { - nodeIDs.formUnion(optimisticTrashVisibility.nodeIDs) - } - return nodeIDs - } - - private func addDiscardPileNodes( - _ nodes: [FileNodeRecord], - snapshot: ScanSnapshot - ) { - let (updatedPile, matchingNodeIDs) = trashFlow.discardPileAdding(nodes, in: snapshot) - reconcileNavigationForHiddenNodes( - hiddenNodeIDs: hiddenNodeIDs(for: snapshot.id).union(matchingNodeIDs), - fileTreeStore: snapshot.treeStore, - preservingSelection: navigationModel.selectedNodeIDs.count == 1 - ) - trashFlow.changeDiscardPile( - updatedPile, - actionName: String(localized: "Add to Discard Pile", comment: "Action for marking one selected item for possible deletion."), - matchingNodeIDs: matchingNodeIDs - ) - } - private func reconcileNavigationForHiddenNodes( hiddenNodeIDs: Set, - fileTreeStore: FileTreeStore, - preservingSelection: Bool = false + fileTreeStore: FileTreeStore ) { guard !hiddenNodeIDs.isEmpty else { return } let hiddenNodes = fileTreeStore.preparedNodeSet(for: hiddenNodeIDs) @@ -3141,8 +3056,7 @@ final class AppModel: ObservableObject { ) } - if !preservingSelection, - navigationModel.selectedNodeIDs.contains(where: { selectedNodeID in + if navigationModel.selectedNodeIDs.contains(where: { selectedNodeID in fileTreeStore.isNodeOrDescendant(selectedNodeID, of: hiddenNodes) }) { navigationModel.clearSelection() diff --git a/Radix/ViewModels/TrashFlowController.swift b/Radix/ViewModels/TrashFlowController.swift index b45713a..7b28f30 100644 --- a/Radix/ViewModels/TrashFlowController.swift +++ b/Radix/ViewModels/TrashFlowController.swift @@ -172,7 +172,6 @@ final class TrashFlowController { var onDiscardPileChange: ((DiscardPileState, Set?) -> Void)? let discardPileUndoManager = UndoManager() - var onDiscardPileHistoryReplay: ((DiscardPileState) -> Void)? private var historyNotifications = Set() @@ -351,18 +350,21 @@ final class TrashFlowController { func discardPileAdding( _ validatedNodes: [FileNodeRecord], in snapshot: ScanSnapshot - ) -> (pile: DiscardPileState, matchingNodeIDs: Set) { + ) -> (pile: DiscardPileState, matchingNodeIDs: Set, addedNodeIDs: Set) { let tree = snapshot.treeStore var updated = discardPile.marks var offsets = Dictionary(uniqueKeysWithValues: updated.enumerated().map { ($0.element.node.id, $0.offset) }) + var addedNodeIDs: Set = [] for node in validatedNodes { let mark = DiscardPileState.Mark(node: node, treeID: tree.contentID) if let offset = offsets[node.id] { - if !updated[offset].matches(node, treeID: tree.contentID) { updated[offset] = mark } + guard !updated[offset].matches(node, treeID: tree.contentID) else { continue } + updated[offset] = mark } else { offsets[node.id] = updated.count updated.append(mark) } + addedNodeIDs.insert(node.id) } let incomingIDs = Set(validatedNodes.map(\.id)) @@ -378,13 +380,14 @@ final class TrashFlowController { incomingIDs.contains(ancestor.id) || incomingIDs.contains(mark.node.id), updated[offset].node.fileIdentity != nil || mark.node.fileIdentity == nil { retainedIDs.remove(mark.node.id) + addedNodeIDs.remove(mark.node.id) return false } parent = tree.parent(of: ancestor.id) } return true } - return (DiscardPileState(marks: retainedMarks), retainedIDs) + return (DiscardPileState(marks: retainedMarks), retainedIDs, addedNodeIDs) } func removeMovedDiscardPileNodes( @@ -434,7 +437,6 @@ final class TrashFlowController { discardPileUndoManager.setActionName(actionName) if !isReplaying { discardPileUndoManager.endUndoGrouping() } publishDiscardPile(state, matchingNodeIDs: matchingNodeIDs) - if isReplaying { onDiscardPileHistoryReplay?(state) } } private func publishDiscardPile( diff --git a/RadixCoreTests/AppModelDependencyTests.swift b/RadixCoreTests/AppModelDependencyTests.swift index 22bbd40..83d2b13 100644 --- a/RadixCoreTests/AppModelDependencyTests.swift +++ b/RadixCoreTests/AppModelDependencyTests.swift @@ -1186,7 +1186,6 @@ struct AppModelDependencyTests { #expect(model.scanState.snapshot?.treeStore.node(id: file.id) != nil) #expect(model.movingToTrashRootNodeIDs == [file.id]) #expect(model.discardPileRootNodeIDs.isEmpty) - #expect(model.workspaceHiddenNodeIDs.contains(file.id)) await probe.finish() @@ -1217,7 +1216,6 @@ struct AppModelDependencyTests { try await probe.waitUntilStarted() #expect(model.movingToTrashRootNodeIDs == [file.id]) - #expect(model.workspaceHiddenNodeIDs.contains(file.id)) await probe.finish() @@ -1225,7 +1223,6 @@ struct AppModelDependencyTests { model.lastErrorMessage != nil } #expect(model.scanState.snapshot?.treeStore.node(id: file.id) != nil) - #expect(!(model.workspaceHiddenNodeIDs.contains(file.id))) #expect(model.movingToTrashRootNodeIDs.isEmpty) } @@ -1290,7 +1287,7 @@ struct AppModelDependencyTests { #expect(movedIDs == [first.id]) #expect(model.usageStats.bytesMovedToTrash == first.allocatedSize) #expect(model.scanState.snapshot?.treeStore.node(id: second.id) != nil) - #expect(!(model.workspaceHiddenNodeIDs.contains(second.id))) + #expect(!(model.movingToTrashRootNodeIDs.contains(second.id))) #expect(model.lastErrorMessage == nil) } @@ -1361,8 +1358,6 @@ struct AppModelDependencyTests { try await probe.waitUntilStarted() let startedURLs = await probe.movedURLs() #expect(startedURLs == [second.url]) - #expect(model.workspaceHiddenNodeIDs.contains(first.id)) - #expect(model.workspaceHiddenNodeIDs.contains(second.id)) #expect(model.movingToTrashRootNodeIDs == [first.id, second.id]) #expect(model.discardPileRootNodeIDs == [first.id, second.id]) @@ -1453,7 +1448,7 @@ struct AppModelDependencyTests { #expect(model.lastErrorMessage == expectedMessage) #expect(model.scanState.snapshot?.treeStore.node(id: file.id) == file) - #expect(!model.workspaceHiddenNodeIDs.contains(file.id)) + #expect(!model.movingToTrashRootNodeIDs.contains(file.id)) #expect(model.usageStats.bytesMovedToTrash == 0) #expect(model.pendingTrashSelection == nil) } @@ -1502,7 +1497,7 @@ struct AppModelDependencyTests { model.scanState.snapshot?.treeStore.node(id: first.id) == nil } #expect(model.scanState.snapshot?.treeStore.node(id: second.id) != nil) - #expect(!(model.workspaceHiddenNodeIDs.contains(second.id))) + #expect(!(model.movingToTrashRootNodeIDs.contains(second.id))) } @Test @@ -1598,6 +1593,33 @@ struct AppModelDependencyTests { #expect(model.lastErrorMessage == (replaced ? FileActionError.changedSinceScan(path: cloudFile.id).localizedDescription : nil)) } + @Test + func testMarkingLocalFileWithAlreadyMarkedCloudFileSkipsCloudConfirmation() throws { + var actions = AppSystemActions.inert + actions.fileExists = { _ in true } + let model = AppModel(dependencies: makeDependencies(systemActions: actions)) + let cloudFile = makeTestFileNode( + id: "/Users/alex/Library/CloudStorage/Dropbox/cloud.bin", name: "cloud.bin" + ) + let localFile = makeTestFileNode(id: "/Users/alex/local.bin", name: "local.bin") + let root = makeTestDirectoryNode(id: "/Users/alex", name: "alex", children: [cloudFile, localFile]) + let store = FileTreeStore(root: root, childrenByID: [root.id: [cloudFile, localFile]]) + let snapshot = makeTestSnapshot(root: root, store: store) + model.scanState.replaceCurrentSnapshot(snapshot) + model.scanState.selectedTarget = snapshot.target + model.navigation.reconcileAfterSnapshotApplied(snapshot) + + try #require(model.addNodesToDiscardPile([cloudFile])) + try #require(model.pendingCloudFileAction?.kind == .addToDiscardPile) + model.confirmPendingCloudFileAction() + try #require(model.discardPile.nodeIDs == [cloudFile.id]) + + model.navigation.select(nodeIDs: [cloudFile.id, localFile.id], primaryNodeID: localFile.id) + #expect(model.addSelectedNodesToDiscardPile()) + #expect(model.pendingCloudFileAction == nil) + #expect(Set(model.discardPile.nodeIDs) == [cloudFile.id, localFile.id]) + } + @Test func testMovingResidentCloudFileToTrashRequiresSecondConfirmation() async throws { let recorder = AppModelActionRecorder() @@ -1850,7 +1872,7 @@ struct AppModelDependencyTests { } @Test - func testPrimaryTrashRejectsStaleDiscardPileSelection() { + func testPrimaryTrashRejectsMarkedSelection() { let recorder = AppModelActionRecorder() var actions = AppSystemActions.inert actions.fileExists = { _ in true } @@ -1879,7 +1901,7 @@ struct AppModelDependencyTests { } @Test - func testSelectedTrashFiltersStaleDiscardPileSelection() { + func testSelectedTrashRejectsMixedSelectionThatIncludesAMark() { var actions = AppSystemActions.inert actions.fileExists = { _ in true } let model = AppModel(dependencies: makeDependencies(systemActions: actions)) @@ -1894,9 +1916,12 @@ struct AppModelDependencyTests { #expect(model.addNodesToDiscardPile([queued])) model.navigation.select(nodeIDs: [queued.id, visible.id], primaryNodeID: visible.id) + #expect(!model.selectionIsInDiscardPile) model.requestMoveSelectedToTrash() - #expect(model.pendingTrashSelection?.nodes.map(\.id) == [visible.id]) + #expect(model.pendingTrashSelection == nil) + #expect(model.navigation.selectedNodeIDs == [queued.id, visible.id]) + #expect(model.lastErrorMessage == FileActionError.unsupported.localizedDescription) #expect(model.discardPile.nodeIDs == [queued.id]) } @@ -1972,7 +1997,7 @@ struct AppModelDependencyTests { } @Test - func testDiscardPileUndoRestoresChildrenAfterAncestorCollapseAndHidesSelection() { + func testDiscardPileUndoRestoresChildrenAfterAncestorCollapseAndKeepsSelection() { let first = makeTestFileNode(id: "/selection/folder/first", name: "first") let second = makeTestFileNode(id: "/selection/folder/second", name: "second") let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [first, second]) @@ -1993,7 +2018,8 @@ struct AppModelDependencyTests { model.navigation.select(nodeIDs: [first.id, second.id], primaryNodeID: first.id) model.undoDiscardPileChange() #expect(model.discardPile.nodeIDs == [second.id, first.id]) - #expect(model.navigation.selectedNodeIDs.isEmpty) + #expect(model.navigation.selectedNodeIDs == [first.id, second.id]) + #expect(model.navigation.focusedNodeID == folder.id) model.redoDiscardPileChange() #expect(model.discardPile.isEmpty) } @@ -2009,7 +2035,7 @@ struct AppModelDependencyTests { #expect(model.canUndoDiscardPileChange) model.undoDiscardPileChange() #expect(model.discardPileSnapshot.nodes == [file]) - #expect(model.workspaceHiddenNodeIDs.isEmpty) + #expect(model.discardPileRootNodeIDs.isEmpty) installSelection(on: model) #expect(model.canRedoDiscardPileChange) @@ -2043,23 +2069,6 @@ struct AppModelDependencyTests { #expect(!(model.discardPileUndoManager.canRedo)) } - @Test - func testPrimaryDiscardPileAddAfterViewUpdateDefersMutation() async throws { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let file = installSelection(on: model) - - model.addPrimarySelectionToDiscardPileAfterViewUpdate() - - #expect(model.discardPile.isEmpty) - #expect(model.navigation.selectedNodeID == file.id) - - try await waitUntil("deferred discard pile add") { - model.discardPile.nodeIDs == [file.id] && model.navigation.selectedNodeID == file.id - } - } - @Test func testDiscardPileAddDefersLivePathValidationUntilTrashRequest() async { let verified = OSAllocatedUnfairLock(initialState: [FileNodeRecord]()) @@ -2255,188 +2264,42 @@ struct AppModelDependencyTests { } @Test - func testDiscardPileAddClearsQueuedMultiSelection() { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let first = makeTestFileNode(id: "/selection/first.txt", name: "first.txt") - let second = makeTestFileNode(id: "/selection/second.txt", name: "second.txt") - let root = makeTestDirectoryNode( - id: "/selection", - name: "selection", - children: [first, second] - ) - let store = FileTreeStore(root: root, childrenByID: [root.id: [first, second]]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) - model.navigation.reconcileAfterSnapshotApplied(snapshot) - model.select(nodeIDs: [first.id, second.id], primaryNodeID: first.id) - - model.addSelectedNodesToDiscardPile() - - #expect(model.navigation.selectedNodeIDs.isEmpty) - #expect(model.navigation.selectedNodeID == nil) - } - - @Test - func testDiscardPileNodeCanBeSelectedButNotFocused() throws { - let recorder = AppModelActionRecorder() - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - actions.reveal = { recorder.revealedURLs.append($0) } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let file = installSelection(on: model) - let rootID = try #require(model.scanState.snapshot?.root.id) - - model.addNodesToDiscardPile([file]) - model.clearSelection() - model.select(nodeID: file.id) - model.focus(nodeID: file.id) - model.revealPrimarySelectionInFinder() - - #expect(model.navigation.selectedNodeID == file.id) - #expect(model.navigation.focusedNodeID == rootID) - #expect(!(model.canZoomIntoSelection)) - #expect(model.selectionIncludesHiddenNodes) - #expect(recorder.revealedURLs == [file.url]) - } - - @Test - func testDiscardPileAddMovesHiddenFocusToVisibleAncestor() { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let child = makeTestFileNode(id: "/selection/folder/child.txt", name: "child.txt") - let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child]) + func testMarkingKeepsBrowserContentsSelectionAndHistory() throws { + let model = AppModel(dependencies: makeDependencies()) + let first = makeTestFileNode(id: "/selection/folder/first.txt", name: "first.txt") + let second = makeTestFileNode(id: "/selection/folder/second.txt", name: "second.txt") + let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [first, second]) let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder]) - let store = FileTreeStore( - root: root, - childrenByID: [ - root.id: [folder], - folder.id: [child], - ]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) + let tree = FileTreeStore(root: root, childrenByID: [root.id: [folder], folder.id: [first, second]]) + let snapshot = makeTestSnapshot(root: root, store: tree) + model.scanState.restoreCompletedSnapshot(snapshot) model.navigation.reconcileAfterSnapshotApplied(snapshot) model.focus(nodeID: folder.id) - model.select(nodeID: child.id) - - model.addNodesToDiscardPile([folder]) - - #expect(model.navigation.focusedNodeID == root.id) - #expect(model.navigation.selectedNodeID == child.id) - #expect(model.navigation.selectedNodeIDs == [child.id]) - #expect(!(model.navigation.canNavigateBack)) - } - - @Test - func testDiscardPileAddPrunesQueuedFolderFromBackHistory() { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let queued = makeTestDirectoryNode( - id: "/selection/queued", - name: "queued", - children: [] - ) - let visible = makeTestDirectoryNode( - id: "/selection/visible", - name: "visible", - children: [] - ) - let root = makeTestDirectoryNode( - id: "/selection", - name: "selection", - children: [queued, visible] - ) - let store = FileTreeStore(root: root, childrenByID: [root.id: [queued, visible]]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) - model.navigation.reconcileAfterSnapshotApplied(snapshot) - model.focus(nodeID: queued.id) - model.focus(nodeID: visible.id) - - model.addNodesToDiscardPile([queued]) - model.navigateBack() + model.select(nodeIDs: [first.id, second.id], primaryNodeID: first.id) + let before = model.navigation.browserContent(excluding: model.movingToTrashRootNodeIDs) - #expect(model.navigation.focusedNodeID == root.id) - #expect(!(model.navigation.canNavigateBack)) - #expect(model.navigation.canNavigateForward) - } + try #require(model.addSelectedNodesToDiscardPile()) + #expect(model.discardPileRootNodeIDs == [first.id, second.id]) + try #require(model.addNodesToDiscardPile([folder])) + #expect(model.discardPileRootNodeIDs == [folder.id]) + let after = model.navigation.browserContent(excluding: model.movingToTrashRootNodeIDs) + #expect(after.nodes.map(\.id) == [first.id, second.id]) + #expect(after.refreshID == before.refreshID) + #expect(model.navigation.focusedNodeID == folder.id) + #expect(model.navigation.selectedNodeIDs == [first.id, second.id]) + #expect(model.navigation.selectedNodeID == first.id) + #expect(model.selectionIsInDiscardPile) - @Test - func testDiscardPileAddPrunesQueuedDescendantFromForwardHistory() { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let descendant = makeTestDirectoryNode( - id: "/selection/queued/descendant", - name: "descendant", - children: [] - ) - let queued = makeTestDirectoryNode( - id: "/selection/queued", - name: "queued", - children: [descendant] - ) - let root = makeTestDirectoryNode( - id: "/selection", - name: "selection", - children: [queued] - ) - let store = FileTreeStore( - root: root, - childrenByID: [ - root.id: [queued], - queued.id: [descendant], - ]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) - model.navigation.reconcileAfterSnapshotApplied(snapshot) - model.focus(nodeID: descendant.id) model.navigateBack() - - model.addNodesToDiscardPile([queued]) - #expect(model.navigation.focusedNodeID == root.id) - #expect(!(model.navigation.canNavigateForward)) + model.undoDiscardPileChange() + model.redoDiscardPileChange() model.navigateForward() - #expect(model.navigation.focusedNodeID == root.id) + #expect(model.navigation.focusedNodeID == folder.id) } @Test - func testDiscardPilePublishesAfterHiddenFocusReconciles() { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let child = makeTestFileNode(id: "/selection/folder/child.txt", name: "child.txt") - let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child]) - let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder]) - let store = FileTreeStore( - root: root, - childrenByID: [ - root.id: [folder], - folder.id: [child], - ]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) - model.navigation.reconcileAfterSnapshotApplied(snapshot) - model.navigation.setFocusedNodeID(folder.id) - model.select(nodeID: folder.id) - - var observedFocusID: FileNodeRecord.ID? - let cancellable = model.trashFlow.$discardPile.dropFirst().sink { _ in - observedFocusID = model.navigation.focusedNodeID - } - defer { cancellable.cancel() } - - model.addNodesToDiscardPile([folder]) - - #expect(observedFocusID == root.id) - } - - @Test - func testSnapshotReplacementPublishesChangedDiscardPileVisibility() throws { + func testSnapshotReplacementPublishesChangedDiscardPileProjection() throws { let model = AppModel(dependencies: makeDependencies()) defer { model.cleanup() } model.showFreeSpaceInDiskMaps = true @@ -2465,7 +2328,7 @@ struct AppModelDependencyTests { } model.scanState.restoreCompletedSnapshot(snapshot(containing: marked)) try #require(model.addNodesToDiscardPile([marked])) - try #require(model.workspaceHiddenNodeIDs.contains(marked.id)) + try #require(model.discardPileRootNodeIDs.contains(marked.id)) var didPublish = false let cancellable = model.objectWillChange.sink { didPublish = true } @@ -2473,39 +2336,10 @@ struct AppModelDependencyTests { model.scanState.replaceCurrentSnapshot(snapshot(containing: replacement)) #expect(didPublish) - #expect(!model.workspaceHiddenNodeIDs.contains(marked.id)) + #expect(!model.discardPileRootNodeIDs.contains(marked.id)) #expect(model.addNodesToDiscardPile([replacement])) - #expect(model.workspaceHiddenNodeIDs.contains(replacement.id)) - } - - @Test - func testDeferredDiscardPileAddMovesFocusedSelectionToVisibleAncestor() async throws { - var actions = AppSystemActions.inert - actions.fileExists = { _ in true } - let model = AppModel(dependencies: makeDependencies(systemActions: actions)) - let child = makeTestFileNode(id: "/selection/folder/child.txt", name: "child.txt") - let folder = makeTestDirectoryNode(id: "/selection/folder", name: "folder", children: [child]) - let sibling = makeTestFileNode(id: "/selection/sibling.txt", name: "sibling.txt") - let root = makeTestDirectoryNode(id: "/selection", name: "selection", children: [folder, sibling]) - let store = FileTreeStore( - root: root, - childrenByID: [ - root.id: [folder, sibling], - folder.id: [child], - ]) - let snapshot = makeTestSnapshot(root: root, store: store) - model.scanState.replaceCurrentSnapshot(snapshot) - model.navigation.reconcileAfterSnapshotApplied(snapshot) - model.navigation.setFocusedNodeID(folder.id) - model.select(nodeID: folder.id) - - model.addPrimarySelectionToDiscardPileAfterViewUpdate() - - try await waitUntil("deferred focused discard pile add") { - model.discardPile.nodeIDs == [folder.id] && model.navigation.focusedNodeID == root.id - && model.navigation.selectedNodeID == folder.id - } + #expect(model.discardPileRootNodeIDs.contains(replacement.id)) } @Test @@ -2520,7 +2354,7 @@ struct AppModelDependencyTests { model.scanState.replaceCurrentSnapshot(firstSnapshot) model.navigation.reconcileAfterSnapshotApplied(firstSnapshot) model.addNodesToDiscardPile([firstFile]) - #expect(model.workspaceHiddenNodeIDs == [firstFile.id]) + #expect(model.discardPileRootNodeIDs == [firstFile.id]) let secondFile = makeTestFileNode(id: "/second/file.txt", name: "file.txt") let secondRoot = makeTestDirectoryNode(id: "/second", name: "second", children: [secondFile]) @@ -2529,7 +2363,7 @@ struct AppModelDependencyTests { model.scanState.replaceCurrentSnapshot(secondSnapshot) #expect(model.discardPileSnapshot.nodes == [firstFile]) - #expect(model.workspaceHiddenNodeIDs.isEmpty) + #expect(model.discardPileRootNodeIDs.isEmpty) } @Test @@ -2602,7 +2436,7 @@ struct AppModelDependencyTests { display("/second", nodes: [failed, unattempted]) try #require(model.addNodesToDiscardPile([failed, unattempted])) #expect(model.discardPileSnapshot.nodes == [first, failed, unattempted]) - #expect(model.workspaceHiddenNodeIDs == [failed.id, unattempted.id]) + #expect(model.discardPileRootNodeIDs == [failed.id, unattempted.id]) try #require(await model.requestMoveDiscardPileToTrash()) #expect(model.pendingTrashSelection?.nodes == [first, failed, unattempted]) @@ -3890,7 +3724,8 @@ struct AppModelDependencyTests { enum ComparisonRevealInterruption: CaseIterable, Equatable { case none case cancelled - case hidden + case marked + case movingToTrash case removed case replacedSnapshot case replacedComparison @@ -3923,10 +3758,12 @@ struct AppModelDependencyTests { break case .cancelled: model.clearSelection() - case .hidden: + case .marked: model.trashFlow.replaceDiscardPile( DiscardPileState(marks: [.init(node: node, treeID: current.treeStore.contentID)]) ) + case .movingToTrash: + model.trashFlow.replaceOptimisticTrashVisibility(nodes: [node.id: node], snapshotID: current.id) case .removed: let removed = try #require(current.removingNode(id: node.id)) model.scanState.replaceCurrentSnapshot(removed) @@ -3949,7 +3786,7 @@ struct AppModelDependencyTests { } for task in pending { await task.value } - if interruption == .none || interruption == .refreshedMetadata { + if interruption == .none || interruption == .marked || interruption == .refreshedMetadata { #expect(model.scanComparison == nil) #expect(model.navigation.selectedNodeID == node.id) #expect(model.navigation.focusedNodeID == current.root.id) diff --git a/RadixCoreTests/AppModelFolderRefreshTests.swift b/RadixCoreTests/AppModelFolderRefreshTests.swift index 1c0592a..1cb0639 100644 --- a/RadixCoreTests/AppModelFolderRefreshTests.swift +++ b/RadixCoreTests/AppModelFolderRefreshTests.swift @@ -286,7 +286,7 @@ struct AppModelFolderRefreshTests { #expect(model.navigation.focusedNodeID == fixture.downloads.id) #expect(model.navigation.selectedNodeID == fixture.keptDownload.id) #expect(model.discardPile.nodeIDs == [fixture.markedDownload.id]) - #expect(model.workspaceHiddenNodeIDs.contains(fixture.markedDownload.id)) + #expect(model.discardPileRootNodeIDs.contains(fixture.markedDownload.id)) #expect(model.scanState.snapshot?.treeStore.node(id: fixture.document.id) == fixture.document) #expect(model.scanState.scanCompletionNotice == nil) diff --git a/RadixCoreTests/DiscardPileVisualizationOverlayTests.swift b/RadixCoreTests/DiscardPileVisualizationOverlayTests.swift index 2cbe0de..1f15b2f 100644 --- a/RadixCoreTests/DiscardPileVisualizationOverlayTests.swift +++ b/RadixCoreTests/DiscardPileVisualizationOverlayTests.swift @@ -19,8 +19,8 @@ struct DiscardPileVisualizationOverlayTests { #expect(overlay.role(for: fixture.folder.id) == .queuedRoot) #expect(overlay.role(for: fixture.child.id) == .queuedDescendant) #expect(overlay.role(for: fixture.sibling.id) == nil) - #expect(!(overlay.allowsChartNodeAction(for: fixture.folder.id))) - #expect(!(overlay.allowsChartNodeAction(for: fixture.child.id))) + #expect(overlay.allowsChartNodeAction(for: fixture.folder.id)) + #expect(overlay.allowsChartNodeAction(for: fixture.child.id)) #expect(overlay.allowsChartNodeAction(for: fixture.sibling.id)) #expect(fixture.store.node(id: fixture.folder.id) != nil) #expect(fixture.store.node(id: fixture.child.id) != nil) @@ -154,7 +154,6 @@ struct DiscardPileVisualizationOverlayTests { #expect(overlay.role(for: fixture.folder.id)?.statusText == "Moving to Trash") #expect(overlay.role(for: fixture.sibling.id)?.statusText == "In Discard Pile") #expect(overlay.isMovingToTrash(fixture.child.id)) - #expect(!(overlay.isQueued(fixture.child.id))) #expect(!(overlay.allowsChartNodeAction(for: fixture.child.id))) } diff --git a/RadixCoreTests/FileBrowserModelTests.swift b/RadixCoreTests/FileBrowserModelTests.swift index fed2bd0..1b31f9e 100644 --- a/RadixCoreTests/FileBrowserModelTests.swift +++ b/RadixCoreTests/FileBrowserModelTests.swift @@ -513,7 +513,7 @@ struct FileBrowserModelTests { @MainActor @Test - func testEntireScanSearchHidesDiscardPileQueuedDescendants() async throws { + func testEntireScanSearchHidesDescendantsOfExcludedFolders() async throws { let hiddenTarget = makeTestFileNode(id: "/root/folder/target-hidden.txt", name: "target-hidden.txt", size: 40) let hiddenFolder = makeTestDirectoryNode(id: "/root/folder", name: "folder", children: [hiddenTarget]) let visibleTarget = makeTestFileNode(id: "/root/target-visible.txt", name: "target-visible.txt", size: 10) diff --git a/RadixCoreTests/GoToPathTests.swift b/RadixCoreTests/GoToPathTests.swift index 1577677..6f4153b 100644 --- a/RadixCoreTests/GoToPathTests.swift +++ b/RadixCoreTests/GoToPathTests.swift @@ -225,13 +225,14 @@ struct GoToPathTests { #expect(model.navigation.currentFocusNode?.id == fixture.root.id) } - @Test(arguments: [false, true]) - func fileJumpOpensItsParentAndSelectsIt(startInParent: Bool) async throws { + @Test(arguments: [false, true], [false, true]) + func fileJumpOpensItsParentAndSelectsIt(startInParent: Bool, isMarked: Bool) async throws { let fixture = PathFixture() - let model = makePathApp(fixture) + let model = makePathApp(fixture, isImported: !isMarked) defer { model.cleanup() } let base = startInParent ? fixture.folder.id : fixture.root.id model.navigation.setFocusedNodeID(base) + if isMarked { try #require(model.addNodesToDiscardPile([fixture.file])) } model.presentGoToPath() let request = try pathRequest(model) #expect(await model.goToPath(fixture.file.url.path, request: request)) @@ -395,7 +396,11 @@ private func pathRecord(id: String, path: String, directory: Bool = false, synth } @MainActor -private func makePathApp(_ fixture: PathFixture, store: FileTreeStore? = nil) -> AppModel { +private func makePathApp( + _ fixture: PathFixture, + store: FileTreeStore? = nil, + isImported: Bool = true +) -> AppModel { let model = AppModel(dependencies: AppDependencies( preferences: TestAppPreferencesStore(preferences: AppPreferences(scan: .defaults, didCompleteOnboarding: true)), recentTargets: RecentTargetStore(persistence: TestRecentTargetPersistence(), isAvailable: { _ in false }), @@ -404,10 +409,10 @@ private func makePathApp(_ fixture: PathFixture, store: FileTreeStore? = nil) -> let snapshot = ScanSnapshot( target: ScanTarget(url: fixture.root.url), treeStore: store ?? fixture.store, startedAt: Date(), finishedAt: Date(), scanWarnings: [], isComplete: true, - source: .imported(ImportedSnapshotContext( + source: isImported ? .imported(ImportedSnapshotContext( sourceURL: URL(filePath: "/saved.radixscan"), pathMode: .absolute, liveActionCapability: .pathValidation - )) + )) : .live ) model.scanState.replaceCurrentSnapshot(snapshot) model.navigation.reconcileAfterSnapshotApplied(snapshot) diff --git a/RadixCoreTests/ScanCoordinatorTests.swift b/RadixCoreTests/ScanCoordinatorTests.swift index 1c804a7..4a37e94 100644 --- a/RadixCoreTests/ScanCoordinatorTests.swift +++ b/RadixCoreTests/ScanCoordinatorTests.swift @@ -1804,7 +1804,7 @@ struct ScanCoordinatorTests { service.finish(scanIndex: 1) try await waitUntil("second sidebar scan finished") { model.scanState.snapshot?.target == secondTarget } #expect(model.discardPileSnapshot.nodes == [kept, replaced]) - #expect(model.workspaceHiddenNodeIDs.isEmpty) + #expect(model.discardPileRootNodeIDs.isEmpty) model.selectSidebarTarget(id: firstTarget.id) if rescansOnReturn { @@ -1816,7 +1816,7 @@ struct ScanCoordinatorTests { let expectedIDs = rescansOnReturn ? [kept.id] : [kept.id, replaced.id] #expect(model.discardPileSnapshot.nodes == [kept, replaced]) - #expect(model.workspaceHiddenNodeIDs == Set(expectedIDs)) + #expect(model.discardPileRootNodeIDs == Set(expectedIDs)) } @Test(arguments: [false, true]) @@ -2115,7 +2115,7 @@ struct ScanCoordinatorTests { model.selectSidebarTarget(id: scope.id) try await waitUntil("scope displays again") { model.scanState.snapshot?.target == scope } try #require(model.discardPileSnapshot.nodes.map(\.id) == [marked.id]) - try #require(model.workspaceHiddenNodeIDs.contains(marked.id)) + try #require(model.discardPileRootNodeIDs.contains(marked.id)) move.finish() try await waitUntil("confirmed move finishes") { model.usageStats.bytesMovedToTrash > 0 } @@ -2123,7 +2123,7 @@ struct ScanCoordinatorTests { try #require(model.discardPileSnapshot.nodes.isEmpty) if model.scanState.fileTreeStore?.node(id: marked.id) != nil { - #expect(model.workspaceHiddenNodeIDs.contains(marked.id)) + #expect(model.movingToTrashRootNodeIDs.contains(marked.id)) } try await waitUntil("moved file is removed from the overlapping scan") { model.scanState.fileTreeStore?.node(id: marked.id) == nil @@ -2195,7 +2195,7 @@ struct ScanCoordinatorTests { } try #require(model.scanState.snapshot?.id == initial.id) #expect(model.discardPileSnapshot.nodes == [marked]) - #expect(model.workspaceHiddenNodeIDs.isEmpty) + #expect(model.discardPileRootNodeIDs.isEmpty) #expect(await model.requestMoveDiscardPileToTrash()) #expect(model.pendingTrashSelection?.nodes.isEmpty == true) #expect(model.pendingTrashSelection?.discardPileBatch?.unavailable.map(\.node) == [marked]) @@ -2255,7 +2255,7 @@ struct ScanCoordinatorTests { try #require(model.scanState.fileTreeStore?.node(id: summarized.id)?.isAutoSummarized == true) try #require(model.scanState.fileTreeStore?.node(id: marked.id) == nil) #expect(model.discardPileSnapshot.nodes == [marked]) - #expect(!model.workspaceHiddenNodeIDs.contains(marked.id)) + #expect(!model.discardPileRootNodeIDs.contains(marked.id)) model.expandSummarizedNode(summarized) {} try await waitUntil("second expansion request") { service.requests.count == 5 }