-
Notifications
You must be signed in to change notification settings - Fork 85
Video Component Models (dark code) #2646
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
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2646 +/- ##
==========================================
- Coverage 78.48% 78.25% -0.24%
==========================================
Files 305 307 +2
Lines 11369 11431 +62
Branches 1577 1581 +4
==========================================
+ Hits 8923 8945 +22
- Misses 1755 1792 +37
- Partials 691 694 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
This makes sense to me! Great job!
val height: UInt, | ||
@get:JvmSynthetic | ||
@Serializable(with = URLSerializer::class) | ||
val url: URL, |
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 wonder... should we allow developers to set a reference to an embedded video? So basically, follow a similar pattern as we do with fonts. If we find the name of the video in the resources for the app, we use that video. If not, we download it. That might help with bandwidth... But then again, it's probably a future improvement we can do without causing a breaking change, so I think this is ok for now.
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.
That is an interesting question. @joshdholtz what do you think?
**This is an automatic release.** ## RevenueCat SDK ### ✨ New Features * Add preferred UI locale override for RevenueCat UI components (#2620) via Josh Holtz (@joshdholtz) ### 🔄 Other Changes * Improve thread safety of setting paywalls preferred locale (#2655) via Josh Holtz (@joshdholtz) * Remove validation for no packages on paywalls (#2653) via Josh Holtz (@joshdholtz) * Video Component Models (dark code) (#2646) via Jacob Rakidzich (@JZDesign) * [EXTERNAL] docs: fixed a typo on documentation about `Purchases.awaitPurchase` by @matteinn in #2593 (#2651) via Toni Rico (@tonidero) * Add warning with 9.x issues to all versions since 9.0.0 in CHANGELOG (#2650) via Toni Rico (@tonidero) * [AUTOMATIC][Paywalls V2] Updates paywall-preview-resources submodule (#2647) via RevenueCat Git Bot (@RCGitBot) * Delete CLAUDE.md (#2648) via Cesar de la Vega (@vegaro) * MON-1193 flatten Transition JSON structure after chatting more thoroughly with team (#2641) via Jacob Rakidzich (@JZDesign) --------- Co-authored-by: revenuecat-ops <ops@revenuecat.com> Co-authored-by: Josh Holtz <me@joshholtz.com>
Checklist
purchases-ios
and hybridsMotivation
Creating a new paywall component can be a VERY large amount of code, I'm breaking things apart to make the PR's easier to review. This PR is just the creation of the VideoComponent model.
Description
Add a video component model and serialization tests
Note
I did not make it a member of the PaywallComponent sealed interface. I intentionally left that part out so I wouldn't have to have a style factory function setup and all of the other stuff that would domino into the PR.
This is just the json -> model