Payment Button Sizing Bug

paypal / paypal-ios

One merchant integration point for all of PayPal's services

Apache License 2.0

59 stars 27 forks source link

Payment Button Sizing Bug #247

Closed stechiu closed 8 months ago

stechiu commented 8 months ago

Reason for changes

There was a sizing issue with the Payment Button when used in SwiftUI apps

The button couldn't be horizontally centered
The button frame took up the entirety of the screen size rather than hugging the button
If a button was added to a Form Section, the button wouldn't resize to fit the Section dimensions; instead, the size remained static

Summary of changes

This PR fixes the above issues
A dev won't be able to set the height to smaller than 38px or width shorter than 300px
PayLater and Credit are also displayed correctly in SwiftUI
The mini button logo wasn't the correct size before, so that has been fixed

Bug	PR

Checklist

[x] Added a changelog entry

Authors

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

@stechiu , @nirvan543

stechiu commented 8 months ago

Just a small comment about updating our docstrings in all 3 inits to note the default in the inits. Also, do we need to make any changes on main to fix the sizing issue for currently released versions of the SDK since this change is going into next major version?

@jaxdesmarais After thinking about it, this PR could go into main since the change isn't breaking and fixes an existing issue. I'll point the branch to main

stechiu commented 8 months ago

Just a small comment about updating our docstrings in all 3 inits to note the default in the inits. Also, do we need to make any changes on main to fix the sizing issue for currently released versions of the SDK since this change is going into next major version?

@jaxdesmarais After thinking about it, this PR could go into main since the change isn't breaking and fixes an existing issue. I'll point the branch to main

Well it looks like pointing to main means introducing all the changes made from next-major-version into main... If we prefer this to go into main, it's probably best to close this PR and open a new one that is branched from main. What do you think?

jaxdesmarais commented 8 months ago

After thinking about it, this PR could go into main since the change isn't breaking and fixes an existing issue. I'll point the branch to main

Yep, now that we separated out the representable changes into a follow up PR we should be able to make the changes in this PR in main. Agree it's likely easier to close this PR and open a new PR off of main so you don't end up with all the changes from next major version.

stechiu commented 8 months ago

After thinking about it, this PR could go into main since the change isn't breaking and fixes an existing issue. I'll point the branch to main

Yep, now that we separated out the representable changes into a follow up PR we should be able to make the changes in this PR in main. Agree it's likely easier to close this PR and open a new PR off of main so you don't end up with all the changes from next major version.

Ok sounds good! I'll make a new PR where I branch from main. Thanks for your help