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

Update Afterpay logos for iOS users #4765

Merged
merged 41 commits into from
Apr 7, 2025

Conversation

joyceqin-stripe
Copy link
Collaborator

@joyceqin-stripe joyceqin-stripe commented Apr 1, 2025

Summary

Rebrand Afterpay in USD to be Cash App Afterpay, update shouldUseClearpayBrand to be determined by currency GBP

Motivation

https://jira.corp.stripe.com/browse/RUN_MOBILESDK-3987

Testing

  • Manually
  • Updated snapshot tests
  • Added snapshot tests

Cash App Afterpay:
image

Regular Afterpay:
image

Clearpay Afterpay:
image

Changelog

[Changed] Updated Afterpay branding in the US to be Cash App Afterpay.

@joyceqin-stripe joyceqin-stripe changed the title start rebranding afterpay Update Afterpay logos for iOS users Apr 2, 2025
@joyceqin-stripe joyceqin-stripe marked this pull request as ready for review April 2, 2025 22:56
@joyceqin-stripe joyceqin-stripe requested review from a team as code owners April 2, 2025 22:56
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look quite right, why is it shifted half way down?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know why it shows up like that, but if you switch to viewing the source diff and back to rich diff it shows up correctly

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the spacing is a little off, probably need an extra space after " ".

Also the info icon in dark mode is hidden.

Simulator Screenshot - iOS 18  - 2025-04-03 at 09 55 59

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In order to add the space before Cash App Afterpay but not Afterpay or Clearpay, we'd need to add a special case— do you think it's needed? Also, the info icon is there but not easily seen because we don't have a white asset for it. Do you know how to get that?

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO I think the Afterpay and Clearpay could use a space too.

To get the info icon to be white, you could try to do what is done in traitCollectionDidChange and set the tint color.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The info icon is a part of a NSAttributedString— I'm not sure how to access the info icon from traitCollectionDidChange

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, did you try?

You can do this by:

  1. Updating attributedStringOfImageWithoutLink to allow setting a tint color of the image attachment.
private func attributedStringOfImageWithoutLink(uiImage: UIImage, font: UIFont, tintColor: UIColor?) -> NSAttributedString {
    let imageAttachment = NSTextAttachment()
    imageAttachment.bounds = boundsOfImage(font: font, uiImage: uiImage)
    if let tintColor {
        imageAttachment.image = uiImage.withTintColor(tintColor, renderingMode: .alwaysTemplate)
    } else {
        imageAttachment.image = uiImage
    }
    return NSAttributedString(attachment: imageAttachment)
}
  1. Update how we build the infoButton in makeAfterPayClearPayString():
let infoButton = attributedStringOfImageWithoutLink(uiImage: infoImage, font: titleFont, tintColor: theme.colors.parentBackground.contrastingColor)
  1. Update traitCollectionDidChange to recompute the attributed string:
override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) {
    super.traitCollectionDidChange(previousTraitCollection)
    afterpayMarkImageView.tintColor = theme.colors.parentBackground.contrastingColor
    afterPayClearPayLabel.attributedText = makeAfterPayClearPayString()
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added a white info icon svg by editing the svg file
Screenshot 2025-04-03 at 11 22 50 AM

Copy link
Collaborator

Choose a reason for hiding this comment

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

Switching between the black and white logo seems fine, but there are some cases where this won't work! Imagine a case where a user is using the Appearance API to force their app into light mode (e.g., their app doesn't support dark mode). Then when the device is in dark mode, their app is in light mode. In this scenario, our info icon would be white and not visible.

Instead of using white for dark mode and black for light mode, we need to tint the image based on the color it is sitting on top of. The suggestion above does this, similar to how the afterpayMarkImageView does this.

@@ -289,6 +289,7 @@ import Foundation
return "multibanco"
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

This needed?

@@ -68,7 +68,7 @@ class PaymentSheetPaymentMethodTypeTest: XCTestCase {
waitForExpectations(timeout: 10)
// A Payment methods without a client-side asset...
let loadExpectation = expectation(description: "Load form spec image")
let image = PaymentSheet.PaymentMethodType.stripe(.amazonPay).makeImage { image in
let image = PaymentSheet.PaymentMethodType.stripe(.amazonPay).makeImage(currency: nil) { image in
Copy link
Collaborator

Choose a reason for hiding this comment

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

To avoid all these changes that aren't really needed, let's make currency default to nil in makeImage. That way you don't need to update many unrelated files. Once you add the default then you can come back to usages like this and revert the changes made.

case .wallet, .withPaymentMethod, .withPaymentDetails:
return Image.link_logo.makeImage()
}
case .external(let paymentMethod, _):
return PaymentSheet.PaymentMethodType.external(paymentMethod).makeImage(
forDarkBackground: traitCollection?.isDarkMode ?? false,
currency: nil,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why aren't we passing in currency here and instead nil even though we have a currency in scope?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currency isn't needed for external payment types, but I guess there's no harm in passing it anyways

}
}

var appearance: PaymentSheet.Appearance = PaymentSheet.Appearance.default {
didSet {
update()
update(currency: currency)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to pass currency into update, don't we have one in scope on self?

Copy link

github-actions bot commented Apr 3, 2025

⚠️ Missing Translations
The following strings have been uploaded to Lokalise but are not yet translated.

Buy now or pay later with

If it's okay for these strings to be unlocalized in master (e.g. this is for an unshipped feature), add the label ship without translations to acknowledge that there are missing translations. Otherwise, wait until translations are available in Lokalise and re-run this job.

New strings are localized on a weekly basis and are downloaded as part of the release process. For more details on how to localize a string, you can refer to this link.

@joyceqin-stripe joyceqin-stripe enabled auto-merge (squash) April 4, 2025 23:39
@joyceqin-stripe joyceqin-stripe merged commit ea1035b into master Apr 7, 2025
7 checks passed
@joyceqin-stripe joyceqin-stripe deleted the joyceqin/RUN_MOBILESDK-3987 branch April 7, 2025 17:16
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