-
Notifications
You must be signed in to change notification settings - Fork 10
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
RevenueCatUI
initial support
#549
Conversation
import androidx.fragment.app.Fragment | ||
import androidx.fragment.app.FragmentActivity | ||
|
||
fun presentPaywallFromFragment(fragment: FragmentActivity, requiredEntitlementIdentifier: String?) { |
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.
Should requiredEntitlementIdentifier
have a default value?
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 guess the main problem with this approach is that we can't share the result of the fragment (whether the user purchased/cancelled mostly) back to the caller... But I guess we can ask the developers to use getCustomerInfo
directly if they need to know the result 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.
Yeah I was going to work on that next. I was planning on passing an observer type here?
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 probably won't work, it's not easy to pass information back from a fragment/activity unfortunately: I recommend reading this, it's dense, but it explains some of the options available: https://developer.android.com/guide/fragments/communicate.
The most common ways to share information between fragments are:
- Use the underlying activity, but we don't have a good way to store data there since we are not the owners.
- Use the fragment result APIs, but again, that will depend on the caller to make sure they handle the result correctly.
I'm not sure how this is handled in RN TBH, so maybe they have another recommended way...
android/build.gradle
Outdated
|
||
api "com.revenuecat.purchases:purchases:$purchases_version" | ||
api "com.revenuecat.purchases:purchases-ui:$purchases_version" |
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.
Mentioned it over slack, but this is causing issues since it's API 24+ but this module is 19+.
android/build.gradle
Outdated
@@ -45,7 +45,8 @@ android { | |||
buildToolsVersion "32.0.0" | |||
|
|||
defaultConfig { | |||
minSdkVersion 19 | |||
// TODO: revert this to 19 | |||
minSdkVersion 24 |
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.
TODO: remove!
f996dc2
to
fc31d95
Compare
Merged commit main at 7774b8a into `paywalls` --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com> Co-authored-by: Toni Rico <toni.rico.diez@revenuecat.com> Co-authored-by: RevenueCat Git Bot <72824662+RCGitBot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
||
public func paywallViewController(_ controller: PaywallViewController, | ||
didFinishPurchasingWith customerInfo: CustomerInfo) { | ||
controller.dismiss(animated: true) |
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 can remove this now, it's done automatically by PaywallView
(and works for PaywallViewController
): RevenueCat/purchases-ios#3517
This merges latest main with current paywalls branch. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com> Co-authored-by: RevenueCat Git Bot <72824662+RCGitBot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Cesar de la Vega <cesarvegaro@gmail.com>
This allows to set a `PaywallViewControllerDelegate` to the `PaywallProxy` to allow to receive purchase events in the hybrids. This is a first step to fix: RevenueCat/purchases-flutter#886 ### TODO - [x] Expose a delegate method that gets called when the PaywallViewController gets dismissed.
This is a pretty big change in terms of changes, but it's mostly moving files around. Until now, the `android` folder was both a module and a project. That's against how gradle projects are usually structured, where you have a root project and one or more modules. In this PR, we change this organization to use the proper one of having a root project with multiple modules. Now, we have a project with 2 modules: - api-tests - hybridcommon Some advantages of this: - Api tests are now actually separated from the library module, so they can more accurately reflect what API is really available to consumers. - Now we can separate the revenuecat ui library much more easily by creating a different module (Will be added in a followup PR)
This PR separates paywalls code into a different module. This way we don't force users of the main hybrid common sdk to have to bump their min version, which is a breaking change. This new dependency can be used with `implementation "com.revenuecat.purchases:purchases-hybrid-common-ui:$phc_version"`
### RevenueCatUI * Separate paywalls into a different module (#604) via Toni Rico (@tonidero) * Add PaywallFragmentViewModel to share data with caller fragment/activity (#602) via Toni Rico (@tonidero) * Paywalls: Expose delegate in PaywallProxy to receive events (#586) via Toni Rico (@tonidero) ### Other Changes * Update paywalls branch with main (#592) via Cesar de la Vega (@vegaro) * Re-organize android library (#603) via Toni Rico (@tonidero) * Update paywalls with latest main (#601) via Toni Rico (@tonidero) * Actually set delegate via NachoSoto (@NachoSoto) * Dismiss paywall automatically via NachoSoto (@NachoSoto) * `displayCloseButton` via NachoSoto (@NachoSoto) * Lint via NachoSoto (@NachoSoto) * [remove me] change `minSdkVersion` via NachoSoto (@NachoSoto) * Fix other platforms via NachoSoto (@NachoSoto) * Optional parameter via NachoSoto (@NachoSoto) * Lint via NachoSoto (@NachoSoto) * Revert SDK version change via NachoSoto (@NachoSoto) * Finished API via NachoSoto (@NachoSoto) * [WIP] `RevenueCatUI` support via NachoSoto (@NachoSoto)
This dependency was added by Android Studio's wizard and I didn't check whether it was needed (it wasn't). It actually caused a breaking change, since this dependency requires apps to target Android 34. For some reason I didn't encounter this when I was testing in flutter using `includeBuild` but was able to repro after publishing 8.1.3-beta.5. I tested this again by publishing to maven local and can confirm the fix now.
This updates the paywalls branch to the latest android version which includes this fix: RevenueCat/purchases-android#1532
Add shouldDisplayDismissButton to launchPaywall so we can add this parameter to the hybrids
Updating paywalls branch with main at 3920db6 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com> Co-authored-by: Toni Rico <toni.rico.diez@revenuecat.com> Co-authored-by: RevenueCat Git Bot <72824662+RCGitBot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
If the `shouldDisplayDismissButtonKey` was missing, the button wasn't being displayed
Added a function to create a `PaywallFooterViewController`
This supports having a listener for the result of presenting a paywall which can be used by the hybrids. Currently it's sent as a string, both in iOS and android and possible values are: - `NOT_PRESENTED` - `CANCELLED` - `PURCHASED` - `RESTORED` - `ERROR` (only in android right now)
This will be used by the `presentPaywall`/`presentPaywallIfNeeded` methods.
We didn't add api tests for the new enum added in #634. This adds those.
Avoid updating hybrids automatically whenever we make a new prerelease. We should only update the hybrids automatically on stable releases. For prereleases, I believe the bump should be performed manually
Only trigger dependent updates on stable releases
I didn't test all ways of presenting a paywall conditionally in #628. This makes sure we return `NOT_PRESENTED` in all cases where appropriate in android. In iOS we were also missing to return a value in `presentPaywallIfNeeded`.
Updating paywalls branch with 6441719 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: NachoSoto <NachoSoto@users.noreply.github.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com> Co-authored-by: Toni Rico <toni.rico.diez@revenuecat.com> Co-authored-by: RevenueCat Git Bot <72824662+RCGitBot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
### RevenueCatUI * Add API tests for paywall result enum in typescript (#636) via Toni Rico (@tonidero) * Fix paywall result not being returned (#639) via Toni Rico (@tonidero) ### Dependency Updates * [AUTOMATIC] iOS 4.31.6 => 4.31.7 (#640) via RevenueCat Git Bot (@RCGitBot) * [AUTOMATIC] Android 7.3.1 => 7.3.2 (#632) via RevenueCat Git Bot (@RCGitBot)
8.10.0-beta.10 |
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 revert this before merging.
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, are we releasing this as 8.10.0
?
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 can manually bump the version after. I was worried that the script that updates the version might miss something if we have inconsistent versions. I think it's better if this PR is just the "feature" change without version changes, etc. and use the regular flow to bump it after.
* [AUTOMATIC] iOS 4.30.5 => 4.31.1 Android 7.2.4 => 7.2.6 (#584) via RevenueCat Git Bot (@RCGitBot) | ||
* Bump cocoapods from 1.14.2 to 1.14.3 (#576) via dependabot[bot] (@dependabot[bot]) | ||
|
||
## 8.0.0 |
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 rebase to remove these changes here.
@@ -1,6 +1,6 @@ | |||
Pod::Spec.new do |s| | |||
s.name = "PurchasesHybridCommon" | |||
s.version = "7.3.3" | |||
s.version = "8.10.0-beta.10" |
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.
Revert this
s.dependency 'RevenueCat', '4.31.7' | ||
s.dependency 'RevenueCatUI', '4.31.7' |
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.
Let's rebase and make sure these are up-to-date.
POM_NAME=purchases-hybrid-common | ||
POM_PACKAGING=aar | ||
POM_ARTIFACT_ID=purchases-hybrid-common | ||
VERSION_NAME=8.10.0-beta.10 |
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.
Revert this before merging.
@@ -15,7 +15,7 @@ | |||
<key>CFBundlePackageType</key> | |||
<string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string> | |||
<key>CFBundleShortVersionString</key> | |||
<string>7.3.3</string> | |||
<string>8.10.0</string> |
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.
Revert this before merging
pod 'RevenueCat', '4.31.7' | ||
pod 'RevenueCatUI', '4.31.7' |
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.
Need to make sure these are up-to-date before merging
@@ -1,24 +1,29 @@ | |||
PODS: | |||
- Nimble (10.0.0) | |||
- Quick (5.0.1) | |||
- RevenueCat (4.30.5) | |||
- RevenueCat (4.31.6) |
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.
And re-generate these
@@ -15,7 +15,7 @@ | |||
<key>CFBundlePackageType</key> | |||
<string>$(PRODUCT_BUNDLE_PACKAGE_TYPE)</string> | |||
<key>CFBundleShortVersionString</key> | |||
<string>7.3.3</string> | |||
<string>8.10.0</string> |
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.
Revert before merging
@@ -1,7 +1,7 @@ | |||
{ | |||
"name": "@revenuecat/purchases-typescript-internal", | |||
"title": "Purchases typescript internal shared code", | |||
"version": "7.3.3", | |||
"version": "8.10.0-beta.10", |
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.
Revert before merging
Re-opened after merging: #647 |
No description provided.