While we're on the subject of `ModalBottomSheetLay...

# compose

[Bradleycorn]

While we're on the subject of ModalBottomSheetLayout ... I have one setup to show a simple Text composable in the bottom sheet. But if I try to pass in the text to show, it doesn't work. The bottom sheet doesn't even show up. But if I "hard code" the text, it works fine. Code posted in the thread ...

[Bradleycorn]

@ExperimentalMaterialApi
@Composable
fun BottomSheetScreen(message: Int?) {
    val bottomSheetState = rememberModalBottomSheetState(initialValue = ModalBottomSheetValue.Hidden)
    val scope = rememberCoroutineScope()

    if (message != null) {
        scope.launch { bottomSheetState.show() }
    }

    ModalBottomSheetLayout(
        modifier = Modifier.fillMaxSize(),
        sheetContent = {
            BottomSheet(message = message)
        },
        sheetState =bottomSheetState
    ) {
        Column(
            modifier = Modifier.fillMaxSize(),
            verticalArrangement = Arrangement.Center,
            horizontalAlignment = Alignment.CenterHorizontally
        ) {
            Text("The message is:")
            Text(message?.let { stringResource(id = it) } ?: "")
        }
    }
}


@Composable
fun BottomSheet(@StringRes message: Int?) {
    val output = message?.let { stringResource(id = it)} ?: ""

    // this doesn't work. I've logged the "output" and verified that is has proper text. 
    Text(text = output, modifier = Modifier.padding(vertical = 50.dp))

    // If I do this instead, it works fine:
    //Text(text = stringResource(R.string.message), modifier = Modifier.padding(vertical = 50.dp)) 
}

[Bradleycorn]

In the above, note that in the BottomSheet composable, if I try to set the text of the Text composable, using the passed in message string resource, it doesn't work. The bottom sheet does not get displayed at all. However, if I don't used the passed in message, and instead just set the Text composable using a "hard coded" string resource, all of a sudden everything works great. What gives?

I have verified via debugging and log messages that the passed in message is a valid string resource identifier, and that the stringResource composable properly converts it to the correct string.

[Se7eN]

I don't know if this should work or not but normally I'd do stuff like this using `remember`:

val output = remember(message) { message?.let { stringResource(id = it)} ?: "" }

[Bradleycorn]

Can't do that ... stringResource is a composable function, and you can't call composables from the block of a remember

[Se7eN]

Oh yeah

[Bradleycorn]

FWIW, the main BottomSheetScreen composable is getting called like this, from the main Acitivity:

class MainActivity : AppCompatActivity() {

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)

        setContent {
            MaterialTheme {
                    Surface(color = MaterialTheme.colors.background) {

                        var message by remember { mutableStateOf<Int?>(null)}

                        rememberCoroutineScope().launch {
                            delay(3_000)
                            message = R.string.message
                        }
                        BottomSheetScreen(message = message)
                    }

                }
            }
        }
}

[jossiwolf]

Hmmm, what Compose version are you on? I just tried to repro this and it works for me.

[Bradleycorn]

beta1

Well that's the thing ... I copied the above to a brand new project and it works there.
But in the project that I wrote it in ... it doesn't ...

[Bradleycorn]

And here's where people reply with, "well then you're doing something wrong in the project where it doesn't work" ...

To which i would reply, it's also a new project, the only difference being it was created as an alpha12 project, and converted to beta1.

[jossiwolf]

Hah, fun... When you're saying the bottom sheet doesn't show at all, can you check if the bottomSheetState.show call throws a CancellationException or completes successfully?

[Bradleycorn]

oooh

[Bradleycorn]

didn't think of that

[jossiwolf]

I've seen some issues with that, but I don't think that's it

[Bradleycorn]

yeah, I wouldn't think so, but you never know ... worth a shot

[jossiwolf]

Random thought: Can you try to create a completely new activity, totally barebones, no special flags and host your code in there?

[Bradleycorn]

omg

[Bradleycorn]

