Paywalls: remove unscrollable spacing in Template 5
#3562
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Motivation As I was implementing this template (with longer content that needs scrolling), the spacing cut off the headline in a weird way. It also takes up a small bit of vertical space unnecessarily. ### Description Instead of doing the previous layout: `header image / even space / scrollable content / even space / footer`, I updated the layout to make the header image and scrollable content in a 0 spacing VStack, with the same existing padding _inside_ the scrollable content to have the same initial layout as before. Probably best described with two videos: Before: https://github.com/RevenueCat/purchases-ios/assets/997637/0961e99c-8a25-4099-a763-6838ff1ac0ed After: https://github.com/RevenueCat/purchases-ios/assets/997637/22274ed2-3ab1-4fc1-894b-83cd8013bce8 Let me know if you have any questions! (@joshdholtz told me to just make a PR, so this is mostly his fault.)
tonidero
approved these changes
Jan 9, 2024
|
There's a slight difference in the default top padding, I'll fix it: https://github.com/RevenueCat/purchases-ios-snapshots/pull/111/files The footer ones do pass. |
NachoSoto
commented
Jan 9, 2024
| } | ||
| } | ||
|
|
||
| private var scrollableContent: some View { | ||
| VStack(spacing: self.defaultVerticalPaddingLength) { | ||
| if self.configuration.mode.isFullScreen { | ||
| Spacer() |
There was a problem hiding this comment.
Added this to make spacing even now.
NachoSoto
pushed a commit
that referenced
this pull request
Jan 10, 2024
**This is an automatic release.** ### RevenueCatUI * `Paywalls`: remove unscrollable spacing in Template 5 (#3562) via NachoSoto (@NachoSoto) * `Paywalls`: improve template 5 checkmark icon (#3559) via NachoSoto (@NachoSoto) ### Bugfixes * Improve sandbox detector for macOS apps (#3549) via Mark Villacampa (@MarkVillacampa) ### Other Changes * `Paywalls`: new `PaywallViewControllerDelegate.paywallViewController(_:didChangeSizeTo:)` (#3563) via Cesar de la Vega (@vegaro) * `Tests`: running tests on `macOS` (#3533) via NachoSoto (@NachoSoto) * `Integration Tests`: split into separate jobs (#3560) via NachoSoto (@NachoSoto)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By @thillsman. See #3561.
Motivation
As I was implementing this template (with longer content that needs scrolling), the spacing cut off the headline in a weird way. It also takes up a small bit of vertical space unnecessarily.
Description
Instead of doing the previous layout:
header image / even space / scrollable content / even space / footer, I updated the layout to make the header image and scrollable content in a 0 spacing VStack, with the same existing padding inside the scrollable content to have the same initial layout as before. Probably best described with two videos:Before:
Screen.Recording.2024-01-08.at.10.10.17.PM.mov
After:
Screen.Recording.2024-01-08.at.10.07.46.PM.mov