Skip to content

Chore(Paywalls): Begin to support min/max size attributes - #4161 - #4217

Draft
JZDesign wants to merge 4 commits into
mainfrom
jzdesign/PW-1359/min-max-simple
Draft

JZDesign wants to merge 4 commits into
mainfrom
jzdesign/PW-1359/min-max-simple

Conversation

@JZDesign

@JZDesign JZDesign commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Checklist

  • If applicable, unit tests
  • If applicable, create follow-up issues for purchases-ios and hybrids

Note

previews were moved to a new branch

Motivation

We want to have more responsive paywalls, to best support that, we should be able to set minimum and maximum sizes.

Description

Wires the new min/max size attributes through the codebase. Puts the bits behind a flag that could cause the SDK to render the changes in production. This way it's a specific opt in until we officially release this. All changes are backward compatible until the flag is on or removed. There are things that need to be iterated on. The flag is there so we can merge this PR and fix those in isolated smaller PRs.
supersedes #4161


Note

Medium Risk
Touches core paywall layout and measurement across stacks and media; behavior is gated at parse time but UI layout changes are substantial and could affect rendering even for paywalls without min/max until the flag is on.

Overview
Adds opt-in paywall min/max sizing via ENABLE_PAYWALL_MIN_MAX_SIZING in local.properties / BuildConfig. When the flag is off, JSON deserialization still accepts min/max on Fit and Fill but strips them so existing paywalls behave unchanged.

When enabled (and in UI either way for layout code paths that read constraints), RevenueCat UI honors limits end-to-end: the custom Modifier.size applies min/max during measure; margins adjust limits on Fit/Fill; image/video sizing uses shared adjustForMedia with aspect-ratio-aware scaling; WebView content sizing clamps resolved dimensions.

Stacks that need more than Row/Column weight switch to ConstrainedFillLayout (detected by needsConstrainedFillLayout—limited Fill children, overrides, or Fit stacks with positive minimums plus Fill/SPACE_* children). That layout allocates main-axis space with min/max, hugs content for Fit-with-minimum, passes resolved sizes through ComponentSizeParentDataModifier, and handles scroll/unbounded cross-axis Fill stretching. Horizontal/vertical stacks were simplified to pass child modifiers explicitly; trackMainAxisUnbounded is now a layout node to avoid intrinsic-measure recomposition loops.

The root Fit→Fill screen hack now preserves min/max when promoting axes to Fill. Broad unit/UI tests cover deserialization gating, modifiers, margins, media sizing, and constrained-fill distributions.

Reviewed by Cursor Bugbot for commit 20d3d9a. Bugbot is set up for automated code reviews on this repo. Configure here.

…layout

Stacks only switch from Row/Column to ConstrainedFillLayout when a min/max
is actually declared on the main axis (including in overrides), so
paywalls without min/max keep the exact same layout path as before.

- Modifier.size honors Fit/Fill min/max
- Margins extend Fit/Fill min/max like they do for Fixed
- Media keep their aspect ratio while being clamped to min/max
- WebView clamps its resolved size
- Root stack Fit->Fill conversion preserves min/max
@RevenueCat-Danger-Bot

RevenueCat-Danger-Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
1 Message
📖 This PR changes 1228 lines of production Kotlin/Java (limit 300); skipped via skip-pr-lines-changed-check label.

Generated by 🚫 Danger

…nded cross axis

A Fit stack inside the root scroll has an unbounded cross axis, so a
cross-axis Fill child had nothing to fill and collapsed to its content.
When the constrained layout has such a child, bound it to its own
intrinsic cross-axis size (single pass, no re-measure) so Fill children
stretch to the largest sibling like flexbox.

Modifier.size now clamps the content size to the constraints it resolved
so intrinsic measurement honors Fixed/min sizes of empty components.
@emerge-tools

emerge-tools Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

📸 Snapshot Test

632 unchanged

Name Added Removed Modified Renamed Unchanged Errored Approval
TestPurchasesUIAndroidCompatibility Paparazzi
com.revenuecat.testpurchasesuiandroidcompatibility.paparazzi
0 0 0 0 285 0 N/A
TestPurchasesUIAndroidCompatibility
com.revenuecat.testpurchasesuiandroidcompatibility
0 0 0 0 347 0 N/A

🛸 Powered by Emerge Tools

@JZDesign JZDesign changed the title WIP Min/Max simplified Feat(Paywalls) Support min/max sizing - #4161 Sep 10, 2026
…ment

Modifier.layout {} blocks also run for intrinsic queries, with unbounded
constraints. Stretching cross-axis Fill children measures the stack's
intrinsic size, so a nested stack tracking its unbounded main axis flipped
its state on every frame and never settled. Implement the tracker as a
LayoutModifierNode whose intrinsics pass straight through.
@JZDesign
JZDesign force-pushed the jzdesign/PW-1359/min-max-simple branch from 0f8fe7d to 20d3d9a Compare September 11, 2026 13:58
@JZDesign JZDesign changed the title Feat(Paywalls) Support min/max sizing - #4161 Chore(Paywalls): Begin to support min/max size attributes - #4161 Sep 11, 2026
@JZDesign JZDesign added pr:other and removed pr:feat A new feature feat:PaywallsV2 labels Sep 11, 2026
@JZDesign

Copy link
Copy Markdown
Contributor Author

@cursor please review

@cursor

cursor Bot commented Sep 11, 2026 •

Copy link
Copy Markdown
Contributor

I posted a review on PR #4217.

The flagged merge looks safe: deserialize strips min/max when ENABLE_PAYWALL_MIN_MAX_SIZING is off, existing paywalls stay on Row/Column, and snapshots are unchanged. The new layout path is well thought through (min-first allocation, Fit hugging, parent-data matching, intrinsic-safe unbounded tracking).

The gaps that will show up once the flag is on: text and other non-stack leaves never call resolveComponentSizeParentData, so size overrides are ignored at measure time; hasFillChild uses style size, so a Fit→Fill override on a Fit(min) parent hugs the child to 0; and root asFill() now keeps max, which can stop the root from filling the screen. I also asked for a test that the flag-off deserialize path actually strips min/max — CI currently skips those cases.

Open in Web Open in Cursor 

@cursor

cursor Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Review

This is a solid incremental landing. The opt-in story is the right shape: ENABLE_PAYWALL_MIN_MAX_SIZING strips min/max at deserialize time, needsConstrainedFillLayout keeps existing paywalls on Row/Column when nothing is limited, and Emerge snapshots staying unchanged is the signal we want for a flagged merge.

The new layout pieces are thoughtfully built — equal-share allocation that honors mins first, then maxes; hugging a Fit stack with a positive minimum; parent-data matching so visible = false children do not shift siblings; and the intrinsic-safe trackMainAxisUnbounded node with a regression test for the recomposition loop.

A few things I would treat as follow-ups before turning the flag on, plus one test gap that is worth fixing in this PR.

1. Text (and other non-stack leaves) never resolve parent data

