Skip to content

Add intrinsicContentSize to CardFormView to allow presentation in SwiftUI to work properly #5134

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

Closed
wants to merge 2 commits into from

Conversation

wooj-stripe
Copy link
Collaborator

@wooj-stripe wooj-stripe commented Jul 9, 2025

Summary

SwiftUI is having trouble rendering this view when surrounded by high priority views. To remedy, returning the intrinsic content size seems to remedy this. Not ideal as having to manually calculate it is difficult. If there are other approaches I'm open to it, but given we only had to re-record a single snapshot test, i'm okay moving forward with this solution.

Motivation

Surrounding the view with high priority views was breaking this view in swift UI

Testing

Existing tests in:

  • STPStackViewWithSeparatorSnapshotTests
  • STPFormViewSnapshotTests
  • STPCardFormViewSnapshotTests
    Manually tested by using the UI Examples app and testing w/ surrounding between two high priority views.

Changelog

Copy link

github-actions bot commented Jul 9, 2025

⚠️ Public API changes detected:

StripePaymentsUI

- public var intrinsicContentSize: CoreFoundation.CGSize {
+ @objc get
+ }

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.

@wooj-stripe wooj-stripe changed the title Different approach Add intrinsicContentSize to CardFormView to allow presentation in SwiftUI to work properly Jul 9, 2025
@wooj-stripe wooj-stripe force-pushed the wooj/cardFormViewPriorityViewSwiftUI branch 3 times, most recently from 2812b78 to a737c9e Compare July 10, 2025 01:39
@wooj-stripe wooj-stripe force-pushed the wooj/cardFormViewPriorityViewSwiftUI branch from a737c9e to 3ed5ccf Compare July 10, 2025 05:44
@wooj-stripe wooj-stripe marked this pull request as ready for review July 10, 2025 06:03
@wooj-stripe wooj-stripe requested review from a team as code owners July 10, 2025 06:03
@wooj-stripe wooj-stripe requested review from davidme-stripe, porter-stripe and gbirch-stripe and removed request for davidme-stripe July 10, 2025 06:03
@@ -65,6 +65,22 @@ public class STPFormView: UIView, STPFormInputValidationObserver {
static let cornerRadius: CGFloat = 6
static let interSectionSpacing: CGFloat = 7

public override var intrinsicContentSize: CGSize {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason we can't just override this in the STPCardFormView.Representable? I think overriding this where we us constraints and intrinsicContentSize may not be correct but not sure.

wooj-stripe added a commit that referenced this pull request Jul 11, 2025
## Summary
This is a much better approach than:
#5134

## Motivation
CardFormView basically shrinks down to nothing if surrounded by high
priority views.

## Testing
Relying on existing screenshot tests and manual testing

## Changelog
<!-- Is this a notable change that affects users? If so, add a line to
`CHANGELOG.md` and prefix the line with one of the following:
    - [Added] for new features.
    - [Changed] for changes in existing functionality.
    - [Deprecated] for soon-to-be removed features.
    - [Removed] for now removed features.
    - [Fixed] for any bug fixes.
    - [Security] in case of vulnerabilities.
-->
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.

2 participants