-
Notifications
You must be signed in to change notification settings - Fork 121
Customer Search: Implement UI #7880
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
57e37ed
7dcc1bc
ca8d350
5b50032
84eaab7
6281456
1605feb
7f42204
a8dd778
da5e401
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| import Foundation | ||
| import Yosemite | ||
|
|
||
| /// Implementation of `SearchUICommand` for Customer search. | ||
| /// | ||
| final class CustomerSearchUICommand: SearchUICommand { | ||
|
|
||
| typealias Model = Customer | ||
| typealias CellViewModel = TitleAndSubtitleAndStatusTableViewCell.ViewModel | ||
| typealias ResultsControllerModel = StorageCustomer | ||
|
|
||
| var searchBarPlaceholder: String = Localization.searchBarPlaceHolder | ||
|
|
||
| var searchBarAccessibilityIdentifier: String = "customer-search-screen-search-field" | ||
|
|
||
| var cancelButtonAccessibilityIdentifier: String = "customer-search-screen-cancel-button" | ||
|
Comment on lines
+14
to
+16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You probably only need to set accessibilityIdentifiers if you're actually using the search in a UI test. Identifiers are nothing to do with VoiceOver and other assistive tech, in practice, they are only used for UI tests. AccessibilityLabels are for VoiceOver and important to have on custom controls or complex groups.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL! |
||
|
|
||
| var resynchronizeModels: (() -> Void) = {} | ||
|
|
||
| var onDidSelectSearchResult: ((Customer) -> Void) | ||
|
|
||
| private let siteID: Int64 | ||
|
|
||
| init(siteID: Int64, onDidSelectSearchResult: @escaping ((Customer) -> Void)) { | ||
| self.siteID = siteID | ||
| self.onDidSelectSearchResult = onDidSelectSearchResult | ||
| } | ||
|
|
||
| func createResultsController() -> ResultsController<StorageCustomer> { | ||
| let storageManager = ServiceLocator.storageManager | ||
| let predicate = NSPredicate(format: "siteID == %lld", siteID) | ||
| let descriptor = NSSortDescriptor(keyPath: \StorageCustomer.customerID, ascending: false) | ||
| return ResultsController<StorageCustomer>(storageManager: storageManager, matching: predicate, sortedBy: [descriptor]) | ||
| } | ||
|
|
||
| func createStarterViewController() -> UIViewController? { | ||
| nil | ||
| } | ||
|
|
||
| func createCellViewModel(model: Customer) -> TitleAndSubtitleAndStatusTableViewCell.ViewModel { | ||
| return CellViewModel( | ||
| id: "\(model.customerID)", | ||
| title: "\(model.firstName ?? "") \(model.lastName ?? "")", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This... might be the right thing to do given our data source, but names are complicated, and simply concatenating them isn't correct in all locales. See
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the link, I definitely didn't know about |
||
| subtitle: model.email, | ||
| accessibilityLabel: "", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Accessibility Label is required here... see what it does in VoiceOver without it: it just announces:
Without any detail about the name of the user or anything else about the search result. If you're moving to another cell, you may not have to do anything, usually we get VoiceOver in cells for free, but in this case we are specific about what the label should be in the view model. |
||
| status: "", | ||
| statusBackgroundColor: .clear | ||
| ) | ||
| } | ||
|
|
||
| func synchronizeModels(siteID: Int64, keyword: String, pageNumber: Int, pageSize: Int, onCompletion: ((Bool) -> Void)?) { | ||
| let action = CustomerAction.searchCustomers(siteID: siteID, keyword: keyword) { result in | ||
| switch result { | ||
| case .success(_): | ||
| onCompletion?(result.isSuccess) | ||
| case .failure(let error): | ||
| DDLogError("Customer Search Failure \(error)") | ||
| } | ||
| } | ||
| ServiceLocator.stores.dispatch(action) | ||
| } | ||
|
|
||
| func didSelectSearchResult(model: Customer, from viewController: UIViewController, reloadData: () -> Void, updateActionButton: () -> Void) { | ||
| // Not implemented yet | ||
| print("1 - Customer tapped") | ||
| print("2 - Customer ID: \(model.customerID) - Name: \(model.firstName ?? ""))") | ||
| // Customer data will go up to EditOrderAddressForm, via OrderCustomerListView completion handler | ||
| onDidSelectSearchResult(model) | ||
| } | ||
|
|
||
| func searchResultsPredicate(keyword: String) -> NSPredicate? { | ||
| return NSPredicate(format: "siteID == %lld AND ANY searchResults.keyword = %@", siteID, keyword) | ||
| } | ||
| } | ||
|
|
||
| private extension CustomerSearchUICommand { | ||
| enum Localization { | ||
| static let searchBarPlaceHolder = NSLocalizedString("Search all customers", | ||
| comment: "Customer Search Placeholder") | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import Foundation | ||
| import Yosemite | ||
| import SwiftUI | ||
|
|
||
| /// `SwiftUI` wrapper for `SearchViewController` using `CustomerSearchUICommand` | ||
| /// TODO: Make it generic | ||
| struct OrderCustomerListView: UIViewControllerRepresentable { | ||
|
|
||
| let siteID: Int64 | ||
|
|
||
| let onCustomerTapped: ((Customer) -> Void) | ||
|
|
||
| func makeUIViewController(context: Context) -> WooNavigationController { | ||
|
|
||
| let viewController = SearchViewController( | ||
| storeID: siteID, | ||
| command: CustomerSearchUICommand(siteID: siteID, onDidSelectSearchResult: onCustomerTapped), | ||
| cellType: TitleAndSubtitleAndStatusTableViewCell.self, | ||
| // Must conform to SearchResultCell. | ||
| // TODO: Proper cell for this cellType. | ||
| cellSeparator: .none | ||
| ) | ||
| let navigationController = WooNavigationController(rootViewController: viewController) | ||
| return navigationController | ||
| } | ||
|
|
||
| func updateUIViewController(_ uiViewController: UIViewControllerType, context: Context) { | ||
| // nope | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy to get a different recommendation for the CellView we want to use here, at the moment I'm using
TitleAndSubtitleAndStatusTableViewCelljust for convenience, as must conform toSearchResultCellthere are several options to choose from without having to create a new one (or I could create a new one as well!)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that you're going to have the avatar in there too, you might want to try adding
SearchResultCellconformance toLeftImageTitleSubtitleTableViewCell?The only thing it won't do is any attributed text in the subtitle for an underlined email as on Android... but I don't personally think that should be a blocker.