From 001936e1b607bd2a4cfa1fb01418b3d08b50a375 Mon Sep 17 00:00:00 2001 From: Morgan Pretty Date: Tue, 9 Jan 2024 12:50:37 +1100 Subject: [PATCH] Updated the in-conversation search to prioritise currently visible results --- .../Conversations/ConversationSearch.swift | 13 +++- Session/Conversations/ConversationVC.swift | 72 ++++++++++--------- .../Conversations/ConversationViewModel.swift | 9 +++ 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/Session/Conversations/ConversationSearch.swift b/Session/Conversations/ConversationSearch.swift index 22c6539b4..14b7522ed 100644 --- a/Session/Conversations/ConversationSearch.swift +++ b/Session/Conversations/ConversationSearch.swift @@ -78,7 +78,7 @@ extension ConversationSearchController: UISearchResultsUpdating { let searchText: String = searchController.searchBar.text?.stripped, searchText.count >= ConversationSearchController.minimumSearchTextLength else { - self.resultsBar.updateResults(results: nil) + self.resultsBar.updateResults(results: nil, visibleItemIds: nil) self.delegate?.conversationSearchController(self, didUpdateSearchResults: nil, searchText: nil) return } @@ -105,7 +105,7 @@ extension ConversationSearchController: UISearchResultsUpdating { guard let strongSelf = self else { return } self?.resultsBar.stopLoading() - self?.resultsBar.updateResults(results: results) + self?.resultsBar.updateResults(results: results, visibleItemIds: self?.delegate?.currentVisibleIds()) self?.delegate?.conversationSearchController(strongSelf, didUpdateSearchResults: results, searchText: searchText) } } @@ -290,13 +290,19 @@ public final class SearchResultsBar: UIView { self.readConnection.mutate { $0 = readConnection } } - func updateResults(results: [Interaction.TimestampInfo]?) { + func updateResults(results: [Interaction.TimestampInfo]?, visibleItemIds: [Int64]?) { // We want to ignore search results that don't match the current searchId (this // will happen when searching large threads with short terms as the shorter terms // will take much longer to resolve than the longer terms) currentIndex = { guard let results: [Interaction.TimestampInfo] = results, !results.isEmpty else { return nil } + // Check if there is a visible item which matches the results and if so use that index (use + // the `lastIndex` as we want to select the message closest to the top of the screen) + if let visibleItemIds: [Int64] = visibleItemIds, let targetIndex: Int = results.lastIndex(where: { visibleItemIds.contains($0.id) }) { + return targetIndex + } + if let currentIndex: Int = currentIndex { return max(0, min(currentIndex, results.count - 1)) } @@ -366,6 +372,7 @@ public final class SearchResultsBar: UIView { // MARK: - ConversationSearchControllerDelegate public protocol ConversationSearchControllerDelegate: UISearchControllerDelegate { + func currentVisibleIds() -> [Int64] func conversationSearchController(_ conversationSearchController: ConversationSearchController, didUpdateSearchResults results: [Interaction.TimestampInfo]?, searchText: String?) func conversationSearchController(_ conversationSearchController: ConversationSearchController, didSelectInteractionInfo: Interaction.TimestampInfo) } diff --git a/Session/Conversations/ConversationVC.swift b/Session/Conversations/ConversationVC.swift index 7dbcc31c7..af0b20a24 100644 --- a/Session/Conversations/ConversationVC.swift +++ b/Session/Conversations/ConversationVC.swift @@ -1812,6 +1812,8 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers hideSearchUI() } + func currentVisibleIds() -> [Int64] { return (fullyVisibleCellViewModels() ?? []).map { $0.id } } + func conversationSearchController(_ conversationSearchController: ConversationSearchController, didUpdateSearchResults results: [Interaction.TimestampInfo]?, searchText: String?) { viewModel.lastSearchedText = searchText tableView.reloadRows(at: tableView.indexPathsForVisibleRows ?? [], with: UITableView.RowAnimation.none) @@ -1939,45 +1941,51 @@ final class ConversationVC: BaseVC, SessionUtilRespondingViewController, Convers self.tableView.scrollToRow(at: targetIndexPath, at: targetPosition, animated: true) } - func markFullyVisibleAndOlderCellsAsRead(interactionInfo: Interaction.TimestampInfo?) { - // We want to mark messages as read on load and while we scroll, so grab the newest message and mark - // everything older as read - // - // Note: For the 'tableVisualBottom' we remove the 'Values.mediumSpacing' as that is the distance - // the table content appears above the input view + func fullyVisibleCellViewModels() -> [MessageViewModel]? { + // We remove the 'Values.mediumSpacing' as that is the distance the table content appears above the input view + let tableVisualTop: CGFloat = tableView.frame.minY//(tableView.frame.minY - (tableView.contentInset.bottom - Values.mediumSpacing)) let tableVisualBottom: CGFloat = (tableView.frame.maxY - (tableView.contentInset.bottom - Values.mediumSpacing)) guard let visibleIndexPaths: [IndexPath] = self.tableView.indexPathsForVisibleRows, let messagesSection: Int = visibleIndexPaths .first(where: { self.viewModel.interactionData[$0.section].model == .messages })? - .section, - let newestCellViewModel: MessageViewModel = visibleIndexPaths - .sorted() - .filter({ $0.section == messagesSection }) - .compactMap({ indexPath -> (frame: CGRect, cellViewModel: MessageViewModel)? in - guard let cell: UITableViewCell = tableView.cellForRow(at: indexPath) else { return nil } - - switch cell { - case is VisibleMessageCell, is CallMessageCell, is InfoMessageCell: - return ( - view.convert(cell.frame, from: tableView), - self.viewModel.interactionData[indexPath.section].elements[indexPath.row] - ) - - case is TypingIndicatorCell, is DateHeaderCell, is UnreadMarkerCell: - return nil + .section + else { return nil } + + return visibleIndexPaths + .sorted() + .filter({ $0.section == messagesSection }) + .compactMap({ indexPath -> (frame: CGRect, cellViewModel: MessageViewModel)? in + guard let cell: UITableViewCell = tableView.cellForRow(at: indexPath) else { return nil } + + switch cell { + case is VisibleMessageCell, is CallMessageCell, is InfoMessageCell: + return ( + view.convert(cell.frame, from: tableView), + self.viewModel.interactionData[indexPath.section].elements[indexPath.row] + ) - default: - SNLog("[ConversationVC] Warning: Processing unhandled cell type when marking as read, this could result in intermittent failures") - return nil - } - }) - // Exclude messages that are partially off the bottom of the screen - .filter({ $0.frame.maxY <= tableVisualBottom }) - .last? - .cellViewModel - else { + case is TypingIndicatorCell, is DateHeaderCell, is UnreadMarkerCell: + return nil + + default: + SNLog("[ConversationVC] Warning: Processing unhandled cell type when marking as read, this could result in intermittent failures") + return nil + } + }) + // Exclude messages that are partially off the the screen + .filter({ $0.frame.minY >= tableVisualTop && $0.frame.maxY <= tableVisualBottom }) + .map { $0.cellViewModel } + } + + func markFullyVisibleAndOlderCellsAsRead(interactionInfo: Interaction.TimestampInfo?) { + // Only retrieve the `fullyVisibleCellViewModels` if the viewModel things we should mark something as read + guard self.viewModel.shouldTryMarkAsRead() else { return } + + // We want to mark messages as read on load and while we scroll, so grab the newest message and mark + // everything older as read + guard let newestCellViewModel: MessageViewModel = fullyVisibleCellViewModels()?.last else { // If we weren't able to get any visible cells for some reason then we should fall back to // marking the provided interactionInfo as read just in case if let interactionInfo: Interaction.TimestampInfo = interactionInfo { diff --git a/Session/Conversations/ConversationViewModel.swift b/Session/Conversations/ConversationViewModel.swift index 177a4e6d4..ad84f5090 100644 --- a/Session/Conversations/ConversationViewModel.swift +++ b/Session/Conversations/ConversationViewModel.swift @@ -725,6 +725,15 @@ public class ConversationViewModel: OWSAudioPlayerDelegate { } } + /// This method indicates whether the client should try to mark the thread or it's messages as read (it's an optimisation for fully read + /// conversations so we can avoid iterating through the visible conversation cells every scroll) + public func shouldTryMarkAsRead() -> Bool { + return ( + (threadData.threadUnreadCount ?? 0) > 0 || + threadData.threadWasMarkedUnread == true + ) + } + /// This method marks a thread as read and depending on the target may also update the interactions within a thread as read public func markAsRead( target: SessionThreadViewModel.ReadTarget,