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

Add custom corner radius for PaymentButtons #229

Merged
merged 7 commits into from
Jan 4, 2024
Merged

Conversation

vradopp
Copy link
Contributor

@vradopp vradopp commented Dec 15, 2023

Reason for changes

To allow consumers of the SDK to use a custom cornerRadius for the PaymentButton

Summary of changes

  • Added a custom(CGFloat) case to PaymentButtonEdges

Checklist

  • Added a changelog entry

Authors

List GitHub usernames for everyone who contributed to this pull request.

@vradopp vradopp requested a review from a team as a code owner December 15, 2023 20:05
CHANGELOG.md Outdated Show resolved Hide resolved
@@ -12,6 +12,9 @@ public enum PaymentButtonEdges: Int {
/// Pill shaped corner radius.
case rounded

/// Custom corner radius.
case custom(CGFloat)

func cornerRadius(for view: UIView) -> CGFloat {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
func cornerRadius(for view: UIView) -> CGFloat {
func cornerRadius(for buttonSize: CGRect) -> CGFloat {

🧶 : Might be overkill, but it would remove need for import UIKit for this enum.

Stepper("Custom Corner Radius: \(customEdge)", value: $customEdge, in: 0...100).onChange(of: customEdge) { _ in
if selectedEdge.description == "custom" {
selectedEdge = PaymentButtonEdges.custom(CGFloat(customEdge))
buttonId += 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

curious q: is button ID used by SwiftUI to re-render?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
buttonId += 1
buttonID += 1

Also side note our team uses ID naming convention for iOS.

Copy link
Contributor Author

@vradopp vradopp Jan 4, 2024

Choose a reason for hiding this comment

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

curious q: is button ID used by SwiftUI to re-render?

Yeah the views use the buttonId as their .id() and it triggers a re-render

Also side note our team uses ID naming convention for iOS.

It was already declared I can change it though

Copy link
Contributor

@stechiu stechiu left a comment

Choose a reason for hiding this comment

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

Reviewed and tested in demo app. LGTM

@scannillo
Copy link
Collaborator

Changes LGTM! Wanted to call out that we technically can target this for main since this is an additive change, not a breaking change.

I also understand if we want to keep all PaymentButton related changes in V2, though

@@ -63,7 +64,12 @@ struct SwiftUIPaymentButtonDemo: View {
selectedEdge = PaymentButtonEdges.allCases[edgesIndex]
buttonId += 1
}

Stepper("Custom Corner Radius: \(customEdge)", value: $customEdge, in: 0...100).onChange(of: customEdge) { _ in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very neat!

@scannillo
Copy link
Collaborator

Looks like this equivalent change went into main (V1) on Android (see PR). We should keep this consistent across platforms.

@vradopp vradopp changed the base branch from next-major-version to main January 4, 2024 20:35
@vradopp vradopp merged commit 9fd8dba into main Jan 4, 2024
4 checks passed
@vradopp vradopp deleted the button-corner-radius branch January 4, 2024 20:44
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.

6 participants