-
Notifications
You must be signed in to change notification settings - Fork 328
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
[Customer Center] Slight improvement to the Customer Center Promotional Offer view #4554
Changes from 1 commit
58364fc
b07df84
6b95557
b2c91e2
91b0935
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 |
---|---|---|
|
@@ -62,6 +62,12 @@ struct PromotionalOfferView: View { | |
|
||
VStack { | ||
if self.viewModel.error == nil { | ||
|
||
AppIconView() | ||
.padding(.top, 100) | ||
.padding(.bottom) | ||
.padding(.horizontal) | ||
|
||
PromotionalOfferHeaderView(viewModel: self.viewModel) | ||
|
||
Spacer() | ||
|
@@ -94,6 +100,33 @@ struct PromotionalOfferView: View { | |
) { | ||
self.onDismissPromotionalOfferView(action) // Forward results to parent view | ||
} | ||
|
||
private struct AppIconView: View { | ||
var body: some View { | ||
if let appIcon = AppIconHelper.getAppIcon() { | ||
Image(uiImage: appIcon) | ||
.resizable() | ||
.aspectRatio(contentMode: .fit) | ||
.frame(width: 100, height: 100) | ||
.clipShape(RoundedRectangle(cornerRadius: 20, style: .continuous)) | ||
.shadow(radius: 10) | ||
} else { | ||
EmptyView() | ||
} | ||
} | ||
} | ||
|
||
private enum AppIconHelper { | ||
static func getAppIcon() -> UIImage? { | ||
guard let iconsDictionary = Bundle.main.infoDictionary?["CFBundleIcons"] as? [String: Any], | ||
let primaryIcons = iconsDictionary["CFBundlePrimaryIcon"] as? [String: Any], | ||
let iconFiles = primaryIcons["CFBundleIconFiles"] as? [String], | ||
let lastIcon = iconFiles.last else { | ||
return nil | ||
} | ||
return UIImage(named: lastIcon) | ||
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. just fetch the app icon, seems to work well in my testing 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. 🤔 looks a bit blurry actually, maybe I can do something to ensure we're always getting the highest-res version |
||
} | ||
} | ||
} | ||
|
||
@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *) | ||
|
@@ -110,7 +143,6 @@ struct PromotionalOfferHeaderView: View { | |
private(set) var viewModel: PromotionalOfferViewModel | ||
|
||
private let spacing: CGFloat = 30 | ||
private let topPadding: CGFloat = 150 | ||
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. moved the padding from here to the icon view so it can be controlled from there |
||
private let horizontalPadding: CGFloat = 40 | ||
|
||
var body: some View { | ||
|
@@ -121,7 +153,7 @@ struct PromotionalOfferHeaderView: View { | |
.font(.title) | ||
.fontWeight(.bold) | ||
.multilineTextAlignment(.center) | ||
.padding(.top, topPadding) | ||
.padding(.top) | ||
|
||
Text(details.subtitle) | ||
.font(.body) | ||
|
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.
Since this padding is always applied, the padding will still be present even if the AppIconView() is just an EmptyView(), which could make the layout look off if the app icon can't be found for some reason. I'd consider making a static property on AppIconView called
canDisplay
and then conditionally setting the top padding based on that value, like: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, I think you switched the missing case from being an EmptyView() to a Color.clear while I was reviewing. Ignore this comment since we want to preserve the space!
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 sorry for the switcheroo, I had uncommitted code. I guess I'm starting to forget how to do PRs properly