-
Notifications
You must be signed in to change notification settings - Fork 338
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
Created PaywallExtensions
: StoreView
and SubscriptionStoreView
overloads for Offering
#2593
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2593 +/- ##
==========================================
- Coverage 86.29% 86.24% -0.05%
==========================================
Files 205 205
Lines 14334 14334
==========================================
- Hits 12369 12362 -7
- Misses 1965 1972 +7 |
} | ||
} else { | ||
ProgressView() | ||
.progressViewStyle(.circular) |
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.
2 questions:
- Will the
ProgressView
use the full available space? Or more importantly, will the transition between this and theSubscriptionStoreView
once it's loaded look good? - Did you/should we get a design review on this?
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.
Yeah this is very WIP.
I'm going to extract CurrentOfferingSubscriptionStoreView
into a separate PR so we can merge the rest, and handle this and the offering failure separately.
} | ||
} | ||
.task { | ||
if let offering = try? await Purchases.shared.offerings().current { |
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.
So if getting offerings fail, we just keep the loading state? I think we should show some error UI. Thoughts?
SubscriptionStoreView(offering: currentOffering, | ||
marketingContent: marketingContent) | ||
} else { | ||
SubscriptionStoreView(offering: currentOffering) |
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.
A couple more questions about these views:
- When purchasing through this view, does it automatically finish the transaction? Or does the developer still have to do it?
- If the user purchases an offering through this view, we won't be getting the
presentedOfferingIdentifier
setup in the SDK... We need to figure out a way to set that.
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.
Will add to this, that anything we do in the purchasePackage/Product
methods won't happen when they use this view. We should check if there are any other unintended side effects of this. Another one I see is that we would not be calling preventPurchasePopupCallFromTriggeringCacheRefresh
so we would never set the offerings cache as stale I think.
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.
When purchasing through this view, does it automatically finish the transaction? Or does the developer still have to do it?
Our SDK handles that :) (though we do need SDK-3170)
If the user purchases an offering through this view, we won't be getting the presentedOfferingIdentifier setup in the SDK... We need to figure out a way to set that.
Oh great call. Just filed SDK-3176 for that.
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.
Will add to this, that anything we do in the purchasePackage/Product methods won't happen when they use this view. We should check if there are any other unintended side effects of this.
I think that's fine. This is basically like our SDK acting in observer mode for a purchase being done from outside.
Another one I see is that we would not be calling
preventPurchasePopupCallFromTriggeringCacheRefresh
so we would never set the offerings cache as stale I think.
Another reason to do SDK-3172 and get rid of that method.
2f8a4b2
to
5ba1822
Compare
@tonidero updated this to leave just the overloads, and listed the 3 tasks that need to be done (working on those today) |
…criptionStoreView` Follow up to #2593.
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.
Looks great! As you said we can work on future improvements in other PRs
5ba1822
to
d747701
Compare
…toreView` (#2595) See #2593 
**This is an automatic release.** ### New Features * New `DebugViewController`: UIKit counterpart for SwiftUI's `debugRevenueCatOverlay` (#2631) via NachoSoto (@NachoSoto) * Created `PaywallExtensions`: `StoreView` and `SubscriptionStoreView` overloads for `Offering` (#2593) via NachoSoto (@NachoSoto) * Introduced `debugRevenueCatOverlay()`: new SwiftUI debug overlay (#2567) via NachoSoto (@NachoSoto) ### Bugfixes * Removed `preventPurchasePopupCallFromTriggeringCacheRefresh`, update caches on `willEnterForeground` (#2623) via NachoSoto (@NachoSoto) * Fixed `Catalyst` build with `Xcode 15 beta 1` (#2586) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump danger from 9.3.0 to 9.3.1 (#2592) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `StoreTransaction`: added new `Storefront` to API testers (#2634) via NachoSoto (@NachoSoto) * `DebugView`: added snapshot tests (#2630) via NachoSoto (@NachoSoto) * `verifyNoUnfinishedTransactions`/`verifyUnfinishedTransaction`: added missing `#file` parameter (#2625) via NachoSoto (@NachoSoto) * `PostReceiptDataOperation`: clean up cache key (#2628) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: also get `Storefront` from SK1 (#2629) via NachoSoto (@NachoSoto) * `CI`: disable iOS 17 for now (#2627) via NachoSoto (@NachoSoto) * `Tests`: fixed crash on iOS 13 (#2624) via NachoSoto (@NachoSoto) * `StoreTransaction`: read `Storefront` from `StoreKit.Transaction` (#2611) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`/`BaseStoreKitIntegrationTests`: also clear transactions after every test (#2616) via NachoSoto (@NachoSoto) * `ErrorCode.networkError`: improved description (#2610) via NachoSoto (@NachoSoto) * `PurchaseTester`: make CI job always point to current version (#2622) via NachoSoto (@NachoSoto) * Improved `finishAllUnfinishedTransactions` (#2615) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: improved `waitForStoreKitTestIfNeeded` (#2614) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: set `continueAfterFailure` to `false` (#2617) via NachoSoto (@NachoSoto) * `PaywallExtensions`: fixed compilation (#2613) via NachoSoto (@NachoSoto) * `CI`: added `iOS 17` job (#2591) via NachoSoto (@NachoSoto) * `Encodable.jsonEncodedData`: fixed tests on iOS 17 due to inconsistent key ordering (#2607) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: added ability to display new `SubscriptionStoreView` (#2595) via NachoSoto (@NachoSoto) * Refactor: extracted all log strings (#2600) via NachoSoto (@NachoSoto) * Changed tests to work around `URL` decoding differences in `iOS 17` (#2605) via NachoSoto (@NachoSoto) * Removed unnecessary `Strings.trimmedOrError` (#2601) via NachoSoto (@NachoSoto) * Fixed test compilation with `Xcode 15` (#2602) via NachoSoto (@NachoSoto) * Tests: added `iOS 17` snapshots (#2603) via NachoSoto (@NachoSoto) * `StoreProductDiscount`: added `description` (#2604) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay` improvements (#2594) via NachoSoto (@NachoSoto) * `Xcode 15`: fixed all documentation warnings (#2596) via NachoSoto (@NachoSoto) * `StoreKitObserverModeIntegrationTests`: fixed and disabled SK2 `testPurchaseInDevicePostsReceipt` (#2589) via NachoSoto (@NachoSoto) * `StoreKit2TransactionListener`: added log when receiving `Transactions.Updates` (#2588) via NachoSoto (@NachoSoto) * `Dictionary.MergeStrategy`: simplify implementation (#2587) via NachoSoto (@NachoSoto) * `Configuration.Builder`: fixed doc reference (#2583) via NachoSoto (@NachoSoto) * `APITesters`: available since iOS 11 (#2581) via NachoSoto (@NachoSoto)
Description
This adds overloads, but there's still a few things pending for the SDK to fully support these:
presentedOfferingIdentifier
when displaying StoreKit2 paywalls: SDK-3176preventPurchasePopupCallFromTriggeringCacheRefresh
: SDK-3172StoreKit2TransactionListener
even on SK1 mode: SDK-3170Example: