From 854525226573aad57eebece742d4a2747ea7b74a Mon Sep 17 00:00:00 2001 From: Maurice Parker Date: Wed, 11 Sep 2019 09:11:33 -0500 Subject: [PATCH] Remove the usage of IndexPaths for the interface between the coordinator and the timeline --- .../MasterTimelineViewController.swift | 109 +++++++------- iOS/SceneCoordinator.swift | 137 ++++++------------ 2 files changed, 92 insertions(+), 154 deletions(-) diff --git a/iOS/MasterTimeline/MasterTimelineViewController.swift b/iOS/MasterTimeline/MasterTimelineViewController.swift index 0d3c6a76a..154729281 100644 --- a/iOS/MasterTimeline/MasterTimelineViewController.swift +++ b/iOS/MasterTimeline/MasterTimelineViewController.swift @@ -146,7 +146,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner // MARK: API func restoreSelectionIfNecessary() { - if let indexPath = coordinator.currentArticleIndexPath { + if let article = coordinator.currentArticle, let indexPath = dataSource.indexPath(for: article) { tableView.selectRowAndScrollIfNotVisible(at: indexPath, animated: false, deselect: coordinator.isRootSplitCollapsed) } } @@ -160,7 +160,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner } func updateArticleSelection(animate: Bool) { - if let indexPath = coordinator.currentArticleIndexPath { + if let article = coordinator.currentArticle, let indexPath = dataSource.indexPath(for: article) { if tableView.indexPathForSelectedRow != indexPath { tableView.selectRowAndScrollIfNotVisible(at: indexPath, animated: true, deselect: coordinator.isRootSplitCollapsed) } @@ -185,7 +185,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner override func tableView(_ tableView: UITableView, trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath) -> UISwipeActionsConfiguration? { - let article = dataSource.itemIdentifier(for: indexPath)! + guard let article = dataSource.itemIdentifier(for: indexPath) else { return nil } // Set up the read action let readTitle = article.status.read ? @@ -193,7 +193,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner NSLocalizedString("Read", comment: "Read") let readAction = UIContextualAction(style: .normal, title: readTitle) { [weak self] (action, view, completionHandler) in - self?.coordinator.toggleRead(for: indexPath) + self?.coordinator.toggleRead(article) completionHandler(true) } @@ -206,7 +206,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner NSLocalizedString("Star", comment: "Star") let starAction = UIContextualAction(style: .normal, title: starTitle) { [weak self] (action, view, completionHandler) in - self?.coordinator.toggleStar(for: indexPath) + self?.coordinator.toggleStar(article) completionHandler(true) } @@ -225,21 +225,21 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner popoverController.sourceRect = CGRect(x: view.frame.size.width/2, y: view.frame.size.height/2, width: 1, height: 1) } - alert.addAction(self.markOlderAsReadAlertAction(indexPath: indexPath, completionHandler: completionHandler)) + alert.addAction(self.markOlderAsReadAlertAction(article, completionHandler: completionHandler)) - if let action = self.discloseFeedAlertAction(indexPath: indexPath, completionHandler: completionHandler) { + if let action = self.discloseFeedAlertAction(article, completionHandler: completionHandler) { alert.addAction(action) } - if let action = self.markAllInFeedAsReadAlertAction(indexPath: indexPath, completionHandler: completionHandler) { + if let action = self.markAllInFeedAsReadAlertAction(article, completionHandler: completionHandler) { alert.addAction(action) } - if let action = self.openInBrowserAlertAction(indexPath: indexPath, completionHandler: completionHandler) { + if let action = self.openInBrowserAlertAction(article, completionHandler: completionHandler) { alert.addAction(action) } - if let action = self.shareAlertAction(indexPath: indexPath, completionHandler: completionHandler) { + if let action = self.shareAlertAction(article, indexPath: indexPath, completionHandler: completionHandler) { alert.addAction(action) } @@ -264,28 +264,30 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner override func tableView(_ tableView: UITableView, contextMenuConfigurationForRowAt indexPath: IndexPath, point: CGPoint) -> UIContextMenuConfiguration? { + guard let article = dataSource.itemIdentifier(for: indexPath) else { return nil } + return UIContextMenuConfiguration(identifier: nil, previewProvider: nil, actionProvider: { [weak self] suggestedActions in guard let self = self else { return nil } var actions = [UIAction]() - actions.append(self.toggleArticleReadStatusAction(indexPath: indexPath)) - actions.append(self.toggleArticleStarStatusAction(indexPath: indexPath)) - actions.append(self.markOlderAsReadAction(indexPath: indexPath)) + actions.append(self.toggleArticleReadStatusAction(article)) + actions.append(self.toggleArticleStarStatusAction(article)) + actions.append(self.markOlderAsReadAction(article)) - if let action = self.discloseFeedAction(indexPath: indexPath) { + if let action = self.discloseFeedAction(article) { actions.append(action) } - if let action = self.markAllInFeedAsReadAction(indexPath: indexPath) { + if let action = self.markAllInFeedAsReadAction(article) { actions.append(action) } - if let action = self.openInBrowserAction(indexPath: indexPath) { + if let action = self.openInBrowserAction(article) { actions.append(action) } - if let action = self.shareAction(indexPath: indexPath) { + if let action = self.shareAction(article, indexPath: indexPath) { actions.append(action) } @@ -297,7 +299,8 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { becomeFirstResponder() - coordinator.selectArticle(indexPath, automated: false) + let article = dataSource.itemIdentifier(for: indexPath) + coordinator.selectArticle(article, automated: false) } // MARK: Notifications @@ -315,8 +318,8 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner let visibleUpdatedArticles = visibleArticles.filter { updatedArticles.contains($0) } for article in visibleUpdatedArticles { - if let articleIndex = coordinator.indexForArticleID(article.articleID) { - if let cell = tableView.cellForRow(at: IndexPath(row: articleIndex, section: 0)) as? MasterTimelineTableViewCell { + if let indexPath = dataSource.indexPath(for: article) { + if let cell = tableView.cellForRow(at: indexPath) as? MasterTimelineTableViewCell { configure(cell, article: article) } } @@ -532,8 +535,7 @@ private extension MasterTimelineViewController { return nil } - func toggleArticleReadStatusAction(indexPath: IndexPath) -> UIAction { - let article = dataSource.itemIdentifier(for: indexPath)! + func toggleArticleReadStatusAction(_ article: Article) -> UIAction { let title = article.status.read ? NSLocalizedString("Mark as Unread", comment: "Mark as Unread") : @@ -541,14 +543,13 @@ private extension MasterTimelineViewController { let image = article.status.read ? AppAssets.circleClosedImage : AppAssets.circleOpenImage let action = UIAction(title: title, image: image) { [weak self] action in - self?.coordinator.toggleRead(for: indexPath) + self?.coordinator.toggleRead(article) } return action } - func toggleArticleStarStatusAction(indexPath: IndexPath) -> UIAction { - let article = dataSource.itemIdentifier(for: indexPath)! + func toggleArticleStarStatusAction(_ article: Article) -> UIAction { let title = article.status.starred ? NSLocalizedString("Mark as Unstarred", comment: "Mark as Unstarred") : @@ -556,34 +557,33 @@ private extension MasterTimelineViewController { let image = article.status.starred ? AppAssets.starOpenImage : AppAssets.starClosedImage let action = UIAction(title: title, image: image) { [weak self] action in - self?.coordinator.toggleStar(for: indexPath) + self?.coordinator.toggleStar(article) } return action } - func markOlderAsReadAction(indexPath: IndexPath) -> UIAction { + func markOlderAsReadAction(_ article: Article) -> UIAction { let title = NSLocalizedString("Mark Older as Read", comment: "Mark Older as Read") let image = coordinator.sortDirection == .orderedDescending ? AppAssets.markOlderAsReadDownImage : AppAssets.markOlderAsReadUpImage let action = UIAction(title: title, image: image) { [weak self] action in - self?.coordinator.markAsReadOlderArticlesInTimeline(indexPath) + self?.coordinator.markAsReadOlderArticlesInTimeline(article) } return action } - func markOlderAsReadAlertAction(indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction { + func markOlderAsReadAlertAction(_ article: Article, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction { let title = NSLocalizedString("Mark Older as Read", comment: "Mark Older as Read") let action = UIAlertAction(title: title, style: .default) { [weak self] action in - self?.coordinator.markAsReadOlderArticlesInTimeline(indexPath) + self?.coordinator.markAsReadOlderArticlesInTimeline(article) completionHandler(true) } return action } - func discloseFeedAction(indexPath: IndexPath) -> UIAction? { - guard let feed = dataSource.itemIdentifier(for: indexPath)?.feed else { - return nil - } + func discloseFeedAction(_ article: Article) -> UIAction? { + guard let feed = article.feed else { return nil } + let title = NSLocalizedString("Select Feed", comment: "Select Feed") let action = UIAction(title: title, image: AppAssets.openInSidebarImage) { [weak self] action in self?.coordinator.discloseFeed(feed) @@ -591,10 +591,9 @@ private extension MasterTimelineViewController { return action } - func discloseFeedAlertAction(indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let feed = dataSource.itemIdentifier(for: indexPath)?.feed else { - return nil - } + func discloseFeedAlertAction(_ article: Article, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { + guard let feed = article.feed else { return nil } + let title = NSLocalizedString("Select Feed", comment: "Select Feed") let action = UIAlertAction(title: title, style: .default) { [weak self] action in self?.coordinator.discloseFeed(feed) @@ -603,11 +602,9 @@ private extension MasterTimelineViewController { return action } - func markAllInFeedAsReadAction(indexPath: IndexPath) -> UIAction? { - guard let feed = dataSource.itemIdentifier(for: indexPath)?.feed else { - return nil - } - + func markAllInFeedAsReadAction(_ article: Article) -> UIAction? { + guard let feed = article.feed else { return nil } + let articles = Array(feed.fetchArticles()) guard articles.canMarkAllAsRead() else { return nil @@ -622,11 +619,9 @@ private extension MasterTimelineViewController { return action } - func markAllInFeedAsReadAlertAction(indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let feed = dataSource.itemIdentifier(for: indexPath)?.feed else { - return nil - } - + func markAllInFeedAsReadAlertAction(_ article: Article, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { + guard let feed = article.feed else { return nil } + let articles = Array(feed.fetchArticles()) guard articles.canMarkAllAsRead() else { return nil @@ -642,24 +637,24 @@ private extension MasterTimelineViewController { return action } - func openInBrowserAction(indexPath: IndexPath) -> UIAction? { - guard let preferredLink = dataSource.itemIdentifier(for: indexPath)?.preferredLink, let _ = URL(string: preferredLink) else { + func openInBrowserAction(_ article: Article) -> UIAction? { + guard let preferredLink = article.preferredLink, let _ = URL(string: preferredLink) else { return nil } let title = NSLocalizedString("Open in Browser", comment: "Open in Browser") let action = UIAction(title: title, image: AppAssets.safariImage) { [weak self] action in - self?.coordinator.showBrowserForArticle(indexPath) + self?.coordinator.showBrowserForArticle(article) } return action } - func openInBrowserAlertAction(indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { - guard let preferredLink = dataSource.itemIdentifier(for: indexPath)?.preferredLink, let _ = URL(string: preferredLink) else { + func openInBrowserAlertAction(_ article: Article, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { + guard let preferredLink = article.preferredLink, let _ = URL(string: preferredLink) else { return nil } let title = NSLocalizedString("Open in Browser", comment: "Open in Browser") let action = UIAlertAction(title: title, style: .default) { [weak self] action in - self?.coordinator.showBrowserForArticle(indexPath) + self?.coordinator.showBrowserForArticle(article) completionHandler(true) } return action @@ -677,8 +672,7 @@ private extension MasterTimelineViewController { present(activityViewController, animated: true) } - func shareAction(indexPath: IndexPath) -> UIAction? { - let article = dataSource.itemIdentifier(for: indexPath)! + func shareAction(_ article: Article, indexPath: IndexPath) -> UIAction? { guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { return nil } @@ -690,8 +684,7 @@ private extension MasterTimelineViewController { return action } - func shareAlertAction(indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { - let article = dataSource.itemIdentifier(for: indexPath)! + func shareAlertAction(_ article: Article, indexPath: IndexPath, completionHandler: @escaping (Bool) -> Void) -> UIAlertAction? { guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { return nil } diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index bbf1d623e..1023f9e5c 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -55,7 +55,6 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { private let fetchAndMergeArticlesQueue = CoalescingQueue(name: "Fetch and Merge Articles", interval: 0.5) private var fetchSerialNumber = 0 private let fetchRequestQueue = FetchRequestQueue() - private var articleRowMap = [String: Int]() // articleID: rowIndex private var animatingChanges = false private var expandedNodes = [Node]() @@ -180,31 +179,31 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } var isPrevArticleAvailable: Bool { - guard let indexPath = currentArticleIndexPath else { + guard let articleRow = currentArticleRow else { return false } - return indexPath.row > 0 + return articleRow > 0 } var isNextArticleAvailable: Bool { - guard let indexPath = currentArticleIndexPath else { + guard let articleRow = currentArticleRow else { return false } - return indexPath.row + 1 < articles.count + return articleRow + 1 < articles.count } - var prevArticleIndexPath: IndexPath? { - guard isPrevArticleAvailable, let indexPath = currentArticleIndexPath else { + var prevArticle: Article? { + guard isPrevArticleAvailable, let articleRow = currentArticleRow else { return nil } - return IndexPath(row: indexPath.row - 1, section: indexPath.section) + return articles[articleRow] } - var nextArticleIndexPath: IndexPath? { - guard isNextArticleAvailable, let indexPath = currentArticleIndexPath else { + var nextArticle: Article? { + guard isNextArticleAvailable, let articleRow = currentArticleRow else { return nil } - return IndexPath(row: indexPath.row + 1, section: indexPath.section) + return articles[articleRow] } var firstUnreadArticleIndexPath: IndexPath? { @@ -216,17 +215,13 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return nil } - var currentArticle: Article? { - if let indexPath = currentArticleIndexPath, indexPath.row < articles.count { - return articles[indexPath.row] - } - return nil - } - - private(set) var currentArticleIndexPath: IndexPath? - private(set) var articles = ArticleArray() - + var currentArticle: Article? + var currentArticleRow: Int? { + guard let article = currentArticle else { return nil } + return articles.firstIndex(of: article) + } + var isTimelineUnreadAvailable: Bool { if let unreadProvider = timelineFetcher as? UnreadCountProvider { return unreadProvider.unreadCount > 0 @@ -602,27 +597,6 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { return indexPathFor(node) } - func indexForArticleID(_ articleID: String?) -> Int? { - guard let articleID = articleID else { return nil } - updateArticleRowMapIfNeeded() - return articleRowMap[articleID] - } - - func indexesForArticleIDs(_ articleIDs: Set) -> IndexSet { - var indexes = IndexSet() - - articleIDs.forEach { (articleID) in - guard let oneIndex = indexForArticleID(articleID) else { - return - } - if oneIndex != NSNotFound { - indexes.insert(oneIndex) - } - } - - return indexes - } - func selectFeed(_ indexPath: IndexPath?, automated: Bool = true) { selectArticle(nil) currentFeedIndexPath = indexPath @@ -673,11 +647,11 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - func selectArticle(_ indexPath: IndexPath?, automated: Bool = true) { - currentArticleIndexPath = indexPath + func selectArticle(_ article: Article?, automated: Bool = true) { + currentArticle = article activityManager.reading(currentArticle) - if indexPath == nil { + if article == nil { if rootSplitViewController.isCollapsed { if masterNavigationController.children.last is DetailViewController { masterNavigationController.popViewController(animated: !automated) @@ -751,14 +725,14 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } func selectPrevArticle() { - if let indexPath = prevArticleIndexPath { - selectArticle(indexPath) + if let article = prevArticle { + selectArticle(article) } } func selectNextArticle() { - if let indexPath = nextArticleIndexPath { - selectArticle(indexPath) + if let article = nextArticle { + selectArticle(article) } } @@ -834,12 +808,12 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } func markAsReadOlderArticlesInTimeline() { - if let indexPath = currentArticleIndexPath { - markAsReadOlderArticlesInTimeline(indexPath) + if let article = currentArticle { + markAsReadOlderArticlesInTimeline(article) } } - func markAsReadOlderArticlesInTimeline(_ indexPath: IndexPath) { - let article = articles[indexPath.row] + + func markAsReadOlderArticlesInTimeline(_ article: Article) { let articlesToMark = articles.filter { $0.logicalDatePublished < article.logicalDatePublished } if articlesToMark.isEmpty { return @@ -865,8 +839,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - func toggleRead(for indexPath: IndexPath) { - let article = articles[indexPath.row] + func toggleRead(_ article: Article) { guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: [article], markingRead: !article.status.read, undoManager: undoManager) else { return @@ -880,8 +853,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - func toggleStar(for indexPath: IndexPath) { - let article = articles[indexPath.row] + func toggleStar(_ article: Article) { guard let undoManager = undoManager, let markReadCommand = MarkStatusCommand(initialArticles: [article], markingStarred: !article.status.starred, undoManager: undoManager) else { return @@ -940,8 +912,8 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } } - func showBrowserForArticle(_ indexPath: IndexPath) { - guard let preferredLink = articles[indexPath.row].preferredLink, let url = URL(string: preferredLink) else { + func showBrowserForArticle(_ article: Article) { + guard let preferredLink = article.preferredLink, let url = URL(string: preferredLink) else { return } UIApplication.shared.open(url, options: [:]) @@ -960,8 +932,8 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { } func navigateToTimeline() { - if currentArticleIndexPath == nil { - selectArticle(IndexPath(row: 0, section: 0)) + if currentArticle == nil && articles.count > 0 { + selectArticle(articles[0]) } masterTimelineViewController?.focus() } @@ -1124,8 +1096,8 @@ private extension SceneCoordinator { @discardableResult func selectPrevUnreadArticleInTimeline() -> Bool { let startingRow: Int = { - if let indexPath = currentArticleIndexPath { - return indexPath.row - 1 + if let articleRow = currentArticleRow { + return articleRow } else { return articles.count - 1 } @@ -1143,7 +1115,7 @@ private extension SceneCoordinator { for i in (0...startingRow).reversed() { let article = articles[i] if !article.status.read { - selectArticle(IndexPath(row: i, section: 0)) + selectArticle(article) return true } } @@ -1231,8 +1203,8 @@ private extension SceneCoordinator { @discardableResult func selectNextUnreadArticleInTimeline() -> Bool { let startingRow: Int = { - if let indexPath = currentArticleIndexPath { - return indexPath.row + 1 + if let articleRow = currentArticleRow { + return articleRow + 1 } else { return 0 } @@ -1250,7 +1222,7 @@ private extension SceneCoordinator { for i in startingRow..