arkivanov / Decompose

Kotlin Multiplatform lifecycle-aware business logic components (aka BLoCs) with routing (navigation) and pluggable UI (Jetpack Compose, SwiftUI, JS React, etc.)
https://arkivanov.github.io/Decompose
Apache License 2.0
2.17k stars 84 forks source link

Handle navigation with Jetpack Compose Modal Bottom Sheets correctly #95

Closed moffpage closed 2 years ago

moffpage commented 2 years ago

I'm receiving a crash when trying to display a modal bottom sheet after it was dismissed (in other words - after first time) when having bottom sheet state (maybe static as well) management delegated to a Component (in Decompose terms). Maybe that's related to Jetpack Compose itself, not really sure. Crash statement: java.lang.IllegalArgumentException: The target value must have an associated anchor.

By the way, there's no hide/dismiss animation and some sort of half-transparent overlay is also visible. Currently, Jetpack Compose notifies component's router to replace the configuration (bottom sheet parent) in confirmStateChange, so that we could replace with configuration that represents bottom sheet in this case. We do however observe the router state in LaunchedEffect then show and hide the bottom sheet looking at the type of current child. Maybe the observations need to be implemented in some different way, so that it works, no idea.

Sample project: https://github.com/moffpage/DecomposeBottomSheetCrash

Kotlin version: 1.6.20 Jetpack Compose version: 1.2.0-alpha08

I think I can sort of "fix" this by creating the component which is displayed via bottom sheet just in time, not lazily, having it as a permanent child via childContext(key = "..."), that was the thing I wanted to avoid but if there isn't any fix that comes to mind, I might do so for now, just so everything works.

arkivanov commented 2 years ago

Thanks for reporting! I haven't check it yet, but it looks like an issue in Compose. Here is a related discussion in the accompanist repository - https://github.com/google/accompanist/issues/910.

Another related discussion - https://stackoverflow.com/questions/69346646/jetpack-compose-bottomsheet-with-empty-sheet-content-is-always-expnaded.

I will check the reproducer and file a bug to Compose if needed. Meantime, as suggested in the linked pages, you should avoid zero-size content inside sheetContent lambda. Just adding the 1.dp size to the dummy Boxes should fix the issue.

arkivanov commented 2 years ago

This appears to be another issue in Compose, which is marked as fixed but seems like not yet released - https://issuetracker.google.com/issues/199509587

Please use if else instead of Elvis operator.

 ModalBottomSheetLayout(
        sheetState = bottomSheetState,
        sheetContent = {
            when (bottomSheetType) {
                BottomSheetType.B -> {
                    if (secondaryChild is SecondaryChild.B) {
                        ContentB(component = secondaryChild.component)
                    } else {
                        Box(
                            modifier = Modifier
                                .background(color = Color.LightGray)
                                .fillMaxSize()
                        )
                    }
                }
...

I have also filed an issue to Compose about the slide-down animation being interrupted (e.g. when swiped down, the animation is interrupted immediately after the finger is lifted) - https://issuetracker.google.com/issues/232107673.

arkivanov commented 2 years ago

Closing this issue as there is nothing to do in Decompose.