-
Notifications
You must be signed in to change notification settings - Fork 34
PW-73 | Add docs for custom variables #1355
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
base: main
Are you sure you want to change the base?
Conversation
facumenzella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the docs! Found one issue with the iOS UIKit example - see inline comment.
facumenzella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the docs!
| let controller = PaywallViewController() | ||
|
|
||
| // Set custom variables | ||
| controller.setCustomVariable(.string("John"), forKey: "player_name") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The setCustomVariable method takes a plain String, not a CustomVariableValue. This example will not compile.
Option 1 - Using the property (preferred for Swift):
import UIKit
import RevenueCat
import RevenueCatUI
class MyViewController: UIViewController {
func showPaywall() {
let controller = PaywallViewController()
// Set custom variables
controller.customVariables = [
"player_name": .string(\"John\"),
"level": .string(\"42\")
]
self.present(controller, animated: true)
}
}
Option 2 - Using the ObjC-compatible method (takes plain String):
controller.setCustomVariable(\"John\", forKey: \"player_name\")
controller.setCustomVariable(\"42\", forKey: \"level\")
facumenzella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review: Android Code Samples Need Updates
Thanks for the docs! Found a few issues with the Android code samples:
1. Wrong import path
The import path for CustomVariableValue is incorrect in both Kotlin files:
// ❌ Current (wrong):
import com.revenuecat.purchases.ui.revenuecatui.data.processed.CustomVariableValue
// ✅ Should be:
import com.revenuecat.purchases.ui.revenuecatui.CustomVariableValue2. PaywallDialog example uses wrong options type
PaywallDialog takes PaywallDialogOptions, not PaywallOptions. The composable example should be:
import androidx.compose.runtime.Composable
import com.revenuecat.purchases.CustomerInfo
import com.revenuecat.purchases.ui.revenuecatui.PaywallDialog
import com.revenuecat.purchases.ui.revenuecatui.PaywallDialogOptions
import com.revenuecat.purchases.ui.revenuecatui.CustomVariableValue
@Composable
fun PaywallWithCustomVariables(
customerInfo: CustomerInfo,
onDismiss: () -> Unit
) {
val options = PaywallDialogOptions.Builder()
.setCustomVariables(
mapOf(
"player_name" to CustomVariableValue.String("John"),
"level" to CustomVariableValue.from(42),
"is_premium" to CustomVariableValue.from(true)
)
)
.setDismissRequest(onDismiss)
.setShouldDisplayBlock { !customerInfo.entitlements.active.containsKey("pro") }
.build()
PaywallDialog(paywallDialogOptions = options)
}3. Activity example needs fixes
import androidx.activity.ComponentActivity
import com.revenuecat.purchases.Offering
import com.revenuecat.purchases.ui.revenuecatui.activity.PaywallActivityLauncher
import com.revenuecat.purchases.ui.revenuecatui.CustomVariableValue
class MyActivity : ComponentActivity() {
private lateinit var paywallActivityLauncher: PaywallActivityLauncher
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
paywallActivityLauncher = PaywallActivityLauncher(this) { result ->
// Handle result
}
}
fun showPaywall(offering: Offering) {
paywallActivityLauncher.launch(
offering = offering,
customVariables = mapOf(
"player_name" to CustomVariableValue.String("John"),
"level" to CustomVariableValue.from(42)
)
)
}
}4. Document CustomVariableValue.from() for non-string values
The Number and Boolean types are internal, so for non-string values developers should use CustomVariableValue.from():
mapOf(
// For strings - use CustomVariableValue.String()
"player_name" to CustomVariableValue.String("John"),
// For numbers and booleans - use CustomVariableValue.from()
"level" to CustomVariableValue.from(42),
"score" to CustomVariableValue.from(99.5),
"is_premium" to CustomVariableValue.from(true)
)Consider adding a note in the docs mentioning that CustomVariableValue.from() accepts String, Int, Long, Float, Double, and Boolean types.
facumenzella
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more note: the public API only supports string values. The .number() and .bool() methods are internal and should not be documented. Please make sure none of the examples or text mention number/boolean types - all custom variable values should be passed as strings.
Correction to my previous reviewIgnore my suggestion about Since mapOf(
"player_name" to CustomVariableValue.String("John"),
"level" to CustomVariableValue.String("42"),
"is_premium" to CustomVariableValue.String("true")
)All custom variable values should be passed as strings in the public API. The SDK handles any necessary type conversions internally. |
Motivation / Description / Changes introduced
Add documentation for custom variables
Do not release till feature is live