From a5a36e098603b2af06caff363ac1fb185e278b20 Mon Sep 17 00:00:00 2001 From: Daniel Jalkut Date: Wed, 28 Aug 2019 11:40:12 -0400 Subject: [PATCH 1/7] Fix for #885: Include 403 status code in list of errors to prompt with 'update credentials' error message, and expand that message to cover the possibility an account is no longer valid with the service, i.e. in the case of an expired Feedbin subscription. --- Frameworks/Account/AccountError.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Frameworks/Account/AccountError.swift b/Frameworks/Account/AccountError.swift index 090cb268d..480871f2b 100644 --- a/Frameworks/Account/AccountError.swift +++ b/Frameworks/Account/AccountError.swift @@ -48,8 +48,8 @@ public enum AccountError: LocalizedError { case .wrappedError(let error, _): switch error { case TransportError.httpError(let status): - if status == 401 { - return NSLocalizedString("Please update your credentials for this account.", comment: "Try later") + if status == 401 || status == 403 { + return NSLocalizedString("Please update your credentials for this account, or ensure that your account with this service is still valid.", comment: "Expired credentials") } else { return NSLocalizedString("Please try again later.", comment: "Try later") } From 9c66f6160e295e8b433ee4850a70caa968cc2c84 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 28 Aug 2019 11:30:40 -0500 Subject: [PATCH 2/7] Clean activities when the associated data is deleted --- Shared/Activity/ActivityManager.swift | 70 ++++++++++++++++++- iOS/MasterFeed/MasterFeedViewController.swift | 6 ++ iOS/Settings/SettingsDetailAccountView.swift | 1 + .../UIKit/DetailAccountViewController.swift | 1 + 4 files changed, 75 insertions(+), 3 deletions(-) diff --git a/Shared/Activity/ActivityManager.swift b/Shared/Activity/ActivityManager.swift index 2c4bd27c6..977108ead 100644 --- a/Shared/Activity/ActivityManager.swift +++ b/Shared/Activity/ActivityManager.swift @@ -41,7 +41,7 @@ class ActivityManager { func selectingFolder(_ folder: Folder) { let localizedText = NSLocalizedString("See articles in “%@”", comment: "See articles in Folder") let title = NSString.localizedStringWithFormat(localizedText as NSString, folder.nameForDisplay) as String - selectingActivity = makeSelectingActivity(type: ActivityType.selectFolder, title: title, identifier: "folder.\(folder.nameForDisplay)") + selectingActivity = makeSelectingActivity(type: ActivityType.selectFolder, title: title, identifier: identifer(for: folder)) selectingActivity!.userInfo = [ ActivityID.accountID.rawValue: folder.account?.accountID ?? "", @@ -55,7 +55,7 @@ class ActivityManager { func selectingFeed(_ feed: Feed) { let localizedText = NSLocalizedString("See articles in “%@”", comment: "See articles in Feed") let title = NSString.localizedStringWithFormat(localizedText as NSString, feed.nameForDisplay) as String - selectingActivity = makeSelectingActivity(type: ActivityType.selectFeed, title: title, identifier: feed.url) + selectingActivity = makeSelectingActivity(type: ActivityType.selectFeed, title: title, identifier: identifer(for: feed)) selectingActivity!.userInfo = [ ActivityID.accountID.rawValue: feed.account?.accountID ?? "", @@ -63,6 +63,16 @@ class ActivityManager { ActivityID.feedID.rawValue: feed.feedID ] + let attributeSet = CSSearchableItemAttributeSet(itemContentType: kUTTypeItem as String) + attributeSet.title = feed.nameForDisplay + attributeSet.keywords = makeKeywords(feed.nameForDisplay) + if let image = appDelegate.feedIconDownloader.icon(for: feed) { + attributeSet.thumbnailData = image.pngData() + } else if let image = appDelegate.faviconDownloader.faviconAsAvatar(for: feed) { + attributeSet.thumbnailData = image.pngData() + } + selectingActivity!.contentAttributeSet = attributeSet + selectingActivity!.becomeCurrent() } @@ -74,6 +84,37 @@ class ActivityManager { readingActivity?.becomeCurrent() } + func cleanUp(_ account: Account) { + var ids = [String]() + + if let folders = account.folders { + for folder in folders { + ids.append(identifer(for: folder)) + } + } + + for feed in account.flattenedFeeds() { + ids.append(contentsOf: identifers(for: feed)) + } + + NSUserActivity.deleteSavedUserActivities(withPersistentIdentifiers: ids) {} + } + + func cleanUp(_ folder: Folder) { + var ids = [String]() + ids.append(identifer(for: folder)) + + for feed in folder.flattenedFeeds() { + ids.append(contentsOf: identifers(for: feed)) + } + + NSUserActivity.deleteSavedUserActivities(withPersistentIdentifiers: ids) {} + } + + func cleanUp(_ feed: Feed) { + NSUserActivity.deleteSavedUserActivities(withPersistentIdentifiers: identifers(for: feed)) {} + } + } // MARK: Private @@ -110,13 +151,13 @@ private extension ActivityManager { activity.isEligibleForSearch = true activity.isEligibleForPrediction = false activity.isEligibleForHandoff = true + activity.persistentIdentifier = identifer(for: article) // CoreSpotlight let attributeSet = CSSearchableItemAttributeSet(itemContentType: kUTTypeCompositeContent as String) attributeSet.title = article.title attributeSet.contentDescription = article.summary attributeSet.keywords = keywords - attributeSet.relatedUniqueIdentifier = article.url if let image = article.avatarImage() { attributeSet.thumbnailData = image.pngData() @@ -131,4 +172,27 @@ private extension ActivityManager { return value?.components(separatedBy: " ").filter { $0.count > 2 } ?? [] } + func identifer(for folder: Folder) -> String { + return "account_\(folder.account!.accountID)_folder_\(folder.nameForDisplay)" + } + + func identifer(for feed: Feed) -> String { + return "account_\(feed.account!.accountID)_feed_\(feed.feedID)" + } + + func identifer(for article: Article) -> String { + return "account_\(article.accountID)_feed_\(article.feedID)_article_\(article.articleID)" + } + + func identifers(for feed: Feed) -> [String] { + var ids = [String]() + ids.append(identifer(for: feed)) + + for article in feed.fetchArticles() { + ids.append(identifer(for: article)) + } + + return ids + } + } diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index 14bd25023..9c2b0fdb1 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -859,6 +859,12 @@ private extension MasterFeedViewController { return } + if let folder = deleteNode.representedObject as? Folder { + ActivityManager.shared.cleanUp(folder) + } else if let feed = deleteNode.representedObject as? Feed { + ActivityManager.shared.cleanUp(feed) + } + var deleteIndexPaths = [indexPath] if coordinator.isExpanded(deleteNode) { for i in 0.. Date: Wed, 28 Aug 2019 11:44:54 -0500 Subject: [PATCH 3/7] Update user activity if a feed has been selected and its best icon has been downloaded --- Shared/Activity/ActivityManager.swift | 41 ++++++++++++++++++++------- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/Shared/Activity/ActivityManager.swift b/Shared/Activity/ActivityManager.swift index 977108ead..459f06da2 100644 --- a/Shared/Activity/ActivityManager.swift +++ b/Shared/Activity/ActivityManager.swift @@ -20,6 +20,10 @@ class ActivityManager { private var selectingActivity: NSUserActivity? = nil private var readingActivity: NSUserActivity? = nil + init() { + NotificationCenter.default.addObserver(self, selector: #selector(feedIconDidBecomeAvailable(_:)), name: .FeedIconDidBecomeAvailable, object: nil) + } + func selectingToday() { let title = NSLocalizedString("See articles for Today", comment: "Today") selectingActivity = makeSelectingActivity(type: ActivityType.selectToday, title: title, identifier: "smartfeed.today") @@ -62,16 +66,8 @@ class ActivityManager { ActivityID.accountName.rawValue: feed.account?.name ?? "", ActivityID.feedID.rawValue: feed.feedID ] - - let attributeSet = CSSearchableItemAttributeSet(itemContentType: kUTTypeItem as String) - attributeSet.title = feed.nameForDisplay - attributeSet.keywords = makeKeywords(feed.nameForDisplay) - if let image = appDelegate.feedIconDownloader.icon(for: feed) { - attributeSet.thumbnailData = image.pngData() - } else if let image = appDelegate.faviconDownloader.faviconAsAvatar(for: feed) { - attributeSet.thumbnailData = image.pngData() - } - selectingActivity!.contentAttributeSet = attributeSet + selectingActivity!.requiredUserInfoKeys = [ActivityID.accountID.rawValue, ActivityID.accountName.rawValue, ActivityID.feedID.rawValue] + updateSelectingActivityFeedSearchAttributes(with: feed) selectingActivity!.becomeCurrent() } @@ -115,6 +111,15 @@ class ActivityManager { NSUserActivity.deleteSavedUserActivities(withPersistentIdentifiers: identifers(for: feed)) {} } + @objc func feedIconDidBecomeAvailable(_ note: Notification) { + guard let feed = note.userInfo?[UserInfoKey.feed] as? Feed, let activityFeedId = selectingActivity?.userInfo?[ActivityID.feedID.rawValue] as? String else { + return + } + if activityFeedId == feed.feedID { + updateSelectingActivityFeedSearchAttributes(with: feed) + } + } + } // MARK: Private @@ -195,4 +200,20 @@ private extension ActivityManager { return ids } + func updateSelectingActivityFeedSearchAttributes(with feed: Feed) { + + let attributeSet = CSSearchableItemAttributeSet(itemContentType: kUTTypeItem as String) + attributeSet.title = feed.nameForDisplay + attributeSet.keywords = makeKeywords(feed.nameForDisplay) + if let image = appDelegate.feedIconDownloader.icon(for: feed) { + attributeSet.thumbnailData = image.pngData() + } else if let image = appDelegate.faviconDownloader.faviconAsAvatar(for: feed) { + attributeSet.thumbnailData = image.pngData() + } + + selectingActivity!.contentAttributeSet = attributeSet + selectingActivity!.needsSave = true + + } + } From d7ec92ef34ef61c23acef70052ffa01aac01cd16 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 28 Aug 2019 11:46:03 -0500 Subject: [PATCH 4/7] Delete unnecessary user activity property value --- Shared/Activity/ActivityManager.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Shared/Activity/ActivityManager.swift b/Shared/Activity/ActivityManager.swift index 459f06da2..9a51a1e7d 100644 --- a/Shared/Activity/ActivityManager.swift +++ b/Shared/Activity/ActivityManager.swift @@ -66,7 +66,6 @@ class ActivityManager { ActivityID.accountName.rawValue: feed.account?.name ?? "", ActivityID.feedID.rawValue: feed.feedID ] - selectingActivity!.requiredUserInfoKeys = [ActivityID.accountID.rawValue, ActivityID.accountName.rawValue, ActivityID.feedID.rawValue] updateSelectingActivityFeedSearchAttributes(with: feed) selectingActivity!.becomeCurrent() From 89a38fa2b5c976ad8443d703d9cd0257b5948475 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 28 Aug 2019 18:06:27 -0500 Subject: [PATCH 5/7] Change Feeds to use diffable data sources --- NetNewsWire.xcodeproj/project.pbxproj | 4 + Shared/Commands/DeleteCommand.swift | 18 -- iOS/AppCoordinator.swift | 60 ++--- iOS/MasterFeed/MasterFeedDataSource.swift | 87 +++++++ iOS/MasterFeed/MasterFeedViewController.swift | 228 +++++------------- 5 files changed, 160 insertions(+), 237 deletions(-) create mode 100644 iOS/MasterFeed/MasterFeedDataSource.swift diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index 25245494f..4fa40a99d 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -122,6 +122,7 @@ 51C452AF2265108300C03939 /* ArticleArray.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84F204DF1FAACBB30076E152 /* ArticleArray.swift */; }; 51C452B42265141B00C03939 /* WebKit.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 51C452B32265141B00C03939 /* WebKit.framework */; }; 51C452B82265178500C03939 /* styleSheet.css in Resources */ = {isa = PBXBuildFile; fileRef = 51C452B72265178500C03939 /* styleSheet.css */; }; + 51CC9B3E231720B2000E842F /* MasterFeedDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51CC9B3D231720B2000E842F /* MasterFeedDataSource.swift */; }; 51D5948722668EFA00DFC836 /* MarkStatusCommand.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84702AA31FA27AC0006B8943 /* MarkStatusCommand.swift */; }; 51D87EE12311D34700E63F03 /* ActivityType.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51D87EE02311D34700E63F03 /* ActivityType.swift */; }; 51E3EB33229AB02C00645299 /* ErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 51E3EB32229AB02C00645299 /* ErrorHandler.swift */; }; @@ -733,6 +734,7 @@ 51C4528B2265095F00C03939 /* AddFolderViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AddFolderViewController.swift; sourceTree = ""; }; 51C452B32265141B00C03939 /* WebKit.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = WebKit.framework; path = Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS12.2.sdk/System/Library/Frameworks/WebKit.framework; sourceTree = DEVELOPER_DIR; }; 51C452B72265178500C03939 /* styleSheet.css */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.css; path = styleSheet.css; sourceTree = ""; }; + 51CC9B3D231720B2000E842F /* MasterFeedDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MasterFeedDataSource.swift; sourceTree = ""; }; 51D87EE02311D34700E63F03 /* ActivityType.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ActivityType.swift; sourceTree = ""; }; 51E3EB32229AB02C00645299 /* ErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorHandler.swift; sourceTree = ""; }; 51E3EB3C229AB08300645299 /* ErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorHandler.swift; sourceTree = ""; }; @@ -1134,6 +1136,7 @@ isa = PBXGroup; children = ( 51C45264226508F600C03939 /* MasterFeedViewController.swift */, + 51CC9B3D231720B2000E842F /* MasterFeedDataSource.swift */, 51C45260226508F600C03939 /* Cell */, ); path = MasterFeed; @@ -2469,6 +2472,7 @@ 51C4529F22650A1900C03939 /* AuthorAvatarDownloader.swift in Sources */, 519E743D22C663F900A78E47 /* SceneDelegate.swift in Sources */, 51E595AD228E1C2100FCC42B /* AddAccountViewController.swift in Sources */, + 51CC9B3E231720B2000E842F /* MasterFeedDataSource.swift in Sources */, 51C452A322650A1E00C03939 /* HTMLMetadataDownloader.swift in Sources */, 51C4528D2265095F00C03939 /* AddFolderViewController.swift in Sources */, 51934CD023108953006127BE /* ActivityID.swift in Sources */, diff --git a/Shared/Commands/DeleteCommand.swift b/Shared/Commands/DeleteCommand.swift index 6d1bda0e3..6c62e0554 100644 --- a/Shared/Commands/DeleteCommand.swift +++ b/Shared/Commands/DeleteCommand.swift @@ -54,24 +54,6 @@ final class DeleteCommand: UndoableCommand { registerUndo() } - func perform(completion: @escaping () -> Void) { - - let group = DispatchGroup() - group.enter() - itemSpecifiers.forEach { - $0.delete() { - group.leave() - } - } - treeController.rebuild() - - group.notify(queue: DispatchQueue.main) { - self.registerUndo() - completion() - } - - } - func undo() { BatchUpdate.shared.perform { diff --git a/iOS/AppCoordinator.swift b/iOS/AppCoordinator.swift index 7df057dad..e87596f8c 100644 --- a/iOS/AppCoordinator.swift +++ b/iOS/AppCoordinator.swift @@ -76,8 +76,12 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return treeController.rootNode } - var numberOfSections: Int { - return shadowTable.count + var allSections: [Int] { + var sections = [Int]() + for (index, _) in shadowTable.enumerated() { + sections.append(index) + } + return sections } private(set) var currentMasterIndexPath: IndexPath? { @@ -313,14 +317,6 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { // MARK: API - func beginUpdates() { - animatingChanges = true - } - - func endUpdates() { - animatingChanges = false - } - func rowsInSection(_ section: Int) -> Int { return shadowTable[section].count } @@ -361,6 +357,10 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return shadowTable[indexPath.section][indexPath.row] } + func nodesFor(section: Int) -> [Node] { + return shadowTable[section] + } + func indexPathFor(_ node: Node) -> IndexPath? { for i in 0.. ()) { - + func expand(section: Int) { guard let expandNode = treeController.rootNode.childAtIndex(section) else { return } @@ -397,11 +396,9 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { animatingChanges = true - var indexPathsToInsert = [IndexPath]() var i = 0 func addNode(_ node: Node) { - indexPathsToInsert.append(IndexPath(row: i, section: section)) shadowTable[section].insert(node, at: i) i = i + 1 } @@ -415,36 +412,26 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - completion(indexPathsToInsert) - animatingChanges = false - } - func expand(_ indexPath: IndexPath, completion: ([IndexPath]) -> ()) { - + func expand(_ indexPath: IndexPath) { let expandNode = shadowTable[indexPath.section][indexPath.row] expandedNodes.append(expandNode) animatingChanges = true - var indexPathsToInsert = [IndexPath]() for i in 0.. ()) { - + func collapse(section: Int) { animatingChanges = true guard let collapseNode = treeController.rootNode.childAtIndex(section) else { @@ -455,20 +442,12 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { expandedNodes.remove(at: removeNode) } - var indexPathsToRemove = [IndexPath]() - for i in 0.. ()) { - + func collapse(_ indexPath: IndexPath) { animatingChanges = true let collapseNode = shadowTable[indexPath.section][indexPath.row] @@ -476,24 +455,13 @@ class AppCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { expandedNodes.remove(at: removeNode) } - var indexPathsToRemove = [IndexPath]() - - for child in collapseNode.childNodes { - if let index = shadowTable[indexPath.section].firstIndex(of: child) { - indexPathsToRemove.append(IndexPath(row: index, section: indexPath.section)) - } - } - for child in collapseNode.childNodes { if let index = shadowTable[indexPath.section].firstIndex(of: child) { shadowTable[indexPath.section].remove(at: index) } } - completion(indexPathsToRemove) - animatingChanges = false - } func indexesForArticleIDs(_ articleIDs: Set) -> IndexSet { diff --git a/iOS/MasterFeed/MasterFeedDataSource.swift b/iOS/MasterFeed/MasterFeedDataSource.swift new file mode 100644 index 000000000..fe7807721 --- /dev/null +++ b/iOS/MasterFeed/MasterFeedDataSource.swift @@ -0,0 +1,87 @@ +// +// MasterFeedDataSource.swift +// NetNewsWire-iOS +// +// Created by Maurice Parker on 8/28/19. +// Copyright © 2019 Ranchero Software. All rights reserved. +// + +import UIKit +import RSCore +import RSTree +import Account + +class MasterFeedDataSource: UITableViewDiffableDataSource where SectionIdentifierType : Hashable, ItemIdentifierType : Hashable { + + private var coordinator: AppCoordinator! + private var errorHandler: ((Error) -> ())! + + init(coordinator: AppCoordinator, errorHandler: @escaping (Error) -> (), tableView: UITableView, cellProvider: @escaping UITableViewDiffableDataSource.CellProvider) { + super.init(tableView: tableView, cellProvider: cellProvider) + self.coordinator = coordinator + self.errorHandler = errorHandler + } + + override func tableView(_ tableView: UITableView, canEditRowAt indexPath: IndexPath) -> Bool { + guard let node = coordinator.nodeFor(indexPath), !(node.representedObject is PseudoFeed) else { + return false + } + return true + } + + override func tableView(_ tableView: UITableView, canMoveRowAt indexPath: IndexPath) -> Bool { + guard let node = coordinator.nodeFor(indexPath) else { + return false + } + return node.representedObject is Feed + } + + override func tableView(_ tableView: UITableView, moveRowAt sourceIndexPath: IndexPath, to destinationIndexPath: IndexPath) { + + guard let sourceNode = coordinator.nodeFor(sourceIndexPath), let feed = sourceNode.representedObject as? Feed else { + return + } + + // Based on the drop we have to determine a node to start looking for a parent container. + let destNode: Node = { + if destinationIndexPath.row == 0 { + return coordinator.rootNode.childAtIndex(destinationIndexPath.section)! + } else { + let movementAdjustment = sourceIndexPath > destinationIndexPath ? 1 : 0 + let adjustedDestIndexPath = IndexPath(row: destinationIndexPath.row - movementAdjustment, section: destinationIndexPath.section) + return coordinator.nodeFor(adjustedDestIndexPath)! + } + }() + + // Now we start looking for the parent container + let destParentNode: Node? = { + if destNode.representedObject is Container { + return destNode + } else { + if destNode.parent?.representedObject is Container { + return destNode.parent! + } else { + return nil + } + } + }() + + // Move the Feed + guard let source = sourceNode.parent?.representedObject as? Container, let destination = destParentNode?.representedObject as? Container else { + return + } + + BatchUpdate.shared.start() + source.account?.moveFeed(feed, from: source, to: destination) { result in + switch result { + case .success: + BatchUpdate.shared.end() + case .failure(let error): + self.errorHandler(error) + } + } + + } + + +} diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index 9c2b0fdb1..7025f8de3 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -18,9 +18,10 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { @IBOutlet private weak var markAllAsReadButton: UIBarButtonItem! @IBOutlet private weak var addNewItemButton: UIBarButtonItem! + private lazy var dataSource = makeDataSource() var undoableCommands = [UndoableCommand]() - weak var coordinator: AppCoordinator! + override var canBecomeFirstResponder: Bool { return true } @@ -36,6 +37,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { navigationItem.rightBarButtonItem = editButtonItem tableView.register(MasterFeedTableViewSectionHeader.self, forHeaderFooterViewReuseIdentifier: "SectionHeader") + tableView.dataSource = dataSource NotificationCenter.default.addObserver(self, selector: #selector(unreadCountDidChange(_:)), name: .UnreadCountDidChange, object: nil) NotificationCenter.default.addObserver(self, selector: #selector(faviconDidBecomeAvailable(_:)), name: .FaviconDidBecomeAvailable, object: nil) @@ -48,6 +50,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { refreshControl!.addTarget(self, action: #selector(refreshAccounts(_:)), for: .valueChanged) updateUI() + applyChanges(animate: false) } @@ -71,38 +74,8 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { // MARK: Notifications @objc func unreadCountDidChange(_ note: Notification) { - updateUI() - - guard let representedObject = note.object else { - return - } - - if let account = representedObject as? Account { - if let node = coordinator.rootNode.childNodeRepresentingObject(account) { - let sectionIndex = coordinator.rootNode.indexOfChild(node)! - if let headerView = tableView.headerView(forSection: sectionIndex) as? MasterFeedTableViewSectionHeader { - headerView.unreadCount = account.unreadCount - } - } - return - } - - var node: Node? = nil - if let coordinator = representedObject as? AppCoordinator, let fetcher = coordinator.timelineFetcher { - node = coordinator.rootNode.descendantNodeRepresentingObject(fetcher as AnyObject) - } else { - node = coordinator.rootNode.descendantNodeRepresentingObject(representedObject as AnyObject) - } - - guard let unwrappedNode = node, let indexPath = coordinator.indexPathFor(unwrappedNode) else { - return - } - - performBlockAndRestoreSelection { - tableView.reloadRows(at: [indexPath], with: .none) - } - + applyChanges(animate: false) } @objc func faviconDidBecomeAvailable(_ note: Notification) { @@ -110,15 +83,12 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } @objc func feedSettingDidChange(_ note: Notification) { - guard let feed = note.object as? Feed, let key = note.userInfo?[Feed.FeedSettingUserInfoKey] as? String else { return } - if key == Feed.FeedSettingKey.homePageURL || key == Feed.FeedSettingKey.faviconURL { configureCellsForRepresentedObject(feed) } - } @objc func userDidAddFeed(_ notification: Notification) { @@ -133,19 +103,11 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } @objc func contentSizeCategoryDidChange(_ note: Notification) { - tableView.reloadData() + applyChanges(animate: false) } // MARK: Table View - override func numberOfSections(in tableView: UITableView) -> Int { - return coordinator.numberOfSections - } - - override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int { - return coordinator.rowsInSection(section) - } - override func tableView(_ tableView: UITableView, heightForHeaderInSection section: Int) -> CGFloat { guard let nameProvider = coordinator.rootNode.childAtIndex(section)?.representedObject as? DisplayNameProvider else { @@ -197,26 +159,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { return UIView(frame: CGRect.zero) } - override func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell { - - let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterFeedTableViewCell - - guard let node = coordinator.nodeFor(indexPath) else { - return cell - } - - configure(cell, node) - return cell - - } - - override func tableView(_ tableView: UITableView, canEditRowAt indexPath: IndexPath) -> Bool { - guard let node = coordinator.nodeFor(indexPath), !(node.representedObject is PseudoFeed) else { - return false - } - return true - } - override func tableView(_ tableView: UITableView, trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath) -> UISwipeActionsConfiguration? { var actions = [UIContextualAction]() @@ -296,13 +238,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { coordinator.selectFeed(indexPath) } - override func tableView(_ tableView: UITableView, canMoveRowAt indexPath: IndexPath) -> Bool { - guard let node = coordinator.nodeFor(indexPath) else { - return false - } - return node.representedObject is Feed - } - override func tableView(_ tableView: UITableView, targetIndexPathForMoveFromRowAt sourceIndexPath: IndexPath, toProposedIndexPath proposedDestinationIndexPath: IndexPath) -> IndexPath { // Adjust the index path so that it will never be in the smart feeds area @@ -360,53 +295,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } - override func tableView(_ tableView: UITableView, moveRowAt sourceIndexPath: IndexPath, to destinationIndexPath: IndexPath) { - - guard let sourceNode = coordinator.nodeFor(sourceIndexPath), let feed = sourceNode.representedObject as? Feed else { - return - } - - // Based on the drop we have to determine a node to start looking for a parent container. - let destNode: Node = { - if destinationIndexPath.row == 0 { - return coordinator.rootNode.childAtIndex(destinationIndexPath.section)! - } else { - let movementAdjustment = sourceIndexPath > destinationIndexPath ? 1 : 0 - let adjustedDestIndexPath = IndexPath(row: destinationIndexPath.row - movementAdjustment, section: destinationIndexPath.section) - return coordinator.nodeFor(adjustedDestIndexPath)! - } - }() - - // Now we start looking for the parent container - let destParentNode: Node? = { - if destNode.representedObject is Container { - return destNode - } else { - if destNode.parent?.representedObject is Container { - return destNode.parent! - } else { - return nil - } - } - }() - - // Move the Feed - guard let source = sourceNode.parent?.representedObject as? Container, let destination = destParentNode?.representedObject as? Container else { - return - } - - BatchUpdate.shared.start() - source.account?.moveFeed(feed, from: source, to: destination) { result in - switch result { - case .success: - BatchUpdate.shared.end() - case .failure(let error): - self.presentError(error) - } - } - - } - // MARK: Actions @IBAction func settings(_ sender: UIBarButtonItem) { @@ -449,18 +337,12 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { if coordinator.isExpanded(sectionNode) { headerView.disclosureExpanded = false - coordinator.collapse(section: sectionIndex) { [weak self] indexPaths in - self?.tableView.beginUpdates() - self?.tableView.deleteRows(at: indexPaths, with: .automatic) - self?.tableView.endUpdates() - } + coordinator.collapse(section: sectionIndex) + self.applyChanges(animate: true) } else { headerView.disclosureExpanded = true - coordinator.expand(section: sectionIndex) { [weak self] indexPaths in - self?.tableView.beginUpdates() - self?.tableView.insertRows(at: indexPaths, with: .automatic) - self?.tableView.endUpdates() - } + coordinator.expand(section: sectionIndex) + self.applyChanges(animate: true) } } @@ -486,7 +368,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { func reloadFeeds() { updateUI() - tableView.reloadData() + applyChanges(animate: true) } func discloseFeed(_ feed: Feed) { @@ -506,13 +388,8 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { return } - coordinator.expand(indexPath) { [weak self] indexPaths in - self?.tableView.beginUpdates() - tableView.reloadRows(at: [indexPath], with: .automatic) - self?.tableView.insertRows(at: indexPaths, with: .automatic) - self?.tableView.endUpdates() - } - + coordinator.expand(indexPath) + self.applyChanges(animate: true) if let indexPath = coordinator.indexPathFor(node) { tableView.scrollToRow(at: indexPath, at: .middle, animated: true) coordinator.selectFeed(indexPath) @@ -544,7 +421,42 @@ private extension MasterFeedViewController { markAllAsReadButton.isEnabled = coordinator.isAnyUnreadAvailable addNewItemButton.isEnabled = !AccountManager.shared.activeAccounts.isEmpty } + + func applyChanges(animate: Bool) { + + let selectedNode: Node? = { + if let selectedIndexPath = tableView.indexPathForSelectedRow { + return coordinator.nodeFor(selectedIndexPath) + } else { + return nil + } + }() + + var snapshot = NSDiffableDataSourceSnapshot() + + let sections = coordinator.allSections + snapshot.appendSections(sections) + for section in sections { + snapshot.appendItems(coordinator.nodesFor(section: section), toSection: section) + } + + dataSource.apply(snapshot, animatingDifferences: animate) + + if let nodeToSelect = selectedNode, let selectingIndexPath = coordinator.indexPathFor(nodeToSelect) { + tableView.selectRow(at: selectingIndexPath, animated: false, scrollPosition: .none) + } + + } + + func makeDataSource() -> UITableViewDiffableDataSource { + return MasterFeedDataSource(coordinator: coordinator, errorHandler: ErrorHandler.present(self), tableView: tableView, cellProvider: { [weak self] tableView, indexPath, node in + let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterFeedTableViewCell + self?.configure(cell, node) + return cell + }) + } + func configure(_ cell: MasterFeedTableViewCell, _ node: Node) { cell.delegate = self @@ -619,32 +531,18 @@ private extension MasterFeedViewController { guard let indexPath = tableView.indexPath(for: cell) else { return } - coordinator.expand(indexPath) { [weak self] indexPaths in - self?.tableView.beginUpdates() - self?.tableView.insertRows(at: indexPaths, with: .automatic) - self?.tableView.endUpdates() - } + coordinator.expand(indexPath) + self.applyChanges(animate: true) } func collapse(_ cell: MasterFeedTableViewCell) { guard let indexPath = tableView.indexPath(for: cell) else { return } - coordinator.collapse(indexPath) { [weak self] indexPaths in - self?.tableView.beginUpdates() - self?.tableView.deleteRows(at: indexPaths, with: .automatic) - self?.tableView.endUpdates() - } + coordinator.collapse(indexPath) + self.applyChanges(animate: true) } - func performBlockAndRestoreSelection(_ block: (() -> Void)) { - let indexPaths = tableView.indexPathsForSelectedRows - block() - indexPaths?.forEach { [weak self] indexPath in - self?.tableView.selectRow(at: indexPath, animated: false, scrollPosition: .none) - } - } - func makeFeedContextMenu(indexPath: IndexPath, includeDeleteRename: Bool) -> UIContextMenuConfiguration { return UIContextMenuConfiguration(identifier: nil, previewProvider: nil, actionProvider: { [ weak self] suggestedActions in @@ -820,7 +718,7 @@ private extension MasterFeedViewController { feed.rename(to: name) { result in switch result { case .success: - break + self?.configureCellsForRepresentedObject(feed) case .failure(let error): self?.presentError(error) } @@ -829,7 +727,7 @@ private extension MasterFeedViewController { folder.rename(to: name) { result in switch result { case .success: - break + self?.configureCellsForRepresentedObject(folder) case .failure(let error): self?.presentError(error) } @@ -851,7 +749,6 @@ private extension MasterFeedViewController { } func delete(indexPath: IndexPath) { - guard let undoManager = undoManager, let deleteNode = coordinator.nodeFor(indexPath), let deleteCommand = DeleteCommand(nodesToDelete: [deleteNode], treeController: coordinator.treeController, undoManager: undoManager, errorHandler: ErrorHandler.present(self)) @@ -865,23 +762,8 @@ private extension MasterFeedViewController { ActivityManager.shared.cleanUp(feed) } - var deleteIndexPaths = [indexPath] - if coordinator.isExpanded(deleteNode) { - for i in 0.. Date: Wed, 28 Aug 2019 20:08:30 -0500 Subject: [PATCH 6/7] Make sure we manually correct some state when using diffable datasources --- iOS/MasterFeed/MasterFeedViewController.swift | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index 7025f8de3..73105174a 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -389,10 +389,16 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } coordinator.expand(indexPath) - self.applyChanges(animate: true) - if let indexPath = coordinator.indexPathFor(node) { - tableView.scrollToRow(at: indexPath, at: .middle, animated: true) - coordinator.selectFeed(indexPath) + + // I don't think this should be necessary when using diffable datasources, but they don't + // always reload cells that you want them too. + configureCellsForRepresentedObject(parent.representedObject) + + self.applyChanges(animate: true) { [weak self] in + if let indexPath = self?.coordinator.indexPathFor(node) { + self?.tableView.scrollToRow(at: indexPath, at: .middle, animated: true) + self?.coordinator.selectFeed(indexPath) + } } } @@ -422,7 +428,7 @@ private extension MasterFeedViewController { addNewItemButton.isEnabled = !AccountManager.shared.activeAccounts.isEmpty } - func applyChanges(animate: Bool) { + func applyChanges(animate: Bool, completion: (() -> Void)? = nil) { let selectedNode: Node? = { if let selectedIndexPath = tableView.indexPathForSelectedRow { @@ -441,10 +447,11 @@ private extension MasterFeedViewController { snapshot.appendItems(coordinator.nodesFor(section: section), toSection: section) } - dataSource.apply(snapshot, animatingDifferences: animate) - - if let nodeToSelect = selectedNode, let selectingIndexPath = coordinator.indexPathFor(nodeToSelect) { - tableView.selectRow(at: selectingIndexPath, animated: false, scrollPosition: .none) + dataSource.apply(snapshot, animatingDifferences: animate) { [weak self] in + if let nodeToSelect = selectedNode, let selectingIndexPath = self?.coordinator.indexPathFor(nodeToSelect) { + self?.tableView.selectRow(at: selectingIndexPath, animated: false, scrollPosition: .none) + } + completion?() } } From be8c14bc65425c80eb3212bbd35c8f52f4da5458 Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 28 Aug 2019 20:21:50 -0500 Subject: [PATCH 7/7] Change to reload the individual row instead of just changing its contents --- iOS/MasterFeed/MasterFeedViewController.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index 73105174a..72bac2de0 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -389,10 +389,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } coordinator.expand(indexPath) - - // I don't think this should be necessary when using diffable datasources, but they don't - // always reload cells that you want them too. - configureCellsForRepresentedObject(parent.representedObject) + reloadNode(parent) self.applyChanges(animate: true) { [weak self] in if let indexPath = self?.coordinator.indexPathFor(node) { @@ -428,6 +425,12 @@ private extension MasterFeedViewController { addNewItemButton.isEnabled = !AccountManager.shared.activeAccounts.isEmpty } + func reloadNode(_ node: Node) { + var snapshot = dataSource.snapshot() + snapshot.reloadItems([node]) + dataSource.apply(snapshot) + } + func applyChanges(animate: Bool, completion: (() -> Void)? = nil) { let selectedNode: Node? = { @@ -725,7 +728,7 @@ private extension MasterFeedViewController { feed.rename(to: name) { result in switch result { case .success: - self?.configureCellsForRepresentedObject(feed) + self?.reloadNode(node) case .failure(let error): self?.presentError(error) } @@ -734,7 +737,7 @@ private extension MasterFeedViewController { folder.rename(to: name) { result in switch result { case .success: - self?.configureCellsForRepresentedObject(folder) + self?.reloadNode(node) case .failure(let error): self?.presentError(error) }