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

Set 'Change >' and sublabel to selected embedded form rows #4538

Merged
merged 10 commits into from
Feb 7, 2025
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -331,14 +331,63 @@ extension EmbeddedPaymentElement: EmbeddedFormViewControllerDelegate {
// If the formViewController was populated with a previous payment option don't reset
if embeddedFormViewController.previousPaymentOption == nil {
embeddedPaymentMethodsView.resetSelectionToLastSelection()
// Show change button if the newly selected row needs it
if let newSelectedType = embeddedPaymentMethodsView.selectedRowButton?.type {
let changeButtonState = getChangeButtonState(for: newSelectedType)
if changeButtonState.shouldShowChangeButton {
embeddedPaymentMethodsView.selectedRowButton?.addChangeButton(sublabel: changeButtonState.sublabel)
}
}
}
embeddedFormViewController.dismiss(animated: true)
}

func embeddedFormViewControllerDidContinue(_ embeddedFormViewController: EmbeddedFormViewController) {
// Show change button if the selected row needs it
if let newSelectedType = embeddedPaymentMethodsView.selectedRowButton?.type {
let changeButtonState = getChangeButtonState(for: newSelectedType)
if changeButtonState.shouldShowChangeButton {
embeddedPaymentMethodsView.selectedRowButton?.addChangeButton(sublabel: changeButtonState.sublabel)
}
}
embeddedFormViewController.dismiss(animated: true)
informDelegateIfPaymentOptionUpdated()
}

func getChangeButtonState(for type: RowButtonType) -> (shouldShowChangeButton: Bool, sublabel: String?) {
guard let _paymentOption, let displayData = paymentOption else {
return (false, nil)
}
// Show change button for new PMs that have a valid form
let shouldShowChangeButton: Bool = {
if case .new = type, selectedFormViewController != nil {
return true
}
return false
}()

// Add a sublabel to the selected row for cards and us bank account like "Visa 4242"
let sublabel: String? = {
switch type.paymentMethodType {
case .stripe(.card):
guard
case .new(confirmParams: let params) = _paymentOption,
let cardNumber = params.paymentMethodParams.card?.number
else {
return nil
}
let brand = STPCardValidator.brand(forNumber: cardNumber)
let brandString = STPCardBrandUtilities.stringFrom(brand)
Copy link
Collaborator

@porter-stripe porter-stripe Feb 4, 2025

Choose a reason for hiding this comment

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

This doesn't seem to work with co-branded cards, when I enter a co-branded (Visa/CB) card and choose CB, the copy says "Visa ...". we need to also check the preferred network (if it exists) on the card params. Can we add a test for this too?

params.paymentMethodParams.card?.networks?.preferred

There's also a weird edge case where you can fill out a form for a co-branded card and don't select a brand since it's optional, so the card brand gets decided on the backend. Not exactly sure what we show there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching this! Fixed it up. Are there things we can do to make it harder to make this mistake?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We did something similar to what you did with func brand(for card: STPPaymentMethodCardParams?) -> STPCardBrand on STPPaymentMethodCard. But this is the first instance AFAIK where we have needed to read it off the STPPaymentMethodCardParams. Maybe we "internally" deprecate STPCardBrandUtilities.stringFrom(brand) somehow, since the card number is not the single source of truth for the card brand for a number.

return [brandString, displayData.label].compactMap({ $0 }).joined(separator: " ")
case .stripe(.USBankAccount):
return displayData.label
default:
return nil
}
}()

return (shouldShowChangeButton: shouldShowChangeButton, sublabel: sublabel)
}
}

