Previewing a component that is scrollable about showkase HOT 14 CLOSED

Just pushed a PR that fixes this problem!

from showkase.

I'm going to give it a shot with the latest version of Showkase that's targeting Compose beta05. Will let you know what I find!

from showkase.

@Dambakk Thanks for flagging this! I haven't tested with this so I would say that its currently unsupported but definitely feel that support for this should exist. I'll take a stab at this later today and report my findings!

from showkase.

It still crashes :(

from showkase.

@Dambakk Any chance you could share your component that used the verticalScroll modifier here so that its easier to repro and fix!

from showkase.

Of course! So this is taken from the official verticalScroll modifier example in the docs:

@Composable
fun VerticalScrollSample() {
    Column(
        modifier = Modifier
            .background(Color.LightGray)
            .fillMaxSize()
            .verticalScroll(rememberScrollState())
    ) {
        repeat(100) {
            Text("Item $it", modifier = Modifier.padding(2.dp))
        }
    }
}

@Preview
@Composable
fun VerticalScrollPreview() {
    VerticalScrollSample()
}

except that the size is fill... instead of fixed. It works fine (that is the browser activity does not crash) when the composable has a fixed height, but crashes with the exception pasted above when the size (or height when using vertical scroll I guess) is fillMax...

@vinaygaba, are you able to reproduce the issue with this snippet?

from showkase.

I was able to reproduce it with the snippet! I think the stacktrace message isn't totally accurate as you are able to scroll in the same direction correctly when you specify a fixed height to the composable(as you rightly pointed out). Think I need to do a bit more digging to find a fix for it. @Dambakk

from showkase.

Cool! Let me know if I can help with anything! 🙋

from showkase.

Hey! How's it going? Any progress yet?

from showkase.

Yeah, I can see the issue too :/

from showkase.

I did some more research about why this error is even thrown. For anyone interested, here's the thread that explains why Google decided to add this check - https://kotlinlang.slack.com/archives/CJLTWPH7S/p1616434089403900?thread_ts=1616403538.380900&cid=CJLTWPH7S

In order to get around this issue, what you need to ensure is that when your component is in preview mode, it isn't trying to use fillMaxHeight. One way to do this would be to make sure that your component can take in a modifier and use a default modifier for the general case. However, for the preview, you pass in the height modifier and that should fix the issue. Here's what your code would look like if I modified the example you shared

@Composable
fun VerticalScrollSample(
    modifier: Modifier = Modifier
) {
    Column(
        modifier = modifier // In the default case, it will continue to use the max size
            .background(Color.LightGray)
            .fillMaxSize()
            .verticalScroll(rememberScrollState())
    ) {
        repeat(100) {
            Text("Item $it", modifier = Modifier.padding(2.dp))
        }
    }
}

@Preview(name = "Scrollable", group = "Debug Scrollable")
@Composable
fun VerticalScrollPreview() {
    VerticalScrollSample(
        Modifier.height(500.dp) // or some other reasonable value
    )
}

Let me know if this fixes the issue for you @Dambakk @L7ColWinters @paulnunezm

from showkase.

I'm on vacation for another week, but my initial thoughts is that what you describe will probably work, but that it feels like a workaround rather than a solution. It would be interesting to know more about how the AS preview handles this situation. Anyone at Google we can ask?

from showkase.

@Dambakk Preview most likely doesn't encounter this issue because they aren't trying to render a scrollable composable inside a scrollable composable. The reason I encounter it is because the Showkase browser app is built using Compose itself and it renders the composable functions inside a LazyColumn. One way to potentially fix it would be to write the Showkase browser app using a RecyclerView but I'd be doing Showkase a disservice if I did that 😅

from showkase.

Right... Hmm.. I havent looked at the showkase source yet, but would it be possible to set like a max height/width for the rendered composable to e.g. the height of the screen? So basically, move the solution you suggested into the library but using a wrapping box rather than providing a modifier. Not sure, I guess it will still have nested scroll, Im just trying to think of solutions. Maybe the modifier is the way to go 🤷‍♂️

from showkase.