google / accompanist

A collection of extension libraries for Jetpack Compose
https://google.github.io/accompanist
Apache License 2.0
7.48k stars 601 forks source link

[Navigation] Bottom sheet scrim still visible after using navigateUp from BottomSheet destination #946

Closed FunkyMuse closed 2 years ago

FunkyMuse commented 2 years ago

Description Bottom sheet scrim still visible after using navigateUp from BottomSheet destination

Steps to reproduce

val bottomSheetNavigator = rememberBottomSheetNavigator()
val navController = rememberAnimatedNavController()
navController.navigatorProvider += bottomSheetNavigator

also using

 ModalBottomSheetLayout(
            bottomSheetNavigator
        ) {
            AnimatedNavHost(
                navController = navController,
                startDestination = "home"
            ) {
                //added the bottomSheet and composable destinations here
            }
        }

https://github.com/google/accompanist/blob/main/sample/src/main/java/com/google/accompanist/sample/navigation/material/BottomSheetNavSample.kt

inside BottomSheet function call navController.navigateUp

Expected behavior The scrim should not be visible once you navigate up

Additional context compose_version = '1.1.0-rc01' accompanist_version = '0.22.0-rc'

FunkyMuse commented 2 years ago

Temporary fix:

 val sheetState = rememberModalBottomSheetState(ModalBottomSheetValue.Hidden, SwipeableDefaults.AnimationSpec)
 val bottomSheetNavigator = rememberBottomSheetNavigatorFix(sheetState)

@Composable
fun rememberBottomSheetNavigatorFix(
    sheetState: ModalBottomSheetState
): BottomSheetNavigator {
    return remember(sheetState) {
        BottomSheetNavigator(sheetState = sheetState)
    }
}

Whenever you call navigateUp just call sheetState.hide() too

LaunchedEffect(navController) {
     sheetState.hide()
     navController.navigateUp()        
    }
FunkyMuse commented 2 years ago

Huh, seems to be fixed with v0.24.1-alpha

I'll investigate more

jossiwolf commented 2 years ago

That would be very surprising but awesome😄 Let us know!

FunkyMuse commented 2 years ago

That would be very surprising but awesome😄 Let us know!

This is now a workaround for #978

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

robbysmeticapps commented 2 years ago

I have the same issue, is there any progress on this issue or should we still use the temporary fix? I'm using v0.27.1

mattiaferigutti commented 2 months ago

I'm still experiencing this issue today, even with the stable version of Compose. It's frustrating that it hasn't been addressed yet. However, I’ve found a workaround in the meantime. I added a resetState() function to my ViewModel, which, as the name suggests, resets all the state within the ViewModel. I then call this function inside the composable using DisposableEffect, like this:

DisposableEffect(Unit) {
  onDispose { viewModel.resetState() }
}