Link v3: Add components for card editing and verification #4158

davidme-stripe · 2024-10-17T22:16:29Z

Summary

Add Card Edit, Payment Method Picker, and Verification components.

Motivation

Link v3

Testing

Added snapshot tests.

Changelog

None

emerge-tools · 2024-10-17T22:27:53Z

⚠️ 1 new unused protocol, 1 build increased size, 5 builds had no size change

Name	Version	Download	Change	Install	Change	Approval
StripeSize com.stripe.StripeSize	1.0 (1)	2.1 MB	⬇️ 6 B	6.8 MB	-	N/A
StripeApplePaySize com.stripe.StripeApplePaySize	1.0 (1)	438.1 kB	-	1.6 MB	-	N/A
StripeFinancialConnectionsSize com.stripe.StripeFinancialConnectionsSize	1.0 (1)	1.3 MB	⬇️ 1 B	4.3 MB	-	N/A
StripePaymentsSize com.stripe.StripePaymentsSize	1.0 (1)	1.2 MB	-	4.1 MB	-	N/A
StripePaymentsUISize com.stripe.StripePaymentsUISize	1.0 (1)	1.9 MB	⬇️ 2 B	6.3 MB	-	N/A
StripePaymentSheetSize com.stripe.StripePaymentSheetSize	1.0 (1)	3.5 MB	⬆️ 59.0 kB (1.7%)	10.6 MB	⬆️ 137.7 kB (1.33%)	N/A

StripeSize 1.0 (1)
com.stripe.StripeSize

No changes to report

StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize

No changes to report

StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize

No changes to report

StripePaymentsSize 1.0 (1)
com.stripe.StripePaymentsSize

No changes to report

StripePaymentsUISize 1.0 (1)
com.stripe.StripePaymentsUISize

No changes to report

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 137.7 kB (1.33%)
Total download size change: ⬆️ 59.0 kB (1.7%)

Largest size changes

Item	Install Size Change
Localizable.strings	⬆️ 4.1 kB
Code Signature	⬆️ 3.3 kB
📝 StripePaymentSheet.LinkPaymentMethodPicker.Cell.setupUI	⬆️ 3.0 kB
📝 StripePaymentSheet.LinkUI.appearance	⬆️ 2.4 kB
📝 StripePaymentSheet.LinkPaymentMethodPicker.CellContentView.iconCo...	⬆️ 2.3 kB

View Treemap

🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

github-actions · 2024-10-17T22:38:45Z

🚨 New dead code detected in this PR:

LinkPaymentMethodPicker-Cell.swift:25 warning: Property 'menuSpacing' is unused
LinkPaymentMethodPicker-Cell.swift:244 warning: Parameter 'sender' is unused
LinkPaymentMethodPicker-Header.swift:71 warning: Property 'cardNumberLabel' is unused
LinkPaymentMethodPicker.swift:175 warning: Parameter 'sender' is unused
LinkPaymentMethodPicker.swift:219 warning: Function 'showLoader(at:)' is unused
LinkPaymentMethodPicker.swift:228 warning: Function 'hideLoader(at:)' is unused
LinkPaymentMethodPicker.swift:237 warning: Function 'setAddPaymentMethodButtonEnabled(_:)' is unused
LinkPaymentMethodPicker.swift:273 warning: Function 'removePaymentMethod(at:animated:)' is unused
LinkCardEditElement.swift:29 warning: Class 'LinkCardEditElement' is unused
LinkCardEditElement.swift:204 warning: Extension 'LinkCardEditElement' is unused
LinkVerificationController.swift:12 warning: Class 'LinkVerificationController' is unused
LinkVerificationController.swift:37 warning: Extension 'LinkVerificationController' is unused
LinkVerificationView-LogoutView.swift:9 warning: Imported module 'StripeCore' is unused
LinkVerificationView.swift:11 warning: Imported module 'StripeCore' is unused
LinkVerificationView.swift:18 warning: Parameter 'view' is unused
LinkVerificationView.swift:153 warning: Parameter 'sender' is unused
LinkVerificationViewController.swift:59 warning: Initializer 'init(mode:linkAccount:)' is unused
LinkVerificationViewController.swift:199 warning: Parameter 'view' is unused
LinkVerificationViewController.swift:236 warning: Class 'TransitioningDelegate' is unused
LinkKeyboardAvoidingScrollView.swift:31 warning: Initializer 'init(contentView:)' is unused

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

github-actions · 2024-10-21T22:51:12Z

⚠️ Public API changes detected:

StripePaymentSheet

