[Paywalls V2] Adds ImageComponentView

[tonidero]

Description

This adds a new component ImageComponentView and the corresponding ImageComponentStyle. It doesn't include transformations from ImageComponent to ImageComponentStyle.

• Placeholder to image loading:

Untitled2.mp4

• Previews

[emerge-tools]

📸 Snapshot Test

3 added, 110 unchanged

Name	Added	Removed	Modified	Unchanged	Errored	Approval
TestPurchasesUIAndroidCompatibility com.revenuecat.testpurchasesuiandroidcompatibility	3	0	0	110	0	✅ Approved

---

🛸 Powered by Emerge Tools

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.87%. Comparing base (7c39fe6) to head (7f632f6).
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1959   +/-   ##
=======================================
  Coverage   81.87%   81.87%           
=======================================
  Files         260      260           
  Lines        8493     8493           
  Branches     1226     1226           
=======================================
  Hits         6954     6954           
  Misses       1041     1041           
  Partials      498      498           

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

[tonidero]

Had to apply the clipping in an outer box... If I applied it to the image itself, it didn't work together with the gradient overlays.

[JayShortway]

You can change the drawWithCache modifier to draw an Outline instead of a Rect. That way we don't need the Box. Might be nice to turn it into a custom modifier (for readability and reusability)? Something like:

@JvmSynthetic
@Stable
internal fun Modifier.overlay(
    color: ColorStyle,
    shape: Shape = RectangleShape,
): Modifier = this then drawWithCache {
    val outline = shape.createOutline(size, layoutDirection, this)

    onDrawWithContent {
        drawContent()
        when (color) {
            is ColorStyle.Solid -> drawOutline(outline, color.color)
            is ColorStyle.Gradient -> drawOutline(outline, color.brush)
        }
    }
}

Which can then be used as:

.applyIfNotNull(style.colorStyle) { colorStyle ->
    overlay(colorStyle, style.shape ?: RectangleShape)
}
// This is now needed, as the Box is no longer clipping the image itself.
.applyIfNotNull(style.shape) { clip(it) }

[tonidero]

Note that this might require some transformations from the ImageComponent to this ImageComponentStyle. For example, this expects a ContentScale but in ImageComponent we have our own FitMode. I believe this translation should be done separately and in advance though.

[tonidero]

I'm not sure if the sizes of the images are affecting anything... Not according to my tests 🤔

[JayShortway]

We can use the width and height to already give the image view the appropriate size before the image is downloaded. That way the layout won't "jump" when the image arrives.

Since width and height are part of ThemeImageUrls, which is provided by ImageComponentStyle, I think we can remove the separate size from ImageComponentStyle.

[tonidero]

Ohhh ok that makes a lot of sense 😅 Thanks!

[tonidero]

Hmm as I was thinking about this, how would that work if we want the image to "Fill" or "Fit" the screen? In the ThemeImageUrls there is no way to pass if we want one of those two right? I think passing the specific width/height only makes sense if we want a "fixed" size...

[JayShortway]

Summary of offline conversation:

• We cannot remove the size as that indicates the size of the image view, not the image itself.
• The ImageUrls width/height come into play when the image view is set to Fit.

[JayShortway]

Really nice job on this! Just a few minor comments.

[JayShortway]

We can use the width and height to already give the image view the appropriate size before the image is downloaded. That way the layout won't "jump" when the image arrives.

Since width and height are part of ThemeImageUrls, which is provided by ImageComponentStyle, I think we can remove the separate size from ImageComponentStyle.

[JayShortway]

Nice!

[JayShortway]

You can change the drawWithCache modifier to draw an Outline instead of a Rect. That way we don't need the Box. Might be nice to turn it into a custom modifier (for readability and reusability)? Something like:

@JvmSynthetic
@Stable
internal fun Modifier.overlay(
    color: ColorStyle,
    shape: Shape = RectangleShape,
): Modifier = this then drawWithCache {
    val outline = shape.createOutline(size, layoutDirection, this)

    onDrawWithContent {
        drawContent()
        when (color) {
            is ColorStyle.Solid -> drawOutline(outline, color.color)
            is ColorStyle.Gradient -> drawOutline(outline, color.brush)
        }
    }
}

Which can then be used as:

.applyIfNotNull(style.colorStyle) { colorStyle ->
    overlay(colorStyle, style.shape ?: RectangleShape)
}
// This is now needed, as the Box is no longer clipping the image itself.
.applyIfNotNull(style.shape) { clip(it) }

[JayShortway]

🤏 this close! Just a comment on px vs dp, and a suggestion.

[JayShortway]

This is nice! 2 comments:

• The values in ImageUrls are pixels, so we should transform them to dp, instead of interpreting them as dp.
• Maybe we can adjust the Size, taking into account the ImageUrls values before it gets to this modifier? That way this modifier doesn't need to know about ImageUrls.

To illustrate, both of those combined could look something like this:

@JvmSynthetic
@Composable
internal fun Size.adjustForImage(imageUrls: ImageUrls): Size {
    val density = LocalDensity.current
    return adjustForImage(imageUrls, density)
}

@JvmSynthetic
internal fun Size.adjustForImage(imageUrls: ImageUrls, density: Density): Size =
    Size(
        width = when (width) {
            is Fit -> Fixed(with(density) { imageUrls.width.toInt().toDp().value.toUInt() })
            is Fill,
            is Fixed,
                -> width
        },
        height = when (height) {
            is Fit -> Fixed(with(density) { imageUrls.height.toInt().toDp().value.toUInt() })
            is Fill,
            is Fixed,
                -> height
        },
    )

[tonidero]

I like that 👍 Thanks!

[tonidero]

Not important, but I think we can make some composables like this read only composables.

[JayShortway]

Yea probably indeed, good call!

[tonidero]

I moved this to the ImageComponentStyle since it felt like a better place to put it than as simple extensions of the Size class, since here is where we actually have the data to adjust the size. Not sure if there are other components that would require a similar adjustment, in which case, I think we can extract it to extension functions as suggested. Wdyt?

[JayShortway]

I like this!

[JayShortway]

Super nice!

[JayShortway]

I like this!