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

Support BDCC in native Link #4692

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Support BDCC in native Link #4692

wants to merge 4 commits into from

Conversation

tillh-stripe
Copy link
Collaborator

Summary

Motivation

Testing

Changelog

Copy link

github-actions bot commented Mar 20, 2025

🚨 New dead code detected in this PR:

ConsumerSession.swift:24 warning: Initializer 'init(clientSecret:emailAddress:redactedFormattedPhoneNumber:unredactedPhoneNumber:phoneNumberCountry:verificationSessions:supportedPaymentDetailsTypes:)' 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.

@@ -16,19 +16,25 @@ final class ConsumerSession: Decodable {
let clientSecret: String
let emailAddress: String
let redactedFormattedPhoneNumber: String
let unredactedPhoneNumber: String?
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We're pulling the unredacted phone number to potentially prefill an empty phone number field.

@tillh-stripe tillh-stripe force-pushed the tillh/link-bdcc branch 10 times, most recently from 051eb11 to b3f9d9e Compare March 21, 2025 01:42
@tillh-stripe tillh-stripe force-pushed the tillh/link-bdcc branch 2 times, most recently from 9ba4d76 to c73ccd6 Compare March 26, 2025 17:58
@@ -385,8 +389,10 @@ extension STPAPIClient {
]

if let details = updateParams.details, case .card(let expiryDate, let billingDetails) = details {
parameters["exp_month"] = expiryDate.month
parameters["exp_year"] = expiryDate.year
if let expiryDate {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the update call work if expiryDate is nil?

let label = UILabel()
label.font = LinkUI.font(forTextStyle: .title)
label.textColor = .linkPrimaryText
label.adjustsFontForContentSizeCategory = true
label.numberOfLines = 0
label.textAlignment = .center
label.text = String.Localized.update_card
label.text = if isBillingDetailsUpdateFlow {
// TODO: Localize me
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make this a localized string before landing?

private lazy var cvcElement: TextFieldElement = {
let cvcElement = TextFieldElement(
configuration: TextFieldElement.CVCConfiguration(
// If we're using a placeholder, we just fill up the view with zeros.
Copy link
Contributor

Choose a reason for hiding this comment

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

We'll render it as "•••" though, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, yes, I see that below now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, but with this ugly workaround 😕 I just noticed that CensoredCVCConfiguration exists, which gives us that functionality for free, but it makes the look different from the other disabled fields. What do you think about this?

Simulator Screenshot - iPhone 16 Pro - 2025-03-28 at 11 26 10

private extension STPElementsSession {
func isCompatible(with configuration: PaymentElementConfiguration) -> Bool {
// We can't collect billing details if we're in the web flow, so turn Link off for those cases.
let nativeLink = deviceCanUseNativeLink(elementsSession: self, configuration: configuration)
Copy link
Contributor

Choose a reason for hiding this comment

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

Kinda wondering what happens if the device fails the attestation check after this — I almost wonder if we should just hide Link entirely if attestation fails instead of falling back to the web flow.

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.

2 participants