- public var paymentMethodLayout: StripePaymentSheet.PaymentSheet.PaymentMethodLayout
- }
- public enum PaymentMethodLayout {
- case horizontal
- case vertical
- case automatic
- public static func == (a: StripePaymentSheet.PaymentSheet.PaymentMethodLayout, b: StripePaymentSheet.PaymentSheet.PaymentMethodLayout) -> Swift.Bool
- public func hash(into hasher: inout Swift.Hasher)
- public var hashValue: Swift.Int {
- get
- }

If you are adding a new public API consider the following:

Do these APIs need to be public or can they be protected with @_spi(STP)?
If these APIs need to be public, assess whether they require an API review.

If you are modifying or removing a public API:

Does this require a breaking version change?
Do these changes require API review?

If you confirm these APIs need to be added/updated and have undergone necessary review, add the label modifies public API to this PR to acknowledge and bypass this check.

ℹ️ If this comment appears to be left in error, make sure your branch is up-to-date with master.

wooj-stripe · 2024-10-25T23:14:11Z

StripePaymentSheet/StripePaymentSheet.xcodeproj/project.pbxproj

+		3147CECA2CC1BF550067B5E4 /* LinkPaymentMethodPicker.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC32CC1BF550067B5E4 /* LinkPaymentMethodPicker.swift */; };
+		3147CECB2CC1BF550067B5E4 /* LinkPaymentMethodPicker-CellContentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC62CC1BF550067B5E4 /* LinkPaymentMethodPicker-CellContentView.swift */; };
+		3147CECC2CC1BF550067B5E4 /* LinkPaymentMethodPicker-Cell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC52CC1BF550067B5E4 /* LinkPaymentMethodPicker-Cell.swift */; };
+		3147CECD2CC1BF550067B5E4 /* LinkPaymentMethodPicker-Header.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC72CC1BF550067B5E4 /* LinkPaymentMethodPicker-Header.swift */; };
+		3147CECE2CC1BF550067B5E4 /* LinkPaymentMethodPicker-RadioButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC82CC1BF550067B5E4 /* LinkPaymentMethodPicker-RadioButton.swift */; };
+		3147CECF2CC1BF550067B5E4 /* LinkPaymentMethodPicker-AddButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3147CEC42CC1BF550067B5E4 /* LinkPaymentMethodPicker-AddButton.swift */; };


Nit: I would prefer these file names to be +CellContentView.swift or +Cell.swift, etc.., but I guess we can keep them using the '-' symbol?

I think + is usually reserved for extensions. (Like UIView+StripeAdditions.) If I were starting over today I would make this a folder structure (e.g. LinkPaymentMethodPicker/AddButton.swift), but the - is fine too

wooj-stripe · 2024-10-25T23:22:26Z

.../Source/Internal/Link/Components/PaymentMethodPicker/LinkPaymentMethodPicker-AddButton.swift

+        override func hitTest(_ point: CGPoint, with event: UIEvent?) -> UIView? {
+            if self.point(inside: point, with: event) {
+                return self
+            }
+
+            return nil
+        }


I think this is fine for now, assuming it "works", but hoping to get rid of it at some point in time. In the past, hitTest() was causing us issues with views that were being recycled -- my guess is that we'll go through a dogfooding/QA cycle, right?

I think that was just because we were calling it during reloadData() in that case? Overriding hitTest in general shouldn't be a problem

wooj-stripe · 2024-10-25T23:23:34Z

...Sheet/Source/Internal/Link/Components/PaymentMethodPicker/LinkPaymentMethodPicker-Cell.swift

