-
Notifications
You must be signed in to change notification settings - Fork 338
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
[Paywalls V2] Optionalizing padding, margin, and corner radius properties for safety #4644
Conversation
@@ -14,7 +14,7 @@ protocol AvailableConfigItems { | |||
|
|||
// CI system adds keys here | |||
extension AvailableConfigItems { | |||
static var apiKey: String { "appl_fpIYNnFHeCcvJRZqibQfQOTUusd" } | |||
static var apiKey: String { "" } |
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.
Ooops
1 build increased size
Paywalls 1.0 (1)
|
Item | Install Size Change |
---|---|
RevenueCatUI.TextComponentViewModel.TextComponentViewModel | ⬆️ 5.8 kB |
RevenueCatUI.StackComponentViewModel.StackComponentViewModel | ⬆️ 5.3 kB |
RevenueCatUI.StickyFooterComponentViewModel.StickyFooterComponent... | ⬆️ 4.0 kB |
RevenueCat.OfferingsFactory.OfferingsFactory | ⬆️ 3.8 kB |
Code Signature | ⬆️ 3.4 kB |
🛸 Powered by Emerge Tools
Comment trigger: Size diff threshold of 100.00kB exceeded
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.
!
Just a question that may be obvious.
EdgeInsets(top: top ?? 0, | ||
leading: leading ?? 0, | ||
bottom: bottom ?? 0, | ||
trailing: trailing ?? 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.
Where is this used? Is this a JSON model? Should there be a test for this? (Sorry if obvious, I'm so deep in the rabbit hole. 😅)
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.
Ah, EdgeInsets
are passed into the SwiftUI views to add padding and margin (so it was being used in SwiftU Previews / Snapshots)
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.
Alright, so what PaddingValues
is in Compose I guess. Thanks for explaining!
} | ||
""" | ||
|
||
_ = try JSONDecoder.default.decode( |
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.
For asserting that an optional is not nil
in a test, there's the XCTUnwrap.
XCTUnwrap(try JSONDecoder.default.decode(...))
(I'm late to the party, but... could still be useful to know)
Motivation
Removing hard requirement for properties that can/should easily default to zero (just for safety)
Description
Made properties optional for:
Padding
CornerRadiuses