ConstrainedFillLayout reads ComponentSizeParentDataModifier to decide who is Fill and what their limits are. The parent attaches ComponentSizeParentDataModifier(item.size) using the style size. Stacks / images / icons / videos / buttons then prepend the resolved size (overrides + margin) via resolveComponentSizeParentData. Text does not:

            Markdown(
                text = text,
                modifier = modifier
                    .size(textState.size, horizontalAlignment = textState.horizontalAlignment)
                    .padding(textState.margin)

Same gap on WebViewComponentView, CarouselComponentView, TabsComponentView, and TimelineComponentView.

Text is the most common Fill child. needsConstrainedFillLayout already walks LocalizedTextPartial.partial.size, so a selected/compact override that adds Fill(max = …) will switch the stack onto ConstrainedFillLayout, then measure that child with the base size. The stack-override test in StackConstrainedFillDistributionTest covers the happy path; a text-override sibling would miss it.

2. shouldFitMainAxis / hasFillChild look at style size, not resolved size

    val hasAnyItemsWithFillWidth = items.any { it.size.width is Fill }
    if (needsConstrainedFillLayout(size, dimension.distribution, items, Orientation.Horizontal)) {
        ConstrainedFillLayout(
            config = ConstrainedFillLayout.Config.Horizontal(
                ...
                fitMainAxis = size.width.shouldFitMainAxis(hasAnyItemsWithFillWidth),

If a child is Fit in the base style and becomes Fill via an applied override, hasFillChild is false. A parent Fit(min > 0) then gets fitMainAxis = true and that Fill child is hugged to its minimum (often 0) instead of taking leftover space. needsConstrainedFillLayout can still enter because it does look at override candidate sizes for limited Fill.

3. Root asFill() now keeps min/max — behavior change when the flag is on

On main, a root Fit stack is rewritten to unlimited Fill() so it fills the screen. This PR copies min/max across. A designer Fit(max = 800) on the root becomes Fill(max = 800) and the paywall will no longer fill the screen. Fine to keep if that is the intended product behavior; worth an explicit decision before the flag comes off, because this hack is still described as temporary.

4. Unused overrides can switch the layout engine

needsConstrainedFillLayout unions every override size, including ones whose conditions are not active. When the flag is on, a compact/selected override that is not currently applied can still move the stack from Row/Column onto ConstrainedFillLayout. That path is close to weight-based layout, but not guaranteed to be pixel-identical (measurement order, SPACE_*, non-Fill leftover). Safer long-term: decide from the currently resolved child sizes, or accept this as “enter the new path too often” only while iterating.

5. The production safety property is untested

SizeTests assumeTrues away every requiresMinMaxSizing case when the flag is false — which is CI and published artifacts. There is no test that {"type":"fill","min":20,"max":100} deserializes to Fill() when ENABLE_PAYWALL_MIN_MAX_SIZING=false. That is the actual backward-compat guarantee. I would add that inverse case here.

Nits / leftover

  • allowsFlexDistribution is only referenced by tests. Dead, or a leftover from an earlier pass.
  • ENABLE_PAYWALL_MIN_MAX_SIZING is not listed in local.properties.example next to ENABLE_EXTRA_REQUEST_LOGGING.
  • The last commit says “conditionally serialize”; only deserialize is gated. serialize() still writes min/max. Fine for inbound paywall JSON; the comment is misleading.
  • Fit.default is applied for WebView placeholders only. The general Modifier.size() path ignores it. Reasonable for this PR if default is only for components with no intrinsic size.

Verdict

Safe to merge with the flag off, given snapshots are unchanged and existing paywalls stay on Row/Column. I would still add the flag-off deserialize test in this PR. The parent-data and hasFillChild gaps are the ones that will bite first once this is opted into with real dashboard paywalls (text + size overrides). Happy to take those as follow-up issues if you want this landed as-is.

@JZDesign
JZDesign marked this pull request as ready for review September 11, 2026 15:40
@JZDesign
JZDesign requested review from a team as code owners September 11, 2026 15:40
@codecov

codecov Bot commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 83.62%. Comparing base (cc253fb) to head (20d3d9a).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...t/purchases/paywalls/components/properties/Size.kt 93.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4217      +/-   ##
==========================================
+ Coverage   83.60%   83.62%   +0.01%     
==========================================
  Files         455      455              
  Lines       18621    18629       +8     
  Branches     2811     2813       +2     
==========================================
+ Hits        15569    15579      +10     
+ Misses       2149     2148       -1     
+ Partials      903      902       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 20d3d9a. Configure here.

val contentWidth = placeable.width.coerceIn(widthConstraints.min, widthConstraints.max)
val contentHeight = placeable.height.coerceIn(heightConstraints.min, heightConstraints.max)
val layoutWidth = contentWidth.coerceIn(incomingConstraints.minWidth, incomingConstraints.maxWidth)
val layoutHeight = contentHeight.coerceIn(incomingConstraints.minHeight, incomingConstraints.maxHeight)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Min size cannot overflow parent

Medium Severity

The size modifier measures a child at its min when that exceeds the parent, then reports a layout size clamped to the incoming max. Components with Fit/Fill minimums larger than their parent therefore stay parent-sized instead of overflowing, which is the behavior SizeModifierTest and constrained fill allocation expect.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 20d3d9a. Configure here.

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