From 91cc902b1d04061740cfd415e4775594f8834d3a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 13 Jun 2018 11:37:01 -0400 Subject: [PATCH 1/3] Update search results. --- .../ConversationSearchViewController.swift | 40 +++++++++++++++++-- .../HomeView/HomeViewController.m | 2 +- .../MediaGalleryViewController.swift | 2 +- 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift index f0ba2e854..7bd36c4e9 100644 --- a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift +++ b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift @@ -7,6 +7,15 @@ import Foundation @objc class ConversationSearchViewController: UITableViewController { + @objc + public var searchText = "" { + didSet { + SwiftAssertIsOnMainThread(#function) + + updateSearchResults(searchText: searchText) + } + } + var searchResultSet: SearchResultSet = SearchResultSet.empty var uiDatabaseConnection: YapDatabaseConnection { @@ -31,7 +40,7 @@ class ConversationSearchViewController: UITableViewController { var blockedPhoneNumberSet = Set() - // MARK: View Lifecyle + // MARK: View Lifecycle override func viewDidLoad() { super.viewDidLoad() @@ -45,6 +54,32 @@ class ConversationSearchViewController: UITableViewController { tableView.register(EmptySearchResultCell.self, forCellReuseIdentifier: EmptySearchResultCell.reuseIdentifier) tableView.register(HomeViewCell.self, forCellReuseIdentifier: HomeViewCell.cellReuseIdentifier()) tableView.register(ContactTableViewCell.self, forCellReuseIdentifier: ContactTableViewCell.reuseIdentifier()) + + NotificationCenter.default.addObserver(self, + selector: #selector(yapDatabaseModified), + name: NSNotification.Name.YapDatabaseModified, + object: OWSPrimaryStorage.shared().dbNotificationObject) + } + + var refreshTimer: Timer? + + @objc internal func yapDatabaseModified(notification: NSNotification) { + SwiftAssertIsOnMainThread(#function) + + if refreshTimer != nil { + // Don't start a new refresh timer if there's already one active. + return + } + + refreshTimer?.invalidate() + refreshTimer = WeakTimer.scheduledTimer(timeInterval: 1, target: self, userInfo: nil, repeats: false) { [weak self] _ in + guard let strongSelf = self else { + return + } + + strongSelf.updateSearchResults(searchText: strongSelf.searchText) + strongSelf.refreshTimer = nil + } } // MARK: UITableViewDelegate @@ -235,8 +270,7 @@ class ConversationSearchViewController: UITableViewController { // MARK: UISearchBarDelegate - @objc - public func updateSearchResults(searchText: String) { + private func updateSearchResults(searchText: String) { guard searchText.stripped.count > 0 else { self.searchResultSet = SearchResultSet.empty self.tableView.reloadData() diff --git a/Signal/src/ViewControllers/HomeView/HomeViewController.m b/Signal/src/ViewControllers/HomeView/HomeViewController.m index 900a696f5..4a2cf83dd 100644 --- a/Signal/src/ViewControllers/HomeView/HomeViewController.m +++ b/Signal/src/ViewControllers/HomeView/HomeViewController.m @@ -910,7 +910,7 @@ NSString *const kArchivedConversationsReuseIdentifier = @"kArchivedConversations OWSAssertIsOnMainThread(); NSString *searchText = self.searchBar.text.ows_stripped; - [self.searchResultsController updateSearchResultsWithSearchText:searchText]; + self.searchResultsController.searchText = searchText; BOOL isSearching = searchText.length > 0; self.searchResultsController.view.hidden = !isSearching; } diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index cbc84f1e2..d450bfb35 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -239,7 +239,7 @@ class MediaGalleryViewController: OWSNavigationController, MediaGalleryDataSourc return super.resignFirstResponder() } - // MARK: View Lifecyle + // MARK: View Lifecycle override func viewDidLoad() { super.viewDidLoad() From 3c50269dbf57f3c9c3b235886efda55571238809 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 13 Jun 2018 11:46:23 -0400 Subject: [PATCH 2/3] Debounce search result updates. --- .../ConversationSearchViewController.swift | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift index 7bd36c4e9..f3874eca8 100644 --- a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift +++ b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift @@ -12,14 +12,14 @@ class ConversationSearchViewController: UITableViewController { didSet { SwiftAssertIsOnMainThread(#function) - updateSearchResults(searchText: searchText) + // Use a slight delay to debounce updates. + refreshSearchResults(delay: 0.25) } } var searchResultSet: SearchResultSet = SearchResultSet.empty var uiDatabaseConnection: YapDatabaseConnection { - // TODO do we want to respond to YapDBModified? Might be hard when there's lots of search results, for only marginal value return OWSPrimaryStorage.shared().uiDatabaseConnection } @@ -61,25 +61,10 @@ class ConversationSearchViewController: UITableViewController { object: OWSPrimaryStorage.shared().dbNotificationObject) } - var refreshTimer: Timer? - @objc internal func yapDatabaseModified(notification: NSNotification) { SwiftAssertIsOnMainThread(#function) - if refreshTimer != nil { - // Don't start a new refresh timer if there's already one active. - return - } - - refreshTimer?.invalidate() - refreshTimer = WeakTimer.scheduledTimer(timeInterval: 1, target: self, userInfo: nil, repeats: false) { [weak self] _ in - guard let strongSelf = self else { - return - } - - strongSelf.updateSearchResults(searchText: strongSelf.searchText) - strongSelf.refreshTimer = nil - } + refreshSearchResults(delay: 1.0) } // MARK: UITableViewDelegate @@ -268,7 +253,28 @@ class ConversationSearchViewController: UITableViewController { } } - // MARK: UISearchBarDelegate + // MARK: Update Search Results + + var refreshTimer: Timer? + + private func refreshSearchResults(delay: TimeInterval) { + SwiftAssertIsOnMainThread(#function) + + if refreshTimer != nil { + // Don't start a new refresh timer if there's already one active. + return + } + + refreshTimer?.invalidate() + refreshTimer = WeakTimer.scheduledTimer(timeInterval: delay, target: self, userInfo: nil, repeats: false) { [weak self] _ in + guard let strongSelf = self else { + return + } + + strongSelf.updateSearchResults(searchText: strongSelf.searchText) + strongSelf.refreshTimer = nil + } + } private func updateSearchResults(searchText: String) { guard searchText.stripped.count > 0 else { @@ -277,9 +283,6 @@ class ConversationSearchViewController: UITableViewController { return } - // TODO: async? - // TODO: debounce? - self.uiDatabaseConnection.read { transaction in self.searchResultSet = self.searcher.results(searchText: searchText, transaction: transaction, contactsManager: self.contactsManager) } From 2db4c96a1c19b48a9925fcb75b28cb878bb636b4 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 13 Jun 2018 13:02:20 -0400 Subject: [PATCH 3/3] Respond to CR. --- .../ConversationSearchViewController.swift | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift index f3874eca8..2039b8372 100644 --- a/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift +++ b/Signal/src/ViewControllers/HomeView/ConversationSearchViewController.swift @@ -13,7 +13,7 @@ class ConversationSearchViewController: UITableViewController { SwiftAssertIsOnMainThread(#function) // Use a slight delay to debounce updates. - refreshSearchResults(delay: 0.25) + refreshSearchResults() } } @@ -64,7 +64,7 @@ class ConversationSearchViewController: UITableViewController { @objc internal func yapDatabaseModified(notification: NSNotification) { SwiftAssertIsOnMainThread(#function) - refreshSearchResults(delay: 1.0) + refreshSearchResults() } // MARK: UITableViewDelegate @@ -257,16 +257,26 @@ class ConversationSearchViewController: UITableViewController { var refreshTimer: Timer? - private func refreshSearchResults(delay: TimeInterval) { + private func refreshSearchResults() { SwiftAssertIsOnMainThread(#function) + guard !searchResultSet.isEmpty else { + // To avoid incorrectly showing the "no results" state, + // always search immediately if the current result set is empty. + refreshTimer?.invalidate() + refreshTimer = nil + + updateSearchResults(searchText: searchText) + return + } + if refreshTimer != nil { // Don't start a new refresh timer if there's already one active. return } refreshTimer?.invalidate() - refreshTimer = WeakTimer.scheduledTimer(timeInterval: delay, target: self, userInfo: nil, repeats: false) { [weak self] _ in + refreshTimer = WeakTimer.scheduledTimer(timeInterval: 0.1, target: self, userInfo: nil, repeats: false) { [weak self] _ in guard let strongSelf = self else { return }