Allow null heights to make the screenshot to wrap content

[fmontesino]

This is very useful when taking screenshots of dynamic height views. We don't always force a view to have a height, and the underlying Facebook implementation allows it by simply not setting a height:

/*
     * Measure and layout the view. In this example we give an exact
     * width but all the height to be WRAP_CONTENT.
     */
    ViewHelpers.setupView(view)
      .setExactWidthDp(300)
      .layout();

Expected behaviour

If I send null as height, I want to take a screenshot with WRAP_CONTENT

Actual behaviour

If I send null as height, I have metrics.heightPixels (fullscreen) as a default

Steps to reproduce

compareScreenshot(
                view = view,
                heightInPx = null,
                widthInPx = 600
            )

Version of the library

4.1+, from the moment shot-android wrapper was released.

[pedrovgs]

Instead of using a null value in the public API I'd go for something different. What if we use the WRAP_CONTENT value or a WRAP_CONTENT constant we can create so we don't introduce a breaking change in the public API? For me, if we use null as height we should not force a wrap_content behavior because you don't specify what's the configuration you expect 🤔 What do you think @Karumi/commanders ?

[fmontesino]

Agreed on sending null might not necessarily be interpreted as wrap_content. What about having WRAP_CONTENT,MATCH_PARENT and EXACT_PX(int) values to send, and then configure the screenshot library accordingly?

You don't have to actually send null explicitly, as it's a default value for compareScreenshot, so maybe just a flag like wrapContent = true for any missing parameter? compareScreenshot(view = myView, widthInPx = 600, wrapContent = true)

[pedrovgs]

I need to think about the public API. Meanwhile, as a workaround, you can use the facebook library used inside the ScreenshotTest interface.

[AlexKrupa]

Any update on this?

I think a sealed class approach would be alright, e.g.

sealed class Size {
    object WrapContent : Size()
    object MatchParent : Size()
    class Exact(@Px size: Int) : Size()
}

The public API could default to current value (i.e. match parent) but it'd probably be necessary to deprecate current compareScreenshot() functions.

[pedrovgs]

@AlexKrupa the only update I can mention is I had no time to develop this feature right now. I'm working on other issues and we need to develop this without a huge breaking change. As a workaround, I mentioned in a previous comment you can still use the internal library Shot uses under the hood to take the screenshot.

I marked this issue as help wanted just in case any other contributor would like to collaborate. Thanks!

[crysxd]

You can easily do this inside your test, just save below in e.g. ScreenshotTestExt:

fun ScreenshotTest.compareScreenshotWrapHeight(
    name: String,
    view: View,
    widthInPx: Int,
) {
    view.measure(
        View.MeasureSpec.makeMeasureSpec(widthInPx, View.MeasureSpec.EXACTLY),
        View.MeasureSpec.makeMeasureSpec(0, View.MeasureSpec.UNSPECIFIED),
    )
    compareScreenshot(
        name = name,
        view = view,
        widthInPx = widthInPx,
        heightInPx = view.measuredHeight
    )
}