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] Implement V2 variables and functions #4633

Merged
merged 11 commits into from
Jan 8, 2025

Conversation

joshdholtz
Copy link
Member

@joshdholtz joshdholtz commented Jan 6, 2025

Motivation

Implements the new Paywalls V2 variables (ex: product.currency_code, product.price, product.periodly)

Description

  • New implementation from Paywalls V1 variables
  • Uses localizations from UIConfig (from offering response)

Back supporting variables

If a variable is not found, it will attempt to find a mapping from UIConfig to use instead

⚠️ New support for functions (but more in a follow up PR)

There will also be some functions that will help format data (

{{ product.period | capitalize }}

Copy link

emerge-tools bot commented Jan 6, 2025

1 build increased size

Name Version Download Change Install Change Approval
Paywalls
com.revenuecat.PaywallsTester
1.0 (1) 10.8 MB ⬆️ 42.5 kB (0.39%) 40.3 MB ⬆️ 163.9 kB (0.41%) N/A

Paywalls 1.0 (1)
com.revenuecat.PaywallsTester

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

Total install size change: ⬆️ 163.9 kB (0.41%)
Total download size change: ⬆️ 42.5 kB (0.39%)

Largest size changes

Item Install Size Change
DYLD.String Table ⬆️ 45.8 kB
RevenueCatUI.TextComponentViewModel.TextComponentViewModel ⬆️ 9.0 kB
RCStoreProductDiscount.Objc Metadata ⬆️ 6.1 kB
DYLD.Exports ⬆️ 4.8 kB
Code Signature ⬆️ 4.0 kB
View Treemap

Image of diff


🛸 Powered by Emerge Tools

Comment trigger: Size diff threshold of 100.00kB exceeded

@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-colors branch from f7b6a15 to f3be655 Compare January 7, 2025 01:45
@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-variable-localizations branch from caac5b3 to 61cba31 Compare January 7, 2025 01:53
@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-colors branch from f3be655 to 0278854 Compare January 7, 2025 12:38
Base automatically changed from paywalls-v2/ui-config-colors to main January 7, 2025 13:20
@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-variable-localizations branch from 6d0827d to 58b3fdd Compare January 7, 2025 15:33
@joshdholtz joshdholtz force-pushed the paywalls-v2/ui-config-variable-localizations branch from 58b3fdd to 2a88bef Compare January 7, 2025 16:56
@joshdholtz joshdholtz marked this pull request as ready for review January 8, 2025 11:02
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.

Very cool stuff! Approved with some minor suggestions and questions.

Comment on lines +96 to +101
// Note: This is temporary while in closed beta and shortly after
let processedWithV2AndV1 = Self.processTextV1(
processedWithV2,
packageContext: packageContext,
locale: locale
)
Copy link
Member

Choose a reason for hiding this comment

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

Just curious why this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

@JayShortway I have a paywall in the wild that has V1 variables 🙃 And a few of our other beta users might since the dashboard originally was reusing V1 variables

Comment on lines +48 to +52
func getLocalizations(for locale: Locale) -> [String: String] {
guard let localizations = self.uiConfig.localizations[locale.identifier] else {
Logger.error("Could not find localizations for '\(locale.identifier)'")
return [:]
}
Copy link
Member

Choose a reason for hiding this comment

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

Should this throw? Or is it not part of the validation step?


}

struct Whisker {
Copy link
Member

Choose a reason for hiding this comment

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

Love the name! What's the inspiration?

Copy link
Member Author

@joshdholtz joshdholtz Jan 8, 2025

Choose a reason for hiding this comment

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

A dumb idea about improving our Handlebars like templating and Handlebars logo is a mustache and cat's whiskers are similar-ish to mustaches 😅

@joshdholtz joshdholtz merged commit fc21382 into main Jan 8, 2025
10 checks passed
@joshdholtz joshdholtz deleted the paywalls-v2/ui-config-variable-localizations branch January 8, 2025 15:17
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.

2 participants