Failure(kotlinx.coroutines.JobCancellationException: ScopeCoroutine was cancelled; job=ScopeCoroutine{Cancelled}@cd2659c)

[Bradleycorn]

errr

[Bradleycorn]

actually, that might be expected

[Bradleycorn]

ah .. no. not expected

[Bradleycorn]

So that is the problem

[Bradleycorn]

But ... why?

[jossiwolf]

Nice

[jossiwolf]

Debugging that one is... fun

[Bradleycorn]

yeah, good times

[jossiwolf]

So it's cancelled whenever the animation is interrupted

[jossiwolf]

Which in most cases happens when another animation is requested

[jossiwolf]

(At least from my experience)

[jossiwolf]

Things you might want to hook breakpoints into:
SwipeableState#processNewAnchors - see if that's called
SwipeableState#snapTo
SwipeableState#animateTo
afaik these are what can cause a new animation to run

[jossiwolf]

Another thing to try: Try delaying the show call by 200ms

[Bradleycorn]

yeah, that "fixes" it

[jossiwolf]

Nice

[Bradleycorn]

hehe I guess

[Bradleycorn]

I still want to understand WHY

[jossiwolf]

ModalBottomSheetLayout measures the sheet content when laying out everything. The composition of the body content is dependent on the sheet's height. I think what's happening is that the sheet's height changes when the text isn't null anymore, causing the body (and the swipeable anchors) to recompose

[Bradleycorn]

yeah, that makes sense.

[Bradleycorn]

Still doesn't explain why it works in the one project and not in the other

[jossiwolf]

When the anchors are set, it tries to settle into the new correct state (which is still the old one)

[jossiwolf]

That's the fun of race conditions 😛

[jossiwolf]

Does your MainActivity have any flags?

[Bradleycorn]

Yeah, that's what it boils down to ... There's a race condition down in there somewhere

