Skip to content

fix(paywalls): clamp negative margins/padding to zero #3559

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.

Sign up for GitHub

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

Merged
facumenzella merged 3 commits into from
Jun 11, 2026
Merged

fix(paywalls): clamp negative margins/padding to zero #3559

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43c3c82
fix: clamp negative margins/padding to zero
facumenzella Jun 8, 2026
fb04cfc
Log a warning on negative padding/margin and add clamping preview
facumenzella Jun 9, 2026
0279786
Only warn on negative padding/margin at style-build time
facumenzella Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 24 additions & 6 deletions ...ecatui/src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/ktx/Padding.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,33 @@ package com.revenuecat.purchases.ui.revenuecatui.components.ktx
import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.ui.unit.dp
import com.revenuecat.purchases.paywalls.components.properties.Padding
import com.revenuecat.purchases.ui.revenuecatui.helpers.Logger

/**
* Converts this Padding to PaddingValues, interpreting all values as [dp].
*
* Negative values are clamped to zero. Android does not support negative padding (and uses padding
* to apply margins too), so any negative values the dashboard sends are coerced to zero rather than
* crashing or rendering incorrectly.
*
* Set [warnIfNegative] to log a warning when clamping occurs. This should only be enabled when
* converting values straight from the dashboard model (e.g. in StyleFactory, once per paywall
* build), not on hot recomposition paths like `derivedStateOf`, to avoid logging the same
* misconfiguration repeatedly on state changes.
*/
@JvmSynthetic
internal fun Padding.toPaddingValues(): PaddingValues =
PaddingValues(
start = leading.dp,
top = top.dp,
end = trailing.dp,
bottom = bottom.dp,
internal fun Padding.toPaddingValues(warnIfNegative: Boolean = false): PaddingValues {
if (warnIfNegative && minOf(top, bottom, leading, trailing) < 0.0) {
Logger.w(
"Received negative padding/margin value(s) " +
"(top=$top, bottom=$bottom, leading=$leading, trailing=$trailing). " +
"Negative padding/margin is not supported on Android; clamping to 0.",
)
}
Comment thread
cursor[bot] marked this conversation as resolved.
return PaddingValues(
start = leading.coerceAtLeast(0.0).dp,
top = top.coerceAtLeast(0.0).dp,
end = trailing.coerceAtLeast(0.0).dp,
bottom = bottom.coerceAtLeast(0.0).dp,
Comment thread
facumenzella marked this conversation as resolved.
)
}
51 changes: 51 additions & 0 deletions ...in/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/stack/StackComponentView.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import com.revenuecat.purchases.ui.revenuecatui.components.PaywallAction
import com.revenuecat.purchases.ui.revenuecatui.components.WithOptionalBackgroundOverlay
import com.revenuecat.purchases.ui.revenuecatui.components.ktx.toAlignment
import com.revenuecat.purchases.ui.revenuecatui.components.ktx.toHorizontalAlignmentOrNull
import com.revenuecat.purchases.ui.revenuecatui.components.ktx.toPaddingValues
import com.revenuecat.purchases.ui.revenuecatui.components.ktx.toShape
import com.revenuecat.purchases.ui.revenuecatui.components.ktx.toVerticalAlignmentOrNull
import com.revenuecat.purchases.ui.revenuecatui.components.modifier.background
Expand Down Expand Up @@ -1003,6 +1004,56 @@ private fun StackComponentView_Preview_Vertical() {
}
}

@Suppress("MagicNumber")
@Preview(uiMode = Configuration.UI_MODE_NIGHT_NO or Configuration.UI_MODE_TYPE_NORMAL)
@Composable
private fun StackComponentView_Preview_NegativePaddingAndMarginClamped() {
// Negative padding/margin is not supported on Android. The dashboard can still send it, so it
// should be clamped to 0 rather than rendering incorrectly. This preview should look identical
// to one with zero padding and margin.
Box(
modifier = Modifier.padding(all = 32.dp),
) {
StackComponentView(
style = StackComponentStyle(
children = previewChildren(),
dimension = Dimension.Vertical(
alignment = HorizontalAlignment.CENTER,
distribution = FlexDistribution.START,
),
visible = true,
size = Size(width = Fit, height = Fit),
spacing = 16.dp,
background = BackgroundStyles.Color(
ColorStyles(
light = ColorStyle.Solid(Color.Red),
dark = ColorStyle.Solid(Color.Yellow),
),
),
padding = Padding(top = -16.0, bottom = -16.0, leading = -16.0, trailing = -16.0).toPaddingValues(),
margin = Padding(top = -16.0, bottom = -16.0, leading = -16.0, trailing = -16.0).toPaddingValues(),
shape = Shape.Rectangle(CornerRadiuses.Dp(all = 20.0)),
border = BorderStyles(width = 2.dp, colors = ColorStyles(light = ColorStyle.Solid(Color.Blue))),
shadow = ShadowStyles(
colors = ColorStyles(ColorStyle.Solid(Color.Black)),
radius = 10.dp,
x = 0.dp,
y = 3.dp,
),
badge = null,
scrollOrientation = null,
rcPackage = null,
tabIndex = null,
countdownDate = null,
countFrom = CountdownComponent.CountFrom.DAYS,
overrides = emptyList(),
),
state = previewEmptyState(),
clickHandler = {},
)
}
}

