Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@ extension CustomerSheet {
guard let matchingPaymentMethod = paymentMethods.first(where: { $0.stripeId == paymentMethodId }) else {
return nil
}
matchingPaymentMethod.preloadCardArtImage(cardArtEnabled: configuration.appearance.cardArtEnabled)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I thought the idea was to block this on downloading the card art, since it's already async?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Doing so would slow down this function as well as be inconsistent with the behavior of payment sheet, so I ultimately decided against doing this.

return CustomerSheet.PaymentOptionSelection.paymentMethod(matchingPaymentMethod)
default:
return nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ extension CustomerSheet {
/// Create a PaymentOptionSelection for a saved payment method.
public static func paymentMethod(_ paymentMethod: STPPaymentMethod) -> PaymentOptionSelection {
let paymentMethodIcon = paymentMethod.makeIcon()
let image = paymentMethod.cardArtImage(cardArtEnabled: _cardArtEnabled) ?? paymentMethodIcon
let image = paymentMethod.cachedCardArtImage(cardArtEnabled: _cardArtEnabled) ?? paymentMethodIcon
let data = PaymentOptionDisplayData(image: image, label: paymentMethod.paymentSheetLabel)
return .paymentMethod(paymentMethod: paymentMethod, paymentOptionDisplayData: data)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ extension PaymentOption {
if let linkedBank = paymentOption?.instantDebitsLinkedBank {
return PaymentSheetImageLibrary.bankIcon(for: PaymentSheetImageLibrary.bankIconCode(for: linkedBank.bankName), iconStyle: iconStyle)
} else {
let cardArtImage = paymentMethod.cardArtImage(cardArtEnabled: cardArtEnabled)
let cardArtImage = paymentMethod.cachedCardArtImage(cardArtEnabled: cardArtEnabled)
return cardArtImage ?? paymentMethod.makeIcon(iconStyle: iconStyle)
}
case .new(let confirmParams):
Expand Down Expand Up @@ -149,8 +149,8 @@ extension STPPaymentMethod {
}
}

func cardArtImage(cardArtEnabled: Bool, downloadManager: DownloadManager = DownloadManager.sharedManager) -> UIImage? {
guard cardArtEnabled, let cardArtURL = cardArtCDNURL(height: 28) else {
func cachedCardArtImage(cardArtEnabled: Bool, downloadManager: DownloadManager = DownloadManager.sharedManager) -> UIImage? {
guard let cardArtURL = vendedCardArtURL(cardArtEnabled: cardArtEnabled) else {
return nil
}
let placeholder = downloadManager.imagePlaceHolder()
Expand All @@ -161,6 +161,25 @@ extension STPPaymentMethod {
)
return image == placeholder ? nil : image.roundedWithBorder(radius: 3)
}

func preloadCardArtImage(cardArtEnabled: Bool, downloadManager: DownloadManager = DownloadManager.sharedManager) {
guard let cardArtURL = vendedCardArtURL(cardArtEnabled: cardArtEnabled) else {
return
}
let updateHandler: ((UIImage) -> Void)? = { _ in }
_ = downloadManager.downloadImage(
url: cardArtURL,
placeholder: nil,
updateHandler: updateHandler
)
}

func vendedCardArtURL(cardArtEnabled: Bool) -> URL? {
guard cardArtEnabled, let cardArtURL = cardArtCDNURL(height: 26) else {
return nil
}
return cardArtURL
}
}

extension STPPaymentMethodParams {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@ final class PaymentSheetLoader {
elementsSession: elementsSession,
defaultPaymentMethod: elementsSession.customer?.getDefaultPaymentMethod()
)

// Pre-fetch card art without waiting for this to finish
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think you said pre-loading payment option images in general was broken, is that still true?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Correct in FlowController.swift, _ = flowController.paymentOption does not seem to have any effect.

if let defaultPaymentMethod = paymentOptionsViewModels.stp_boundSafeObject(at: defaultSelectedIndex),
case .saved(let stpPaymentMethod) = defaultPaymentMethod {
stpPaymentMethod.preloadCardArtImage(cardArtEnabled: configuration.appearance.cardArtEnabled)
}
loadTimings.logEnd("makeViewModels")

// Send load finished analytic
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,32 @@ final class STPPaymentMethodCardArtImageTest: APIStubbedTestCase {

func testCardArtImage_returnsNilWhenCardArtDisabled() {
let pm = STPPaymentMethod._testCardWithCardArt()
XCTAssertNil(pm.cardArtImage(cardArtEnabled: false, downloadManager: downloadManager))
XCTAssertNil(pm.cachedCardArtImage(cardArtEnabled: false, downloadManager: downloadManager))
}

func testCardArtImage_returnsNilWhenNoCardArt() {
let pm = STPPaymentMethod._testCard()
XCTAssertNil(pm.cardArtImage(cardArtEnabled: true, downloadManager: downloadManager))
XCTAssertNil(pm.cachedCardArtImage(cardArtEnabled: true, downloadManager: downloadManager))
}

func testCardArtImage_returnsNilWhenImageNotCached() {
let pm = STPPaymentMethod._testCardWithCardArt()
XCTAssertNil(pm.cardArtImage(cardArtEnabled: true, downloadManager: downloadManager))
XCTAssertNil(pm.cachedCardArtImage(cardArtEnabled: true, downloadManager: downloadManager))
}

func testCardArtImage_returnsImageWhenCached() {
let pm = STPPaymentMethod._testCardWithCardArt()
let cardArtURL = pm.cardArtCDNURL(height: 28)!
let cardArtURL = pm.cardArtCDNURL(height: 26)!

// Pre-seed the URL cache so downloadImage returns the real image
let imageData = generateUIImage(size: CGSize(width: 100, height: 28)).pngData()!
let imageData = generateUIImage(size: CGSize(width: 100, height: 26)).pngData()!
seedURLCache(url: cardArtURL, data: imageData)

let result = pm.cardArtImage(cardArtEnabled: true, downloadManager: downloadManager)
let result = pm.cachedCardArtImage(cardArtEnabled: true, downloadManager: downloadManager)
XCTAssertNotNil(result)

// The returned image should be the same as the one we created
XCTAssertEqual(result?.size, CGSize(width: 100, height: 28))
XCTAssertEqual(result?.size, CGSize(width: 100, height: 26))
}

// MARK: - Helpers
Expand Down
Loading