From 50508446bb4b4cdad36c4a49bd269d5d27364bf7 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Mon, 15 Jun 2020 18:03:20 -0500 Subject: [PATCH] Use immutable object for diffable datasource on Feeds. Issues #1901, #2031, #2124 --- Frameworks/Account/AccountManager.swift | 28 +++ NetNewsWire.xcodeproj/project.pbxproj | 8 +- ...erFeedTableViewCellSectionIdentifier.swift | 18 ++ .../Cell/MasterFeedTableViewIdentifier.swift | 98 ++++++++ iOS/MasterFeed/MasterFeedDataSource.swift | 4 +- .../MasterFeedDataSourceOperation.swift | 6 +- .../MasterFeedViewController+Drag.swift | 6 +- .../MasterFeedViewController+Drop.swift | 64 ++--- iOS/MasterFeed/MasterFeedViewController.swift | 233 ++++++++++-------- iOS/SceneCoordinator.swift | 66 +++-- 10 files changed, 357 insertions(+), 174 deletions(-) create mode 100644 iOS/MasterFeed/Cell/MasterFeedTableViewCellSectionIdentifier.swift create mode 100644 iOS/MasterFeed/Cell/MasterFeedTableViewIdentifier.swift diff --git a/Frameworks/Account/AccountManager.swift b/Frameworks/Account/AccountManager.swift index c2b1a5da0..7b2999529 100644 --- a/Frameworks/Account/AccountManager.swift +++ b/Frameworks/Account/AccountManager.swift @@ -169,6 +169,34 @@ public final class AccountManager: UnreadCountProvider { return accountsDictionary[accountID] } + public func existingContainer(with containerID: ContainerIdentifier) -> Container? { + switch containerID { + case .account(let accountID): + return existingAccount(with: accountID) + case .folder(let accountID, let folderName): + return existingAccount(with: accountID)?.existingFolder(with: folderName) + default: + break + } + return nil + } + + public func existingFeed(with feedID: FeedIdentifier) -> Feed? { + switch feedID { + case .folder(let accountID, let folderName): + if let account = existingAccount(with: accountID) { + return account.existingFolder(with: folderName) + } + case .webFeed(let accountID, let webFeedID): + if let account = existingAccount(with: accountID) { + return account.existingWebFeed(withWebFeedID: webFeedID) + } + default: + break + } + return nil + } + public func suspendNetworkAll() { isSuspended = true accounts.forEach { $0.suspendNetwork() } diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index 9131725af..09ccdf60e 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -109,6 +109,7 @@ 513C5D0A232574D2003D4054 /* RSWeb.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 84C37FA320DD8D0500CA8CF5 /* RSWeb.framework */; }; 513C5D0C232574DA003D4054 /* RSTree.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 84C37F9520DD8CFE00CA8CF5 /* RSTree.framework */; }; 513C5D0E232574E4003D4054 /* SyncDatabase.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 51554C01228B6EB50055115A /* SyncDatabase.framework */; }; + 513CCF2524880C1500C55709 /* MasterFeedTableViewIdentifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 513CCF08248808BA00C55709 /* MasterFeedTableViewIdentifier.swift */; }; 5141E7392373C18B0013FF27 /* WebFeedInspectorViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5141E7382373C18B0013FF27 /* WebFeedInspectorViewController.swift */; }; 5142192A23522B5500E07E2C /* ImageViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5142192923522B5500E07E2C /* ImageViewController.swift */; }; 514219372352510100E07E2C /* ImageScrollView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 514219362352510100E07E2C /* ImageScrollView.swift */; }; @@ -1461,6 +1462,7 @@ 513C5CE8232571C2003D4054 /* ShareViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ShareViewController.swift; sourceTree = ""; }; 513C5CEB232571C2003D4054 /* Base */ = {isa = PBXFileReference; lastKnownFileType = file.storyboard; name = Base; path = Base.lproj/MainInterface.storyboard; sourceTree = ""; }; 513C5CED232571C2003D4054 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; + 513CCF08248808BA00C55709 /* MasterFeedTableViewIdentifier.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MasterFeedTableViewIdentifier.swift; sourceTree = ""; }; 5141E7382373C18B0013FF27 /* WebFeedInspectorViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WebFeedInspectorViewController.swift; sourceTree = ""; }; 5141E7552374A2890013FF27 /* DetailIconSchemeHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DetailIconSchemeHandler.swift; sourceTree = ""; }; 5142192923522B5500E07E2C /* ImageViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ImageViewController.swift; sourceTree = ""; }; @@ -2269,10 +2271,11 @@ 51C45260226508F600C03939 /* Cell */ = { isa = PBXGroup; children = ( - 512E08F722688F7C00BDCFDD /* MasterFeedTableViewSectionHeader.swift */, - 516AE9B22371C372007DEEAA /* MasterFeedTableViewSectionHeaderLayout.swift */, 51C45262226508F600C03939 /* MasterFeedTableViewCell.swift */, 51C45263226508F600C03939 /* MasterFeedTableViewCellLayout.swift */, + 513CCF08248808BA00C55709 /* MasterFeedTableViewIdentifier.swift */, + 512E08F722688F7C00BDCFDD /* MasterFeedTableViewSectionHeader.swift */, + 516AE9B22371C372007DEEAA /* MasterFeedTableViewSectionHeaderLayout.swift */, 51C45261226508F600C03939 /* MasterFeedUnreadCountView.swift */, ); path = Cell; @@ -4481,6 +4484,7 @@ 511D4419231FC02D00FB1562 /* KeyboardManager.swift in Sources */, 51A1699D235E10D700EB091F /* SettingsViewController.swift in Sources */, 51C45293226509C800C03939 /* StarredFeedDelegate.swift in Sources */, + 513CCF2524880C1500C55709 /* MasterFeedTableViewIdentifier.swift in Sources */, 51D6A5BC23199C85001C27D8 /* MasterTimelineDataSource.swift in Sources */, 51934CCB230F599B006127BE /* InteractiveNavigationController.swift in Sources */, 769F2ED513DA03EE75B993A8 /* NewsBlurAccountViewController.swift in Sources */, diff --git a/iOS/MasterFeed/Cell/MasterFeedTableViewCellSectionIdentifier.swift b/iOS/MasterFeed/Cell/MasterFeedTableViewCellSectionIdentifier.swift new file mode 100644 index 000000000..87e298f75 --- /dev/null +++ b/iOS/MasterFeed/Cell/MasterFeedTableViewCellSectionIdentifier.swift @@ -0,0 +1,18 @@ +// +// MasterFeedTableViewCellSectionIdentifier.swift +// NetNewsWire-iOS +// +// Created by Maurice Parker on 6/3/20. +// Copyright © 2020 Ranchero Software. All rights reserved. +// + +import Foundation +import RSTree + +struct MasterFeedTableViewSectionIdentifier: Hashable { + + init(node: Node) { + + } + +} diff --git a/iOS/MasterFeed/Cell/MasterFeedTableViewIdentifier.swift b/iOS/MasterFeed/Cell/MasterFeedTableViewIdentifier.swift new file mode 100644 index 000000000..ebeefe245 --- /dev/null +++ b/iOS/MasterFeed/Cell/MasterFeedTableViewIdentifier.swift @@ -0,0 +1,98 @@ +// +// MasterFeedTableViewIdentifier.swift +// NetNewsWire-iOS +// +// Created by Maurice Parker on 6/3/20. +// Copyright © 2020 Ranchero Software. All rights reserved. +// + +import Foundation +import Account +import RSTree + +final class MasterFeedTableViewIdentifier: NSObject, NSCopying { + + let feedID: FeedIdentifier? + let containerID: ContainerIdentifier? + let parentContainerID: ContainerIdentifier? + + let isEditable: Bool + let isPsuedoFeed: Bool + let isFolder: Bool + let isWebFeed: Bool + + let nameForDisplay: String + let url: String? + let unreadCount: Int + let childCount: Int + + init(node: Node, unreadCount: Int) { + let feed = node.representedObject as! Feed + self.feedID = feed.feedID + self.containerID = (node.representedObject as? Container)?.containerID + self.parentContainerID = (node.parent?.representedObject as? Container)?.containerID + + self.isEditable = !(node.representedObject is PseudoFeed) + self.isPsuedoFeed = node.representedObject is PseudoFeed + self.isFolder = node.representedObject is Folder + self.isWebFeed = node.representedObject is WebFeed + self.nameForDisplay = feed.nameForDisplay + + if let webFeed = node.representedObject as? WebFeed { + self.url = webFeed.url + } else { + self.url = nil + } + + self.unreadCount = unreadCount + self.childCount = node.numberOfChildNodes + } + + override func isEqual(_ object: Any?) -> Bool { + guard let otherIdentifier = object as? MasterFeedTableViewIdentifier else { return false } + if self === otherIdentifier { return true } + return feedID == otherIdentifier.feedID + } + + override var hash: Int { + return feedID.hashValue + } + +// override func isEqual(_ object: Any?) -> Bool { +// guard let otherIdentifier = object as? MasterFeedTableViewIdentifier else { return false } +// if self === otherIdentifier { return true } +// +// return feedID == otherIdentifier.feedID && +// containerID == otherIdentifier.containerID && +// parentContainerID == otherIdentifier.parentContainerID && +// isEditable == otherIdentifier.isEditable && +// isPsuedoFeed == otherIdentifier.isPsuedoFeed && +// isFolder == otherIdentifier.isFolder && +// isWebFeed == otherIdentifier.isWebFeed && +// nameForDisplay == otherIdentifier.nameForDisplay && +// url == otherIdentifier.url && +// unreadCount == otherIdentifier.unreadCount && +// childCount == otherIdentifier.childCount +// } +// +// override var hash: Int { +// var hasher = Hasher() +// hasher.combine(feedID) +// hasher.combine(containerID) +// hasher.combine(parentContainerID) +// hasher.combine(isEditable) +// hasher.combine(isPsuedoFeed) +// hasher.combine(isFolder) +// hasher.combine(isWebFeed) +// hasher.combine(nameForDisplay) +// hasher.combine(url) +// hasher.combine(unreadCount) +// hasher.combine(childCount) +// return hasher.finalize() +// } +// + func copy(with zone: NSZone? = nil) -> Any { + return self + } + +} diff --git a/iOS/MasterFeed/MasterFeedDataSource.swift b/iOS/MasterFeed/MasterFeedDataSource.swift index 150644fc5..dff3d79c1 100644 --- a/iOS/MasterFeed/MasterFeedDataSource.swift +++ b/iOS/MasterFeed/MasterFeedDataSource.swift @@ -10,10 +10,10 @@ import UIKit import RSTree import Account -class MasterFeedDataSource: UITableViewDiffableDataSource { +class MasterFeedDataSource: UITableViewDiffableDataSource { override func tableView(_ tableView: UITableView, canEditRowAt indexPath: IndexPath) -> Bool { - guard let node = itemIdentifier(for: indexPath), !(node.representedObject is PseudoFeed) else { + guard let identifier = itemIdentifier(for: indexPath), identifier.isEditable else { return false } return true diff --git a/iOS/MasterFeed/MasterFeedDataSourceOperation.swift b/iOS/MasterFeed/MasterFeedDataSourceOperation.swift index f96310346..ac04c1946 100644 --- a/iOS/MasterFeed/MasterFeedDataSourceOperation.swift +++ b/iOS/MasterFeed/MasterFeedDataSourceOperation.swift @@ -19,11 +19,11 @@ class MasterFeedDataSourceOperation: MainThreadOperation { public var name: String? = "MasterFeedDataSourceOperation" public var completionBlock: MainThreadOperation.MainThreadOperationCompletionBlock? - private var dataSource: UITableViewDiffableDataSource - private var snapshot: NSDiffableDataSourceSnapshot + private var dataSource: UITableViewDiffableDataSource + private var snapshot: NSDiffableDataSourceSnapshot private var animating: Bool - init(dataSource: UITableViewDiffableDataSource, snapshot: NSDiffableDataSourceSnapshot, animating: Bool) { + init(dataSource: UITableViewDiffableDataSource, snapshot: NSDiffableDataSourceSnapshot, animating: Bool) { self.dataSource = dataSource self.snapshot = snapshot self.animating = animating diff --git a/iOS/MasterFeed/MasterFeedViewController+Drag.swift b/iOS/MasterFeed/MasterFeedViewController+Drag.swift index 65bd16610..7c089b796 100644 --- a/iOS/MasterFeed/MasterFeedViewController+Drag.swift +++ b/iOS/MasterFeed/MasterFeedViewController+Drag.swift @@ -13,11 +13,11 @@ import Account extension MasterFeedViewController: UITableViewDragDelegate { func tableView(_ tableView: UITableView, itemsForBeginning session: UIDragSession, at indexPath: IndexPath) -> [UIDragItem] { - guard let node = dataSource.itemIdentifier(for: indexPath), let webFeed = node.representedObject as? WebFeed else { + guard let identifier = dataSource.itemIdentifier(for: indexPath), identifier.isWebFeed, let url = identifier.url else { return [UIDragItem]() } - let data = webFeed.url.data(using: .utf8) + let data = url.data(using: .utf8) let itemProvider = NSItemProvider() itemProvider.registerDataRepresentation(forTypeIdentifier: kUTTypeURL as String, visibility: .ownProcess) { completion in @@ -26,7 +26,7 @@ extension MasterFeedViewController: UITableViewDragDelegate { } let dragItem = UIDragItem(itemProvider: itemProvider) - dragItem.localObject = node + dragItem.localObject = identifier return [dragItem] } diff --git a/iOS/MasterFeed/MasterFeedViewController+Drop.swift b/iOS/MasterFeed/MasterFeedViewController+Drop.swift index 318157523..8692e3020 100644 --- a/iOS/MasterFeed/MasterFeedViewController+Drop.swift +++ b/iOS/MasterFeed/MasterFeedViewController+Drop.swift @@ -21,12 +21,12 @@ extension MasterFeedViewController: UITableViewDropDelegate { guard let destIndexPath = destinationIndexPath, destIndexPath.section > 0, tableView.hasActiveDrag, - let destNode = dataSource.itemIdentifier(for: destIndexPath), + let destIdentifier = dataSource.itemIdentifier(for: destIndexPath), let destCell = tableView.cellForRow(at: destIndexPath) else { return UITableViewDropProposal(operation: .forbidden) } - if destNode.representedObject is Folder { + if destIdentifier.isFolder { if session.location(in: destCell).y >= 0 { return UITableViewDropProposal(operation: .move, intent: .insertIntoDestinationIndexPath) } else { @@ -40,27 +40,28 @@ extension MasterFeedViewController: UITableViewDropDelegate { func tableView(_ tableView: UITableView, performDropWith dropCoordinator: UITableViewDropCoordinator) { guard let dragItem = dropCoordinator.items.first?.dragItem, - let sourceNode = dragItem.localObject as? Node, - let webFeed = sourceNode.representedObject as? WebFeed, + let sourceIdentifier = dragItem.localObject as? MasterFeedTableViewIdentifier, + let sourceParentContainerID = sourceIdentifier.parentContainerID, + let source = AccountManager.shared.existingContainer(with: sourceParentContainerID), let destIndexPath = dropCoordinator.destinationIndexPath else { return } let isFolderDrop: Bool = { - if let propDestNode = dataSource.itemIdentifier(for: destIndexPath), let propCell = tableView.cellForRow(at: destIndexPath) { - return propDestNode.representedObject is Folder && dropCoordinator.session.location(in: propCell).y >= 0 + if let propDestIdentifier = dataSource.itemIdentifier(for: destIndexPath), let propCell = tableView.cellForRow(at: destIndexPath) { + return propDestIdentifier.isFolder && dropCoordinator.session.location(in: propCell).y >= 0 } return false }() // Based on the drop we have to determine a node to start looking for a parent container. - let destNode: Node? = { + let destIdentifier: MasterFeedTableViewIdentifier? = { if isFolderDrop { return dataSource.itemIdentifier(for: destIndexPath) } else { if destIndexPath.row == 0 { - return coordinator.rootNode.childAtIndex(destIndexPath.section)! + return dataSource.itemIdentifier(for: IndexPath(row: 0, section: destIndexPath.section)) } else if destIndexPath.row > 0 { return dataSource.itemIdentifier(for: IndexPath(row: destIndexPath.row - 1, section: destIndexPath.section)) } else { @@ -71,24 +72,19 @@ extension MasterFeedViewController: UITableViewDropDelegate { }() // Now we start looking for the parent container - let destParentNode: Node? = { - if destNode?.representedObject is Container { - return destNode + let destinationContainer: Container? = { + if let containerID = destIdentifier?.containerID ?? destIdentifier?.parentContainerID { + return AccountManager.shared.existingContainer(with: containerID) } else { - if destNode?.parent?.representedObject is Container { - return destNode!.parent! - } else { - return nil - } + return nil } }() - // Move the Web Feed - guard let source = sourceNode.parent?.representedObject as? Container, let destination = destParentNode?.representedObject as? Container else { - return - } + guard let destination = destinationContainer else { return } + guard case .webFeed(_, let webFeedID) = sourceIdentifier.feedID else { return } + guard let webFeed = source.existingWebFeed(withWebFeedID: webFeedID) else { return } - if sameAccount(sourceNode, destParentNode!) { + if source.account == destination.account { moveWebFeedInAccount(feed: webFeed, sourceContainer: source, destinationContainer: destination) } else { moveWebFeedBetweenAccounts(feed: webFeed, sourceContainer: source, destinationContainer: destination) @@ -97,32 +93,6 @@ extension MasterFeedViewController: UITableViewDropDelegate { } - private func sameAccount(_ node: Node, _ parentNode: Node) -> Bool { - if let accountID = nodeAccountID(node), let parentAccountID = nodeAccountID(parentNode) { - if accountID == parentAccountID { - return true - } - } - return false - } - - private func nodeAccount(_ node: Node) -> Account? { - if let account = node.representedObject as? Account { - return account - } else if let folder = node.representedObject as? Folder { - return folder.account - } else if let webFeed = node.representedObject as? WebFeed { - return webFeed.account - } else { - return nil - } - - } - - private func nodeAccountID(_ node: Node) -> String? { - return nodeAccount(node)?.accountID - } - func moveWebFeedInAccount(feed: WebFeed, sourceContainer: Container, destinationContainer: Container) { guard sourceContainer !== destinationContainer else { return } diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index a240e4b58..0faf32797 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -102,8 +102,10 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { node = coordinator.rootNode.descendantNodeRepresentingObject(representedObject as AnyObject) } - if let node = node, dataSource.indexPath(for: node) != nil { - self.reloadNode(node) + guard let unreadCountNode = node else { return } + let identifier = makeIdentifier(unreadCountNode) + if dataSource.indexPath(for: identifier) != nil { + self.reload(identifier) } } @@ -228,13 +230,13 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { renameAction.backgroundColor = UIColor.systemOrange actions.append(renameAction) - if let webFeed = dataSource.itemIdentifier(for: indexPath)?.representedObject as? WebFeed { + if let identifier = dataSource.itemIdentifier(for: indexPath), identifier.isWebFeed { let moreTitle = NSLocalizedString("More", comment: "More") let moreAction = UIContextualAction(style: .normal, title: moreTitle) { [weak self] (action, view, completion) in if let self = self { - let alert = UIAlertController(title: webFeed.nameForDisplay, message: nil, preferredStyle: .actionSheet) + let alert = UIAlertController(title: identifier.nameForDisplay, message: nil, preferredStyle: .actionSheet) if let popoverController = alert.popoverPresentationController { popoverController.sourceView = view popoverController.sourceRect = CGRect(x: view.frame.size.width/2, y: view.frame.size.height/2, width: 1, height: 1) @@ -280,28 +282,27 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } override func tableView(_ tableView: UITableView, contextMenuConfigurationForRowAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? { - guard let node = dataSource.itemIdentifier(for: indexPath) else { + guard let identifier = dataSource.itemIdentifier(for: indexPath) else { return nil } - if node.representedObject is WebFeed { - return makeFeedContextMenu(node: node, indexPath: indexPath, includeDeleteRename: true) - } else if node.representedObject is Folder { - return makeFolderContextMenu(node: node, indexPath: indexPath) - } else if node.representedObject is PseudoFeed { - return makePseudoFeedContextMenu(node: node, indexPath: indexPath) + if identifier.isWebFeed { + return makeWebFeedContextMenu(identifier: identifier, indexPath: indexPath, includeDeleteRename: true) + } else if identifier.isFolder { + return makeFolderContextMenu(identifier: identifier, indexPath: indexPath) + } else if identifier.isPsuedoFeed { + return makePseudoFeedContextMenu(identifier: identifier, indexPath: indexPath) } else { return nil } } override func tableView(_ tableView: UITableView, previewForHighlightingContextMenuWithConfiguration configuration: UIContextMenuConfiguration) -> UITargetedPreview? { - guard let nodeUniqueId = configuration.identifier as? Int, - let node = coordinator.rootNode.descendantNode(where: { $0.uniqueID == nodeUniqueId }), - let indexPath = dataSource.indexPath(for: node), + guard let identifier = configuration.identifier as? MasterFeedTableViewIdentifier, + let indexPath = dataSource.indexPath(for: identifier), let cell = tableView.cellForRow(at: indexPath) else { return nil } - + return UITargetedPreview(view: cell, parameters: CroppingPreviewParameters(view: cell)) } @@ -320,13 +321,19 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { return coordinator.cappedIndexPath(proposedDestinationIndexPath) }() - guard let draggedNode = dataSource.itemIdentifier(for: sourceIndexPath) else { + guard let draggedIdentifier = dataSource.itemIdentifier(for: sourceIndexPath), + let draggedFeedID = draggedIdentifier.feedID, + let draggedNode = coordinator.nodeFor(feedID: draggedFeedID) else { assertionFailure("This should never happen") return sourceIndexPath } // If there is no destination node, we are dragging onto an empty Account - guard let destNode = dataSource.itemIdentifier(for: destIndexPath), let parentNode = destNode.parent else { + guard let destIdentifier = dataSource.itemIdentifier(for: destIndexPath), + let destFeedID = destIdentifier.feedID, + let destNode = coordinator.nodeFor(feedID: destFeedID), + let destParentContainerID = destIdentifier.parentContainerID, + let destParentNode = coordinator.nodeFor(containerID: destParentContainerID) else { return proposedDestinationIndexPath } @@ -336,13 +343,13 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } // If we are dragging around in the same container, just return the original source - if parentNode.childNodes.contains(draggedNode) { + if destParentNode.childNodes.contains(draggedNode) { return sourceIndexPath } // Suggest to the user the best place to drop the feed // Revisit if the tree controller can ever be sorted in some other way. - let nodes = parentNode.childNodes + [draggedNode] + let nodes = destParentNode.childNodes + [draggedNode] var sortedNodes = nodes.sortedAlphabeticallyWithFoldersAtEnd() let index = sortedNodes.firstIndex(of: draggedNode)! @@ -350,10 +357,11 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { if index == 0 { - if parentNode.representedObject is Account { + if destParentNode.representedObject is Account { return IndexPath(row: 0, section: destIndexPath.section) } else { - let candidateIndexPath = dataSource.indexPath(for: sortedNodes[index])! + let identifier = makeIdentifier(sortedNodes[index]) + let candidateIndexPath = dataSource.indexPath(for: identifier)! let movementAdjustment = sourceIndexPath < destIndexPath ? 1 : 0 return IndexPath(row: candidateIndexPath.row - movementAdjustment, section: candidateIndexPath.section) } @@ -361,12 +369,14 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } else { if index >= sortedNodes.count { - let lastSortedIndexPath = dataSource.indexPath(for: sortedNodes[sortedNodes.count - 1])! + let identifier = makeIdentifier(sortedNodes[sortedNodes.count - 1]) + let lastSortedIndexPath = dataSource.indexPath(for: identifier)! let movementAdjustment = sourceIndexPath > destIndexPath ? 1 : 0 return IndexPath(row: lastSortedIndexPath.row + movementAdjustment, section: lastSortedIndexPath.section) } else { let movementAdjustment = sourceIndexPath < destIndexPath ? 1 : 0 - return dataSource.indexPath(for: sortedNodes[index - movementAdjustment])! + let identifer = makeIdentifier(sortedNodes[index - movementAdjustment]) + return dataSource.indexPath(for: identifer)! } } @@ -429,8 +439,8 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } @objc func expandSelectedRows(_ sender: Any?) { - if let indexPath = coordinator.currentFeedIndexPath, let node = dataSource.itemIdentifier(for: indexPath) { - coordinator.expand(node) + if let indexPath = coordinator.currentFeedIndexPath, let containerID = dataSource.itemIdentifier(for: indexPath)?.containerID { + coordinator.expand(containerID) self.applyChanges(animated: true) { self.reloadAllVisibleCells() } @@ -438,8 +448,8 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } @objc func collapseSelectedRows(_ sender: Any?) { - if let indexPath = coordinator.currentFeedIndexPath, let node = dataSource.itemIdentifier(for: indexPath) { - coordinator.collapse(node) + if let indexPath = coordinator.currentFeedIndexPath, let containerID = dataSource.itemIdentifier(for: indexPath)?.containerID { + coordinator.collapse(containerID) self.applyChanges(animated: true) { self.reloadAllVisibleCells() } @@ -614,22 +624,27 @@ private extension MasterFeedViewController { filterButton?.accLabelText = NSLocalizedString("Filter Read Feeds", comment: "Filter Read Feeds") } - func reloadNode(_ node: Node) { + func makeIdentifier(_ node: Node) -> MasterFeedTableViewIdentifier { + let unreadCount = coordinator.unreadCountFor(node) + return MasterFeedTableViewIdentifier(node: node, unreadCount: unreadCount) + } + + func reload(_ identifier: MasterFeedTableViewIdentifier) { var snapshot = dataSource.snapshot() - snapshot.reloadItems([node]) + snapshot.reloadItems([identifier]) queueApply(snapshot: snapshot, animatingDifferences: false) { [weak self] in self?.restoreSelectionIfNecessary(adjustScroll: false) } } func applyChanges(animated: Bool, adjustScroll: Bool = false, completion: (() -> Void)? = nil) { - var snapshot = NSDiffableDataSourceSnapshot() - let sectionNodes = coordinator.rootNode.childNodes - snapshot.appendSections(sectionNodes) + var snapshot = NSDiffableDataSourceSnapshot() + let sectionIdentifiers = Array(0...coordinator.rootNode.childNodes.count - 1) + snapshot.appendSections(sectionIdentifiers) - for (index, sectionNode) in sectionNodes.enumerated() { - let shadowTableNodes = coordinator.shadowNodesFor(section: index) - snapshot.appendItems(shadowTableNodes, toSection: sectionNode) + for sectionIdentifer in sectionIdentifiers { + let identifiers = coordinator.shadowNodesFor(section: sectionIdentifer).map { makeIdentifier($0) } + snapshot.appendItems(identifiers, toSection: sectionIdentifer) } queueApply(snapshot: snapshot, animatingDifferences: animated) { [weak self] in @@ -638,7 +653,7 @@ private extension MasterFeedViewController { } } - func queueApply(snapshot: NSDiffableDataSourceSnapshot, animatingDifferences: Bool = true, completion: (() -> Void)? = nil) { + func queueApply(snapshot: NSDiffableDataSourceSnapshot, animatingDifferences: Bool = true, completion: (() -> Void)? = nil) { let operation = MasterFeedDataSourceOperation(dataSource: dataSource, snapshot: snapshot, animating: animatingDifferences) operation.completionBlock = { [weak self] _ in self?.enableTableViewSelection() @@ -659,9 +674,9 @@ private extension MasterFeedViewController { } func makeDataSource() -> MasterFeedDataSource { - let dataSource = MasterFeedDataSource(tableView: tableView, cellProvider: { [weak self] tableView, indexPath, node in + let dataSource = MasterFeedDataSource(tableView: tableView, cellProvider: { [weak self] tableView, indexPath, cellContents in let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterFeedTableViewCell - self?.configure(cell, node) + self?.configure(cell, cellContents) return cell }) dataSource.defaultRowAnimation = .middle @@ -679,22 +694,27 @@ private extension MasterFeedViewController { tableView.estimatedRowHeight = layout.height } - func configure(_ cell: MasterFeedTableViewCell, _ node: Node) { + func configure(_ cell: MasterFeedTableViewCell, _ identifier: MasterFeedTableViewIdentifier) { cell.delegate = self - if node.representedObject is Folder { + if identifier.isFolder { cell.indentationLevel = 0 } else { cell.indentationLevel = 1 } - cell.setDisclosure(isExpanded: coordinator.isExpanded(node), animated: false) - cell.isDisclosureAvailable = node.canHaveChildNodes - cell.name = nameFor(node) - cell.unreadCount = coordinator.unreadCountFor(node) - configureIcon(cell, node) + if let containerID = identifier.containerID { + cell.setDisclosure(isExpanded: coordinator.isExpanded(containerID), animated: false) + cell.isDisclosureAvailable = true + } else { + cell.isDisclosureAvailable = false + } - guard let indexPath = dataSource.indexPath(for: node) else { return } + cell.name = identifier.nameForDisplay + cell.unreadCount = identifier.unreadCount + configureIcon(cell, identifier) + + guard let indexPath = dataSource.indexPath(for: identifier) else { return } let rowsInSection = tableView.numberOfRows(inSection: indexPath.section) if indexPath.row == rowsInSection - 1 { cell.isSeparatorShown = false @@ -704,12 +724,20 @@ private extension MasterFeedViewController { } - func configureIcon(_ cell: MasterFeedTableViewCell, _ node: Node) { - cell.iconImage = imageFor(node) + func configureIcon(_ cell: MasterFeedTableViewCell, _ identifier: MasterFeedTableViewIdentifier) { + cell.iconImage = imageFor(identifier) } - func imageFor(_ node: Node) -> IconImage? { - if let webFeed = node.representedObject as? WebFeed { + func imageFor(_ identifier: MasterFeedTableViewIdentifier) -> IconImage? { + guard let feedID = identifier.feedID else { return nil } + + if let smartFeed = SmartFeedsController.shared.find(by: feedID) { + return smartFeed.smallIcon + } + + guard let feed = AccountManager.shared.existingFeed(with: feedID) else { return nil } + + if let webFeed = feed as? WebFeed { let feedIconImage = appDelegate.webFeedIconDownloader.icon(for: webFeed) if feedIconImage != nil { @@ -722,7 +750,7 @@ private extension MasterFeedViewController { } - if let smallIconProvider = node.representedObject as? SmallIconProvider { + if let smallIconProvider = feed as? SmallIconProvider { return smallIconProvider.smallIcon } @@ -740,20 +768,20 @@ private extension MasterFeedViewController { applyToCellsForRepresentedObject(representedObject, configure) } - func applyToCellsForRepresentedObject(_ representedObject: AnyObject, _ completion: (MasterFeedTableViewCell, Node) -> Void) { - applyToAvailableCells { (cell, node) in - if node.representedObject === representedObject { - completion(cell, node) + func applyToCellsForRepresentedObject(_ representedObject: AnyObject, _ completion: (MasterFeedTableViewCell, MasterFeedTableViewIdentifier) -> Void) { + applyToAvailableCells { (cell, identifier) in + if let representedFeed = representedObject as? Feed, representedFeed.feedID == identifier.feedID { + completion(cell, identifier) } } } - func applyToAvailableCells(_ completion: (MasterFeedTableViewCell, Node) -> Void) { + func applyToAvailableCells(_ completion: (MasterFeedTableViewCell, MasterFeedTableViewIdentifier) -> Void) { tableView.visibleCells.forEach { cell in - guard let indexPath = tableView.indexPath(for: cell), let node = dataSource.itemIdentifier(for: indexPath) else { + guard let indexPath = tableView.indexPath(for: cell), let identifier = dataSource.itemIdentifier(for: indexPath) else { return } - completion(cell as! MasterFeedTableViewCell, node) + completion(cell as! MasterFeedTableViewCell, identifier) } } @@ -762,9 +790,9 @@ private extension MasterFeedViewController { reloadCells(visibleNodes, completion: completion) } - private func reloadCells(_ nodes: [Node], completion: (() -> Void)? = nil) { + private func reloadCells(_ identifiers: [MasterFeedTableViewIdentifier], completion: (() -> Void)? = nil) { var snapshot = dataSource.snapshot() - snapshot.reloadItems(nodes) + snapshot.reloadItems(identifiers) queueApply(snapshot: snapshot, animatingDifferences: false) { [weak self] in self?.restoreSelectionIfNecessary(adjustScroll: false) completion?() @@ -801,23 +829,23 @@ private extension MasterFeedViewController { } func expand(_ cell: MasterFeedTableViewCell) { - guard let indexPath = tableView.indexPath(for: cell), let node = dataSource.itemIdentifier(for: indexPath) else { + guard let indexPath = tableView.indexPath(for: cell), let containerID = dataSource.itemIdentifier(for: indexPath)?.containerID else { return } - coordinator.expand(node) + coordinator.expand(containerID) applyChanges(animated: true) } func collapse(_ cell: MasterFeedTableViewCell) { - guard let indexPath = tableView.indexPath(for: cell), let node = dataSource.itemIdentifier(for: indexPath) else { + guard let indexPath = tableView.indexPath(for: cell), let containerID = dataSource.itemIdentifier(for: indexPath)?.containerID else { return } - coordinator.collapse(node) + coordinator.collapse(containerID) applyChanges(animated: true) } - func makeFeedContextMenu(node: Node, indexPath: IndexPath, includeDeleteRename: Bool) -> UIContextMenuConfiguration { - return UIContextMenuConfiguration(identifier: node.uniqueID as NSCopying, previewProvider: nil, actionProvider: { [ weak self] suggestedActions in + func makeWebFeedContextMenu(identifier: MasterFeedTableViewIdentifier, indexPath: IndexPath, includeDeleteRename: Bool) -> UIContextMenuConfiguration { + return UIContextMenuConfiguration(identifier: identifier as NSCopying, previewProvider: nil, actionProvider: { [ weak self] suggestedActions in guard let self = self else { return nil } @@ -854,8 +882,8 @@ private extension MasterFeedViewController { } - func makeFolderContextMenu(node: Node, indexPath: IndexPath) -> UIContextMenuConfiguration { - return UIContextMenuConfiguration(identifier: node.uniqueID as NSCopying, previewProvider: nil, actionProvider: { [weak self] suggestedActions in + func makeFolderContextMenu(identifier: MasterFeedTableViewIdentifier, indexPath: IndexPath) -> UIContextMenuConfiguration { + return UIContextMenuConfiguration(identifier: identifier as NSCopying, previewProvider: nil, actionProvider: { [weak self] suggestedActions in guard let self = self else { return nil } @@ -872,12 +900,12 @@ private extension MasterFeedViewController { }) } - func makePseudoFeedContextMenu(node: Node, indexPath: IndexPath) -> UIContextMenuConfiguration? { + func makePseudoFeedContextMenu(identifier: MasterFeedTableViewIdentifier, indexPath: IndexPath) -> UIContextMenuConfiguration? { guard let markAllAction = self.markAllAsReadAction(indexPath: indexPath) else { return nil } - return UIContextMenuConfiguration(identifier: node.uniqueID as NSCopying, previewProvider: nil, actionProvider: { suggestedActions in + return UIContextMenuConfiguration(identifier: identifier as NSCopying, previewProvider: nil, actionProvider: { suggestedActions in return UIMenu(title: "", children: [markAllAction]) }) } @@ -908,9 +936,9 @@ private extension MasterFeedViewController { } func copyFeedPageAction(indexPath: IndexPath) -> UIAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - let feed = node.representedObject as? WebFeed, - let url = URL(string: feed.url) else { + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, + let webFeed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed, + let url = URL(string: webFeed.url) else { return nil } @@ -922,9 +950,9 @@ private extension MasterFeedViewController { } func copyFeedPageAlertAction(indexPath: IndexPath, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - let feed = node.representedObject as? WebFeed, - let url = URL(string: feed.url) else { + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, + let webFeed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed, + let url = URL(string: webFeed.url) else { return nil } @@ -937,9 +965,9 @@ private extension MasterFeedViewController { } func copyHomePageAction(indexPath: IndexPath) -> UIAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - let feed = node.representedObject as? WebFeed, - let homePageURL = feed.homePageURL, + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, + let webFeed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed, + let homePageURL = webFeed.homePageURL, let url = URL(string: homePageURL) else { return nil } @@ -952,9 +980,9 @@ private extension MasterFeedViewController { } func copyHomePageAlertAction(indexPath: IndexPath, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - let feed = node.representedObject as? WebFeed, - let homePageURL = feed.homePageURL, + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, + let webFeed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed, + let homePageURL = webFeed.homePageURL, let url = URL(string: homePageURL) else { return nil } @@ -968,9 +996,10 @@ private extension MasterFeedViewController { } func markAllAsReadAlertAction(indexPath: IndexPath, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - coordinator.unreadCountFor(node) > 0, - let feed = node.representedObject as? WebFeed, + guard let identifier = dataSource.itemIdentifier(for: indexPath), + identifier.unreadCount > 0, + let feedID = identifier.feedID, + let feed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed, let articles = try? feed.fetchArticles(), let contentView = self.tableView.cellForRow(at: indexPath)?.contentView else { return nil } @@ -1009,7 +1038,7 @@ private extension MasterFeedViewController { } func getInfoAction(indexPath: IndexPath) -> UIAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), let feed = node.representedObject as? WebFeed else { + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, let feed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed else { return nil } @@ -1037,7 +1066,7 @@ private extension MasterFeedViewController { } func getInfoAlertAction(indexPath: IndexPath, completion: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), let feed = node.representedObject as? WebFeed else { + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, let feed = AccountManager.shared.existingFeed(with: feedID) as? WebFeed else { return nil } @@ -1050,18 +1079,18 @@ private extension MasterFeedViewController { } func markAllAsReadAction(indexPath: IndexPath) -> UIAction? { - guard let node = dataSource.itemIdentifier(for: indexPath), - coordinator.unreadCountFor(node) > 0 else { + guard let identifier = dataSource.itemIdentifier(for: indexPath), identifier.unreadCount > 0 else { return nil } - guard let articleFetcher = node.representedObject as? Feed, - let fetchedArticles = try? articleFetcher.fetchArticles() else { + guard let feedID = identifier.feedID, + let feed = AccountManager.shared.existingFeed(with: feedID), + let fetchedArticles = try? feed.fetchArticles() else { return nil } let articles = Array(fetchedArticles) - return markAllAsReadAction(articles: articles, nameForDisplay: articleFetcher.nameForDisplay, indexPath: indexPath) + return markAllAsReadAction(articles: articles, nameForDisplay: feed.nameForDisplay, indexPath: indexPath) } func markAllAsReadAction(account: Account) -> UIAction? { @@ -1091,8 +1120,9 @@ private extension MasterFeedViewController { } func rename(indexPath: IndexPath) { - - let name = (dataSource.itemIdentifier(for: indexPath)?.representedObject as? DisplayNameProvider)?.nameForDisplay ?? "" + guard let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, let feed = AccountManager.shared.existingFeed(with: feedID) else { return } + + let name = dataSource.itemIdentifier(for: indexPath)?.nameForDisplay ?? "" let formatString = NSLocalizedString("Rename “%@”", comment: "Feed finder") let title = NSString.localizedStringWithFormat(formatString as NSString, name) as String @@ -1104,14 +1134,12 @@ private extension MasterFeedViewController { let renameTitle = NSLocalizedString("Rename", comment: "Rename") let renameAction = UIAlertAction(title: renameTitle, style: .default) { [weak self] action in - guard let node = self?.dataSource.itemIdentifier(for: indexPath), - let name = alertController.textFields?[0].text, - !name.isEmpty else { - return + guard let name = alertController.textFields?[0].text, !name.isEmpty else { + return } - if let feed = node.representedObject as? WebFeed { - feed.rename(to: name) { result in + if let webFeed = feed as? WebFeed { + webFeed.rename(to: name) { result in switch result { case .success: break @@ -1119,7 +1147,7 @@ private extension MasterFeedViewController { self?.presentError(error) } } - } else if let folder = node.representedObject as? Folder { + } else if let folder = feed as? Folder { folder.rename(to: name) { result in switch result { case .success: @@ -1147,7 +1175,8 @@ private extension MasterFeedViewController { func delete(indexPath: IndexPath) { guard let undoManager = undoManager, - let deleteNode = dataSource.itemIdentifier(for: indexPath), + let feedID = dataSource.itemIdentifier(for: indexPath)?.feedID, + let deleteNode = coordinator.nodeFor(feedID: feedID), let deleteCommand = DeleteCommand(nodesToDelete: [deleteNode], undoManager: undoManager, errorHandler: ErrorHandler.present(self)) else { return diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index 637c46974..ac7f86ced 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -68,7 +68,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { private var wasRootSplitViewControllerCollapsed = false private let fetchAndMergeArticlesQueue = CoalescingQueue(name: "Fetch and Merge Articles", interval: 0.5) - private let rebuildBackingStoresQueue = CoalescingQueue(name: "Rebuild The Backing Stores", interval: 1.0) + private let rebuildBackingStoresQueue = CoalescingQueue(name: "Rebuild The Backing Stores", interval: 0.5) private var fetchSerialNumber = 0 private let fetchRequestQueue = FetchRequestQueue() @@ -443,9 +443,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { guard notification.object is AccountManager else { return } - if isReadFeedsFiltered { - rebuildBackingStores() - } + rebuildBackingStores() treeControllerDelegate.resetFilterExceptions() } @@ -614,6 +612,26 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return shadowTable[section] } + func nodeFor(containerID: ContainerIdentifier) -> Node? { + return treeController.rootNode.descendantNode(where: { node in + if let container = node.representedObject as? Container { + return container.containerID == containerID + } else { + return false + } + }) + } + + func nodeFor(feedID: FeedIdentifier) -> Node? { + return treeController.rootNode.descendantNode(where: { node in + if let feed = node.representedObject as? Feed { + return feed.feedID == feedID + } else { + return false + } + }) + } + func articleFor(_ articleID: String) -> Article? { return idToAticleDictionary[articleID] } @@ -646,9 +664,13 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } + func isExpanded(_ containerID: ContainerIdentifier) -> Bool { + return expandedTable.contains(containerID) + } + func isExpanded(_ containerIdentifiable: ContainerIdentifiable) -> Bool { if let containerID = containerIdentifiable.containerID { - return expandedTable.contains(containerID) + return isExpanded(containerID) } return false } @@ -660,17 +682,18 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return false } - func expand(_ node: Node) { - guard let containerIdentifiable = node.representedObject as? ContainerIdentifiable else { - return - } - - markExpanded(containerIdentifiable) + func expand(_ containerID: ContainerIdentifier) { + markExpanded(containerID) animatingChanges = true rebuildShadowTable() animatingChanges = false } + func expand(_ node: Node) { + guard let containerID = (node.representedObject as? ContainerIdentifiable)?.containerID else { return } + expand(containerID) + } + func expandAllSectionsAndFolders() { for sectionNode in treeController.rootNode.childNodes { markExpanded(sectionNode) @@ -685,14 +708,19 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { animatingChanges = false } - func collapse(_ node: Node) { - unmarkExpanded(node) + func collapse(_ containerID: ContainerIdentifier) { + unmarkExpanded(containerID) animatingChanges = true rebuildShadowTable() animatingChanges = false clearTimelineIfNoLongerAvailable() } + func collapse(_ node: Node) { + guard let containerID = (node.representedObject as? ContainerIdentifiable)?.containerID else { return } + collapse(containerID) + } + func collapseAllFolders() { for sectionNode in treeController.rootNode.childNodes { unmarkExpanded(sectionNode) @@ -1519,9 +1547,13 @@ private extension SceneCoordinator { self.showIcons = false } + func markExpanded(_ containerID: ContainerIdentifier) { + expandedTable.insert(containerID) + } + func markExpanded(_ containerIdentifiable: ContainerIdentifiable) { if let containerID = containerIdentifiable.containerID { - expandedTable.insert(containerID) + markExpanded(containerID) } } @@ -1531,9 +1563,13 @@ private extension SceneCoordinator { } } + func unmarkExpanded(_ containerID: ContainerIdentifier) { + expandedTable.remove(containerID) + } + func unmarkExpanded(_ containerIdentifiable: ContainerIdentifiable) { if let containerID = containerIdentifiable.containerID { - expandedTable.remove(containerID) + unmarkExpanded(containerID) } }