From 809a792aaefe378c5638ef85835dd7737eeb5abf Mon Sep 17 00:00:00 2001 From: Michelle Linington Date: Tue, 18 Oct 2022 12:14:49 -0700 Subject: [PATCH] Fix stale thread selection in chat list See [here](https://github.com/signalapp/Signal-iOS/pull/5432). --- .../ChatListViewController+Actions.swift | 2 -- .../ChatListViewController+Helpers.swift | 2 -- .../Chat List/ChatListViewController.h | 4 ++- .../Chat List/ChatListViewController.m | 8 ++++++ .../Chat List/ChatListViewController.swift | 26 +++++++++++++++++++ .../ConversationSplitViewController.swift | 2 +- 6 files changed, 38 insertions(+), 6 deletions(-) diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Actions.swift b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Actions.swift index e8053ee794..af293a13ac 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Actions.swift +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Actions.swift @@ -22,7 +22,6 @@ extension ChatListViewController { if let previousIndexPath = renderState.indexPath(beforeThread: currentThread), let thread = self.thread(forIndexPath: previousIndexPath) { self.present(thread, action: .compose, animated: true) - tableView.selectRow(at: previousIndexPath, animated: true, scrollPosition: .none) } } @@ -41,7 +40,6 @@ extension ChatListViewController { if let nextIndexPath = renderState.indexPath(afterThread: currentThread), let thread = self.thread(forIndexPath: nextIndexPath) { self.present(thread, action: .compose, animated: true) - tableView.selectRow(at: nextIndexPath, animated: true, scrollPosition: .none) } } } diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Helpers.swift b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Helpers.swift index cf3219fbe1..31d8882548 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Helpers.swift +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController+Helpers.swift @@ -146,8 +146,6 @@ public extension ChatListViewController { owsFailDebug("Missing threadViewModel.") return nil } - self.lastViewedThread = threadViewModel.threadRecord - let vc = ConversationViewController(threadViewModel: threadViewModel, action: .none, focusMessageId: nil) diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.h b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.h index fead90a6ca..5481e4e8d3 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.h +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.h @@ -29,7 +29,9 @@ NS_ASSUME_NONNULL_BEGIN - (void)unarchiveSelectedConversation; @property (nonatomic, readonly) CLVViewState *viewState; -@property (nonatomic) TSThread *lastViewedThread; + +/// Used to update the selected cell for split view and maintain scroll positions for reappearing collapsed views. +- (void)updateLastViewedThread:(TSThread *)thread animated:(BOOL)animated; // For use by Swift extension. - (void)updateBarButtonItems; diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.m b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.m index 829fee7207..d96c440a08 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.m +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.m @@ -53,6 +53,8 @@ NSString *const kArchiveButtonPseudoGroup = @"kArchiveButtonPseudoGroup"; @property (nonatomic) BOOL hasEverPresentedExperienceUpgrade; +@property (nonatomic, nullable) TSThread *lastViewedThread; + @end #pragma mark - @@ -696,6 +698,12 @@ NSString *const kArchiveButtonPseudoGroup = @"kArchiveButtonPseudoGroup"; [self.searchResultsController viewWillDisappear:animated]; } +- (void)updateLastViewedThread:(TSThread *)thread animated:(BOOL)animated +{ + self.lastViewedThread = thread; + [self ensureSelectedThread:thread animated:animated]; +} + #pragma mark - - (void)pullToRefreshPerformed:(UIRefreshControl *)refreshControl diff --git a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift index 26361e0eea..90dd830846 100644 --- a/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift +++ b/Signal/src/ViewControllers/HomeView/Chat List/ChatListViewController.swift @@ -497,6 +497,32 @@ public extension ChatListViewController { navigationController.setViewControllers(viewControllers, animated: false) presentFormSheet(navigationController, animated: true, completion: completion) } + + /// Verifies that the currently selected cell matches the provided thread. + /// If it does or if the user's in multi-select: Do nothing. + /// If it doesn't: Select the first cell matching the provided thread, if one exists. Otherwise, deselect the current row. + @objc + func ensureSelectedThread(_ targetThread: TSThread, animated: Bool) { + // Ignore any updates if we're in multiselect mode. I don't think this can happen, + // but if it does let's avoid stepping over the user's manual selection. + let currentSelection = tableView.indexPathsForSelectedRows ?? [] + guard viewState.multiSelectState.isActive == false, currentSelection.count < 2 else { + return + } + + let currentlySelectedThread = currentSelection.first.flatMap { + self.tableDataSource.thread(forIndexPath: $0, expectsSuccess: false) + } + + if currentlySelectedThread?.uniqueId != targetThread.uniqueId { + if let targetPath = tableDataSource.renderState.indexPath(forUniqueId: targetThread.uniqueId) { + tableView.selectRow(at: targetPath, animated: animated, scrollPosition: .none) + tableView.scrollToRow(at: targetPath, at: .none, animated: animated) + } else if let stalePath = currentSelection.first { + tableView.deselectRow(at: stalePath, animated: animated) + } + } + } } extension ChatListViewController: BadgeExpirationSheetDelegate { diff --git a/Signal/src/ViewControllers/HomeView/ConversationSplitViewController.swift b/Signal/src/ViewControllers/HomeView/ConversationSplitViewController.swift index 0c9b70e1de..5dc97c22b1 100644 --- a/Signal/src/ViewControllers/HomeView/ConversationSplitViewController.swift +++ b/Signal/src/ViewControllers/HomeView/ConversationSplitViewController.swift @@ -184,7 +184,7 @@ class ConversationSplitViewController: UISplitViewController, ConversationSplit // Update the last viewed thread on the conversation list so it // can maintain its scroll position when navigating back. - homeVC.chatListViewController.lastViewedThread = thread + homeVC.chatListViewController.updateLastViewedThread(thread, animated: animated) let threadViewModel = databaseStorage.read { return ThreadViewModel(thread: thread,