-
Notifications
You must be signed in to change notification settings - Fork 109
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
Changes from 3 commits
7a734cf
e9df779
7ac67b0
27ec9b5
e23c6d2
a542b76
1706a3f
1e1819b
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 |
---|---|---|
@@ -1,19 +1,28 @@ | ||
import SwiftUI | ||
|
||
struct OrderCustomFieldsDetails: View { | ||
struct CustomFieldsDetails: View { | ||
@Environment(\.presentationMode) var presentationMode | ||
|
||
let customFields: [OrderCustomFieldsViewModel] | ||
let viewEditCustomFieldsInProductsAndOrdersEnabled: Bool | ||
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. Super nit: the naming is a bit verbose making it hard to follow. How about 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. 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) | ||
} | ||
|
@@ -90,13 +99,75 @@ private struct CustomFieldRow: View { | |
Spacer() | ||
} | ||
.padding([.top, .bottom], Constants.hStackPadding) | ||
.padding(.horizontal, Constants.hStackPadding) | ||
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. Nit: this can be combined into 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. 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) | ||
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. Same comment as above about padding. 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. 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") | ||
} | ||
|
@@ -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 | ||
} | ||
} | ||
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. Nit: instead of duplicating the constants, you can bring the enum to 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. 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/")) | ||
]) | ||
} | ||
} | ||
|
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.
Nit: Please rename the view to clarify that it's a view. The postfix
View
is usually used for this.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.
Sure, updated in 27ec9b5