Skip to content
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

Add PaywallFragmentViewModel to share data with caller fragment/activity #602

Merged
merged 3 commits into from
Dec 20, 2023

Conversation

tonidero
Copy link
Contributor

This is an attempt to be able to have some sort of completion listener from the calling fragment/activity. The idea is to use a shared view model, which doesn't need to be exposed, to share data using the underlying activity. The flow would be:

  • The calling activity/fragment would pass us a new PaywallResultListener to listen to the result of the paywall
  • Internally, we create a PaywallFragmentViewModel using the underlying activity and set that listener there
  • Then, in the PaywallFragment itself, we also get the same instance of the PaywallFragmentViewModel, giving us access to the same result listener.

Note that I haven't exposed a PaywallListener directly. The reason for that is that we're using a different activity for the paywall, so we can't use the underlying activity to communicate with that (we would need the underlying application context). This would require making the Paywall activity as a fragment instead of an activity in purchases-android.

@tonidero
Copy link
Contributor Author

Tested this in flutter. I was able to make it work!! (get the result returned through the VM)

@tonidero tonidero marked this pull request as ready for review December 20, 2023 17:25
@tonidero tonidero requested a review from a team December 20, 2023 17:25
internal class PaywallFragment(
private val requiredEntitlementIdentifier: String?,
) : Fragment(), PaywallResultHandler {
internal class PaywallFragment : Fragment(), PaywallResultHandler {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that since we're using a fragment, this does require the hybrid to use some subclass of a FragmentActivity... But most hybrids provide some of those, without having to provide our own version.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember that we found out that we needed to use an activity on Flutter, as they say that's the preferred method. Are you thinking of forcing users to use our own fragment parent class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the idea is for the developer to use FlutterFragmentActivity (provided by flutter) instead of the PurchasesFlutterActivity. That way, the current presentPaywallFromFragment method we have here should take care of launching this fragment and from there the activity.

I believe the reason we created that custom activity was to try to get a result back from the paywall activity right? @NachoSoto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be solved with the view model we're adding here

Copy link
Contributor

@NachoSoto NachoSoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this works then sounds great!

@tonidero
Copy link
Contributor Author

tonidero commented Dec 20, 2023

If this works then sounds great!

Yup it works well! The only thing I see is that the activity in Android has the title bar, which it didn't use to have, but I think that's unrelated to this change and possibly related to RevenueCat/purchases-android#1511. Want to take a look at that tomorrow

@tonidero tonidero merged commit b3a4631 into paywalls Dec 20, 2023
9 checks passed
@tonidero tonidero deleted the add-paywall-fragment-view-model branch December 20, 2023 18:56
tonidero added a commit that referenced this pull request Dec 22, 2023
### 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)
tonidero added a commit to RevenueCat/purchases-flutter that referenced this pull request Dec 22, 2023
After RevenueCat/purchases-hybrid-common#602, we
can now use the `PaywallHelpers.presentPaywallFromFragment` method to
present the paywall and receive a result back using an internal view
model.

This makes it so the `PurchasesFlutterActivity` isn't needed anymore and
we can make use of that functionality, shared with other hybrids.

Note that this does mean developers need to make sure their android
activity inherits from `FlutterFragmentActivity` (or other
`FragmentActivity`) and not `FlutterActivity`, as we need to present the
fragment.

### TODO
- [x] Hide action bar:
RevenueCat/purchases-android#1532
- [x] Update PHC with those 2 changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants