From 9818278c9b4db555f50e35f1bf79c77eb09ac836 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sun, 5 Nov 2017 12:14:36 -0800 Subject: [PATCH] Make undo deleting feeds/folders work. --- Commands/DeleteFromSidebarCommand.swift | 301 ++++++++++-------- .../Sidebar/SidebarViewController.swift | 5 +- Frameworks/Account/Account.swift | 10 +- 3 files changed, 185 insertions(+), 131 deletions(-) diff --git a/Commands/DeleteFromSidebarCommand.swift b/Commands/DeleteFromSidebarCommand.swift index 230000e0a..b384f283b 100644 --- a/Commands/DeleteFromSidebarCommand.swift +++ b/Commands/DeleteFromSidebarCommand.swift @@ -14,7 +14,7 @@ import Data final class DeleteFromSidebarCommand: UndoableCommand { - let undoManager: UndoManager + let undoManager: UndoManager let undoActionName: String var redoActionName: String { get { @@ -22,40 +22,40 @@ final class DeleteFromSidebarCommand: UndoableCommand { } } - private let itemSpecifiers: [SidebarItemSpecifier] - + private let itemSpecifiers: [SidebarItemSpecifier] + init?(nodesToDelete: [Node], undoManager: UndoManager) { - guard DeleteFromSidebarCommand.canDelete(nodesToDelete) else { - return nil - } - guard let actionName = DeleteActionName.name(for: nodesToDelete) else { - return nil - } - - self.undoActionName = actionName + guard DeleteFromSidebarCommand.canDelete(nodesToDelete) else { + return nil + } + guard let actionName = DeleteActionName.name(for: nodesToDelete) else { + return nil + } + + self.undoActionName = actionName self.undoManager = undoManager - - let itemSpecifiers = nodesToDelete.flatMap{ SidebarItemSpecifier(node: $0) } - guard !itemSpecifiers.isEmpty else { - return nil - } - self.itemSpecifiers = itemSpecifiers + + let itemSpecifiers = nodesToDelete.flatMap{ SidebarItemSpecifier(node: $0) } + guard !itemSpecifiers.isEmpty else { + return nil + } + self.itemSpecifiers = itemSpecifiers } func perform() { - BatchUpdate.shared.perform { - itemSpecifiers.forEach { $0.delete() } - } + BatchUpdate.shared.perform { + itemSpecifiers.forEach { $0.delete() } + } registerUndo() } func undo() { - BatchUpdate.shared.perform { - - } + BatchUpdate.shared.perform { + itemSpecifiers.forEach { $0.restore() } + } registerRedo() } @@ -88,133 +88,176 @@ final class DeleteFromSidebarCommand: UndoableCommand { private struct SidebarItemSpecifier { private weak var account: Account? - private let parentFolder: Folder? + private let parentFolder: Folder? private let folder: Folder? private let feed: Feed? private let path: ContainerPath - - private var container: Container? { - get { - if let parentFolder = parentFolder { - return parentFolder - } - if let account = account { - return account - } - return nil - } - } - + + private var container: Container? { + get { + if let parentFolder = parentFolder { + return parentFolder + } + if let account = account { + return account + } + return nil + } + } + init?(node: Node) { var account: Account? - - self.parentFolder = node.parentFolder() - if let feed = node.representedObject as? Feed { + self.parentFolder = node.parentFolder() + + if let feed = node.representedObject as? Feed { self.feed = feed - self.folder = nil - account = feed.account + self.folder = nil + account = feed.account } else if let folder = node.representedObject as? Folder { - self.feed = nil + self.feed = nil self.folder = folder account = folder.account } - else { - return nil - } - if account == nil { - return nil - } + else { + return nil + } + if account == nil { + return nil + } self.account = account! - self.path = ContainerPath(account: account!, folders: node.containingFolders()) + self.path = ContainerPath(account: account!, folders: node.containingFolders()) + } + + func delete() { + + guard let container = container else { + return + } + + if let feed = feed { + container.deleteFeed(feed) + } + else if let folder = folder { + container.deleteFolder(folder) + } + } + + func restore() { + + if let feed = feed { + restoreFeed() + } + else if let folder = folder { + restoreFolder() + } + } + + private func restoreFeed() { + + guard let account = account, let feed = feed else { + return + } + + let feedToUse = uniquedFeed(feed) + account.addFeed(feedToUse, to: resolvedFolder()) + } + + private func restoreFolder() { + + guard let account = account, let folder = folder else { + return + } + account.addFolder(folder, to: nil) + } + + private func uniquedFeed(_ feed: Feed) -> Feed { + + // A Feed may appear in multiple places in a given account, + // but it’s best if they’re the same Feed instance. + // Usually this will return the same Feed that was passed-in, + // but not necessarily always. + + return account?.existingFeed(with: feed.feedID) ?? feed + } + + private func resolvedFolder() -> Folder? { + + return path.resolveContainer() as? Folder } - - func delete() { - - guard let container = container else { - return - } - - if let feed = feed { - container.deleteFeed(feed) - } - else if let folder = folder { - container.deleteFolder(folder) - } - } } private extension Node { - - func parentFolder() -> Folder? { - - guard let parentNode = self.parent else { - return nil - } - if parentNode.isRoot { - return nil - } - if let folder = parentNode.representedObject as? Folder { + + func parentFolder() -> Folder? { + + guard let parentNode = self.parent else { + return nil + } + if parentNode.isRoot { + return nil + } + if let folder = parentNode.representedObject as? Folder { return folder - } - return nil - } - - func containingFolders() -> [Folder] { - - var nomad = self.parent - var folders = [Folder]() - - while nomad != nil { - if let folder = nomad!.representedObject as? Folder { - folders += [folder] - } - else { - break - } - nomad = nomad!.parent - } - - return folders.reversed() - } - + } + return nil + } + + func containingFolders() -> [Folder] { + + var nomad = self.parent + var folders = [Folder]() + + while nomad != nil { + if let folder = nomad!.representedObject as? Folder { + folders += [folder] + } + else { + break + } + nomad = nomad!.parent + } + + return folders.reversed() + } + } private struct DeleteActionName { - - private static let deleteFeed = NSLocalizedString("Delete Feed", comment: "command") - private static let deleteFeeds = NSLocalizedString("Delete Feeds", comment: "command") - private static let deleteFolder = NSLocalizedString("Delete Folder", comment: "command") - private static let deleteFolders = NSLocalizedString("Delete Folders", comment: "command") - private static let deleteFeedsAndFolders = NSLocalizedString("Delete Feeds and Folders", comment: "command") - - static func name(for nodes: [Node]) -> String? { - - var numberOfFeeds = 0 - var numberOfFolders = 0 - - for node in nodes { - if let _ = node.representedObject as? Feed { - numberOfFeeds += 1 - } - else if let _ = node.representedObject as? Folder { - numberOfFolders += 1 - } - else { - return nil // Delete only Feeds and Folders. - } - } - - if numberOfFolders < 1 { - return numberOfFeeds == 1 ? deleteFeed : deleteFeeds - } - if numberOfFeeds < 1 { - return numberOfFolders == 1 ? deleteFolder : deleteFolders - } - - return deleteFeedsAndFolders - } + + private static let deleteFeed = NSLocalizedString("Delete Feed", comment: "command") + private static let deleteFeeds = NSLocalizedString("Delete Feeds", comment: "command") + private static let deleteFolder = NSLocalizedString("Delete Folder", comment: "command") + private static let deleteFolders = NSLocalizedString("Delete Folders", comment: "command") + private static let deleteFeedsAndFolders = NSLocalizedString("Delete Feeds and Folders", comment: "command") + + static func name(for nodes: [Node]) -> String? { + + var numberOfFeeds = 0 + var numberOfFolders = 0 + + for node in nodes { + if let _ = node.representedObject as? Feed { + numberOfFeeds += 1 + } + else if let _ = node.representedObject as? Folder { + numberOfFolders += 1 + } + else { + return nil // Delete only Feeds and Folders. + } + } + + if numberOfFolders < 1 { + return numberOfFeeds == 1 ? deleteFeed : deleteFeeds + } + if numberOfFeeds < 1 { + return numberOfFolders == 1 ? deleteFolder : deleteFolders + } + + return deleteFeedsAndFolders + } } diff --git a/Evergreen/MainWindow/Sidebar/SidebarViewController.swift b/Evergreen/MainWindow/Sidebar/SidebarViewController.swift index acebca5fa..3459c8104 100644 --- a/Evergreen/MainWindow/Sidebar/SidebarViewController.swift +++ b/Evergreen/MainWindow/Sidebar/SidebarViewController.swift @@ -20,6 +20,7 @@ import RSCore TreeController(delegate: treeControllerDelegate) }() var undoableCommands = [UndoableCommand]() + private var animatingChanges = false //MARK: NSViewController @@ -79,11 +80,13 @@ import RSCore let selectedRows = outlineView.selectedRowIndexes + animatingChanges = true outlineView.beginUpdates() outlineView.removeItems(at: selectedRows, inParent: nil, withAnimation: [.slideDown]) outlineView.endUpdates() runCommand(deleteCommand) + animatingChanges = false } // MARK: Navigation @@ -170,7 +173,7 @@ private extension SidebarViewController { func rebuildTreeAndReloadDataIfNeeded() { - if !BatchUpdate.shared.isPerforming { + if !animatingChanges && !BatchUpdate.shared.isPerforming { treeController.rebuild() outlineView.reloadData() } diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index e3e44d9cd..9fdc1640f 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -276,7 +276,15 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, @discardableResult public func addFolder(_ folder: Folder, to parentFolder: Folder?) -> Bool { - return false // TODO + // TODO: support subfolders, maybe, some day, if one of the sync systems + // supports subfolders. But, for now, parentFolder is ignored. + + if objectIsChild(folder) { + return true + } + children += [folder] + postChildrenDidChangeNotification() + return true } public func importOPML(_ opmlDocument: RSOPMLDocument) {