Skip to content
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

Custom Fields: Prepare list view and viewmodel to be reusable #13639

Merged
merged 8 commits into from
Aug 21, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ final class OrderDetailsViewModel {
private let stores: StoresManager
private let storageManager: StorageManagerType
private let currencyFormatter: CurrencyFormatter
let featureFlagService: FeatureFlagService

private(set) var order: Order

Expand All @@ -29,11 +30,13 @@ final class OrderDetailsViewModel {
init(order: Order,
stores: StoresManager = ServiceLocator.stores,
storageManager: StorageManagerType = ServiceLocator.storageManager,
currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings)) {
currencyFormatter: CurrencyFormatter = CurrencyFormatter(currencySettings: ServiceLocator.currencySettings),
featureFlagService: FeatureFlagService = ServiceLocator.featureFlagService) {
self.order = order
self.stores = stores
self.storageManager = storageManager
self.currencyFormatter = currencyFormatter
self.featureFlagService = featureFlagService
self.configurationLoader = CardPresentConfigurationLoader(stores: stores)
self.dataSource = OrderDetailsDataSource(order: order,
cardPresentPaymentsConfiguration: configurationLoader.configuration)
Expand Down Expand Up @@ -477,9 +480,12 @@ extension OrderDetailsViewModel {
case .customFields:
ServiceLocator.analytics.track(.orderViewCustomFieldsTapped)
let customFields = order.customFields.map {
OrderCustomFieldsViewModel(metadata: $0)
CustomFieldsViewModel(metadata: $0)
}
let customFieldsView = UIHostingController(rootView: OrderCustomFieldsDetails(customFields: customFields))
let customFieldsView = UIHostingController(
rootView: CustomFieldsDetails(
viewEditCustomFieldsInProductsAndOrdersEnabled: featureFlagService.isFeatureFlagEnabled(.viewEditCustomFieldsInProductsAndOrders),
customFields: customFields))
viewController.present(customFieldsView, animated: true)
case .seeReceipt:
let countryCode = configurationLoader.configuration.countryCode
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,28 @@
import SwiftUI

struct OrderCustomFieldsDetails: View {
struct CustomFieldsDetails: View {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Please rename the view to clarify that it's a view. The postfix View is usually used for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, updated in 27ec9b5

@Environment(\.presentationMode) var presentationMode

let customFields: [OrderCustomFieldsViewModel]
let viewEditCustomFieldsInProductsAndOrdersEnabled: Bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super nit: the naming is a bit verbose making it hard to follow. How about isEditable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in e23c6d2

let customFields: [CustomFieldsViewModel]

var body: some View {
NavigationView {
GeometryReader { geometry in
ScrollView {
VStack(alignment: .leading) {
ForEach(customFields) { customField in
CustomFieldRow(title: customField.title,
content: customField.content,
contentURL: customField.contentURL)

if viewEditCustomFieldsInProductsAndOrdersEnabled {
EditableCustomFieldRow(title: customField.title,
content: customField.content,
contentURL: customField.contentURL)
} else {
CustomFieldRow(title: customField.title,
content: customField.content,
contentURL: customField.contentURL)
}

Divider()
.padding(.leading)
}
Expand Down Expand Up @@ -90,13 +99,75 @@ private struct CustomFieldRow: View {
Spacer()
}
.padding([.top, .bottom], Constants.hStackPadding)
.padding(.horizontal, Constants.hStackPadding)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: this can be combined into .padding(Constants.hStackPadding) to set the same padding for all edges.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as part of 1706a3f

.frame(minHeight: Constants.height)
}
}

private struct EditableCustomFieldRow: View {
/// Custom Field title
///
let title: String

/// Custom Field content
///
let content: String

/// Optional URL to link the content
///
let contentURL: URL?

/// URL to display in `SafariSheet` in app
///
@State private var displayedURL: URL?

/// Action to open URL with system handler
///
@Environment(\.openURL) private var openURL

var body: some View {
HStack {
VStack(alignment: .leading,
spacing: Constants.spacing) {
Text(title)

if let url = contentURL { // Display content as a link if URL is provided
Text(content)
.font(.footnote)
.foregroundColor(Color(.textLink))
.safariSheet(url: $displayedURL)
.onTapGesture {
switch url.scheme {
case "http", "https":
displayedURL = url // Open in `SafariSheet` in app
default:
openURL(url) // Open in associated app for URL scheme
}
}
} else { // Display content as plain text
Text(content)
.footnoteStyle()
.lineLimit(3)
}
}.padding([.leading, .trailing], Constants.vStackPadding)

Spacer()

// Chevron icon
Image(uiImage: .chevronImage)
.flipsForRightToLeftLayoutDirection(true)
.foregroundStyle(Color(.textTertiary))
}
.padding([.top, .bottom], Constants.hStackPadding)
.padding(.horizontal, Constants.hStackPadding)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above about padding.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as part of 1706a3f

.frame(minHeight: Constants.height)
}
}


// MARK: - Constants
//
extension OrderCustomFieldsDetails {
extension CustomFieldsDetails {
enum Localization {
static let title = NSLocalizedString("Custom Fields", comment: "Title for the order custom fields list")
}
Expand All @@ -111,13 +182,25 @@ private extension CustomFieldRow {
}
}


private extension EditableCustomFieldRow {
enum Constants {
static let spacing: CGFloat = 8
static let vStackPadding: CGFloat = 16
static let hStackPadding: CGFloat = 10
static let height: CGFloat = 64
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: instead of duplicating the constants, you can bring the enum to CustomFieldsDetails extension and reuse it for both CustomFieldRow and EditableCustomFieldRow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated as part of 1706a3f


#if DEBUG

struct OrderCustomFieldsDetails_Previews: PreviewProvider {
static var previews: some View {
OrderCustomFieldsDetails(customFields: [
OrderCustomFieldsViewModel(id: 0, title: "First Title", content: "First Content"),
OrderCustomFieldsViewModel(id: 1, title: "Second Title", content: "Second Content", contentURL: URL(string: "https://woocommerce.com/"))
CustomFieldsDetails(
viewEditCustomFieldsInProductsAndOrdersEnabled: false,
customFields: [
CustomFieldsViewModel(id: 0, title: "First Title", content: "First Content"),
CustomFieldsViewModel(id: 1, title: "Second Title", content: "Second Content", contentURL: URL(string: "https://woocommerce.com/"))
])
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import Foundation
import Yosemite

/// ViewModel for `OrderCustomFieldsDetails`
struct OrderCustomFieldsViewModel: Identifiable {
/// ViewModel for `CustomFieldsDetails`
struct CustomFieldsViewModel: Identifiable {
/// Unique identifier, required by `SwiftUI`
///
let id: Int64
Expand Down
Loading