Skip to content

[Paywalls V2] Add full cover option to Paywalls Tester#4745

Merged
joshdholtz merged 2 commits into
mainfrom
paywalls-v2/add-full-cover-option-to-paywalls-tester
Jan 30, 2025
Merged

[Paywalls V2] Add full cover option to Paywalls Tester#4745
joshdholtz merged 2 commits into
mainfrom
paywalls-v2/add-full-cover-option-to-paywalls-tester

Conversation

@joshdholtz

@joshdholtz joshdholtz commented Jan 30, 2025

Copy link
Copy Markdown
Member

Motivation

Changes here were made to test #4744

Description

Make a separate full screen for:

  • Full cover
  • Sheet
Screen.Recording.2025-01-29.at.5.53.51.PM.mov

@emerge-tools

emerge-tools Bot commented Jan 30, 2025

Copy link
Copy Markdown

1 build increased size

Name Version Download Change Install Change Approval
Paywalls
com.revenuecat.PaywallsTester
1.0 (1) 10.8 MB ⬆️ 7.5 kB (0.07%) 40.8 MB ⬆️ 34.7 kB (0.09%) N/A

Paywalls 1.0 (1)
com.revenuecat.PaywallsTester

⚖️ Compare build
📦 Install build
⏱️ Analyze build performance

Total install size change: ⬆️ 34.7 kB (0.09%)
Total download size change: ⬆️ 7.5 kB (0.07%)

Largest size changes

Item Install Size Change
DYLD.String Table ⬆️ 13.9 kB
Code Signature ⬆️ 936 B
DYLD.Exports ⬆️ 912 B
📝 RCPurchases.applicationDidBecomeActive ⬆️ 900 B
RCPurchases.Objc Metadata ⬇️ -873 B
View Treemap

Image of diff


🛸 Powered by Emerge Tools

@joshdholtz joshdholtz changed the base branch from main to paywalls-v2/ignore-top-edges-for-image January 30, 2025 03:06
@joshdholtz joshdholtz force-pushed the paywalls-v2/ignore-top-edges-for-image branch from 0ccb313 to b73ce2f Compare January 30, 2025 03:43
@joshdholtz joshdholtz force-pushed the paywalls-v2/add-full-cover-option-to-paywalls-tester branch from eec3175 to cc82b4f Compare January 30, 2025 03:46
@joshdholtz joshdholtz requested review from a team January 30, 2025 03:48
// CI system adds keys here
extension AvailableConfigItems {
static var apiKey: String { "" }
static var apiKey: String { "appl_SYkbbYptqjDadVkQeBUtXxVxCcw" }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Added by mistake?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Totally lol

@JayShortway JayShortway left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM! No additional comments besides Antonio's.

Comment on lines +11 to +16
enum PaywallTesterViewMode: String {
case fullScreen = "Full Screen"
case sheet = "Sheet"
case footer = "Footer"
case condensedFooter = "Condensed Footer"
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

More a question to get context than a comment. Are those rawValue coming from somewhere? Like, are those used for decoding / encoding? Because I see the name uses the same strings.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ahhh, I was going to use them as a display name in the UI but ended up making with a name variable in the extension instead 😊 Going to remove these!

@joshdholtz joshdholtz force-pushed the paywalls-v2/add-full-cover-option-to-paywalls-tester branch from cc82b4f to 1e2e423 Compare January 30, 2025 10:54
@joshdholtz joshdholtz marked this pull request as ready for review January 30, 2025 10:57
Base automatically changed from paywalls-v2/ignore-top-edges-for-image to main January 30, 2025 11:04
@joshdholtz joshdholtz force-pushed the paywalls-v2/add-full-cover-option-to-paywalls-tester branch from 1e2e423 to 8abb7fa Compare January 30, 2025 11:06
.onRestoreCompleted { _ in
viewModel.dismissPaywall()
}
.id(viewModel.presentedPaywall?.hashValue) //FIXME: This should not be required, issue is in Paywallview

@MarkVillacampa MarkVillacampa Jan 30, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I hit a similar issue when working on the paywall reload, the reason being:

https://github.com/RevenueCat/purchases-ios/blob/main/RevenueCatUI/Templates/V2/PaywallsV2View.swift#L125-L137

Which is fine because normally paywall info will not change while it is presented.

@joshdholtz joshdholtz merged commit f1c4205 into main Jan 30, 2025
@joshdholtz joshdholtz deleted the paywalls-v2/add-full-cover-option-to-paywalls-tester branch January 30, 2025 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants