`Paywalls`: improve landscape support of all templates by NachoSoto · Pull Request #3471 · RevenueCat/purchases-ios

NachoSoto · 2023-11-21T23:30:21Z

Template 2

Before

After

Template 3

Before

After

Template 4

Before

After

Template 5

Before

After

NachoSoto · 2023-11-21T23:43:15Z

    }

+    var isVerticalSizeCompact: Bool {
+        #if os(tvOS)


We don't support tvOS yet, but this is what we'll want to do anyway.

NachoSoto · 2023-11-21T23:43:45Z

+            .padding(.top, self.defaultVerticalPaddingLength)
+    }
+
+    // TODO: snapshots


TODO: need snapshots for landscape paywalls.

guido732 · 2023-11-22T14:32:03Z

first of all, looks great!

I'm just wondering about that close button X and the alignment so it doesn't overlap with the content at any point. Is there a chance to adjust it or move it to the side outside of the content?

aboedo · 2023-11-22T15:14:47Z

ohhh that is really cool

NachoSoto · 2023-11-23T02:58:12Z

I'm just wondering about that close button X and the alignment so it doesn't overlap with the content at any point. Is there a chance to adjust it or move it to the side outside of the content?

Thanks for taking a close look at this!

I'll check in more detail, but that's actually the default alignment for both the close button (navigation bar buttons) and the main content.

NachoSoto · 2023-12-19T17:46:17Z

    var userInterfaceIdiom: UserInterfaceIdiom { get }

+    /// `UserInterfaceSizeClass` is only available for macOS/watchOS/tvOS with Xcode 15
+    #if swift(>=5.9) || (!os(macOS) && !os(watchOS) && !os(tvOS))


Ideally we mark TemplateViewType as unavailable in these platforms (since it is), but that leads to a SwiftUI warning:

_modify accessor cannot be more available than unavailable enclosing scope

And having a warning breaks CocoaPods linting.

what's the story with visionOS for these? wondering if it'd be easier or safer to allow-list iOS rather than deny-list the rest

It is available in visionOS.
I went with deny-list because presumedly we'll remove these eventually when adding support for them (other than watchOS which uses a separate template)

aboedo

looking great!

aboedo · 2023-12-19T19:47:46Z

+        // tvOS never reports UserInterfaceSizeClass.compact
+        // but for the purposes of template layouts, we consider landscape
+        // on tvOS as compact to produce horizontal layouts.


this is a fair point, but it makes me wonder whether the flag should be named something more representative, like shouldUseLandscapeLayout or something, given that it's not really "compact" for tvOS and that on macOS it's neither compact nor not compact, right?

Yup, I'll rename.

aboedo · 2023-12-19T19:48:29Z

+    @ViewBuilder
+    private var horizontalContent: some View {
+        VStack {
+            HStack {
+                VStack {
+                    Spacer()
+                    self.iconImage
+                    Spacer()
+                    self.title
+                    Spacer()
+                    self.subtitle
+                    Spacer()
+                }
+                .scrollableIfNecessary()
+                .frame(maxHeight: .infinity)
+
+                VStack(spacing: self.defaultVerticalPaddingLength) {
+                    Spacer()
+
+                    self.packages
+                        .scrollableIfNecessary()
+
+                    Spacer(minLength: self.defaultVerticalPaddingLength)
+
+                    self.subscribeButton
+                }
+                .frame(maxHeight: .infinity)
+            }
+
+            self.footer
+        }


this just looks so nice

aboedo · 2023-12-19T19:50:43Z

    var userInterfaceIdiom: UserInterfaceIdiom { get }

+    /// `UserInterfaceSizeClass` is only available for macOS/watchOS/tvOS with Xcode 15
+    #if swift(>=5.9) || (!os(macOS) && !os(watchOS) && !os(tvOS))


what's the story with visionOS for these? wondering if it'd be easier or safer to allow-list iOS rather than deny-list the rest

@tonidero

**This is an automatic release.** ### RevenueCatUI * `Paywalls`: add `PaywallFooterViewController` (#3486) via Toni Rico (@tonidero) * `Paywalls`: improve landscape support of all templates (#3471) via NachoSoto (@NachoSoto) * `Paywalls`: ensure footer links open in full-screen sheets (#3524) via NachoSoto (@NachoSoto) * `Paywalls`: improve `FooterView` text alignment (#3525) via NachoSoto (@NachoSoto) * Paywalls: Add dismissal method in `PaywallViewControllerDelegate` (#3493) via Toni Rico (@tonidero)

NachoSoto added the pr:RevenueCatUI label Nov 21, 2023

NachoSoto requested a review from a team November 21, 2023 23:30

NachoSoto force-pushed the paywalls-landscape-2 branch from f2857b9 to d97ac9a Compare November 21, 2023 23:31

NachoSoto commented Nov 21, 2023

View reviewed changes

Comment thread RevenueCatUI/Templates/Template3View.swift Outdated

Base automatically changed from paywalls-landscape to main November 23, 2023 01:18

NachoSoto force-pushed the paywalls-landscape-2 branch from d97ac9a to 3861659 Compare November 23, 2023 02:58

NachoSoto force-pushed the paywalls-landscape-2 branch from 3861659 to 2bea659 Compare December 7, 2023 00:24

NachoSoto marked this pull request as draft December 8, 2023 20:52

NachoSoto force-pushed the paywalls-landscape-2 branch from 2bea659 to c25d819 Compare December 18, 2023 21:39

NachoSoto marked this pull request as ready for review December 18, 2023 22:22

NachoSoto changed the title ~~[WIP] Paywalls: improve landscape support of all templates~~ Paywalls: improve landscape support of all templates Dec 18, 2023

NachoSoto requested a review from a team December 18, 2023 22:22

NachoSoto force-pushed the paywalls-landscape-2 branch from 3cdd597 to 0b5f399 Compare December 18, 2023 23:12

NachoSoto changed the base branch from main to paywalls-footer-text-alignment December 18, 2023 23:29

NachoSoto force-pushed the paywalls-landscape-2 branch from 48bc434 to 0cc0f84 Compare December 18, 2023 23:43

Base automatically changed from paywalls-footer-text-alignment to main December 19, 2023 16:33

NachoSoto added 7 commits December 19, 2023 08:41

Paywalls: improve landscape support of all templates

18aaead

Snapshot tests

fb13eee

Handle some TODOs

d42c324

Restore TestData

ecbed69

Fix macOS

6f36469

Update snapshots

9d59b6a

Fix watchOS

501b604

NachoSoto force-pushed the paywalls-landscape-2 branch from 0cc0f84 to 501b604 Compare December 19, 2023 16:41

Fix tvOS

4454a38

NachoSoto commented Dec 19, 2023

View reviewed changes

aboedo approved these changes Dec 19, 2023

View reviewed changes

isVerticalSizeCompact -> shouldUseLandscapeLayout

9dd05e1

NachoSoto enabled auto-merge (squash) December 19, 2023 20:09

NachoSoto merged commit c5e039a into main Dec 19, 2023

NachoSoto deleted the paywalls-landscape-2 branch December 19, 2023 20:24

RCGitBot mentioned this pull request Dec 19, 2023

[AUTOMATIC] Release/4.31.5 #3527

Merged

MarkVillacampa mentioned this pull request Dec 21, 2023

Release/4.32.0-beta1 #3535

Closed

Uh oh!

Conversation

NachoSoto commented Nov 21, 2023

Template 2

Before

After

Template 3

Before

After

Template 4

Before

After

Template 5

Before

After

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guido732 commented Nov 22, 2023

Uh oh!

aboedo commented Nov 22, 2023

Uh oh!

NachoSoto commented Nov 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aboedo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants