Skip to content

Commit

Permalink
Search was broken due to concurrent search requests
Browse files Browse the repository at this point in the history
This change ensures only one search request executes at a time.
Additionally, the change ensures proper search lifecycle handling.
  • Loading branch information
mohamede1945 committed Dec 18, 2023
1 parent 5d5e5f9 commit 4ec969e
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 130 deletions.
29 changes: 9 additions & 20 deletions Features/SearchFeature/SearchTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,19 @@
//

import Foundation
import QuranText

enum SearchUIState {
case entry
case autocomplete
case loading
case searchResults
case search(_ term: String)
}

enum SearchTerm {
case autocomplete(_ term: String)
case noAction(_ term: String)

// MARK: Internal

var term: String {
switch self {
case .autocomplete(let term), .noAction(let term): return term
}
}
enum SearchState {
case searching
case searchResult(_ results: [SearchResults])
}

var isAutocomplete: Bool {
switch self {
case .autocomplete: return true
case .noAction: return false
}
}
enum KeyboardState {
case open
case closed
}
117 changes: 66 additions & 51 deletions Features/SearchFeature/SearchView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ struct SearchView: View {
var body: some View {
SearchViewUI(
error: $viewModel.error,
state: viewModel.uiState,
term: viewModel.searchTerm.term,
uiState: viewModel.uiState,
searchState: viewModel.searchState,
term: viewModel.searchTerm,
recents: viewModel.recents,
populars: viewModel.populars,
autocompletions: viewModel.autocompletions,
searchResults: viewModel.searchResults,
start: { await viewModel.start() },
search: { viewModel.search(for: $0) },
selectSearchResult: { viewModel.select(searchResult: $0.result, source: $0.source) }
Expand All @@ -34,60 +34,40 @@ struct SearchView: View {
private struct SearchViewUI: View {
@Binding var error: Error?

let state: SearchUIState
let uiState: SearchUIState
let searchState: SearchState

let term: String
let recents: [String]
let populars: [String]
let autocompletions: [String]
let searchResults: [SearchResults]

let start: AsyncAction
let search: AsyncItemAction<String>
let selectSearchResult: ItemAction<(result: SearchResult, source: SearchResults.Source)>

var body: some View {
Group {
switch state {
switch uiState {
case .entry:
entry
case .autocomplete:
autocompletionsView
case .loading:
NoorList {
LoadingView()
if autocompletions.isEmpty {
entry
} else {
autocompletionsView
}
case .searchResults:
searchResultsView
}
}
.task(start)
.errorAlert(error: $error)
}

@ViewBuilder
var searchResultsView: some View {
if !searchResults.isEmpty {
NoorList {
ForEach(searchResults) { result in
let plainTitle = title(of: result)
let title = lFormat("search.result.count", plainTitle, result.items.count)
NoorSection(title: title, result.items) { item in
let localizedVerse = item.ayah.localizedName
let arabicSuraName = item.ayah.sura.arabicSuraName
NoorListItem(
subheading: "\(localizedVerse) \(sura: arabicSuraName)",
title: searchResultText(of: item),
accessory: .text(NumberFormatter.shared.format(item.ayah.page.pageNumber))
) {
selectSearchResult((item, result.source))
}
case .search:
switch searchState {
case .searching:
NoorList {
LoadingView()
}
case .searchResult(let results):
searchResultsView(searchResults: results)
}
}
} else {
noResultsData
}
.task(start)
.errorAlert(error: $error)
}

var noResultsData: some View {
Expand Down Expand Up @@ -135,6 +115,31 @@ private struct SearchViewUI: View {
}
}

@ViewBuilder
func searchResultsView(searchResults: [SearchResults]) -> some View {
if !searchResults.isEmpty {
NoorList {
ForEach(searchResults) { result in
let plainTitle = title(of: result)
let title = lFormat("search.result.count", plainTitle, result.items.count)
NoorSection(title: title, result.items) { item in
let localizedVerse = item.ayah.localizedName
let arabicSuraName = item.ayah.sura.arabicSuraName
NoorListItem(
subheading: "\(localizedVerse) \(sura: arabicSuraName)",
title: searchResultText(of: item),
accessory: .text(NumberFormatter.shared.format(item.ayah.page.pageNumber))
) {
selectSearchResult((item, result.source))
}
}
}
}
} else {
noResultsData
}
}

func title(of searchResults: SearchResults) -> String {
switch searchResults.source {
case .translation(let translation):
Expand Down Expand Up @@ -185,20 +190,21 @@ struct SearchView_Previews: PreviewProvider {
),
])

@State var results = [populatedResults]
@State var state = SearchUIState.searchResults
@State var uiState = SearchUIState.search("abc")
@State var searchState = SearchState.searching
@State var error: Error?
@State var autocompletions: [String] = []

var body: some View {
NavigationView {
SearchViewUI(
error: $error,
state: state,
uiState: uiState,
searchState: searchState,
term: "is",
recents: ["Recent 1", "Recent 2"],
populars: ["Popular 1", "Popular 2"],
autocompletions: [Self.englishText, Self.arabicText],
searchResults: results,
autocompletions: autocompletions,
start: { },
search: { _ in },
selectSearchResult: { _ in }
Expand All @@ -207,17 +213,26 @@ struct SearchView_Previews: PreviewProvider {
.toolbar {
ScrollView(.horizontal) {
HStack {
Button("Entry") { state = .entry }
Button("Autocomplete") { state = .autocomplete }
Button("Entry") {
uiState = .entry
autocompletions = []
}
Button("Autocomplete") {
uiState = .entry
autocompletions = [Self.englishText, Self.arabicText]
}
Button("Search") {
state = .searchResults
results = [Self.populatedResults]
uiState = .search("abc")
searchState = .searchResult([Self.populatedResults])
}
Button("No Results") {
state = .searchResults
results = []
uiState = .search("abc")
searchState = .searchResult([])
}
Button("Loading") {
uiState = .search("abc")
searchState = .searching
}
Button("Loading") { state = .loading }
Button("Error") { error = URLError(.notConnectedToInternet) }
}
}
Expand Down
45 changes: 31 additions & 14 deletions Features/SearchFeature/SearchViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import Combine
import Foundation
import Localization
import SwiftUI
import VLogging

final class SearchViewController: UIViewController, UISearchResultsUpdating, UISearchBarDelegate {
final class SearchViewController: UIViewController, UISearchBarDelegate {
// MARK: Lifecycle

init(viewModel: SearchViewModel) {
Expand Down Expand Up @@ -44,43 +45,59 @@ final class SearchViewController: UIViewController, UISearchResultsUpdating, UIS
searchController.obscuresBackgroundDuringPresentation = false
searchController.hidesNavigationBarDuringPresentation = true
searchController.searchBar.placeholder = l("search.placeholder.text")
searchController.searchResultsUpdater = self
searchController.searchBar.delegate = self
navigationItem.searchController = searchController
navigationItem.hidesSearchBarWhenScrolling = false
definesPresentationContext = true

viewModel.$searchTerm
.map(\.term)
.receive(on: DispatchQueue.main)
.sink { [weak self] searchTerm in
if searchTerm != self?.searchController.searchBar.text {
self?.searchController.searchBar.text = searchTerm
}
self?.searchController.searchBar.text = searchTerm
}
.store(in: &cancellables)

viewModel.resignSearchBar
viewModel.$keyboardState
.removeDuplicates()
.receive(on: DispatchQueue.main)
.sink { [weak self] in
self?.searchController.searchBar.endEditing(true)
.sink { [weak self] state in
switch state {
case .closed:
self?.searchController.searchBar.endEditing(true)
case .open:
self?.searchController.searchBar.becomeFirstResponder()
}
}
.store(in: &cancellables)
}

// MARK: - Search delegate methods

func updateSearchResults(for searchController: UISearchController) {
let term = searchController.searchBar.text ?? ""
if viewModel.searchTerm.term != term {
viewModel.searchTerm = .autocomplete(term)
}
func searchBar(_ searchBar: UISearchBar, textDidChange searchText: String) {
logger.info("[Search] textDidChange to \(searchText)")
viewModel.autocomplete(searchText)
}

func searchBarTextDidBeginEditing(_ searchBar: UISearchBar) {
logger.info("[Search] searchBarTextDidBeginEditing \(searchBar.text ?? "")")
viewModel.keyboardState = .open
}

func searchBarTextDidEndEditing(_ searchBar: UISearchBar) {
logger.info("[Search] searchBarTextDidEndEditing \(searchBar.text ?? "")")
viewModel.keyboardState = .closed
}

func searchBarSearchButtonClicked(_ searchBar: UISearchBar) {
logger.info("[Search] searchBarSearchButtonClicked \(searchBar.text ?? "")")
viewModel.searchForUserTypedTerm()
}

func searchBarCancelButtonClicked(_ searchBar: UISearchBar) {
logger.info("[Search] searchBarCancelButtonClicked")
viewModel.reset()
}

// MARK: Private

private var cancellables: Set<AnyCancellable> = []
Expand Down
Loading

0 comments on commit 4ec969e

Please sign in to comment.