Paywalls: use HEIC images#3496
Conversation
f969a00 to
a088595
Compare
|
looks great! Maybe we should fall back to the old key if the new one is not found while we figure out how to migrate the existing images? |
|
might not be worth it given that it's a bigger refactor, depending on how things go migration-wise |
1dee37e to
ba75c5a
Compare
ba75c5a to
401c3ba
Compare
|
@aboedo done |
Performance optimization equivalent to RevenueCat/purchases-ios#3496
|
|
||
| set { | ||
| self._imagesHeic = newValue | ||
| self._legacyImages = nil |
There was a problem hiding this comment.
what if some of the images are in the old format but not in heic like in https://github.com/RevenueCat/purchases-ios/pull/3496/files#diff-61501df9f82fee7c24f6504108b2ade66c4990e1d72725577a1723c5cb69987e
There was a problem hiding this comment.
Also, when would this setter be called?
There was a problem hiding this comment.
This preserves the semantics of the setter.
Imagine a given configuration:
var config = Configuration(
imagesHeic: Images(...),
legacyImages: Images(...)
)Doing this:
config.images.background = "new"Is equivalent in Swift to this:
var newImages = config.images // This is getting the "derived" getter, merging both
newImages.background = "new"
config.images = newImagesThis implementation is tested in a new test to verify its correctness too. Basically, not removing _legacyImages would mean you can't do this:
config.images.background = nilBecause it would potentially still find the fallback in legacyImages. This setter is mainly used for tests. Everywhere else we use immutable PaywallData so the setter isn't accessible anyway.
There was a problem hiding this comment.
I see! Thanks for taking the time to write that up
There was a problem hiding this comment.
Learning (some) Kotlin made me realize that these sorts of things are nothing but obvious when learning a new language 😇
| "display_restore_purchases" : true, | ||
| "images" : { | ||
| "images_heic" : { | ||
| "background" : "background.jpg", |
There was a problem hiding this comment.
do these need to be updated to background.heic?
There was a problem hiding this comment.
That comes from this:
static let backgroundImage = "background.jpg"The background image embedded in the framework is jpg. When creating a custom PaywallData.Configuration I chose to set images_heic instead of images, so it gets serialized like this.
| expect(paywall.config.images) == .init( | ||
| header: "header.jpg", | ||
| header: "header.heic", | ||
| background: "background.jpg", |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3496 +/- ##
==========================================
+ Coverage 86.06% 86.07% +0.01%
==========================================
Files 237 237
Lines 17212 17226 +14
==========================================
+ Hits 14813 14827 +14
Misses 2399 2399 ☔ View full report in Codecov by Sentry. |
**This is an automatic release.** ### RevenueCatUI * `Paywalls`: improve image caching (#3498) via NachoSoto (@NachoSoto) * `Paywalls`: change style of CTA button to be consistent with other platforms (#3507) via NachoSoto (@NachoSoto) * `Paywalls`: open footer links on Safari on Catalyst (#3508) via NachoSoto (@NachoSoto) * `Paywalls`: fix compilation on Xcode < 14.3 (#3513) via NachoSoto (@NachoSoto) * Fix typo in zh-Hans localization of RevenueCatUI (#3512) via Francis Feng (@francisfeng) * `Paywalls`: fix PaywallViewControllerDelegate access from Objective-C (#3510) via noncenz (@noncenz) * `Paywalls`: open Terms and Privacy Policy links in-app (#3475) via Andy Boedo (@aboedo) * `Paywalls`: fix empty description when using `custom` package type and `{{ sub_period }}` (#3495) via Andy Boedo (@aboedo) * `Paywalls`: use `HEIC` images (#3496) via NachoSoto (@NachoSoto) * Paywalls: disable the close button when an action is in progress (#3474) via Andy Boedo (@aboedo) * `Paywalls`: adjusted German translations (#3476) via Tensei (@tensei) * Paywalls: Improve Chinese localization (#3489) via Andy Boedo (@aboedo) ### Other Changes * `CircleCI`: add git credentials to snapshot generation (#3506) via NachoSoto (@NachoSoto) * `StoreProduct`: improve `priceFormatter` documentation (#3500) via NachoSoto (@NachoSoto) * `Paywalls`: fix tests (#3499) via NachoSoto (@NachoSoto) * `Optimization`: changed `first` to `first(where:)` (#3467) via NachoSoto (@NachoSoto)
This is a performance optimization.