Skip to content

Commit

Permalink
Search was broken due to concurrent search requests (#608)
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
mohamede1945 authored Dec 19, 2023
1 parent 5d5e5f9 commit 7512f4c
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
View file
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
View file
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
View file
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 7512f4c

Please sign in to comment.