-
Notifications
You must be signed in to change notification settings - Fork 320
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
Remove presentCodeRedemptionSheet from PaymentQueueWrapper & Use SK2 APIs #4378
base: main
Are you sure you want to change the base?
Remove presentCodeRedemptionSheet from PaymentQueueWrapper & Use SK2 APIs #4378
Conversation
…per' of https://github.com/RevenueCat/purchases-ios into remove-presentCodeRedemptionSheet-from-paymentqueuewrapper
…d for ipad on macos
} | ||
|
||
guard let windowScene else { | ||
Logger.error(Strings.storeKit.error_displaying_offer_code_redemption_sheet_no_window_scene) |
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.
It might be a little awkward that this is possible, but I thought it was important to keep the provided window scene optional for two reasons:
- So that developers using SwiftUI can use easily the API
- To keep the new
Purchases.presentCodeRedemptionSheet
API as similar as possible to the deprecated one
Sources/Purchasing/StoreKitAbstractions/OfferCodeRedemptionSheetPresenter.swift
Outdated
Show resolved
Hide resolved
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.
generally looks great! Left one question
* "`This function doesn’t affect Mac apps built with Mac Catalyst.`" | ||
* when, in fact, it crashes when called both from Catalyst and also when running as "Designed for iPad". | ||
* This is why RevenueCat's SDK makes it unavailable in mac catalyst. | ||
*/ |
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.
Does this mean calling this function of the SDK has been crashing on Catalyst and iOS apps running on macOS?
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.
For Catalyst, nope! Luckily the old version of presentCodeRedemptionSheet()
was marked as unavailable on Catalyst (see the definition in the current main branch here.
For "Designed for iPad" on macOS, I haven't tested it myself, but I wouldn't be surprised if the existing SDK function is crashing, since this PR introduces the isiOSAppOnMac
check for the first time in the code redemption sheet flow
@@ -679,6 +679,28 @@ public protocol PurchasesType: AnyObject { | |||
|
|||
#if os(iOS) || VISION_OS |
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.
Was the presentCodeRedemptionSheet
function moved inside this conditional intentionally? I think this might be a breaking change since it changes where the function is compiled (even if unavailable) so you cannot use runtime availability checks with it.
…app_extension message
Motivation
As a part of supporting win-back offers with Streamlined Purchasing disabled, we need to ensure that we aren't using both the
PurchaseIntents
API and thepaymentQueue(_:shouldAddStorePayment:for:)
functions at the same time to comply with this warning from Apple's PurchaseIntent docs:To meet this requirement, we want to ensure that we're not using the
PaymentQueueWrapper
class when on iOS >=16.4 (when the PurchaseIntents API was introduced) and the SDK is configured to use StoreKit2.Description
This PR removes the
presentCodeRedemptionSheet
function from thePaymentQueueWrapperType
and replaces its usages with a newOfferCodeRedemptionSheetPresenter
struct.API Changes
The PR deprecates the existing public
Purchases.presentCodeRedemptionSheet()
function and replaces it with a new function that more closely matches Apple's new API for presenting offer code redemption sheets:The notable changes for this function signature include:
UIWindowScene
that allows the developer to specify which scene to present the sheet over. If one is not provided, we display the sheet over the currently active scene.Mac Catalyst & "Designed for iPad" Apps on macOS
According to Apple's presentOfferCodeRedeemSheet docs:
In our testing, the
StoreKitError.unknown
error is also thrown on iOS apps running as "Designed for iPad" on macOS. Due to this error, our new functions:Implications for the Hybrids
For now, the hybrids should be okay to continue using the deprecated
presentCodeRedemptionSheet
function. We can update them to use the newpresentCodeRedemptionSheet
API by doing the following:Task
uiWindowScene
and just presenting over the currently active window scenePresenting the Offer Code Redemption Sheet on macOS
Apple introduced a new API for showing the code redemption sheet on native (non-iOS, non-catalyst) macOS apps. To keep the scope of this PR down, that functionality will be added in a follow-up PR.