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

Paywalls: open Terms and Privacy Policy links in-app #3475

Merged
merged 5 commits into from
Dec 8, 2023

Conversation

aboedo
Copy link
Member

@aboedo aboedo commented Nov 27, 2023

Updated the terms and Privacy Policy links to open within the app, without exiting to Safari

RocketSim_Recording_iPhone_15_Pro_Max_6.7_2023-12-08_10.42.51.mp4

@aboedo aboedo requested a review from a team November 27, 2023 18:24
@aboedo aboedo self-assigned this Nov 27, 2023
@rkotzy
Copy link
Member

rkotzy commented Nov 27, 2023

Since we don't know what the navigation stack looks like, what if we always open the links in a fullscreen modal? We could use the system setting for light/dark mode to set a light/dark header.

I think this approach would work with the footer views too since we're always overlaying the fullscreen, and the close action is always dismissing the view. No navigation push/pop.

@tonidero
Copy link
Contributor

what if we always open the links in a fullscreen modal?

I agree that makes more sense. Just not sure if you can present a modal on top of a modal, in case the developer is presenting the paywall as a modal already?

@aboedo
Copy link
Member Author

aboedo commented Nov 28, 2023

Yeah, the modal thing might feel weird if you're already in a modal. One alternative is to make this remotely controllable from a variable

@NachoSoto
Copy link
Contributor

How does this look on iPad, and visionOS?

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.

I think this makes sense and it's a better UX 👍🏻

@@ -257,6 +257,23 @@ private struct LinkButton: View {

}

@available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *)
@available(watchOS, unavailable)
Copy link
Contributor

Choose a reason for hiding this comment

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

FooterView is used on watchOS too, so I think we need to keep the existing implementation for watchOS

Copy link
Contributor

@NachoSoto NachoSoto Dec 6, 2023

Choose a reason for hiding this comment

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

And maybe visionOS and Catalyst?

return WKWebView()
}

func updateUIView(_ uiView: WKWebView, context: Context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When the state of your app changes, SwiftUI updates the portions of your interface affected by those changes. SwiftUI calls this method for any changes affecting the corresponding UIKit view. Use this method to update the configuration of your view to match the new state information provided in the context parameter.

Since this doesn't depend on the context, we should load the url in makeUIView. Otherwise this might be loaded multiple times unnecessarily.

RevenueCatUI/Views/FooterView.swift Show resolved Hide resolved
@NachoSoto
Copy link
Contributor

Just watched the video. Perhaps we should present this as a sheet. Right now it shows a navigation bar which doesn't make a lot of sense since the controller doesn't have a title.

@@ -13,6 +13,7 @@

import RevenueCat
import SwiftUI
import WebKit
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't exist on watchOS 😅

}
}
#else
Link(destination: self.url) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping the existing implementation if WebKit isn't supported.

@NachoSoto
Copy link
Contributor

Updated to use a sheet. Presenting a sheet on top of a sheet is a common pattern supported by iOS (see video)

@NachoSoto NachoSoto changed the title Paywalls: open Terms and Privacy Policy links in-app Paywalls: open Terms and Privacy Policy links in-app Dec 8, 2023
@NachoSoto NachoSoto enabled auto-merge (squash) December 8, 2023 18:49
@NachoSoto NachoSoto merged commit 2a9e190 into main Dec 8, 2023
20 checks passed
@NachoSoto NachoSoto deleted the andy/open_toc_links_in_app branch December 8, 2023 21:46
NachoSoto added a commit that referenced this pull request Dec 8, 2023
Follow up to #3475.
This new implementation doesn't make sense (and doesn't work very well) on Catalyst. It will now fall back to the previous implementation and open the link on a new window (using the default system browser).
NachoSoto pushed a commit that referenced this pull request Dec 12, 2023
**This is an automatic release.**

### RevenueCatUI
* `Paywalls`: improve image caching (#3498) via NachoSoto (@NachoSoto)
* `Paywalls`: change style of CTA button to be consistent with other
platforms (#3507) via NachoSoto (@NachoSoto)
* `Paywalls`: open footer links on Safari on Catalyst (#3508) via
NachoSoto (@NachoSoto)
* `Paywalls`: fix compilation on Xcode < 14.3 (#3513) via NachoSoto
(@NachoSoto)
* Fix typo in zh-Hans localization of RevenueCatUI (#3512) via Francis
Feng (@francisfeng)
* `Paywalls`: fix PaywallViewControllerDelegate access from Objective-C
(#3510) via noncenz (@noncenz)
* `Paywalls`: open Terms and Privacy Policy links in-app (#3475) via
Andy Boedo (@aboedo)
* `Paywalls`: fix empty description when using `custom` package type and
`{{ sub_period }}` (#3495) via Andy Boedo (@aboedo)
* `Paywalls`: use `HEIC` images (#3496) via NachoSoto (@NachoSoto)
* Paywalls: disable the close button when an action is in progress
(#3474) via Andy Boedo (@aboedo)
* `Paywalls`: adjusted German translations (#3476) via Tensei (@tensei)
* Paywalls: Improve Chinese localization (#3489) via Andy Boedo
(@aboedo)
### Other Changes
* `CircleCI`: add git credentials to snapshot generation (#3506) via
NachoSoto (@NachoSoto)
* `StoreProduct`: improve `priceFormatter` documentation (#3500) via
NachoSoto (@NachoSoto)
* `Paywalls`: fix tests (#3499) via NachoSoto (@NachoSoto)
* `Optimization`: changed `first` to `first(where:)` (#3467) via
NachoSoto (@NachoSoto)
@stuart-j-williams
Copy link

This is broken on iPad as it opens the target in the sidebar of a split screen. Do I need to log a new issue for this? Thanks
IMG_0003 2

@NachoSoto
Copy link
Contributor

Oh thanks! We'll get that fixed 🙏🏻

@NachoSoto
Copy link
Contributor

@stuart-j-williams I'm unable to reproduce this, could you please file an issue including:

  • iOS version
  • Xcode version
  • Code with how you're presenting the paywall.

Thanks!

NachoSoto added a commit that referenced this pull request Dec 18, 2023
NachoSoto added a commit that referenced this pull request Dec 18, 2023
These seemed to be the default before, but it broke with #3475.
NachoSoto added a commit that referenced this pull request Dec 19, 2023
These seemed to be the default before, but it broke with #3475.

Difference (before and after):
![Screenshot 2023-12-18 at 15 09
51](https://github.com/RevenueCat/purchases-ios/assets/685609/1e2e8c69-51b0-4500-8e30-28e4c357907d)

This restores the layout shown on the left side.
NachoSoto added a commit that referenced this pull request Dec 19, 2023
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.

5 participants