Skip to content

Commit 5457de6

Browse files
cyndichincyndichin
committed
Add FXIOS-13648 [Trending Searches] initial recent searches UI
1 parent 3590cdf commit 5457de6

File tree

8 files changed

+144
-31
lines changed

8 files changed

+144
-31
lines changed

‎firefox-ios/Client/Frontend/Browser/BrowserViewController/Views/BrowserViewController.swift‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,8 +353,9 @@ class BrowserViewController: UIViewController,
353353
return featureFlags.isFeatureEnabled(.tabScrollRefactorFeature, checking: .buildOnly)
354354
}
355355

356-
private var isPreSearchEnabled: Bool {
356+
private var isRecentOrTrendingSearchEnabled: Bool {
357357
let isTrendingSearchEnabled = featureFlags.isFeatureEnabled(.trendingSearches, checking: .buildOnly)
358+
let isRecentSearchEnabled = featureFlags.isFeatureEnabled(.recentSearches, checking: .buildOnly)
358359
return isTrendingSearchEnabled || isRecentSearchEnabled
359360
}
360361

@@ -3804,9 +3805,11 @@ class BrowserViewController: UIViewController,
38043805

38053806
/// Zero search describes the state in which the user highlights the address bar, but no
38063807
/// text has been entered.
3807-
/// We only want to display if webview, user has either trending searches or recent searches enabled.
3808+
/// We only want to display if webview, user has either trending searches or recent searches enabled
3809+
/// and is not private mode.
38083810
private func showZeroSearchView() {
3809-
guard contentContainer.hasWebView, isPreSearchEnabled else {
3811+
let isPrivateTab = tabManager.selectedTab?.isPrivate ?? false
3812+
guard contentContainer.hasWebView, isRecentOrTrendingSearchEnabled, !isPrivateTab else {
38103813
hideSearchController()
38113814
return
38123815
}
@@ -3856,9 +3859,11 @@ class BrowserViewController: UIViewController,
38563859
toast.removeFromSuperview()
38573860
}
38583861
// Instead of showing homepage when user enters overlay mode,
3859-
// we want to show the zero search state if trending searches or recent searches is enabled
3860-
if !isPreSearchEnabled {
3861-
showEmbeddedHomepage(inline: false, isPrivate: tabManager.selectedTab?.isPrivate ?? false)
3862+
// we want to show the zero search state if trending searches or recent searches is enabled or if in private mode
3863+
// we handle that in `showZeroSearchView()` and avoid embedding homepage here.
3864+
let isPrivateMode = tabManager.selectedTab?.isPrivate ?? false
3865+
if !isRecentOrTrendingSearchEnabled || isPrivateMode {
3866+
showEmbeddedHomepage(inline: false, isPrivate: isPrivateMode)
38623867
}
38633868
}
38643869

‎firefox-ios/Client/Frontend/Browser/Search/SearchListSection.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@
55
import Foundation
66

77
enum SearchListSection: Int, CaseIterable {
8+
// Pre search state
9+
case recentSearches
810
case trendingSearches
11+
12+
// User typed search term state
913
case searchSuggestions
1014
case firefoxSuggestions
1115
case openedTabs

‎firefox-ios/Client/Frontend/Browser/Search/SearchViewController.swift‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,14 @@ class SearchViewController: SiteTableViewController,
459459
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
460460
searchTelemetry?.engagementType = .tap
461461
switch SearchListSection(rawValue: indexPath.section)! {
462+
case .recentSearches:
463+
guard let defaultEngine = viewModel.searchEnginesManager?.defaultEngine else { return }
464+
465+
guard let recentSearch = viewModel.recentSearches[safe: indexPath.row],
466+
let url = defaultEngine.searchURLForQuery(recentSearch)
467+
else { return }
468+
searchDelegate?.searchViewController(self, didSelectURL: url, searchTerm: recentSearch)
469+
462470
case .trendingSearches:
463471
guard let defaultEngine = viewModel.searchEnginesManager?.defaultEngine else { return }
464472

@@ -547,13 +555,20 @@ class SearchViewController: SiteTableViewController,
547555

548556
var title: String
549557
switch section {
558+
case SearchListSection.recentSearches.rawValue:
559+
guard !viewModel.recentSearches.isEmpty else { return nil }
560+
// TODO: FXIOS-13644 - Add proper strings when finalized
561+
title = "Recent Searches"
562+
550563
case SearchListSection.trendingSearches.rawValue:
564+
guard !viewModel.trendingSearches.isEmpty else { return nil }
551565
// TODO: FXIOS-13644 - Add proper strings when finalized
552566
if let engineName = viewModel.searchEnginesManager?.defaultEngine?.shortName {
553567
title = "Trending on \(engineName)"
554568
} else {
555569
title = ""
556570
}
571+
557572
case SearchListSection.firefoxSuggestions.rawValue:
558573
title = .Search.SuggestSectionTitle
559574
case SearchListSection.searchSuggestions.rawValue:
@@ -597,6 +612,8 @@ class SearchViewController: SiteTableViewController,
597612
override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
598613
if let section = SearchListSection(rawValue: indexPath.section) {
599614
switch section {
615+
case .recentSearches, .trendingSearches:
616+
break
600617
case .searchSuggestions:
601618
if let site = viewModel.suggestions?[indexPath.row] {
602619
if searchTelemetry?.visibleSuggestions.contains(site) == false {
@@ -639,29 +656,29 @@ class SearchViewController: SiteTableViewController,
639656
searchTelemetry?.visibleFirefoxSuggestions.append(firefoxSuggestion)
640657
}
641658
}
642-
case .trendingSearches:
643-
break
644659
}
645660
}
646661
}
647662

648663
override func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
649664
switch SearchListSection(rawValue: section)! {
665+
case .recentSearches:
666+
return viewModel.shouldShowRecentSearches ? viewModel.recentSearches.count : 0
667+
case .trendingSearches:
668+
return viewModel.shouldShowTrendingSearches ? viewModel.trendingSearches.count : 0
650669
case .searchSuggestions:
651670
guard let count = viewModel.suggestions?.count else { return 0 }
652671
return count < 4 ? count : 4
653672
case .openedTabs:
654-
return !viewModel.searchQuery.isEmpty ? viewModel.filteredOpenedTabs.count : 0
673+
return !viewModel.isZeroSearchState ? viewModel.filteredOpenedTabs.count : 0
655674
case .remoteTabs:
656675
return viewModel.shouldShowSyncedTabsSuggestions ? viewModel.filteredRemoteClientTabs.count : 0
657676
case .history:
658677
return viewModel.shouldShowBrowsingHistorySuggestions ? viewModel.historySites.count : 0
659678
case .bookmarks:
660679
return viewModel.shouldShowBookmarksSuggestions ? viewModel.bookmarkSites.count : 0
661680
case .firefoxSuggestions:
662-
return viewModel.firefoxSuggestions.count
663-
case .trendingSearches:
664-
return viewModel.shouldShowTrendingSearches ? viewModel.trendingSearches.count : 0
681+
return !viewModel.isZeroSearchState ? viewModel.firefoxSuggestions.count : 0
665682
}
666683
}
667684

@@ -724,6 +741,16 @@ class SearchViewController: SiteTableViewController,
724741
_ indexPath: IndexPath) -> UITableViewCell {
725742
var cell = UITableViewCell()
726743
switch section {
744+
case .recentSearches:
745+
if let recentSearch = viewModel.recentSearches[safe: indexPath.row] {
746+
let oneLineCellViewModel = oneLineCellModelForSearch(
747+
with: recentSearch,
748+
and: StandardImageIdentifiers.Large.history
749+
)
750+
oneLineCell.configure(viewModel: oneLineCellViewModel)
751+
cell = oneLineCell
752+
}
753+
727754
case .trendingSearches:
728755
if let trendingSearch = viewModel.trendingSearches[safe: indexPath.row] {
729756
let arrowImageName = StandardImageIdentifiers.Large.arrowTrending

‎firefox-ios/Client/Frontend/Browser/Search/SearchViewModel.swift‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,10 +113,11 @@ class SearchViewModel: FeatureFlaggable, LoaderListener {
113113
@MainActor
114114
var shouldShowSearchEngineSuggestions: Bool {
115115
let shouldShowSuggestions = searchEnginesManager?.shouldShowSearchSuggestions ?? false
116-
return shouldShowSuggestions && !searchQuery.isEmpty
116+
return shouldShowSuggestions && !isZeroSearchState
117117
}
118118

119119
var shouldShowSyncedTabsSuggestions: Bool {
120+
guard !isZeroSearchState else { return false }
120121
return shouldShowFirefoxSuggestions(
121122
model.shouldShowSyncedTabsSuggestions
122123
)
@@ -157,7 +158,7 @@ class SearchViewModel: FeatureFlaggable, LoaderListener {
157158
/// Does not show when search term in url is empty (aka zero search state).
158159
@MainActor
159160
var hasFirefoxSuggestions: Bool {
160-
guard !searchQuery.isEmpty else { return false }
161+
guard !isZeroSearchState else { return false }
161162
return hasBookmarksSuggestions
162163
|| hasHistorySuggestions
163164
|| hasHistoryAndBookmarksSuggestions
@@ -169,6 +170,7 @@ class SearchViewModel: FeatureFlaggable, LoaderListener {
169170

170171
// MARK: - Zero Search State Variables
171172
// Determines whether we should zero search state based on searchQuery being empty.
173+
// Zero search state is when the user has not entered a search term in the address bar.
172174
@MainActor
173175
var isZeroSearchState: Bool {
174176
return searchQuery.isEmpty
@@ -214,6 +216,9 @@ class SearchViewModel: FeatureFlaggable, LoaderListener {
214216
@MainActor
215217
func shouldShowHeader(for section: Int) -> Bool {
216218
switch section {
219+
case SearchListSection.recentSearches.rawValue:
220+
guard !recentSearches.isEmpty else { return false }
221+
return shouldShowRecentSearches
217222
case SearchListSection.trendingSearches.rawValue:
218223
guard !trendingSearches.isEmpty else { return false }
219224
return shouldShowTrendingSearches
@@ -465,8 +470,9 @@ class SearchViewModel: FeatureFlaggable, LoaderListener {
465470
}
466471

467472
/// Determines if a suggestion should be shown based on the view model's privacy mode and
468-
/// the specific suggestion's status.
473+
/// the specific suggestion's status. We do not show if in zero search state.
469474
private func shouldShowFirefoxSuggestions(_ suggestion: Bool) -> Bool {
475+
guard !isZeroSearchState else { return false }
470476
model.shouldShowPrivateModeFirefoxSuggestions = true
471477
return isPrivate ?
472478
(suggestion && model.shouldShowPrivateModeFirefoxSuggestions) :

‎firefox-ios/Client/Frontend/Browser/SearchEngines/RecentSearchProvider.swift‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ protocol RecentSearchProvider {
1313
}
1414

1515
/// A provider that manages recent search terms from a user's history storage.
16-
struct DefaultRecentSearchProvider: RecentSearchProvider {
16+
final class DefaultRecentSearchProvider: RecentSearchProvider {
1717
private let historyStorage: HistoryHandler
1818
private let logger: Logger
1919
private let nimbus: FxNimbus
@@ -58,11 +58,12 @@ struct DefaultRecentSearchProvider: RecentSearchProvider {
5858
/// We don't have an interface to fetch only a certain amount, so we follow what Android does for now.
5959
func loadRecentSearches(completion: @escaping ([String]) -> Void) {
6060
// TODO: FXIOS-13782 Use get_most_recent method to fetch history
61-
historyStorage.getHistoryMetadataSince(since: Int64.min) { result in
61+
historyStorage.getHistoryMetadataSince(since: Int64.min) { [weak self] result in
6262
if case .success(let historyMetadata) = result {
63-
let searches = historyMetadata.compactMap { $0.searchTerm }
64-
let recentSearches = Array(searches.prefix(maxNumberOfSuggestions))
65-
completion(recentSearches)
63+
let uniqueSearchTermResult = historyMetadata.compactMap { $0.searchTerm }
64+
.uniqued()
65+
.prefix(self?.maxNumberOfSuggestions ?? 5)
66+
completion(Array(uniqueSearchTermResult))
6667
} else {
6768
completion([])
6869
}

‎firefox-ios/Client/Frontend/Browser/Toolbars/AddressToolbarContainer.swift‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -501,9 +501,14 @@ final class AddressToolbarContainer: UIView,
501501
let locationText = shouldShowSuggestions ? searchTerm : nil
502502
enterOverlayMode(locationText, pasted: false, search: false)
503503

504-
// We want to show suggestions if we turn on the trending searches
505-
// which displays the zero search state.
506-
let isZeroSearchEnabled = featureFlags.isFeatureEnabled(.trendingSearches, checking: .buildOnly)
504+
// We want to show suggestions if we turn on the trending searches or recent searches
505+
// which displays the zero search state. Only if not in private mode.
506+
let isTrendingSearchEnabled = featureFlags.isFeatureEnabled(.trendingSearches, checking: .buildOnly)
507+
let isRecentSearchEnabled = featureFlags.isFeatureEnabled(.recentSearches, checking: .buildOnly)
508+
let isRecentOrTrendingSearchEnabled = isTrendingSearchEnabled || isRecentSearchEnabled
509+
let isPrivateMode = model?.isPrivateMode ?? false
510+
let isZeroSearchEnabled = isRecentOrTrendingSearchEnabled && !isPrivateMode
511+
507512
if shouldShowSuggestions || isZeroSearchEnabled {
508513
delegate?.openSuggestions(searchTerm: locationText ?? "")
509514
}

‎firefox-ios/firefox-ios-tests/Tests/ClientTests/Search/MockRecentSearchProvider.swift‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@ import Foundation
88
/// A mock for testing that methods are called.
99
final class MockRecentSearchProvider: RecentSearchProvider {
1010
private(set) var addRecentSearchCalledCount = 0
11-
private(set) var recentSearchesCalledCount = 0
12-
private(set) var loadRecentSearchesCalledCount = 0
11+
private let searchTerms: [String] = ["search term 1", "search term 2"]
1312

1413
func addRecentSearch(_ term: String, url: String?) {
1514
addRecentSearchCalledCount += 1
1615
}
1716

1817
func loadRecentSearches(completion: @escaping ([String]) -> Void) {
19-
loadRecentSearchesCalledCount += 1
18+
completion(searchTerms)
2019
}
2120
}

‎firefox-ios/firefox-ios-tests/Tests/ClientTests/Search/SearchViewModelTests.swift‎

Lines changed: 71 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ final class SearchViewModelTests: XCTestCase {
395395
setupNimbusTrendingSearchesTesting(isEnabled: true)
396396
let subject = createSubject()
397397
subject.searchQuery = "hello"
398-
let shouldShowHeader = subject.shouldShowHeader(for: 0)
398+
let trendingSearchesSectionIndex = 1
399+
let shouldShowHeader = subject.shouldShowHeader(for: trendingSearchesSectionIndex)
399400
XCTAssertFalse(shouldShowHeader)
400401
}
401402

@@ -406,7 +407,8 @@ final class SearchViewModelTests: XCTestCase {
406407
let subject = createSubject(mockTrendingClient: mockClient)
407408
await subject.retrieveTrendingSearches()
408409
subject.searchQuery = ""
409-
let shouldShowHeader = subject.shouldShowHeader(for: 0)
410+
let trendingSearchesSectionIndex = 1
411+
let shouldShowHeader = subject.shouldShowHeader(for: trendingSearchesSectionIndex)
410412
XCTAssertEqual(subject.trendingSearches, ["foo", "bar"])
411413
XCTAssertTrue(shouldShowHeader)
412414
}
@@ -416,7 +418,8 @@ final class SearchViewModelTests: XCTestCase {
416418
setupNimbusTrendingSearchesTesting(isEnabled: true)
417419
let subject = createSubject()
418420
subject.searchQuery = ""
419-
let shouldShowHeader = subject.shouldShowHeader(for: 0)
421+
let trendingSearchesSectionIndex = 1
422+
let shouldShowHeader = subject.shouldShowHeader(for: trendingSearchesSectionIndex)
420423
XCTAssertEqual(subject.trendingSearches, [])
421424
XCTAssertFalse(shouldShowHeader)
422425
}
@@ -425,7 +428,8 @@ final class SearchViewModelTests: XCTestCase {
425428
func test_shouldShowHeader_forTrendingSearches_withoutFeatureFlagOn_doesNotShowHeader() async {
426429
setupNimbusTrendingSearchesTesting(isEnabled: false)
427430
let subject = createSubject()
428-
let shouldShowHeader = subject.shouldShowHeader(for: 0)
431+
let trendingSearchesSectionIndex = 1
432+
let shouldShowHeader = subject.shouldShowHeader(for: trendingSearchesSectionIndex)
429433
XCTAssertEqual(subject.trendingSearches, [])
430434
XCTAssertFalse(shouldShowHeader)
431435
}
@@ -455,12 +459,74 @@ final class SearchViewModelTests: XCTestCase {
455459
XCTAssertEqual(subject.trendingSearches, [])
456460
}
457461

462+
// MARK: - Recent Searches
463+
@MainActor
464+
func test_shouldShowHeader_forRecentSearches_withFFOn_andSearchTerm_doesNotShowHeader() async {
465+
setupNimbusRecentSearchesTesting(isEnabled: true)
466+
let subject = createSubject()
467+
subject.searchQuery = "hello"
468+
let recentSearchesSectionIndex = 0
469+
let shouldShowHeader = subject.shouldShowHeader(for: recentSearchesSectionIndex)
470+
XCTAssertFalse(shouldShowHeader)
471+
}
472+
473+
@MainActor
474+
func test_shouldShowHeader_withRecentSearches_withFFOn_andSearchTermEmpty_showsHeader() async {
475+
setupNimbusRecentSearchesTesting(isEnabled: true)
476+
let mockRecentSearchProvider = MockRecentSearchProvider()
477+
let subject = createSubject(mockRecentSearchProvider: mockRecentSearchProvider)
478+
subject.retrieveRecentSearches()
479+
subject.searchQuery = ""
480+
let recentSearchesSectionIndex = 0
481+
let expectation = XCTestExpectation(description: "Recent Searches have been fetched")
482+
483+
let shouldShowHeader = subject.shouldShowHeader(for: recentSearchesSectionIndex)
484+
485+
mockRecentSearchProvider.loadRecentSearches { result in
486+
XCTAssertEqual(result, ["search term 1", "search term 2"])
487+
expectation.fulfill()
488+
}
489+
490+
await fulfillment(of: [expectation], timeout: 1.0)
491+
XCTAssertTrue(shouldShowHeader)
492+
}
493+
494+
@MainActor
495+
func test_shouldShowHeader_withNoRecentSearches_withFFOn_andSearchTermEmpty_doesNotShowHeader() async {
496+
setupNimbusRecentSearchesTesting(isEnabled: true)
497+
let subject = createSubject()
498+
subject.searchQuery = ""
499+
let recentSearchesSectionIndex = 0
500+
let shouldShowHeader = subject.shouldShowHeader(for: recentSearchesSectionIndex)
501+
XCTAssertEqual(subject.recentSearches, [])
502+
XCTAssertFalse(shouldShowHeader)
503+
}
504+
505+
@MainActor
506+
func test_shouldShowHeader_forRecentSearches_withoutFeatureFlagOn_doesNotShowHeader() async {
507+
setupNimbusRecentSearchesTesting(isEnabled: false)
508+
let subject = createSubject()
509+
let recentSearchesSectionIndex = 0
510+
let shouldShowHeader = subject.shouldShowHeader(for: recentSearchesSectionIndex)
511+
XCTAssertEqual(subject.recentSearches, [])
512+
XCTAssertFalse(shouldShowHeader)
513+
}
514+
458515
func test_retrieveRecentSearches_withSuccess_hasExpectedList() {
459516
setupNimbusRecentSearchesTesting(isEnabled: true)
460517
let mockRecentSearchProvider = MockRecentSearchProvider()
461518
let subject = createSubject(mockRecentSearchProvider: mockRecentSearchProvider)
519+
520+
let expectation = XCTestExpectation(description: "Recent Searches have been fetched")
521+
462522
subject.retrieveRecentSearches()
463-
XCTAssertEqual(mockRecentSearchProvider.loadRecentSearchesCalledCount, 1)
523+
524+
mockRecentSearchProvider.loadRecentSearches { result in
525+
XCTAssertEqual(result, ["search term 1", "search term 2"])
526+
expectation.fulfill()
527+
}
528+
529+
wait(for: [expectation], timeout: 1.0)
464530
}
465531

466532
func test_retrieveRecentSearches_withNilProvider_hasEmptyList() {

0 commit comments

Comments
 (0)