-
Notifications
You must be signed in to change notification settings - Fork 54
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] Improves image previews #2029
Conversation
📸 Snapshot Test3 modified, 10 added, 3 removed, 124 unchanged
🛸 Powered by Emerge Tools |
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.
LGTM!
light = ImageUrls( | ||
original = URL("https://preview"), | ||
webp = URL("https://preview"), | ||
webpLowRes = URL("https://preview"), |
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.
Hmm should we change these for somewhat more valid URLs? Though I guess it doesn't fail so it doesn't really matter 🤷
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 usually make these as invalid as possible to avoid accidentally hitting a live, untrusted endpoint. I guess we can use a domain we own to limit that risk, but we really don't want a network connection here. So maybe that's not worth it? This also screams "fake", which is good in this case I think haha.
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.
Sounds good, yeah I don’t feel too strongly about this at all
.../main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/image/ImageComponentView.kt
Show resolved
Hide resolved
* This is not production-quality code and should only be used for Previews. If we ever have a need for this, it's | ||
* better to use the Accompanist Drawable Painter library directly. | ||
*/ | ||
private class DrawablePainter( |
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'm wondering if it's worth mark it as experimental even if it's private code, so people don't use it without looking at the doc above. Wdyt?
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 like it! Will make the change.
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.
Done here: a6959c2. I split it out in a separate method so we can @OptIn
as narrowly as possible. Let me know what you think!
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2029 +/- ##
=======================================
Coverage 81.88% 81.88%
=======================================
Files 260 260
Lines 8513 8513
Branches 1226 1226
=======================================
Hits 6971 6971
Misses 1043 1043
Partials 499 499 ☔ View full report in Codecov by Sentry. |
**This is an automatic release.** ## RevenueCat SDK ### 📦 Dependency Updates * Updates the Play Billing Library to 7.1.1. (#2032) via JayShortway (@JayShortway) ### 🔄 Other Changes * [Paywalls V2] Fixes click consumption in text buttons (#2033) via JayShortway (@JayShortway) * [Paywalls V2] Handles gradient percentages in the 0..100 range (#2030) via JayShortway (@JayShortway) * [Paywalls V2] Add `Badge` property to StackComponent (#2003) via Toni Rico (@tonidero) * [Paywalls V2] Improves image previews (#2029) via JayShortway (@JayShortway) * Make `PurchasesError` `java.io.Serializable` (#2031) via Toni Rico (@tonidero) * [Paywalls V2] Fixes `ImageComponentView` size when axes are Fit and Fixed (#2024) via JayShortway (@JayShortway) * [Paywalls V2] Image backgrounds support `fit_mode` and `color_overlay` (#2021) via JayShortway (@JayShortway) * [Paywalls V2] Fixes various rendering issues (#2020) via JayShortway (@JayShortway) * [Paywalls V2] Fixes various deserialization issues (#2018) via JayShortway (@JayShortway) * [Paywalls V2] Simplifies validation of `PaywallComponentsData` (#2017) via JayShortway (@JayShortway) * [Paywalls V2] Moves click handling from `ButtonComponentStyle` to `ButtonComponentView` (#2016) via JayShortway (@JayShortway) * [Paywalls V2] Adds `PackageComponentView` (#2014) via JayShortway (@JayShortway) * [Paywalls V2] Adds `ImageComponentState` and override functionality (#2012) via JayShortway (@JayShortway) Co-authored-by: revenuecat-ops <[email protected]>
Motivation
The current mechanisms to show remote images in Compose Previews did not allow us to specify the size of the remote image. The replacement image was a solid color or a vector drawable, both of which resize uniformly to the view they're displayed in. However, in the real world, Paywalls V2 will receive images with a fixed size. We needed a way to preview resizing and scaling behavior with these kinds of images.
Description
This PR adds a new
previewImageLoader
parameter, which can be used to return whatever kind of image we want. This is then used inImageComponentView
previews to check its behavior for images with a fixed size. It's slightly hacky, but the upside is that it will only run in the preview environment. The alternative approach involves updating to Coil 3 (#2028), but that bumps Kotlin to 2.0.21. So that's left for a later date.This PR also fixes a rendering bug when the width is
Fill
and the height isFit
, by introducing anaspectRatio
modifier. These size constraints are often used for header images. The changes to the preview mechanism allowed us to write a regression "test" (preview) for this.Note
This changes some existing previews for
ImageComponentView
, since the preview image is now no longer scaled uniformly.