@Suppress("MagicNumber")
@Preview
@Composable
Expand Down
34 changes: 17 additions & 17 deletions ...src/main/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/style/StyleFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,8 +895,8 @@ internal class StyleFactory(
size = component.size,
spacing = (component.spacing ?: DEFAULT_SPACING).dp,
background = background,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
shape = component.shape ?: DEFAULT_SHAPE,
border = borderStyles,
shadow = shadowStyles,
Expand Down Expand Up @@ -962,8 +962,8 @@ internal class StyleFactory(
backgroundColor = backgroundColor,
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
rcPackage = rcPackage,
resolvedOffer = resolvedOffer,
tabIndex = tabControlIndex,
Expand Down Expand Up @@ -998,8 +998,8 @@ internal class StyleFactory(
sources,
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
shape = component.maskShape?.toShape(),
border = border,
shadow = shadow,
Expand Down Expand Up @@ -1053,8 +1053,8 @@ internal class StyleFactory(
shadow = shadow,
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
padding = component.padding?.toPaddingValues() ?: PaddingValues(),
margin = component.margin?.toPaddingValues() ?: PaddingValues(),
padding = component.padding?.toPaddingValues(warnIfNegative = true) ?: PaddingValues(),
margin = component.margin?.toPaddingValues(warnIfNegative = true) ?: PaddingValues(),
rcPackage = rcPackage,
resolvedOffer = resolvedOffer,
tabIndex = tabControlIndex,
Expand Down Expand Up @@ -1091,8 +1091,8 @@ internal class StyleFactory(
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
color = colorStyles,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
iconBackground = background,
rcPackage = rcPackage,
resolvedOffer = resolvedOffer,
Expand All @@ -1119,8 +1119,8 @@ internal class StyleFactory(
iconAlignment = component.iconAlignment,
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
items = items,
rcPackage = rcPackage,
resolvedOffer = resolvedOffer,
Expand All @@ -1145,7 +1145,7 @@ internal class StyleFactory(
if (connectorColor != null) {
TimelineComponentStyle.ConnectorStyle(
width = connector.width,
margin = connector.margin.toPaddingValues(),
margin = connector.margin.toPaddingValues(warnIfNegative = true),
color = connectorColor,
)
} else {
Expand Down Expand Up @@ -1191,8 +1191,8 @@ internal class StyleFactory(
pagePeek = component.pagePeek?.dp ?: 0.dp,
pageSpacing = (component.pageSpacing ?: DEFAULT_SPACING).dp,
background = background,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
shape = component.shape ?: DEFAULT_SHAPE,
border = borderStyles,
shadow = shadowStyles,
Expand Down Expand Up @@ -1278,8 +1278,8 @@ internal class StyleFactory(
TabsComponentStyle(
visible = component.visible ?: DEFAULT_VISIBILITY,
size = component.size,
padding = component.padding.toPaddingValues(),
margin = component.margin.toPaddingValues(),
padding = component.padding.toPaddingValues(warnIfNegative = true),
margin = component.margin.toPaddingValues(warnIfNegative = true),
background = backgroundColor,
shape = component.shape ?: DEFAULT_SHAPE,
border = border,
Expand Down
59 changes: 59 additions & 0 deletions ...ui/src/test/kotlin/com/revenuecat/purchases/ui/revenuecatui/components/ktx/PaddingTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
package com.revenuecat.purchases.ui.revenuecatui.components.ktx

import androidx.compose.foundation.layout.PaddingValues
import androidx.compose.ui.unit.dp
import com.revenuecat.purchases.InternalRevenueCatAPI
import com.revenuecat.purchases.paywalls.components.properties.Padding
import org.assertj.core.api.Assertions.assertThat
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner

@OptIn(InternalRevenueCatAPI::class)
@RunWith(RobolectricTestRunner::class)
internal class PaddingTest {
Comment thread
facumenzella marked this conversation as resolved.

@Test
fun `positive values are converted as-is`() {
val padding = Padding(top = 10.0, bottom = 20.0, leading = 30.0, trailing = 40.0)

val actual = padding.toPaddingValues()

assertThat(actual).isEqualTo(
PaddingValues(start = 30.dp, top = 10.dp, end = 40.dp, bottom = 20.dp),
)
}

@Test
fun `negative values are clamped to zero`() {
val padding = Padding(top = -10.0, bottom = -20.0, leading = -30.0, trailing = -40.0)

val actual = padding.toPaddingValues()

assertThat(actual).isEqualTo(
PaddingValues(start = 0.dp, top = 0.dp, end = 0.dp, bottom = 0.dp),
)
}

@Test
fun `only negative values are clamped, positive values are preserved`() {
val padding = Padding(top = -10.0, bottom = 20.0, leading = -30.0, trailing = 40.0)

val actual = padding.toPaddingValues()

assertThat(actual).isEqualTo(
PaddingValues(start = 0.dp, top = 0.dp, end = 40.dp, bottom = 20.dp),
)
}

@Test
fun `negative values are still clamped when warnIfNegative is enabled`() {
val padding = Padding(top = -10.0, bottom = -20.0, leading = -30.0, trailing = -40.0)

val actual = padding.toPaddingValues(warnIfNegative = true)

assertThat(actual).isEqualTo(
PaddingValues(start = 0.dp, top = 0.dp, end = 0.dp, bottom = 0.dp),
)
}
}