-
Notifications
You must be signed in to change notification settings - Fork 48
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: Add simple paywall and use in tester app #1223
Paywalls: Add simple paywall and use in tester app #1223
Conversation
private val offeringId = savedStateHandle.get<String?>(PaywallScreenViewModel.OFFERING_ID_KEY) | ||
|
||
init { | ||
updateOffering() |
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.
We need to refetch the offering, since after navigating to this screen, we only have the offeringId.
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.
We are also loading in the PaywallViewModel
right? Do we need to do it in both places?
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 is in the paywall tester app, the other is in the library itself. We could offer an API so users can pass an offering id, and not an offering... But that seems worse, since they might send other strings, so I would prefer not to if possible.
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.
Got it! Passing an offering and getting the current if null makes sense!
import com.revenuecat.purchases.Offering | ||
|
||
@Composable | ||
internal fun InternalPaywallView( |
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.
Moved the UI to an "internal" view, so we can separate the actual UI from the API.
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 have something similar since the public one will have to do loading and processing of the config. The internal one is simpler if it depends on the actual final requirements.
import androidx.lifecycle.ViewModelProvider | ||
import com.revenuecat.purchases.Offering | ||
|
||
class PaywallViewModelFactory(private val offering: Offering?) : ViewModelProvider.NewInstanceFactory() { |
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.
To pass parameters to the view model, a factory like this is required... One of the problems of architecture component's viewModel.
Log.e("PaywallTester", "Error purchasing package: $error") | ||
}, | ||
onSuccess = { purchase, _ -> | ||
Log.i("PaywallTester", "Purchased package: $purchase") |
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.
We should at least display some sort of UI, but in this PR, I'm just logging. Will follow-up with some alerts in the next PR.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## paywalls #1223 +/- ##
===========================================
Coverage ? 85.25%
===========================================
Files ? 186
Lines ? 6319
Branches ? 911
===========================================
Hits ? 5387
Misses ? 586
Partials ? 346 ☔ View full report in Codecov by Sentry. |
@@ -54,6 +64,7 @@ dependencies { | |||
implementation project(path: ':purchases') | |||
implementation project(path: ':feature:amazon') | |||
implementation project(path: ':ui:debugview') |
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.
🎉
Text(text = "Error: ${state.errorMessage}") | ||
} | ||
is PaywallScreenState.Loaded -> { | ||
PaywallView(PaywallViewOptions.Builder().setOffering(state.offering).build()) |
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.
Nice
|
||
import com.revenuecat.purchases.Offering | ||
|
||
sealed class PaywallScreenState { |
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.
Love it. I wonder if you'll need to move it to the UI framework to handle these 3 states there instead.
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.
There is a state in the library as well, but that's internal. This is needed because we need to refetch the Offering
since we only get the offering_id
in this screen.
} | ||
|
||
private fun updateOffering() { | ||
Purchases.sharedInstance.getOfferingsWith( |
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.
Would it be possible to use the coroutines? 😅
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.
Yup! I wasn't thinking about it but we totally could! Will do that in a followup PR
import com.revenuecat.purchases.Offering | ||
|
||
@Composable | ||
internal fun InternalPaywallView( |
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 have something similar since the public one will have to do loading and processing of the config. The internal one is simpler if it depends on the actual final requirements.
Column( | ||
modifier = Modifier | ||
.fillMaxSize() | ||
.padding(16.dp), |
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 not perfect at this, but I'd recommend starting to extract some of these magic numbers.
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.
Honestly, this is too early in the UI 😅, since this whole UI is probably going away. But yeah, we should extract these.
} | ||
|
||
override fun purchasePackage(activity: Activity, packageToPurchase: Package) { | ||
Purchases.sharedInstance.purchaseWith( |
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.
Is it possible to use coroutines here too?
It will make it easier to update the internal state to avoid multiple concurrent purchases (to disable the buttons on the UI).
See PurchaseHandler
.
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.
Yup as mentioned above, will do that in a followup PR
cda6832
to
bba0dc1
Compare
ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/PaywallViewModel.kt
Show resolved
Hide resolved
### Description This PR adds a simple paywall to the revenuecatui library and uses it to display the paywall in paywall tester. Still pretty barebones but it's a start :P https://github.com/RevenueCat/purchases-android/assets/808417/b4a4b85c-68dc-4cc0-9019-2baefb50e2e3
Description
This PR adds a simple paywall to the revenuecatui library and uses it to display the paywall in paywall tester. Still pretty barebones but it's a start :P
Screen_recording_20230906_145136.mp4