Paywall views: Support updating options state after initial layout #1582
Conversation
ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/views/PaywallView.kt
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1582 +/- ##
=======================================
Coverage 83.73% 83.73%
=======================================
Files 218 218
Lines 7248 7248
Branches 1007 1007
=======================================
Hits 6069 6069
Misses 788 788
Partials 391 391 ☔ View full report in Codecov by Sentry. |
ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/views/PaywallView.kt
Outdated
Show resolved
Hide resolved
| offeringSelection = OfferingSelection.OfferingId(newOfferingId) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we need to do this not only for the offering id but also for the font provider and the dismiss button... can we do something similar with the whole state?
There was a problem hiding this comment.
@vegaro
I think that'll help our team a lot since we won't have to fork off this repository for fontProvider changes :D
There was a problem hiding this comment.
I took a stab at that in #1586 in case you're interested
There was a problem hiding this comment.
awesome
https://github.com/Jjastiny/purchases-android/pull/1/files
Just curious, any concerns or thoughts on using mutableStateOf outside of the composable function?
| .setShouldDisplayDismissButton(shouldDisplayDismissButton ?: false) | ||
| .build()) | ||
| } | ||
| updateOfferingId = { |
There was a problem hiding this comment.
nit: Sorry to bother you guys again, but I highly recommend staying away from unstable lambdas, as it may cause unforeseen recompositions and performance hits.
There was a problem hiding this comment.
|
Took another approach in #1586 |
tonidero
changed the title
tonidero
changed the title
| } else { | ||
| OfferingSelection.OfferingId(offeringId) | ||
| } | ||
| paywallOptionsState.value = paywallOptionsState.value.copy(offeringSelection = offeringSelection) |
There was a problem hiding this comment.
There is quite a bit of repetition on these, we probably need to refactor these to avoid it.
| options = paywallOptions, | ||
| ) | ||
| val paywallOptions by remember { | ||
| paywallOptionsState |
There was a problem hiding this comment.
@vegaro
Wouldn't this not update after OfferingSelection changes once? Not a big deal, but I think it may lead to future bugs 😭
There was a problem hiding this comment.
Hi @Jjastiny, I believe the changes in this PR would update the state every time the setOfferingId method is called, even if it's called multiple times. Can you explain why you think this would only work once?
There was a problem hiding this comment.
@tonidero
hhm yeah, I stand corrected. It does look like it's recomposing for me. Sorry for the misunderstanding!
**This is an automatic release.** ### RevenueCatUI * `Paywalls`: fix template 5 title alignment (#1585) via NachoSoto (@NachoSoto) * `Paywalls`: replace `TextAlign.Left` with `TextAlign.Start` to better support RTL (#1584) via NachoSoto (@NachoSoto) * Paywall views: Support updating options state after initial layout (#1582) via Toni Rico (@tonidero) * `Paywalls`: improve `PaywallData.configForLocale()` disambiguation (#1579) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump fastlane-plugin-revenuecat_internal from `e6ba247` to `9c82c7a` (#1583) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `OfferingsFactory`: debug logs when creating `Offerings` (#1576) via NachoSoto (@NachoSoto) * `Paywalls`: log error when creating `PaywallState.Error` (#1574) via NachoSoto (@NachoSoto) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Description
This will make it so calling the setOfferingId method after the initial layout updates the state causing a recomposition of the paywall.