Skip to content

Paywalls: Add dismissal method in PaywallViewControllerDelegate#3493

Merged
tonidero merged 3 commits into
mainfrom
toni/add-paywall-dismiss-delegate-method
Dec 18, 2023
Merged

Paywalls: Add dismissal method in PaywallViewControllerDelegate#3493
tonidero merged 3 commits into
mainfrom
toni/add-paywall-dismiss-delegate-method

Conversation

@tonidero

@tonidero tonidero commented Dec 5, 2023

Copy link
Copy Markdown
Contributor

Description

This adds a new method to the PaywallViewControllerDelegate to indicate when the PaywallViewController is dismissed.

This will be useful to listen to responses in the hybrids, since we need to present the paywall and wait until it's been purchased/dismissed before returning a value for the hybrids.

This will be needed to fix: RevenueCat/purchases-flutter#886


public override func viewDidDisappear(_ animated: Bool) {
if isBeingDismissed {
self.delegate?.paywallViewControllerDismissed?(self)

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.

I'm not certain this is the best way to do this. Afaik, in iOS, the presenter is usually in charge of dismissing the presented VC, but not sure if the same applies to modals, where it can be dismissed by user interaction. And we want to know when it's dismissed so the hybrids can return a result at that point in time. Any feedback would be welcome @aboedo @NachoSoto

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the footer wouldn't own the modal anyway, right? I think we should just leave it up to the owner of the view that has a footer in it to dismiss the whole thing, which in turn will kill the footer

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.

Right, this is a bit confusing, but this fix is for the old approach of presenting the paywall as a modal in the hybrids and fix the issue I mentioned in the description... But yeah, if we end up moving to using views, this becomes unnecessary.

@tonidero tonidero force-pushed the toni/add-paywall-dismiss-delegate-method branch from ba41e5b to ec31b28 Compare December 5, 2023 14:37
@codecov

codecov Bot commented Dec 5, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ad0f9d9) 86.06% compared to head (ec31b28) 86.07%.
Report is 18 commits behind head on main.

❗ Current head ec31b28 differs from pull request most recent head c459bad. Consider uploading reports for the commit c459bad to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3493      +/-   ##
==========================================
+ Coverage   86.06%   86.07%   +0.01%     
==========================================
  Files         237      237              
  Lines       17211    17211              
==========================================
+ Hits        14812    14814       +2     
+ Misses       2399     2397       -2     

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

@NachoSoto NachoSoto 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.

This makes sense 👍

Comment thread RevenueCatUI/UIKit/PaywallViewController.swift Outdated
Comment thread RevenueCatUI/UIKit/PaywallViewController.swift Outdated
@tonidero tonidero marked this pull request as ready for review December 18, 2023 11:37
@tonidero tonidero requested a review from NachoSoto December 18, 2023 11:37
Comment thread RevenueCatUI/UIKit/PaywallViewController.swift Outdated
@tonidero tonidero requested a review from NachoSoto December 18, 2023 16:40
@tonidero tonidero merged commit 61a38d3 into main Dec 18, 2023
@tonidero tonidero deleted the toni/add-paywall-dismiss-delegate-method branch December 18, 2023 17:23
NachoSoto pushed a commit that referenced this pull request Dec 20, 2023
**This is an automatic release.**

### RevenueCatUI
* `Paywalls`: add `PaywallFooterViewController` (#3486) via Toni Rico
(@tonidero)
* `Paywalls`: improve landscape support of all templates (#3471) via
NachoSoto (@NachoSoto)
* `Paywalls`: ensure footer links open in full-screen sheets (#3524) via
NachoSoto (@NachoSoto)
* `Paywalls`: improve `FooterView` text alignment (#3525) via NachoSoto
(@NachoSoto)
* Paywalls: Add dismissal method in `PaywallViewControllerDelegate`
(#3493) via Toni Rico (@tonidero)
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.

presentPaywallIfNeeded always return Null

3 participants