Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navigating to bottom sheet and touching the sheet as it opens results in ANR #660

Closed
ColtonIdle opened this issue Aug 18, 2021 · 6 comments · Fixed by #680
Closed

Navigating to bottom sheet and touching the sheet as it opens results in ANR #660

ColtonIdle opened this issue Aug 18, 2021 · 6 comments · Fixed by #680

Comments

@ColtonIdle
Copy link

ColtonIdle commented Aug 18, 2021

I have an empty screen with a box and a clickable modifier.

onClick = {
 if (navController.currentDestination!!.route != "bottom_sheet") {
    navController.navigate("bottom_sheet")
...

This works fine 99% of the time unless the user presses on the bottom sheet while it's opening. The bottom sheet cannot be dismissed which results in an ANR. It's a "little" tricky to repro because you have to click quickly, but here's a video of it in action.

bottomDEMO.mp4
@jossiwolf
Copy link
Collaborator

Thanks for the report! I'll look into this as soon as possible!

@jossiwolf
Copy link
Collaborator

@ColtonIdle Could you please attach minimal repro?

@ColtonIdle
Copy link
Author

I'll try to whip something up, but it's happening in all of my 3 apps that use this. The trick is to be able to quickly click on the bottom sheet as it's opening.

@ColtonIdle
Copy link
Author

Repro:

@AndroidEntryPoint
class MainActivity : ComponentActivity() {

    @OptIn(ExperimentalMaterialNavigationApi::class)
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)


        setContent {
            MyTheme {

                val navController = rememberNavController()
                val bottomSheetNavigator = rememberBottomSheetNavigator()
                navController.navigatorProvider += bottomSheetNavigator
                ModalBottomSheetLayout(bottomSheetNavigator) {
                    NavHost(
                        navController = navController,
                        startDestination = Screen.MyHome.route
                    ) {
                        composable(Screen.MyHome.route) {
                            MyHome(onClick = { id ->
                                if (navController.currentDestination!!.route != "bottom_sheet") {
                                    navController.navigate("bottom_sheet")
                                }
                            })
                        }
                        bottomSheet(route = "bottom_sheet") {
                            Text("This is a cool bottom sheet!", modifier = Modifier.height(300.dp))
                        }
                    }
                }
            }
        }
    }
}

@jossiwolf
Copy link
Collaborator

@ColtonIdle just following up on that, can you post the MyHome composable as well, please?

@ColtonIdle
Copy link
Author

@Composable
fun MyHome(
    onClick: (String) -> Unit,
) {
    Box(
        modifier = Modifier
            .fillMaxSize()
            .clickable { onClick("blah") },
    ) {
    }
}

jossiwolf added a commit to jossiwolf/accompanist that referenced this issue Aug 20, 2021
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes google#660
jossiwolf added a commit to jossiwolf/accompanist that referenced this issue Aug 25, 2021
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes google#660
jossiwolf added a commit to jossiwolf/accompanist that referenced this issue Sep 28, 2021
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes google#660
jossiwolf added a commit to jossiwolf/accompanist that referenced this issue Sep 28, 2021
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes google#660
jossiwolf added a commit to jossiwolf/accompanist that referenced this issue Sep 28, 2021
Our `internalShow` method could end up in an infinite loop sometimes (yay, recursion!). We now use the official `show` API which *should* work. Unfortunately, our `show` calls all get cancelled but Swipeable still settles in the closest state. This seems to always be the expanded/half-expanded state. We look at the state's targetValue in our try's `finally` block to figure out if the state will settle in an expanded state and call the `onSheetShown` callback based on that.

Fixes google#660
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants