Paywalls: new PaywallIcon
#1274
Conversation
NachoSoto
commented
Sep 20, 2023
NachoSoto
commented
NachoSoto
force-pushed
the
nacho/pwl-266-ability-to-render-svg-icons
branch
from
6b521bd to
fbd5ad0
Compare
| /** | ||
| * An icon that can be displayed inside a RevenueCat paywall. | ||
| */ | ||
| enum class PaywallIconName(val value: String) { |
There was a problem hiding this comment.
We need to expose this for https://linear.app/revenuecat/issue/PWL-194/icons /cc @vegaro
So we can iterate through the icons inside of features and verify that they're all valid.
There was a problem hiding this comment.
Hmm still not clear to me... Are we performing the validation in the purchases module? If so, should we move this to the purchases module as well and make it public there? If we make it internal here, we can still perform the validation in this module I think.
There was a problem hiding this comment.
Sorry I read your first comment as "private", I keep forgetting that Kotlin's default is public and not internal like in Swift.
| TRANSFER("transfer"), | ||
| TWO_WAY_ARROWS("two_way_arrows"), | ||
| KEY("key"), | ||
| WARNING("warning"), |
There was a problem hiding this comment.
I believe the strings always match the enum but uppercased right? If so, we could just remove the string value and in the fromValue, we can do PaywallIconName.valueOf(value.uppercased()) I think. Or create a map from the lowercased strings to the enums as well, to avoid linear lookup
|
|
||
| @Preview(showBackground = true, widthDp = 300) | ||
| @Composable | ||
| internal fun PaywallIconPreview() { |
| modifier = Modifier | ||
| .aspectRatio(1.0f) | ||
| .fillMaxSize() | ||
| .then(modifier), |
There was a problem hiding this comment.
So this would apply the modifiers in the modifier parameter AFTER the aspectRation/fillMaxSize modifiers. That might be ok depending on the usage. Most times, when there is a modifier parameter, I've seen it used instead of the Modifier name, so this would look like:
modifier = modifier
.aspectRatio(1.0f)
.fillMaxSize(),
But again, this might be correct if we need to apply the modifiers in the parameters last.
There was a problem hiding this comment.
Yeah I don't think there's a right answer, but I was thinking that this way you can override the aspect ratio, etc. if needed.
| LazyVerticalGrid(columns = GridCells.Adaptive(40.dp)) { | ||
| items(icons.size) { | ||
| Box(modifier = Modifier.background(randomColor())) { | ||
| PaywallIcon(icon = icons[it], tintColor = Color.Black) |
There was a problem hiding this comment.
I guess there is a chance the background color is also black? I guess it's probably not a big deal though.
There was a problem hiding this comment.
Sure 😅 it's just a way to make them easier to see in a grid.
NachoSoto
force-pushed
the
nacho/pwl-266-ability-to-render-svg-icons
branch
from
fbd5ad0 to
7b3cbb2
Compare
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## paywalls #1274 +/- ##
=========================================
Coverage 85.46% 85.46%
=========================================
Files 189 189
Lines 6376 6376
Branches 929 929
=========================================
Hits 5449 5449
Misses 568 568
Partials 359 359 ☔ View full report in Codecov by Sentry. |
