Skip to content

PaywallActivityLauncher: new constructor for a generic ActivityResultCaller#1441

Merged
NachoSoto merged 2 commits into
mainfrom
paywalls-present-if-needed-activity-result-caller
Nov 8, 2023
Merged

PaywallActivityLauncher: new constructor for a generic ActivityResultCaller#1441
NachoSoto merged 2 commits into
mainfrom
paywalls-present-if-needed-activity-result-caller

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Improvements:

  • Remove duplicated registerForActivityResult call
  • Allow using any instance, like AppCompatActivity

…ultCaller`

#### Improvements:
- Remove duplicated `registerForActivityResult` call
- Allow using any instance, like `AppCompatActivity`
@NachoSoto NachoSoto requested a review from a team November 6, 2023 19:16
@NachoSoto

Copy link
Copy Markdown
Contributor Author

I think I need this for ReactNative paywalls.

@codecov

codecov Bot commented Nov 6, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9219386) 83.99% compared to head (6faccc5) 83.99%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1441   +/-   ##
=======================================
  Coverage   83.99%   83.99%           
=======================================
  Files         203      203           
  Lines        6717     6717           
  Branches      975      975           
=======================================
  Hits         5642     5642           
  Misses        698      698           
  Partials      377      377           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

/**
* Creates a new PaywallActivityLauncher from an [ActivityResultCaller] instance.
*/
constructor(resultCaller: ActivityResultCaller, resultHandler: PaywallResultHandler) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm considering both ComponentActivity and Fragment implement ActivityResultCaller, we could potentially remove them and just leave this constructor... Originally I thought to separate them so it's easier to understand, so I think it's better to leave them though... We can reconsider later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good point, I removed the other 2 since they're redundant.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tonidero can you confirm this looks good to you?

@NachoSoto NachoSoto requested a review from tonidero November 7, 2023 23:51

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

@NachoSoto NachoSoto merged commit 36f8da8 into main Nov 8, 2023
@NachoSoto NachoSoto deleted the paywalls-present-if-needed-activity-result-caller branch November 8, 2023 06:18
NachoSoto added a commit that referenced this pull request Nov 8, 2023
**This is an automatic release.**

### RevenueCatUI
* `PaywallActivityLauncher`: new constructor for a generic
`ActivityResultCaller` (#1441) via NachoSoto (@NachoSoto)
* Improve fullscreen templates in landscape orientation (#1435) via Toni
Rico (@tonidero)
* `Paywalls`: improve Japanese localization (#1439) via NachoSoto
(@NachoSoto)
### Other Changes
* Remove side effect from setting purchasesUpdatedListener (#1443) via
Cesar de la Vega (@vegaro)
* Paywalls: Store paywall events on disk and API (1) (#1436) via Toni
Rico (@tonidero)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
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