+            contentView.paymentMethod = paymentMethod
+            updateAccessibilityContent()
+
+            guard let paymentMethod = paymentMethod else {


nit:
guard let paymentMethod else {
}

wooj-stripe · 2024-10-25T23:25:39Z

...Sheet/Source/Internal/Link/Components/PaymentMethodPicker/LinkPaymentMethodPicker-Cell.swift

+                case .bankAccount:
+                    return false
+                case .unparsable:
+                    return false


nit: can be combined to
case .bankAccount, .unparsable:

wooj-stripe · 2024-10-25T23:27:11Z

...e/Internal/Link/Components/PaymentMethodPicker/LinkPaymentMethodPicker-CellContentView.swift

+                bankIconView.leadingAnchor.constraint(greaterThanOrEqualTo: view.leadingAnchor),
+                bankIconView.topAnchor.constraint(greaterThanOrEqualTo: view.topAnchor),
+                bankIconView.trailingAnchor.constraint(lessThanOrEqualTo: view.trailingAnchor),
+                bankIconView.bottomAnchor.constraint(lessThanOrEqualTo: view.bottomAnchor),


i would have expected these to all be just equalTo not greaterThanOr/lessThanOr

I think it makes sense, you want the icon to be:

Centered (with the centerXAnchor and centerYAnchor)

To follow their intrinsic size

To not extend beyond the leading, trailing, top, or bottom anchors

If you set this as equalTo, you'd be requiring that the view extend to the edges of the containing view, which may not be correct depending on its intrinsic size.

...tSheet/StripePaymentSheet/Source/Internal/Link/Verification/LinkVerificationController.swift

wooj-stripe · 2024-10-26T00:06:46Z

...PaymentSheet/StripePaymentSheet/Source/Internal/Link/Verification/LinkVerificationView.swift

+        let codeField = OneTimeCodeTextField(configuration:
+                .init(numberOfDigits: 6),


nit: spacing looks off.

wooj-stripe · 2024-10-26T00:07:41Z

...ource/Internal/Link/Verification/LinkVerificationViewController-PresentationController.swift

+        }
+
+        override var frameOfPresentedViewInContainerView: CGRect {
+            guard let containerView = containerView else {


nit: remove = containerView

wooj-stripe · 2024-10-26T00:07:48Z

...ource/Internal/Link/Verification/LinkVerificationViewController-PresentationController.swift

+        }
+
+        private func calculateModalFrame(forContainerSize containerSize: CGSize) -> CGRect {
+            guard let contentView = contentView else {


nit: remove = contentView

wooj-stripe · 2024-10-26T00:08:55Z

...ource/Internal/Link/Verification/LinkVerificationViewController-PresentationController.swift

+        ///
+        ///  This is always equals to the container view safe area minus `padding` on eat edge.
+        private var safeFrame: CGRect {
+            guard let containerView = containerView else {


nit: remove = containerView

davidme-stripe changed the base branch from master to relink/datamodel October 17, 2024 22:16

davidme-stripe force-pushed the relink/vcs branch from 319a3d8 to 5c6f1bd Compare October 17, 2024 22:28

davidme-stripe force-pushed the relink/vcs branch from 5c6f1bd to 7023828 Compare October 17, 2024 22:43

davidme-stripe added the skip dead code check label Oct 21, 2024

davidme-stripe marked this pull request as ready for review October 23, 2024 19:35

davidme-stripe requested review from a team as code owners October 23, 2024 19:35

davidme-stripe force-pushed the relink/datamodel branch from e69c8a6 to dcc3894 Compare October 25, 2024 17:52

Base automatically changed from relink/datamodel to master October 25, 2024 22:11

davidme-stripe force-pushed the relink/vcs branch from 7023828 to b00afa9 Compare October 25, 2024 22:50

Add view components

9f7cc8d

davidme-stripe force-pushed the relink/vcs branch from b00afa9 to 9f7cc8d Compare October 25, 2024 22:53

davidme-stripe requested a review from wooj-stripe October 25, 2024 23:12

wooj-stripe reviewed Oct 26, 2024

View reviewed changes

Updates based on feedback

de9c74c

wooj-stripe previously approved these changes Oct 26, 2024

View reviewed changes

davidme-stripe enabled auto-merge (squash) October 26, 2024 00:57

Updates based on feedback

7420f18

davidme-stripe dismissed wooj-stripe’s stale review via 7420f18 October 26, 2024 00:59

wooj-stripe approved these changes Oct 26, 2024

View reviewed changes

davidme-stripe merged commit bf85c49 into master Oct 26, 2024
7 checks passed

davidme-stripe deleted the relink/vcs branch October 26, 2024 01:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Link v3: Add components for card editing and verification #4158

Link v3: Add components for card editing and verification #4158

davidme-stripe commented Oct 17, 2024

emerge-tools bot commented Oct 17, 2024 •

edited

Loading

github-actions bot commented Oct 17, 2024 •

edited

Loading

github-actions bot commented Oct 21, 2024

wooj-stripe Oct 25, 2024

davidme-stripe Oct 26, 2024

wooj-stripe Oct 25, 2024

davidme-stripe Oct 26, 2024

wooj-stripe Oct 25, 2024

wooj-stripe Oct 25, 2024

wooj-stripe Oct 25, 2024

davidme-stripe Oct 26, 2024

wooj-stripe Oct 26, 2024

wooj-stripe Oct 26, 2024

wooj-stripe Oct 26, 2024

wooj-stripe Oct 26, 2024

		let codeField = OneTimeCodeTextField(configuration:
		.init(numberOfDigits: 6),

Link v3: Add components for card editing and verification #4158

Link v3: Add components for card editing and verification #4158

Conversation

davidme-stripe commented Oct 17, 2024

Summary

Motivation

Testing

Changelog

emerge-tools bot commented Oct 17, 2024 • edited Loading

⚠️ 1 new unused protocol, 1 build increased size, 5 builds had no size change

StripeSize 1.0 (1)com.stripe.StripeSize

StripeApplePaySize 1.0 (1)com.stripe.StripeApplePaySize

StripeFinancialConnectionsSize 1.0 (1)com.stripe.StripeFinancialConnectionsSize

StripePaymentsSize 1.0 (1)com.stripe.StripePaymentsSize

StripePaymentsUISize 1.0 (1)com.stripe.StripePaymentsUISize

StripePaymentSheetSize 1.0 (1)com.stripe.StripePaymentSheetSize

github-actions bot commented Oct 17, 2024 • edited Loading

github-actions bot commented Oct 21, 2024

StripePaymentSheet

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

emerge-tools bot commented Oct 17, 2024 •

edited

Loading

StripeSize 1.0 (1)
com.stripe.StripeSize

StripeApplePaySize 1.0 (1)
com.stripe.StripeApplePaySize

StripeFinancialConnectionsSize 1.0 (1)
com.stripe.StripeFinancialConnectionsSize

StripePaymentsSize 1.0 (1)
com.stripe.StripePaymentsSize

StripePaymentsUISize 1.0 (1)
com.stripe.StripePaymentsUISize

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

github-actions bot commented Oct 17, 2024 •

edited

Loading