No. Really about the ONLY difference between the 2 activities is that in the one where it is broken, I have added Hilt to the project, so the activity is marked as an @AndroidEntryPoint (though I'm not actually having anything injected into it yet).

[jossiwolf]

When you mark the other one as @AndroidEntryPoint, can you repro it there?

[Bradleycorn]

haven't tried yet, but I will

[Bradleycorn]

Gotta jump through all the Hilt setup hoops

[Bradleycorn]

But I mean ... the implementation I have ... for a case where your bottom sheet content isn't static ... It sure seems like a decent implementation .... right?

[jossiwolf]

In any case, file an issue! cc @matvei, look! Somebody encountered the bug I created with the first version of my CL

[jossiwolf]

Yup

[Bradleycorn]

yeah, I'll file an issue. Trying to get a consistent repoducable version. I hate to file without cause as we've seen, if you just create a new project and put the code it, it's fine.

[BenjO]

As an alternative, you can copy your project and strip everything but this composable. If it still doesn't work you have your sample to fill the bug tracker ^^

[Bradleycorn]

yeah. At the moment, I'm going the opposite route ... copying the broken project to the new, piece by piece, seeing which piece breaks it.

Adding Hilt did not break it.

[jossiwolf]

Some more questions:
• You were saying hardcoding the text works. When you hardcode it in your activity (just set a String instead of a resource Int) and pass that down instead, I'm guessing the issue doesn't happen?
• Do you have a lot of strings?

[Bradleycorn]

I've tried so many things, that I don't remember exactly ... I have tried using pure strings instead of resources and still ran into the problem ... but, at that point I had some additional complexities where there were some Flows and States involved. I may not have done a basic test with pure strings.

As for my string resources ... no, there's only like 3 strings in the xml file

[Bradleycorn]

Tested with pure strings instead of using string resources ... issue persists

[Shakil Karim]

@Bradleycorn I had a same issue i think that's because your calling

if (message != null) {
        scope.launch { bottomSheetState.show() }
    }

before setting the bottomSheetState to ModalBottomSheetLayout that's why applying delay works.

[Bradleycorn]

@Shakil Karim -- I tried moving the coroutine down AFTER calling the ModalBottomSheetLayout composable, and that had no effect. Also, that wouldn't explain why the code DOES work in a fresh project

[Shakil Karim]

@Bradleycorn Ah ok, then it's a separate issue.

[jossiwolf]

Hm, yeah, the question was a bit of a shot in the dark. I was thinking if the retrieval of the stringResource could be holding things up

[Bradleycorn]

yeah

[Bradleycorn]

Holy cow I found it!

[Bradleycorn]

@jossiwolf you were right all along

[Bradleycorn]

ModalBottomSheetLayout measures the sheet content when laying out everything. The composition of the body content is dependent on the sheet's height. I think what's happening is that the sheet's height changes when the text isn't null anymore, causing the body (and the swipeable anchors) to recompose

[Bradleycorn]

it works in my demo app because the string to be displayed was short and fit on a single line (i.e. the same height as when it's an empty string).
In my other app, the string to be displayed was a longer error message that wraps to a 2nd line ... thus the height of the bottom sheet changes ... and ... 💥.

[jossiwolf]

Yayyy

[jossiwolf]

Lol I'm happy I was right and not at the same time 😄

[jossiwolf]

Let's see what the Compose folks say when they see the ticket

[justasm]

As mentioned — https://kotlinlang.slack.com/archives/CJLTWPH7S/p1614537608220200?thread_ts=1614530232.195400&cid=CJLTWPH7S

Delay the show, e.g.

val scope = rememberCoroutineScope()
val sheetState = rememberModalBottomSheetState()

// in onClick or other callback..
scope.launch {
  delay(1)
  sheetState.show()
}

// alternatively, in composition like in the example in this thread
LaunchedEffect(message) {
  delay(1)
  sheetState.show()
}

[matvei]

Yeah, nasty issue. We need to fix it as some point soon.

This is the issue to track: http://issuetracker.google.com/180977981

The crash is not the same, but the idea us the same.

The crash is different because we trigger show at the different point of time, the one in the bug is more correct.

the rule of thumb with coroutine launches is that you should never launch directly from the composition (like in your example above). We have special tools e.g LaunchedEffect to launch smth as a side effect, or DisposableEffect and SideEffect to just call a lambda as a side effects, where you can later do scope.launch . Please use those instead of calling launch from the composition.

[Alex Bieliaiev]

I have the very same problem, but with a slightly different setup - NavHost inside a ModalBottomSheet . My solution is a bit more verbose, since I don't really like delay s:

/**
 * That is horrible, but I don't have a better option ATM.
 * Please refer to [androidx.compose.material.SwipeableState.processNewAnchors],
 * specifically that part:
 * val targetOffset = newAnchors.getOffset(currentValue)
 * ?: newAnchors.keys.minByOrNull { abs(it - offset.value) }!!
 * try {
 *     animateInternalToOffset(targetOffset, animationSpec)
 * } catch (c: CancellationException) {
 *     // If the animation was interrupted for any reason, snap as a last resort.
 *     snapInternalToOffset(targetOffset)
 * }
 * The problem is that [androidx.compose.material.SwipeableState.processNewAnchors]
 * cancels expanding animation, then I have to re-launch it first after the initial
 * animateInternalToOffset(targetOffset, animationSpec) and then once again after
 * the snapInternalToOffset(targetOffset).
 */
LaunchedEffect(Unit) {
    try {
        modalBottomSheetState.animateTo(ModalBottomSheetValue.Expanded)
    } finally {
        try {
            modalBottomSheetState.animateTo(ModalBottomSheetValue.Expanded)
        } finally {
            modalBottomSheetState.animateTo(ModalBottomSheetValue.Expanded)
        }
    }
}

[matvei]

uuuf, that is quite bad 🙂 Please file a bug, we shouldn't be that "safe" in the swipeable implementation about that

[Bradleycorn]

I've already created an issue, here:
https://issuetracker.google.com/issues/181593642

[Bradleycorn]

@Alex Bieliaiev (and others too) you might want to ⭐ that issue