UIViewRepresentable for embedded #4520

porter-stripe · 2025-01-31T00:36:34Z

Summary

Adds EmbeddedViewRepresentable which is a SwiftUI view that can display the embedded payment element's view in SwiftUI
Adds EmbeddedPaymentElementView, which is the public API of the EmbeddedViewRepresentable
Adds a snapshot test for it

Motivation

Embedded SwiftUI

Testing

Manual
New snapshots

Changelog

N/A

porter-stripe · 2025-01-31T15:11:29Z

...PaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedViewRepresentable.swift

+    }
+}
+
+extension UIViewController {


I tested this VC stuff out in a very barebones SwiftUI app and confirmed it works. https://github.com/porter-stripe/swift-ui-embedded/tree/main

porter-stripe · 2025-01-31T17:15:46Z

...mentSheet/StripePaymentSheetTests/PaymentSheet/EmbeddedPaymentMethodsViewSnapshotTests.swift

@@ -74,7 +74,7 @@ class EmbeddedPaymentMethodsViewSnapshotTests: STPSnapshotTestCase {
        appearance.embeddedPaymentElement.row.additionalInsets = 20

        let embeddedView = EmbeddedPaymentMethodsView(initialSelection: nil,
-                                                      paymentMethodTypes: [.stripe(.card), .stripe(.cashApp), .stripe(.klarna)],
+                                                      paymentMethodTypes: [.stripe(.card), .stripe(.cashApp), .stripe(.afterpayClearpay)],


Context on this change: https://stripe.slack.com/archives/C02HQBW38JX/p1738343322384659

yuki-stripe · 2025-02-03T17:48:03Z

...ntSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement+SwiftUI.swift

+    private func calculateAndPublishHeight() {
+        guard let embeddedPaymentElement else { return }


nit: take an embeddedPaymentElement and we can remove the need to think (eg wonder how we end up in that situation, if we should assert, etc) about it being nil.

yuki-stripe · 2025-02-03T17:54:42Z

...PaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedViewRepresentable.swift

+}
+
+extension UIViewController {
+    var topMostViewController: UIViewController {


We have a couple methods already that do this (search "topmost" in the repo), should we use one of them?

Yeah, I tried the one in StripeUICore, but it actually doesn't work, it can't find the topmost VC and hits an assert that we do not have a presenting VC. We have a few others in FinancialConnections and one in the PaymentSheet+SwiftUI. All three of these use pretty much the same logic as the StripeUICore one. They don't seem to account for NavigationControllers or TabBarControllers, which may be the issue.

I think it'd be best to consolidate all our logic for this kind of work, but I'm a little worried about changing it for the others in this PR and verifying we didn't break anything because I'm not sure of the context they're being used in. Apparently, they work for their use cases, but makes me question if they actually work given they don't work in our example app. Maybe a run ticket to investigate the others and see if they can be updated safely? WDYT?

Sounds good, but can you own following up? I want to try and leave less tech debt behind in projects and I'm not sure there's a reason to punt on this one. Do we not know why this method works but the StripeUICore one doesn't?

Based on quick debugging, it appears that failing to handle the NavigationController case is causing the failure.

if let nav = self as? UINavigationController { // Use visibleViewController for navigation stacks return nav.visibleViewController?.topMostViewController ?? nav

I updated the SwiftUICore implementation to handle this case now, I did a quick test of everywhere that calls this function and things appear fine.

yuki-stripe · 2025-02-03T17:57:01Z

...ntSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement+SwiftUI.swift

+
+    public var body: some View {
+        EmbeddedViewRepresentable(viewModel: viewModel)
+            .frame(height: viewModel.height)


Curious to understand this better (is this the standard way to deal with UIViews that can change size? what happens if you don't do this?), maybe we can chat live.

If you remove this line SwiftUI thinks our view has a height of zero, so the layout of things is totally busted! SwiftUI doesn't seem to play very nice when it comes to view sizing and AutoLayout. It looks at the intrinsicContentSize of the view to determine the height (if you don't set it explicitly like we're doing). The intrinsicContentSize of our embedded view always has a height of zero, I believe this is b/c we use AutoLayout to handle the height. The width is computed correctly on the intrinsicContentSize but height is always zero.

An alternative approach would be to define the intrinsicContentSize on the container view and give it a non-zero height, we could compute the height the same way we currently do. This works and the view sizes itself correctly, however the tradeoff becomes that there doesn't seem to be a way to animate these height changes smoothly. The view jumps between heights.

Those are the two standard ways I've found, here's someone proposing what I am for a similar question. What I have in the PR seems to be the most reliable from what I have found, providing smooth animations.

There is one another option sizeThatFits(_:uiView:context:). But requires iOS 16+, and I haven't found it to be super easy to use, mainly due to the "proposed size" being somewhat confusing/not explained well in the docs. We can chat more about it.

Here's what sizeThatFits(_:uiView:context:)
look like, then we remove the .height you've flagged. The implementation is a more complex IMO, but maybe there's a better way to do it. This gives us a nice animation too.

@available(iOS 16.0, *) func sizeThatFits(_ proposal: ProposedViewSize, uiView: UIView, context: Context) -> CGSize? { guard let embeddedView = uiView.subviews.first else { return nil } // Create a target size from the proposal. // If no width or height is provided, fall back to UIView's compressed size. let targetSize = CGSize( width: proposal.width ?? UIView.layoutFittingCompressedSize.width, height: UIView.layoutFittingCompressedSize.height ) // Determine fitting priorities: // If a dimension isn’t specified, use a lower priority to allow flexibility. let horizontalPriority: UILayoutPriority = proposal.width == nil ? .defaultLow : .required let verticalPriority: UILayoutPriority = proposal.height == nil ? .defaultLow : .required // Use the embedded view's Auto Layout to determine its optimal size. return embeddedView.systemLayoutSizeFitting( targetSize, withHorizontalFittingPriority: horizontalPriority, verticalFittingPriority: verticalPriority ) }

Thanks for the details! Our view should have a valid intrinsicContentSize but interestingly it sounds like that makes SwiftUI automatically lay out the superview without animation. Is that right?

Yeah, intrinsicContentSize seems not viable due to animation limitations. Given that, I think it's between what is proposed in the PR or the iOS 16 approach commented above.

With intrinsicContentSize our view does resize properly, but I can't find any way to make it animate smoothly. It's abrupt height changes.

I think I'm leaning towards the iOS 14 compatible approach, the implementation is easier to understand IMO. Lmk what you think, we can also discuss during co-working hours too.

Yep agreed. It's a shame that correctly defining intrinsicContentSize on our embedded UIKit view causes SwiftUI to automatically lay out when it changes (good) but not animate with no way to make it animated.

We should override intrinsicContentSize in our embedded view, make it return super.intrinsicContentSize, and add a comment explaining why we can't actually implement it correctly. O/w some enterprising future Stripe might implement it and not realize.

cc @davidme-stripe fyi

Good idea, I added the comment in both the EmbeddedPaymentMethodsView and EmbeddedPaymentElementContainerView.

I also confirmed our snapshot tests added in this PR would catch this.

Oh woah how does the snapshot test catch it? I'd expect the final snapshot to be the same, regardless of whether it was animated or not.

Oh you're probably right, I hardcoded in a bad size of 200x200 to test it, but if it had the actual "correct" size it probably wouldn't catch it!

yuki-stripe · 2025-02-03T17:58:13Z

...PaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedViewRepresentable.swift

+
+    init(viewModel: EmbeddedPaymentElementViewModel) {
+        self.viewModel = viewModel
+        STPAnalyticsClient.sharedClient.addClass(toProductUsageIfNecessary: EmbeddedSwiftUIProduct.self)


Why add it here vs the EmbeddedPaymentElementViewModel init?

I very slightly prefer to put this here, since it's an internal class, and keep the public EmbeddedPaymentElementViewModel init empty. In an effort to keep our public classes a bit cleaner in case anyone is looking through them, kinda like EmbeddedPaymentElement+Internal motivation.

I see. I haven't really thought about that before. Like, a developer is looking at our source code but doesn't want to see our implementation?

I think conceptually it makes more sense in the EmbeddedPaymentElementViewModel init, since that is the class that best maps to the product. It's possible for a checkout to use Embedded without ever initializing this view if they have a FlowController-style UX and the customer never changes their payment method, in which case we'd currently fail to report product usage.

Putting it in EmbeddedPaymentElementViewModel also lets us get rid of the EmbeddedSwiftUIProduct workaround class, too.

Hmm, I see, I'll move it.

yuki-stripe · 2025-02-03T17:59:23Z

...PaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedViewRepresentable.swift

+        containerView.backgroundColor = .clear
+        containerView.layoutMargins = .zero
+
+        guard let embeddedPaymentElement = viewModel.embeddedPaymentElement else { return containerView }


Should this error?

Yeah, that's a good thought. I think it should! This case should only happen if you show the EmbeddedPaymentElementView in your SwiftUI view before isLoaded is true on the view model. According to the docs makeUIView is only ever called once, so this looks safe and appears so from testing in an example integration.

yuki-stripe · 2025-02-03T19:35:35Z

...PaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedViewRepresentable.swift

+// MARK: UIWindow and UIViewController helpers
+
+extension UIWindow {
+    static var current: UIWindow? {


This one is also similar to stp_hackilyFumbleAroundUntilYouFindAKeyWindow

Ah didn't see that, I tested this out in our example app and in the barebones SwiftUI app and it works!

yuki-stripe · 2025-02-04T19:57:29Z

...ntSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElement+SwiftUI.swift

@@ -50,7 +51,9 @@ import Combine
    // MARK: - Public APIs

    /// Creates an empty view model. Call `load` to initialize the `EmbeddedPaymentElementViewModel`
-    public init() {}
+    public init() {
+        STPAnalyticsClient.sharedClient.addClass(toProductUsageIfNecessary: SwiftUIProduct.self)


Let's pass self and get rid of SwiftUIProduct?

...et/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentElementContainerView.swift

...aymentSheet/StripePaymentSheet/Source/PaymentSheet/Embedded/EmbeddedPaymentMethodsView.swift

…dded/EmbeddedPaymentElementContainerView.swift Co-authored-by: Yuki <[email protected]>

…dded/EmbeddedPaymentMethodsView.swift Co-authored-by: Yuki <[email protected]>

porter-stripe added 3 commits January 30, 2025 15:58

Start on SwiftUI/UIViewRepresentable

dcd15d0

Remove some SwiftUI hacks

924c06d

Create a helper function

f7d0787

porter-stripe added the embedded label Jan 31, 2025

Improve VC traversing

b2aaa7c

porter-stripe commented Jan 31, 2025

View reviewed changes

porter-stripe added 3 commits January 31, 2025 08:20

Improve test

63175e0

Find a better way to disable animations

c4fa13a

Remove StripeCore import

4fdb385

stripe deleted a comment from github-actions bot Jan 31, 2025

porter-stripe and others added 4 commits January 31, 2025 08:50

Merge branch 'master' into porter/MOBILESDK-3001

2370543

Update [email protected]

4145d0a

Swap test to use Afterpay

7c6a892

Merge branch 'master' into porter/MOBILESDK-3001

076f84e

porter-stripe commented Jan 31, 2025

View reviewed changes

porter-stripe added 4 commits January 31, 2025 10:17

Use Afterpay in snapshots

2f6c6a6

Re-record snapshots

8973855

Don't make window key and visible

bcb89c5

Update test name

ab7fde2

porter-stripe marked this pull request as ready for review January 31, 2025 20:48

porter-stripe requested review from a team as code owners January 31, 2025 20:48

porter-stripe requested a review from yuki-stripe January 31, 2025 20:53

porter-stripe assigned yuki-stripe Jan 31, 2025

yuki-stripe reviewed Feb 3, 2025

View reviewed changes

porter-stripe added 2 commits February 3, 2025 11:24

Pass embeddedPaymentElement instead of unwrapping

1a1eee5

Assert on integration error

cb7e290

yuki-stripe reviewed Feb 3, 2025

View reviewed changes

porter-stripe and others added 8 commits February 3, 2025 12:41

Move product usage

baf977c

Import StripeCore

f902d80

Use stp_hackilyFumbleAroundUntilYouFindAKeyWindow instead

8b98616

Consolidate and improve UIViewController traversing

4c21ed7

Merge branch 'master' into porter/MOBILESDK-3001

71d1fb0

Add comment to intrinsicContentSize

e4b4a51

Update comment

f339af1

Merge branch 'master' into porter/MOBILESDK-3001

e50396a

yuki-stripe reviewed Feb 4, 2025

View reviewed changes

porter-stripe and others added 3 commits February 4, 2025 13:03

Send EmbeddedPaymentElementViewModel product usage

953ce47

Update StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embe…

90b2572

…dded/EmbeddedPaymentElementContainerView.swift Co-authored-by: Yuki <[email protected]>

Update StripePaymentSheet/StripePaymentSheet/Source/PaymentSheet/Embe…

b37bcdc

…dded/EmbeddedPaymentMethodsView.swift Co-authored-by: Yuki <[email protected]>

porter-stripe enabled auto-merge (squash) February 4, 2025 21:11

yuki-stripe approved these changes Feb 4, 2025

View reviewed changes

porter-stripe merged commit c597258 into master Feb 4, 2025
6 checks passed

porter-stripe deleted the porter/MOBILESDK-3001 branch February 4, 2025 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UIViewRepresentable for embedded #4520

UIViewRepresentable for embedded #4520

porter-stripe commented Jan 31, 2025 •

edited

Loading

porter-stripe Jan 31, 2025

porter-stripe Jan 31, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025 •

edited

Loading

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025 •

edited

Loading

yuki-stripe Feb 3, 2025

porter-stripe Feb 4, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025 •

edited

Loading

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 3, 2025

porter-stripe Feb 3, 2025

yuki-stripe Feb 4, 2025

porter-stripe Feb 4, 2025

		private func calculateAndPublishHeight() {
		guard let embeddedPaymentElement else { return }

UIViewRepresentable for embedded #4520

UIViewRepresentable for embedded #4520

Conversation

porter-stripe commented Jan 31, 2025 • edited Loading

Summary

Motivation

Testing

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

porter-stripe Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

porter-stripe Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuki-stripe Feb 3, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

porter-stripe commented Jan 31, 2025 •

edited

Loading

porter-stripe Feb 3, 2025 •

edited

Loading

porter-stripe Feb 3, 2025 •

edited

Loading

yuki-stripe Feb 3, 2025 •

edited

Loading