Populate timeline asynchronously and restore timeline after search. Issues #1307 & #1308

This commit is contained in:
Maurice Parker 2019-11-19 11:16:43 -06:00
parent 188c1f8d8e
commit bdf9add8f1
3 changed files with 196 additions and 166 deletions

View File

@ -146,7 +146,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
@objc func contentSizeCategoryDidChange(_ note: Notification) {
resetEstimatedRowHeight()
applyChanges(animate: false)
applyChanges(animated: false)
}
@objc func willEnterForeground(_ note: Notification) {
@ -384,11 +384,11 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
if sectionNode.isExpanded {
headerView.disclosureExpanded = false
coordinator.collapse(sectionNode)
self.applyChanges(animate: true)
self.applyChanges(animated: true)
} else {
headerView.disclosureExpanded = true
coordinator.expand(sectionNode)
self.applyChanges(animate: true)
self.applyChanges(animated: true)
}
}
@ -429,7 +429,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
@objc func expandSelectedRows(_ sender: Any?) {
if let indexPath = coordinator.currentFeedIndexPath, let node = dataSource.itemIdentifier(for: indexPath) {
coordinator.expand(node)
self.applyChanges(animate: true) {
self.applyChanges(animated: true) {
self.reloadAllVisibleCells()
}
}
@ -438,7 +438,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
@objc func collapseSelectedRows(_ sender: Any?) {
if let indexPath = coordinator.currentFeedIndexPath, let node = dataSource.itemIdentifier(for: indexPath) {
coordinator.collapse(node)
self.applyChanges(animate: true) {
self.applyChanges(animated: true) {
self.reloadAllVisibleCells()
}
}
@ -446,14 +446,14 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
@objc func expandAll(_ sender: Any?) {
coordinator.expandAllSectionsAndFolders()
self.applyChanges(animate: true) {
self.applyChanges(animated: true) {
self.reloadAllVisibleCells()
}
}
@objc func collapseAllExceptForGroupItems(_ sender: Any?) {
coordinator.collapseAllFolders()
self.applyChanges(animate: true) {
self.applyChanges(animated: true) {
self.reloadAllVisibleCells()
}
}
@ -488,7 +488,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
// We have to reload all the visible cells because if we got here by doing a table cell move,
// then the table itself is in a weird state. This is because we do unusual things like allowing
// drops on a "folder" that should cause the dropped cell to disappear.
applyChanges(animate: true) { [weak self] in
applyChanges(animated: true) { [weak self] in
self?.reloadAllVisibleCells()
}
}
@ -500,7 +500,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
if !sectionNode.isExpanded {
coordinator.expand(sectionNode)
self.applyChanges(animate: true) {
self.applyChanges(animated: true) {
completion?()
}
} else {
@ -530,10 +530,11 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
coordinator.expand(parent)
reloadNode(parent)
self.applyChanges(animate: true, adjustScroll: true) { [weak self] in
self.applyChanges(animated: true, adjustScroll: true) { [weak self] in
if let indexPath = self?.dataSource.indexPath(for: node) {
self?.coordinator.selectFeed(indexPath, animated: animated)
completion?()
self?.coordinator.selectFeed(indexPath, animated: animated) {
completion?()
}
}
}
@ -609,7 +610,7 @@ private extension MasterFeedViewController {
}
}
func applyChanges(animate: Bool, adjustScroll: Bool = false, completion: (() -> Void)? = nil) {
func applyChanges(animated: Bool, adjustScroll: Bool = false, completion: (() -> Void)? = nil) {
var snapshot = NSDiffableDataSourceSnapshot<Node, Node>()
let sectionNodes = coordinator.rootNode.childNodes
snapshot.appendSections(sectionNodes)
@ -619,7 +620,7 @@ private extension MasterFeedViewController {
snapshot.appendItems(shadowTableNodes, toSection: sectionNode)
}
dataSource.apply(snapshot, animatingDifferences: animate) { [weak self] in
dataSource.apply(snapshot, animatingDifferences: animated) { [weak self] in
self?.restoreSelectionIfNecessary(adjustScroll: adjustScroll)
completion?()
}
@ -753,7 +754,7 @@ private extension MasterFeedViewController {
return
}
coordinator.expand(node)
applyChanges(animate: true) { [weak self] in
applyChanges(animated: true) { [weak self] in
self?.reloadNode(node)
}
}
@ -763,7 +764,7 @@ private extension MasterFeedViewController {
return
}
coordinator.collapse(node)
applyChanges(animate: true) { [weak self] in
applyChanges(animated: true) { [weak self] in
self?.reloadNode(node)
}
}

View File

@ -69,8 +69,7 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner
resetEstimatedRowHeight()
resetUI()
applyChanges(animate: false)
applyChanges(animated: false)
// Restore the scroll position if we have one stored
if let restoreIndexPath = coordinator.timelineMiddleIndexPath {
@ -127,6 +126,10 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner
// MARK: API
func restoreTimelinePosition() {
}
func restoreSelectionIfNecessary(adjustScroll: Bool) {
if let article = coordinator.currentArticle, let indexPath = dataSource.indexPath(for: article) {
if adjustScroll {
@ -141,8 +144,8 @@ class MasterTimelineViewController: UITableViewController, UndoableCommandRunner
resetUI()
}
func reloadArticles(animate: Bool) {
applyChanges(animate: animate)
func reloadArticles(animated: Bool) {
applyChanges(animated: animated)
}
func updateArticleSelection(animated: Bool) {
@ -499,28 +502,29 @@ private extension MasterTimelineViewController {
}
func updateTitleUnreadCount() {
if let unreadCountProvider = coordinator.timelineFeed as? UnreadCountProvider {
self.titleView?.unreadCountView.unreadCount = unreadCountProvider.unreadCount
}
self.titleView?.unreadCountView.unreadCount = coordinator.unreadCount
}
func applyChanges(animate: Bool, completion: (() -> Void)? = nil) {
func applyChanges(animated: Bool, completion: (() -> Void)? = nil) {
var snapshot = NSDiffableDataSourceSnapshot<Int, Article>()
snapshot.appendSections([0])
snapshot.appendItems(coordinator.articles, toSection: 0)
dataSource.apply(snapshot, animatingDifferences: animate) { [weak self] in
dataSource.apply(snapshot, animatingDifferences: animated) { [weak self] in
self?.restoreSelectionIfNecessary(adjustScroll: false)
completion?()
}
}
func makeDataSource() -> UITableViewDiffableDataSource<Int, Article> {
return MasterTimelineDataSource(coordinator: coordinator, tableView: tableView, cellProvider: { [weak self] tableView, indexPath, article in
let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterTimelineTableViewCell
self?.configure(cell, article: article)
return cell
})
let dataSource: UITableViewDiffableDataSource<Int, Article> =
MasterTimelineDataSource(coordinator: coordinator, tableView: tableView, cellProvider: { [weak self] tableView, indexPath, article in
let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterTimelineTableViewCell
self?.configure(cell, article: article)
return cell
})
dataSource.defaultRowAnimation = .left
return dataSource
}
func configure(_ cell: MasterTimelineTableViewCell, article: Article) {

View File

@ -64,7 +64,8 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
private var lastSearchString = ""
private var lastSearchScope: SearchScope? = nil
private var isSearching: Bool = false
private var searchArticleIds: Set<String>? = nil
private var savedSearchArticles: ArticleArray? = nil
private var savedSearchArticleIds: Set<String>? = nil
var isTimelineViewControllerPending = false
var isArticleViewControllerPending = false
@ -128,28 +129,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
return (timelineFeed as? SmallIconProvider)?.smallIcon
}
var timelineFeed: Feed? {
didSet {
timelineMiddleIndexPath = nil
if timelineFeed is WebFeed {
showFeedNames = false
} else {
showFeedNames = true
}
if isSearching {
fetchAndReplaceArticlesAsync {
self.masterTimelineViewController?.reinitializeArticles()
}
} else {
fetchAndReplaceArticlesSync()
masterTimelineViewController?.reinitializeArticles()
}
}
}
private(set) var timelineFeed: Feed?
var timelineMiddleIndexPath: IndexPath?
@ -323,18 +303,20 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
}
func handle(_ activity: NSUserActivity) {
selectFeed(nil, animated: false)
selectFeed(nil, animated: false) {
guard let activityType = ActivityType(rawValue: activity.activityType) else { return }
switch activityType {
case .selectFeed:
self.handleSelectFeed(activity.userInfo)
case .nextUnread:
self.selectFirstUnreadInAllUnread()
case .readArticle:
self.handleReadArticle(activity.userInfo)
case .addFeedIntent:
self.showAdd(.feed)
}
guard let activityType = ActivityType(rawValue: activity.activityType) else { return }
switch activityType {
case .selectFeed:
handleSelectFeed(activity.userInfo)
case .nextUnread:
selectFirstUnreadInAllUnread()
case .readArticle:
handleReadArticle(activity.userInfo)
case .addFeedIntent:
showAdd(.feed)
}
}
@ -359,15 +341,17 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
}
func selectFirstUnreadInAllUnread() {
selectFeed(IndexPath(row: 1, section: 0), animated: false)
selectFirstUnreadArticleInTimeline()
selectFeed(IndexPath(row: 1, section: 0), animated: false) {
self.selectFirstUnreadArticleInTimeline()
}
}
func showSearch() {
selectFeed(nil, animated: false)
installTimelineControllerIfNecessary(animated: false)
DispatchQueue.main.asyncAfter(deadline: .now()) {
self.masterTimelineViewController!.showSearchAll()
selectFeed(nil, animated: false) {
self.installTimelineControllerIfNecessary(animated: false)
DispatchQueue.main.asyncAfter(deadline: .now()) {
self.masterTimelineViewController!.showSearchAll()
}
}
}
@ -393,42 +377,61 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
}
@objc func accountStateDidChange(_ note: Notification) {
if timelineFetcherContainsAnyPseudoFeed() {
fetchAndReplaceArticlesSync()
}
guard let account = note.userInfo?[Account.UserInfoKey.account] as? Account else {
assertionFailure()
return
}
let rebuildAndExpand = {
guard let account = note.userInfo?[Account.UserInfoKey.account] as? Account else {
assertionFailure()
return
}
rebuildBackingStores() {
// If we are activating an account, then automatically expand it
if account.isActive, let node = self.treeController.rootNode.childNodeRepresentingObject(account) {
node.isExpanded = true
self.rebuildBackingStores() {
// If we are activating an account, then automatically expand it
if account.isActive, let node = self.treeController.rootNode.childNodeRepresentingObject(account) {
node.isExpanded = true
}
}
}
if timelineFetcherContainsAnyPseudoFeed() {
fetchAndReplaceArticlesAsync {
rebuildAndExpand()
}
} else {
rebuildAndExpand()
}
}
@objc func userDidAddAccount(_ note: Notification) {
if timelineFetcherContainsAnyPseudoFeed() {
fetchAndReplaceArticlesSync()
}
rebuildBackingStores() {
// Automatically expand any new accounts
if let account = note.userInfo?[Account.UserInfoKey.account] as? Account,
let node = self.treeController.rootNode.childNodeRepresentingObject(account) {
node.isExpanded = true
let rebuildAndExpand = {
self.rebuildBackingStores() {
// Automatically expand any new accounts
if let account = note.userInfo?[Account.UserInfoKey.account] as? Account,
let node = self.treeController.rootNode.childNodeRepresentingObject(account) {
node.isExpanded = true
}
}
}
if timelineFetcherContainsAnyPseudoFeed() {
fetchAndReplaceArticlesAsync {
rebuildAndExpand()
}
} else {
rebuildAndExpand()
}
}
@objc func userDidDeleteAccount(_ note: Notification) {
if timelineFetcherContainsAnyPseudoFeed() {
fetchAndReplaceArticlesSync()
fetchAndReplaceArticlesAsync {
self.rebuildBackingStores()
}
} else {
rebuildBackingStores()
}
rebuildBackingStores()
}
@objc func userDefaultsDidChange(_ note: Notification) {
@ -520,24 +523,36 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
return indexPathFor(node)
}
func selectFeed(_ indexPath: IndexPath?, animated: Bool) {
guard indexPath != currentFeedIndexPath else { return }
func selectFeed(_ indexPath: IndexPath?, animated: Bool, completion: (() -> Void)? = nil) {
guard indexPath != currentFeedIndexPath else {
completion?()
return
}
selectArticle(nil)
currentFeedIndexPath = indexPath
masterFeedViewController.updateFeedSelection(animated: animated)
emptyTheTimeline()
selectArticle(nil)
if let ip = indexPath, let node = nodeFor(ip), let feed = node.representedObject as? Feed {
timelineFeed = feed
activityManager.selecting(feed: feed)
installTimelineControllerIfNecessary(animated: animated)
} else {
timelineFeed = nil
activityManager.invalidateSelecting()
if rootSplitViewController.isCollapsed && navControllerForTimeline().viewControllers.last is MasterTimelineViewController {
navControllerForTimeline().popViewController(animated: animated)
self.activityManager.selecting(feed: feed)
self.installTimelineControllerIfNecessary(animated: animated)
setTimelineFeed(feed) {
completion?()
}
} else {
setTimelineFeed(nil) {
self.activityManager.invalidateSelecting()
if self.rootSplitViewController.isCollapsed && self.navControllerForTimeline().viewControllers.last is MasterTimelineViewController {
self.navControllerForTimeline().popViewController(animated: animated)
}
completion?()
}
}
}
@ -615,22 +630,26 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
func beginSearching() {
isSearching = true
searchArticleIds = Set(articles.map { $0.articleID })
timelineFeed = nil
savedSearchArticles = articles
savedSearchArticleIds = Set(articles.map { $0.articleID })
setTimelineFeed(nil)
selectArticle(nil)
}
func endSearching() {
isSearching = false
lastSearchString = ""
lastSearchScope = nil
searchArticleIds = nil
if let ip = currentFeedIndexPath, let node = nodeFor(ip), let feed = node.representedObject as? Feed {
timelineFeed = feed
masterTimelineViewController?.reinitializeArticles()
replaceArticles(with: savedSearchArticles!, animate: true)
} else {
timelineFeed = nil
setTimelineFeed(nil)
}
lastSearchString = ""
lastSearchScope = nil
savedSearchArticleIds = nil
savedSearchArticles = nil
isSearching = false
selectArticle(nil)
}
@ -639,7 +658,7 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
guard isSearching else { return }
if searchString.count < 3 {
timelineFeed = nil
setTimelineFeed(nil)
return
}
@ -647,9 +666,9 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider {
switch searchScope {
case .global:
timelineFeed = SmartFeed(delegate: SearchFeedDelegate(searchString: searchString))
setTimelineFeed(SmartFeed(delegate: SearchFeedDelegate(searchString: searchString)))
case .timeline:
timelineFeed = SmartFeed(delegate: SearchTimelineFeedDelegate(searchString: searchString, articleIDs: searchArticleIds!))
setTimelineFeed(SmartFeed(delegate: SearchTimelineFeedDelegate(searchString: searchString, articleIDs: savedSearchArticleIds!)))
}
lastSearchString = searchString
@ -968,10 +987,6 @@ extension SceneCoordinator: UINavigationControllerDelegate {
return
}
// Restore any bars hidden by the previous controller
showStatusBar()
navigationController.setNavigationBarHidden(false, animated: true)
navigationController.setToolbarHidden(false, animated: true)
// If we are showing the Feeds and only the feeds start clearing stuff
if viewController === masterFeedViewController && !isThreePanelMode && !isTimelineViewControllerPending {
@ -989,6 +1004,11 @@ extension SceneCoordinator: UINavigationControllerDelegate {
currentArticle = nil
masterTimelineViewController?.updateArticleSelection(animated: animated)
activityManager.invalidateReading()
// Restore any bars hidden by the article controller
showStatusBar()
navigationController.setNavigationBarHidden(false, animated: true)
navigationController.setToolbarHidden(false, animated: true)
return
}
@ -1092,7 +1112,23 @@ private extension SceneCoordinator {
return indexPathFor(node)
}
func updateShowIcons() {
func setTimelineFeed(_ feed: Feed?, completion: (() -> Void)? = nil) {
timelineFeed = feed
timelineMiddleIndexPath = nil
fetchAndReplaceArticlesAsync {
self.masterTimelineViewController?.reinitializeArticles()
completion?()
}
}
func updateShowNamesAndIcons() {
if timelineFeed is WebFeed {
showFeedNames = false
} else {
showFeedNames = true
}
if showFeedNames {
self.showIcons = true
@ -1351,14 +1387,15 @@ private extension SceneCoordinator {
func replaceArticles(with unsortedArticles: Set<Article>, animate: Bool) {
let sortedArticles = Array(unsortedArticles).sortedByDate(sortDirection, groupByFeed: groupByFeed)
replaceArticles(with: sortedArticles, animate: animate)
}
func replaceArticles(with sortedArticles: ArticleArray, animate: Bool) {
if articles != sortedArticles {
articles = sortedArticles
updateShowIcons()
updateShowNamesAndIcons()
updateUnreadCount()
masterTimelineViewController?.reloadArticles(animate: animate)
masterTimelineViewController?.reloadArticles(animated: animate)
}
}
@ -1395,46 +1432,21 @@ private extension SceneCoordinator {
fetchRequestQueue.cancelAllRequests()
}
func fetchAndReplaceArticlesSync() {
// To be called when the user has made a change of selection in the sidebar.
// It blocks the main thread, so that theres no async delay,
// so that the entire display refreshes at once.
// Its a better user experience this way.
cancelPendingAsyncFetches()
guard let timelineFetcher = timelineFeed else {
emptyTheTimeline()
return
}
let fetchedArticles = fetchUnsortedArticlesSync(for: [timelineFetcher])
replaceArticles(with: fetchedArticles, animate: false)
}
func fetchAndReplaceArticlesAsync(completion: @escaping () -> Void) {
// To be called when we need to do an entire fetch, but an async delay is okay.
// Example: we have the Today feed selected, and the calendar day just changed.
cancelPendingAsyncFetches()
guard let timelineFetcher = timelineFeed else {
emptyTheTimeline()
completion()
return
}
fetchUnsortedArticlesAsync(for: [timelineFetcher]) { [weak self] (articles) in
self?.replaceArticles(with: articles, animate: true)
completion()
}
}
func fetchUnsortedArticlesSync(for representedObjects: [Any]) -> Set<Article> {
cancelPendingAsyncFetches()
let articleFetchers = representedObjects.compactMap{ $0 as? ArticleFetcher }
if articleFetchers.isEmpty {
return Set<Article>()
}
var fetchedArticles = Set<Article>()
for articleFetcher in articleFetchers {
fetchedArticles.formUnion(articleFetcher.fetchArticles())
}
return fetchedArticles
}
func fetchUnsortedArticlesAsync(for representedObjects: [Any], callback: @escaping ArticleSetBlock) {
@ -1442,13 +1454,16 @@ private extension SceneCoordinator {
// if its been superseded by a newer fetch, or the timeline was emptied, etc., it wont get called.
precondition(Thread.isMainThread)
cancelPendingAsyncFetches()
let fetchOperation = FetchRequestOperation(id: fetchSerialNumber, representedObjects: representedObjects) { [weak self] (articles, operation) in
precondition(Thread.isMainThread)
guard !operation.isCanceled, let strongSelf = self, operation.id == strongSelf.fetchSerialNumber else {
callback(Set<Article>())
return
}
callback(articles)
}
fetchRequestQueue.add(fetchOperation)
}
@ -1499,7 +1514,7 @@ private extension SceneCoordinator {
masterTimelineViewController!.coordinator = self
navControllerForTimeline().pushViewController(masterTimelineViewController!, animated: animated)
masterTimelineViewController?.reloadArticles(animate: false)
masterTimelineViewController?.reloadArticles(animated: false)
}
}
@ -1676,7 +1691,7 @@ private extension SceneCoordinator {
return
}
if restoreFeed(userInfo, accountID: accountID, articleID: articleID) {
if restoreFeed(userInfo, accountID: accountID, webFeedID: webFeedID, articleID: articleID) {
return
}
@ -1689,7 +1704,7 @@ private extension SceneCoordinator {
}
}
func restoreFeed(_ userInfo: [AnyHashable : Any], accountID: String, articleID: String) -> Bool {
func restoreFeed(_ userInfo: [AnyHashable : Any], accountID: String, webFeedID: String, articleID: String) -> Bool {
guard let feedIdentifierUserInfo = userInfo[UserInfoKey.feedIdentifier] as? [AnyHashable : Any],
let articleFetcherType = FeedIdentifier(userInfo: feedIdentifierUserInfo) else {
return false
@ -1701,8 +1716,9 @@ private extension SceneCoordinator {
guard let smartFeed = SmartFeedsController.shared.find(by: identifier) else { return false }
if smartFeed.fetchArticles().contains(accountID: accountID, articleID: articleID) {
if let indexPath = indexPathFor(smartFeed) {
selectFeed(indexPath, animated: false)
selectArticleInCurrentFeed(articleID)
selectFeed(indexPath, animated: false) {
self.selectArticleInCurrentFeed(articleID)
}
return true
}
}
@ -1717,15 +1733,14 @@ private extension SceneCoordinator {
return false
}
if folderFeed.fetchArticles().contains(accountID: accountID, articleID: articleID) {
if let indexPath = indexPathFor(folderNode) {
selectFeed(indexPath, animated: false)
selectArticleInCurrentFeed(articleID)
return true
}
return selectFeedAndArticle(feedNode: folderNode, articleID: articleID)
}
case .webFeed:
return false
guard let accountNode = findAccountNode(accountID: accountID), let webFeedNode = findWebFeedNode(webFeedID: webFeedID, beginningAt: accountNode) else {
return false
}
return selectFeedAndArticle(feedNode: webFeedNode, articleID: articleID)
}
@ -1758,6 +1773,16 @@ private extension SceneCoordinator {
return nil
}
func selectFeedAndArticle(feedNode: Node, articleID: String) -> Bool {
if let feedIndexPath = indexPathFor(feedNode) {
selectFeed(feedIndexPath, animated: false) {
self.selectArticleInCurrentFeed(articleID)
}
return true
}
return false
}
func selectArticleInCurrentFeed(_ articleID: String) {
if let article = self.articles.first(where: { $0.articleID == articleID }) {
self.selectArticle(article)