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

Link v3: Assets and component views #4114

Merged
merged 15 commits into from
Oct 25, 2024
Merged

Conversation

davidme-stripe
Copy link
Contributor

@davidme-stripe davidme-stripe commented Oct 8, 2024

Summary

Bring back a grab bag of assets, colors, and component views for Link. These are all self-contained and don't change the current functionality, but will be used by the ViewControllers in later PRs.

The black and white Link logo in the snapshot tests is a known issue — we'll have updated logo assets before launch.

This code was all reviewed previously, but we should give it another look as we merge it back in.

Motivation

Link v2

Testing

Resurrected some snapshot tests

Changelog

None

Copy link

emerge-tools bot commented Oct 9, 2024

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

Name Version Download Change Install Change Approval
StripeSize
com.stripe.StripeSize
1.0 (1) 2.4 MB ⬆️ 441 B (0.02%) 7.8 MB ⬆️ 320 B N/A
StripeApplePaySize
com.stripe.StripeApplePaySize
1.0 (1) 437.5 kB - 1.5 MB - N/A
StripeFinancialConnectionsSize
com.stripe.StripeFinancialConnectionsSize
1.0 (1) 1.3 MB ⬆️ 4 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 ⬆️ 701 B (0.04%) 6.3 MB ⬆️ 320 B N/A
StripePaymentSheetSize
com.stripe.StripePaymentSheetSize
1.0 (1) 3.5 MB ⬆️ 10.7 kB (0.31%) 10.4 MB ⬆️ 30.5 kB (0.3%) N/A

StripeSize 1.0 (1)
com.stripe.StripeSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 320 B
Total download size change: ⬆️ 441 B (0.02%)

Largest size changes

Item Install Size Change
📝 StripePaymentsUI.CardBrandView.init(showCVC,centerHorizontally) ⬆️ 1.8 kB
🗑 StripePaymentsUI.CardBrandView.init(showCVC) ⬇️ -1.7 kB
Other ⬆️ 228 B
View Treemap

Image of diff

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

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 320 B
Total download size change: ⬆️ 701 B (0.04%)

Largest size changes

Item Install Size Change
📝 StripePaymentsUI.CardBrandView.init(showCVC,centerHorizontally) ⬆️ 1.8 kB
🗑 StripePaymentsUI.CardBrandView.init(showCVC) ⬇️ -1.7 kB
Other ⬆️ 228 B
View Treemap

Image of diff

StripePaymentSheetSize 1.0 (1)
com.stripe.StripePaymentSheetSize

⚖️ Compare build
⏱️ Analyze build performance

Total install size change: ⬆️ 30.5 kB (0.3%)
Total download size change: ⬆️ 10.7 kB (0.31%)

Largest size changes

Item Install Size Change
Packed Asset ⬆️ 0 B
Other ⬆️ 3.3 kB
📝 StripePaymentSheet.LinkNavigationBar.LinkNavigationBar ⬆️ 3.0 kB
📝 StripePaymentsUI.CardBrandView.init(showCVC,centerHorizontally) ⬆️ 1.8 kB
🗑 StripePaymentsUI.CardBrandView.init(showCVC) ⬇️ -1.7 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Copy link

github-actions bot commented Oct 9, 2024

🚨 New dead code detected in this PR:

LinkBadgeView.swift:15 warning: Class 'LinkBadgeView' is unused
LinkBadgeView.swift:104 warning: Extension 'BadgeType' is unused
LinkNavigationBar.swift:18 warning: Class 'LinkNavigationBar' is unused
LinkNoticeView.swift:17 warning: Class 'LinkNoticeView' is unused
LinkNoticeView.swift:90 warning: Extension 'NoticeType' is unused
LinkToast.swift:11 warning: Imported module 'StripeCore' is unused
LinkToast.swift:17 warning: Class 'LinkToast' is unused
LinkToast.swift:96 warning: Extension 'LinkToast' is unused
LinkToast.swift:171 warning: Extension 'ToastType' is unused
Button+Link.swift:31 warning: Function 'linkSecondary()' is unused
Button+Link.swift:42 warning: Function 'linkPlain()' is unused
Button+Link.swift:52 warning: Function 'linkBordered()' is unused
ConfirmButton+Link.swift:13 warning: Function 'makeLinkButton(callToAction:compact:didTap:)' is unused
UIColor+Link.swift:70 warning: Property 'linkControlHighlight' is unused
UIColor+Link.swift:78 warning: Property 'linkControlLightPlaceholder' is unused
UIColor+Link.swift:84 warning: Property 'linkToastBackground' is unused
UIColor+Link.swift:95 warning: Property 'linkSecondaryButtonForeground' is unused
UIColor+Link.swift:101 warning: Property 'linkSecondaryButtonBackground' is unused
UIColor+Link.swift:107 warning: Property 'linkNeutralBackground' is unused
UIColor+Link.swift:119 warning: Property 'linkDangerBackground' is unused
LinkInstantDebitMandateView.swift:28 warning: Struct 'Constants' is unused
LinkInstantDebitMandateView.swift:39 warning: Property 'textView' is unused
LinkInstantDebitMandateView.swift:59 warning: Initializer 'init(delegate:)' is unused
LinkInstantDebitMandateView.swift:69 warning: Function 'formattedLegalText()' is unused
LinkKeyboardAvoidingScrollView.swift:15 warning: Class 'LinkKeyboardAvoidingScrollView' is unused
LinkKeyboardAvoidingScrollView.swift:57 warning: Extension 'LinkKeyboardAvoidingScrollView' 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.

Copy link

github-actions bot commented Oct 9, 2024

⚠️ Public API changes detected:

StripePaymentSheet

- public var disabledBackgroundColor: UIKit.UIColor?
- public var disabledTextColor: UIKit.UIColor?
- public init(mode: StripePaymentSheet.PaymentSheet.IntentConfiguration.Mode, paymentMethodTypes: [Swift.String]? = nil, onBehalfOf: Swift.String? = nil, paymentMethodConfigurationId: Swift.String? = nil, confirmHandler: @escaping StripePaymentSheet.PaymentSheet.IntentConfiguration.ConfirmHandler, requireCVCRecollection: Swift.Bool = false)
+ public init(mode: StripePaymentSheet.PaymentSheet.IntentConfiguration.Mode, paymentMethodTypes: [Swift.String]? = nil, onBehalfOf: Swift.String? = nil, paymentMethodConfigurationId: Swift.String? = nil, confirmHandler: @escaping StripePaymentSheet.PaymentSheet.IntentConfiguration.ConfirmHandler)
- public var requireCVCRecollection: Swift.Bool

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.

@davidme-stripe davidme-stripe marked this pull request as ready for review October 10, 2024 00:28
@davidme-stripe davidme-stripe requested review from a team as code owners October 10, 2024 00:28
@davidme-stripe davidme-stripe changed the title Link v2: Assets and component views Link v3: Assets and component views Oct 10, 2024
Comment on lines 71 to 75
"By continuing, you agree to authorize payments pursuant to <terms>these terms</terms>." = "By continuing, you agree to authorize payments pursuant to <terms>these terms</terms>.";

/* Mandate text displayed when paying via Link instant debit. */
"By continuing, you agree to authorize payments pursuant to these <terms>terms</terms>." = "By continuing, you agree to authorize payments pursuant to these <terms>terms</terms>.";

Copy link
Collaborator

Choose a reason for hiding this comment

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

probably outside the scope of this PR, but for ACH, it's:

<terms>these terms</terms>.

and for this one it is:

these <terms>terms</terms>.


/// Hides toast.
///
/// You normally don't need to call this, as the toast will hide on its own after an specific duration.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This comment is vague-- it explains that we don't normally need to call this, but doesn't describe when we should be calling it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's for if you want to hide it manually. Like maybe you have a "success" toast, then an error immediately occurs and you want to cancel the first toast?

Comment on lines 74 to 76
let formattedString = NSMutableAttributedString()

STPStringUtils.parseRanges(from: string, withTags: Set<String>(links.keys)) { string, matches in
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be able to leverage:
STPStringUtils.applyLinksToString

At the very least add a todo, if you feel it is out of scope for this PR

Copy link
Contributor Author

@davidme-stripe davidme-stripe Oct 24, 2024

Choose a reason for hiding this comment

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

Good call, this code predates that

@davidme-stripe davidme-stripe merged commit 5faf38f into master Oct 25, 2024
6 checks passed
@davidme-stripe davidme-stripe deleted the relink/assets-and-button branch October 25, 2024 17:30
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