Skip to content

fix crash from using legacy UI from cashappafterpay locale#288

Merged
squarepaul merged 1 commit into
masterfrom
fix-crash-from-unauthorized-ui
Jun 18, 2025
Merged

fix crash from using legacy UI from cashappafterpay locale#288
squarepaul merged 1 commit into
masterfrom
fix-crash-from-unauthorized-ui

Conversation

@squarepaul

@squarepaul squarepaul commented Jun 17, 2025

Copy link
Copy Markdown
Collaborator

Summary of Changes

  • Previously we crashed if legacy UI was used in an unsupported region, here we just hide the UI to prevent crashes

Items of Note

Submission Checklist

  • Tests are included.
  • Documentation is changed or added.

@kruegermj kruegermj left a comment

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.

What would the state of these views be if the region is not supported?

@@ -12,9 +12,12 @@ import UIKit
public class BadgeView: AfterpayLogo {
override public init(colorScheme: ColorScheme = .static(.default)) {
if Afterpay.isCashAppAfterpayRegion {

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.

This may read a little better than running the same isCashAppAfterpayRegion check twice.

Suggested change
if Afterpay.isCashAppAfterpayRegion {
if Afterpay.isCashAppAfterpayRegion {
self.isHidden = true
#if DEBUG
assertionFailure("Cash App Afterpay compact badge is not supported")
#endif
}
super.init(colorScheme: colorScheme)

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.

yes I agree, but self must first be init'd before we can set it to hidden

@@ -12,9 +12,12 @@ import UIKit
public class CompactBadgeView: AfterpayLogo {
override public init(colorScheme: ColorScheme = .static(.default)) {
if Afterpay.isCashAppAfterpayRegion {

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.

Same for this view

imageView.translatesAutoresizingMaskIntoConstraints = false
addSubview(imageView)
setImageViewConstraints()
setupConstraints()

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.

Should the other constraints added in setupConstraints still be added if there is no image? What does this view render like with no image?

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.

Currently, since we use attributes in the image to calculate the constraints (the ratio) we shouldn't setup constraints unless the image exists. Could be changed to use a more modern layout method

@squarepaul

squarepaul commented Jun 18, 2025

Copy link
Copy Markdown
Collaborator Author

What would the state of these views be if the region is not supported?

The idea is in debug, it will crash so the implementers will know not to use that UI, but if for some reason they still end up using it we don't want to crash it production albeit less than ideal UI (it's hidden).

@squarepaul squarepaul force-pushed the fix-crash-from-unauthorized-ui branch from 5265192 to 74e255b Compare June 18, 2025 21:29
@squarepaul squarepaul merged commit ecf98af into master Jun 18, 2025
3 checks passed
@squarepaul squarepaul deleted the fix-crash-from-unauthorized-ui branch June 18, 2025 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants