-
Notifications
You must be signed in to change notification settings - Fork 45
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
: new TestData
to store fake paywalls
#1239
Conversation
@Preview(showBackground = true) | ||
@Composable | ||
fun Template2PaywallPreview() { | ||
InternalPaywallView(offering = TestData.template2Offering) |
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.
This doesn't actually work, and I have no idea why. Preview fails to load:
java.lang.ClassNotFoundException: com.revenuecat.purchases.Package at java.lang.ClassLoader.loadClass at java.lang.ClassLoader.loadClass at java.lang.Class.getDeclaredMethods0 at java.lang.Class.privateGetDeclaredMethods at java.lang.Class.getDeclaredMethods at androidx.compose.ui.tooling.ComposableInvoker.getDeclaredCompatibleMethod ... (ComposableInvoker.kt:54) at androidx.compose.ui.tooling.ComposableInvoker.findComposableMethod(ComposableInvoker.kt:74)
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.
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.
I fixed the data now so all packages are found. But now I'm running into this from PurchaseButton
:
java.lang.IllegalStateException: Error finding activity
@Preview(showBackground = true) | ||
@Composable | ||
fun Template2PaywallPreview() { | ||
InternalPaywallView(offering = TestData.template2Offering) |
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.
ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/templates/Template2.kt
Outdated
Show resolved
Hide resolved
import com.revenuecat.purchases.paywalls.PaywallData | ||
import java.net.URL | ||
|
||
class TestData { |
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.
Same here, we could make this internal
ui/revenuecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/data/TestData.kt
Outdated
Show resolved
Hide resolved
Also, looks like you might need to run detekt to autocorrect some issues, you can just do |
5373f71
to
b2ab8e9
Compare
81383cc
to
e978586
Compare
e978586
to
c5feb8f
Compare
}?.value | ||
} | ||
|
||
private val Locale.languageWithFallback: String |
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.
Maybe add a comment mentioning this is to fix testing and previews
@@ -22,7 +24,7 @@ internal interface PaywallViewModel { | |||
val state: StateFlow<PaywallViewState> | |||
|
|||
fun selectPackage(packageToSelect: Package) | |||
fun purchaseSelectedPackage(activity: Activity) | |||
fun purchaseSelectedPackage(context: Context) |
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.
hmm, I worry about this becuase there's no indication to the developers that this should be a context with an Activity
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.
This is to fix the previews right? 🤔
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.
Yes, it's to fix the previews. Basically, it can't find an activity in the preview context so it fails... Any ideas of how to solve this? I can add a comment to the function so at least we are aware when using this function... And well, this context is usually an activity... I don't think LocalContext.current
would return any service context or anything like that afaik.
This allows us to create previews for each template so we can easily iterate on them.
3f01733
to
c4018b5
Compare
@tonidero this should be ready now :) |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## paywalls #1239 +/- ##
===========================================
Coverage ? 85.45%
===========================================
Files ? 187
Lines ? 6351
Branches ? 923
===========================================
Hits ? 5427
Misses ? 567
Partials ? 357 ☔ View full report in Codecov by Sentry. |
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.
Nice job! This will make iterating much faster 💪
val images = PaywallData.Configuration.Images( | ||
header = "9a17e0a7_1689854430..jpeg", | ||
background = "9a17e0a7_1689854342..jpg", | ||
icon = "9a17e0a7_1689854430..jpeg", |
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.
Is it normal these have two dots?
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.
That was a bug in how the backend was storing these initially (/cc @joshdholtz 😅)
It's since been fixed, but these test images were uploaded early on.
This allows us to create previews for each template so we can easily iterate on them.