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

[Paywalls V2] Fix vstack and hstack growing size when fit #4646

Merged
merged 6 commits into from
Jan 13, 2025

Conversation

joshdholtz
Copy link
Member

Motivation

Vertical and horizontal stacks would act as fill when set as fit

Description

  • Remove frame infinity
  • Added new SwiftUI preview for snapshot
Before After
Screenshot 2025-01-10 at 11 24 32 AM Screenshot 2025-01-10 at 11 24 47 AM

// self.modifier(SizeModifier(size: size,
// hortizontalAlignment: alignment,
// verticalAlignment: alignment))
// }

Choose a reason for hiding this comment

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

Perhaps this commented out code can be removed

@@ -58,7 +58,8 @@ struct TextComponentView: View {
.multilineTextAlignment(style.textAlignment)
.foregroundColorScheme(style.color, uiConfigProvider: self.viewModel.uiConfigProvider)
.padding(style.padding)
.size(style.size, alignment: style.horizontalAlignment)
.size(style.size,
horizontalAlignment: style.horizontalAlignment)

Choose a reason for hiding this comment

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

Don't we need to also pass the verticalAlignment parameter here?

.size(style.size,
      horizontalAlignment: style.horizontalAlignment,
      verticalAlignment: style.verticalAlignment)

Copy link
Member Author

Choose a reason for hiding this comment

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

I also thought the same thing at first too 😅 But... text doesn't have a vertical alignment property for its content 🤷‍♂️

Choose a reason for hiding this comment

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

Oh, you're totally right! Thanks for the clarification!

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, you're totally right! Thanks for the clarification!

Np np! Thanks for asking about that because it does seem like an odd change at first 😇

Copy link
Member

@JayShortway JayShortway left a comment

Choose a reason for hiding this comment

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

Looks good! No comments besides Antonio's.

@joshdholtz joshdholtz merged commit c100009 into main Jan 13, 2025
10 checks passed
@joshdholtz joshdholtz deleted the paywalls-v2/fix-stack-infinity branch January 13, 2025 11:33
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.

4 participants