extension EmbeddedPaymentElement {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ public final class EmbeddedPaymentElement {

internal private(set) lazy var paymentHandler: STPPaymentHandler = STPPaymentHandler(apiClient: configuration.apiClient)

private init(
internal init(
configuration: Configuration,
loadResult: PaymentSheetLoader.LoadResult,
analyticsHelper: PaymentSheetAnalyticsHelper
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,33 @@ class EmbeddedPaymentMethodsView: UIView {
private let customer: PaymentSheet.CustomerConfiguration?
private var previousSelectedRowButton: RowButton? {
didSet {
previousSelectedRowButton?.isSelected = false
guard let previousSelectedRowButton, selectedRowButton?.type != previousSelectedRowButton.type else {
return
}
previousSelectedRowButton.isSelected = false
// Clear out the 'Change >' button and any sublabel (eg 4242) we set for new PM rows
switch previousSelectedRowButton.type {
case .new(paymentMethodType: let paymentMethodType):
let isCardOrUSBankAccount = paymentMethodType == .stripe(.card) || paymentMethodType == .stripe(.USBankAccount)
previousSelectedRowButton.removeChangeButton(shouldClearSublabel: isCardOrUSBankAccount)
default:
break
}
}
}
private(set) var selectedRowButton: RowButton? {
didSet {
previousSelectedRowButton = oldValue
selectedRowButton?.isSelected = true
updateMandate()
if oldValue?.type != selectedRowButton?.type {
delegate?.embeddedPaymentMethodsViewDidUpdateSelection()
}
if let selectedRowButton {
selectedRowButton.isSelected = true
}
updateMandate()
}
}

private let mandateProvider: MandateTextProvider
private let shouldShowMandate: Bool
private let analyticsHelper: PaymentSheetAnalyticsHelper
Expand Down Expand Up @@ -369,7 +383,7 @@ class EmbeddedPaymentMethodsView: UIView {
func makePaymentMethodRowButton(paymentMethodType: PaymentSheet.PaymentMethodType, savedPaymentMethods: [STPPaymentMethod]) -> RowButton {
// We always add a hidden accessory button ("Change >") so we can show/hide it easily
let accessoryButton = RowButton.RightAccessoryButton(
accessoryType: appearance.embeddedPaymentElement.row.style == .flatWithCheckmark ? .changeWithChevron : .change,
accessoryType: appearance.embeddedPaymentElement.row.style == .flatWithCheckmark ? .change : .changeWithChevron,
appearance: appearance,
didTap: { [weak self] in
guard let self, let selectedRowButton else { return }
Expand Down Expand Up @@ -425,3 +439,22 @@ extension Array where Element == STPPaymentMethod {
return !self.filter { $0.type == .card }.isEmpty
}
}

extension RowButton {
func addChangeButton(sublabel: String?) {
rightAccessoryView?.isHidden = false
if let sublabel {
self.sublabel.text = sublabel
self.sublabel.isHidden = sublabel.isEmpty
}
makeSameHeightAsOtherRowButtonsIfNecessary()
}

func removeChangeButton(shouldClearSublabel: Bool) {
rightAccessoryView?.isHidden = true
if shouldClearSublabel {
sublabel.text = nil
sublabel.isHidden = true
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ class FormSpecProvider {
}
}
}

func load() async {
await withCheckedContinuation { continuation in
load { _ in
continuation.resume()
}
}
}

/// Allows overwriting of formSpecs given a NSDictionary. Typically, the specs comes
/// from the sessions endpoint.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,12 @@ extension RowButton {
self.appearance = appearance
self.didTap = didTap
super.init(frame: .zero)
addAndPinSubview(stackView)
if accessoryType == .change || accessoryType == .viewMore {
directionalLayoutMargins = .insets(top: 8)
} else {
directionalLayoutMargins = .zero
}
porter-stripe marked this conversation as resolved.
Show resolved Hide resolved
addAndPinSubview(stackView, directionalLayoutMargins: directionalLayoutMargins)

accessibilityLabel = accessoryType.text
accessibilityIdentifier = accessoryType.text
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,11 @@ class RowButton: UIView {

// Label and sublabel
label.isAccessibilityElement = false
let labelsStackView = UIStackView(arrangedSubviews: [
label, sublabel, isFlatWithCheckmarkStyle ? rightAccessoryView : nil // add accessory view below labels if in checkmark style
].compactMap { $0 })
let labelsStackView = UIStackView(arrangedSubviews: [label, sublabel])
// add accessory view below labels if in checkmark style
if let rightAccessoryView, isFlatWithCheckmarkStyle {
labelsStackView.addArrangedSubview(rightAccessoryView)
}
labelsStackView.axis = .vertical
labelsStackView.alignment = .leading

Expand Down Expand Up @@ -214,12 +216,6 @@ class RowButton: UIView {
]

if isFlatWithCheckmarkStyle, let rightAccessoryView, !rightAccessoryView.isHidden {
// In flat_with_checkmark, we need additional vertical space around the View More / Change accessory view.
if sublabel.isHidden {
labelsStackView.setCustomSpacing(8, after: label)
} else {
labelsStackView.setCustomSpacing(8, after: sublabel)
}
imageViewConstraints.append(imageView.centerYAnchor.constraint(equalTo: label.centerYAnchor))
} else {
imageViewConstraints.append(imageView.centerYAnchor.constraint(equalTo: centerYAnchor))
Expand Down Expand Up @@ -321,7 +317,7 @@ class RowButton: UIView {
}

// MARK: Tap handling
@objc private func handleTap() {
@objc func handleTap() {
guard isEnabled else { return }
if shouldAnimateOnPress {
// Fade the text and icon out and back in
Expand Down Expand Up @@ -355,10 +351,12 @@ class RowButton: UIView {
// Don't do this if we are flat_with_checkmark style and have an accessory view - this row button is allowed to be taller than the rest
let isDisplayingRightAccessoryView = rightAccessoryView?.isHidden == false
if isFlatWithCheckmarkStyle && isDisplayingRightAccessoryView {
heightConstraint?.isActive = false
return
}
// Don't do this if we *are* the tallest variant; otherwise we'll infinite loop!
guard sublabel.text?.isEmpty ?? true else {
heightConstraint?.isActive = false
return
}
heightConstraint = heightAnchor.constraint(equalToConstant: Self.calculateTallestHeight(appearance: appearance, isEmbedded: isEmbedded))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,88 @@ class EmbeddedPaymentElementSnapshotTests: STPSnapshotTestCase, EmbeddedPaymentE
XCTAssertEqual(sut.view.directionalLayoutMargins, .zero)
XCTAssertFalse(sut.view.hasAmbiguousLayout)
}

// MARK: - 'Change >' button and sublabel (eg Visa 4242)

func testShowsChangeButton_flatRadio() async throws {
await _testShowsChangeButton(rowStyle: .flatWithRadio)
}

func testShowsChangeButton_floatingButton() async throws {
await _testShowsChangeButton(rowStyle: .floatingButton)

}

func testShowsChangeButton_flatCheckmark() async throws {
await _testShowsChangeButton(rowStyle: .flatWithCheckmark)
}

func _testShowsChangeButton(rowStyle: PaymentSheet.Appearance.EmbeddedPaymentElement.Row.Style) async {
var configuration = configuration
configuration.appearance.embeddedPaymentElement.row.style = rowStyle
var paymentIntentConfig = paymentIntentConfig
paymentIntentConfig.paymentMethodTypes = ["card", "us_bank_account", "afterpay_clearpay"]
let loadResult = PaymentSheetLoader.LoadResult(
intent: .deferredIntent(intentConfig: paymentIntentConfig),
elementsSession: ._testValue(paymentMethodTypes: ["card", "us_bank_account", "afterpay_clearpay"]),
savedPaymentMethods: [],
paymentMethodTypes: [.stripe(.card), .stripe(.USBankAccount), .stripe(.afterpayClearpay)]
)
await AddressSpecProvider.shared.loadAddressSpecs()
await FormSpecProvider.shared.load()
let sut = EmbeddedPaymentElement(
configuration: configuration,
loadResult: loadResult,
analyticsHelper: ._testValue()
)
sut.view.autosizeHeight(width: 300)
sut.delegate = self
sut.presentingViewController = UIViewController()
// There are 3 variations of adding a 'Change >' button and sublabel to a selected row
// 1️⃣
// ...tapping card and filling out the form...
sut.embeddedPaymentMethodsView.getRowButton(accessibilityIdentifier: "Card").handleTap()
let cardForm = sut.formCache[.stripe(.card)]!
cardForm.getTextFieldElement("Card number")?.setText("4242424242424242")
cardForm.getTextFieldElement("MM / YY").setText("1232")
cardForm.getTextFieldElement("CVC").setText("123")
cardForm.getTextFieldElement("ZIP").setText("65432")
sut.selectedFormViewController?.didTapPrimaryButton()

// ...should show the card row w/ the 'Change >' + "Visa 4242"
STPSnapshotVerifyView(sut.view, identifier: "card")

// 2️⃣
// Tapping US Bank Account...
sut.embeddedPaymentMethodsView.getRowButton(accessibilityIdentifier: "US bank account").handleTap()
// ...and backing out...
sut.embeddedFormViewControllerDidCancel(sut.selectedFormViewController!)
// ...should keep the card row selected...
// (this tests that setting the selection back to the previous works)
STPSnapshotVerifyView(sut.view, identifier: "us_bank_account_canceled")

// Filling out US Bank account...
sut.embeddedPaymentMethodsView.getRowButton(accessibilityIdentifier: "US bank account").handleTap()
let bankForm = sut.formCache[.stripe(.USBankAccount)] as! USBankAccountPaymentMethodElement
bankForm.getTextFieldElement("Full name").setText("Name")
bankForm.getTextFieldElement("Email").setText("[email protected]")
bankForm.linkedBank = FinancialConnectionsLinkedBank(sessionId: "123", accountId: "123", displayName: "Success", bankName: "StripeBank", last4: "6789", instantlyVerified: true)
sut.selectedFormViewController?.didTapPrimaryButton()
// ...should show the row w/ 'Change >' + "6789" (the last bank 4)
STPSnapshotVerifyView(sut.view, identifier: "us_bank_account_continue")

// 3️⃣
// Tapping Afterpay and filling out the form...
sut.embeddedPaymentMethodsView.getRowButton(accessibilityIdentifier: "Afterpay").handleTap()
let afterpayForm = sut.formCache[.stripe(.afterpayClearpay)]!
afterpayForm.getTextFieldElement("Full name")?.setText("Tester")
afterpayForm.getTextFieldElement("Email")?.setText("[email protected]")
afterpayForm.getTextFieldElement("Address line 1").setText("asdf")
afterpayForm.getTextFieldElement("City").setText("asdf")
afterpayForm.getTextFieldElement("ZIP").setText("12345")
sut.selectedFormViewController?.didTapPrimaryButton()

// ...should show the row w/ 'Change >'
STPSnapshotVerifyView(sut.view, identifier: "afterpay")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,20 @@ class EmbeddedPaymentMethodsViewSnapshotTests: STPSnapshotTestCase {
}
}

extension EmbeddedPaymentMethodsViewSnapshotTests: EmbeddedPaymentMethodsViewDelegate {
func embeddedPaymentMethodsViewDidUpdateHeight() {
}

func embeddedPaymentMethodsViewDidTapPaymentMethodRow() {
}

func embeddedPaymentMethodsViewDidUpdateSelection() {
}

func embeddedPaymentMethodsViewDidTapViewMoreSavedPaymentMethods(selectedSavedPaymentMethod: StripePayments.STPPaymentMethod?) {
}
}

class MockMandateProvider: MandateTextProvider {
private let mandateResolver: (PaymentSheet.PaymentMethodType?) -> (NSAttributedString?)

Expand All @@ -1124,14 +1138,14 @@ class MockMandateProvider: MandateTextProvider {

extension EmbeddedPaymentMethodsView {
convenience init(
initialSelection: RowButtonType?,
paymentMethodTypes: [PaymentSheet.PaymentMethodType],
savedPaymentMethod: STPPaymentMethod?,
appearance: PaymentSheet.Appearance,
shouldShowApplePay: Bool,
shouldShowLink: Bool,
savedPaymentMethodAccessoryType: RowButton.RightAccessoryButton.AccessoryType?,
mandateProvider: MandateTextProvider,
initialSelection: RowButtonType? = nil,
paymentMethodTypes: [PaymentSheet.PaymentMethodType] = [.stripe(.card), .stripe(.cashApp)],
savedPaymentMethod: STPPaymentMethod? = nil,
appearance: PaymentSheet.Appearance = .default,
shouldShowApplePay: Bool = true,
shouldShowLink: Bool = true,
savedPaymentMethodAccessoryType: RowButton.RightAccessoryButton.AccessoryType? = nil,
mandateProvider: MandateTextProvider = MockMandateProvider(),
shouldShowMandate: Bool = true,
savedPaymentMethods: [STPPaymentMethod] = [],
customer: PaymentSheet.CustomerConfiguration? = nil,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ import UIKit
didUpdate: DidUpdateSelectedIndex? = nil,
didTapClose: DidTapClose? = nil
) {
assert(!items.filter{!$0.isDisabled}.isEmpty, "`items` must contain at least one non-disabled item")
stpAssert(!items.filter{!$0.isDisabled}.isEmpty, "`items` must contain at least one non-disabled item; if this is a test, you might need to set AddressSpecProvider.shared.loadAddressSpecs")

self.label = label
self.theme = theme
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ let addressDataFilename = "localized_address_data"
}
}
}

public func loadAddressSpecs() async {
await withCheckedContinuation { continuation in
loadAddressSpecs {
continuation.resume()
}
}
}

func addressSpec(for country: String) -> AddressSpec {
guard let spec = addressSpecs[country] else {
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...ementSnapshotTests/[email protected]
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading