[Customer Center] Make colors nullable#4134
Conversation
Generated by 🚫 Danger |
fire-at-will
left a comment
There was a problem hiding this comment.
The changes here look good to me!
| Button("Didn't receive purchase") {} | ||
| .buttonStyle(ManageSubscriptionsButtonStyle()) | ||
| .environment(\.appearance, CustomerCenterConfigData.Appearance(mode: .system)) | ||
| .environment(\.appearance, CustomerCenterConfigData.Appearance( |
There was a problem hiding this comment.
Nit: It'd be nice to extract this CustomerCenterConfigData.Appearance to a constant somewhere so that it can be reused across previews
tonidero
left a comment
There was a problem hiding this comment.
Not super-familiar with swift UI but changes LGTM!
| textColor: .init(light: "#000000", dark: "#ffffff"), | ||
| backgroundColor: .init(light: "#000000", dark: "#ffffff"), | ||
| buttonTextColor: .init(light: "#000000", dark: "#ffffff"), | ||
| buttonBackgroundColor: .init(light: "#000000", dark: "#ffffff") |
There was a problem hiding this comment.
Should the defaults be all null? So we fallback to the system colors?
There was a problem hiding this comment.
right right, not sure what I was thinking!
| .buttonStyle(.borderedProminent) | ||
| .controlSize(.large) | ||
| .applyIf(background != nil, apply: { $0.tint(background) }) | ||
| .applyIf(textColor != nil, apply: { $0.foregroundColor(textColor) }) |
There was a problem hiding this comment.
I see we're just not applying custom colors if they are null... But I'm wondering if we should customize the default style of these to be the accentColor or something like that... But I'm ok going with just the defaults for now.
There was a problem hiding this comment.
That's actually done by .boderedProminent and it's the reason why I am using that style by default, otherwise it would just look like text. So I think we are covered
a8442e3 to
d779ca1
Compare
.buttonStyle(.borderedProminent)so they look native by default