-
Notifications
You must be signed in to change notification settings - Fork 16
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
Rewrite the MobileBuyIntegration sample controllers in SwiftUI #244
Conversation
8729f84
to
b55f14b
Compare
b55f14b
to
4523601
Compare
6a5dbda
to
21e85dc
Compare
INFOPLIST_KEY_UIApplicationSupportsIndirectInputEvents = YES; | ||
INFOPLIST_KEY_UILaunchStoryboardName = LaunchScreen; | ||
INFOPLIST_KEY_UISupportedInterfaceOrientations = UIInterfaceOrientationPortrait; | ||
INFOPLIST_KEY_UISupportedInterfaceOrientations_iPad = "UIInterfaceOrientationLandscapeLeft UIInterfaceOrientationLandscapeRight UIInterfaceOrientationPortrait UIInterfaceOrientationPortraitUpsideDown"; | ||
IPHONEOS_DEPLOYMENT_TARGET = 13.0; |
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.
iOS 13 is obsolete and no longer receiving security updates
2bc019f
to
dec9a3d
Compare
dec9a3d
to
ed603d9
Compare
811428d
to
39d48bb
Compare
@@ -29,6 +29,10 @@ extension Storefront.MoneyV2 { | |||
let formatter = NumberFormatter() | |||
formatter.numberStyle = .currency | |||
formatter.currencyCode = currencyCode.rawValue | |||
return formatter.string(from: NSDecimalNumber(decimal: amount)) | |||
return isFree() ? "Free" : formatter.string(from: NSDecimalNumber(decimal: amount)) |
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.
Gonna steal this for android 👍
@State private var product: Storefront.Product | ||
@State private var handle: String? | ||
@State private var loading = false | ||
@State private var imageLoaded: Bool = false |
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.
Can't tell if it's Github or the indentation is actually a bit off?
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.
Yeah it's actually off. #247 will fix it
Text(product.description) | ||
.font(.body) | ||
.foregroundColor(.gray) | ||
.lineLimit(descriptionExpanded ? 10 : 3) |
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.
I think 10 for the expanded text is still bit too low for some of the products in the store (e.g. looking at Bird of paradise)
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.
Yeah, probably. I'll account for this in the full redesign task
guard let variant = product.variants.nodes.first else { return } | ||
|
||
loading = true | ||
let start = Date() |
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.
Continuous clock looks interesting.. Although I'm not sure how easy it is to use with callbacks.
let clock = ContinuousClock()
let elapsed = clock.measure {
someWork()
}
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.
Oh interesting 🤔
Toggle("Handle Cart URLs", isOn: $appConfiguration.universalLinks.cart) | ||
Toggle("Handle Product URLs", isOn: $appConfiguration.universalLinks.products) | ||
Toggle("Handle all Universal Links", isOn: $appConfiguration.universalLinks.handleAllURLsInApp) | ||
Toggle("Handle Checkout URLs", isOn: $config.universalLinks.checkout) |
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.
We also need to catch android up here at some point
What changes are you making?
plant-sample-app.mp4
Before you merge
Important