Merge pull request #1244 from jbw0033/bottom_sheet_destroy

[Navigation Material] Mark transitionsInProgress complete in BottomSheetNavigator
google · Jul 13, 2022 · 701b3d6 · 701b3d6
2 parents ca1b3d5 + e32a4e2
commit 701b3d6
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 2 deletions.
diff --git a/...c/androidTest/java/com/google/accompanist/navigation.material/BottomSheetNavigatorTest.kt b/...c/androidTest/java/com/google/accompanist/navigation.material/BottomSheetNavigatorTest.kt
@@ -17,10 +17,13 @@
 package com.google.accompanist.navigation.material
 
 import android.os.Bundle
+import androidx.activity.OnBackPressedDispatcher
+import androidx.activity.compose.LocalOnBackPressedDispatcherOwner
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.size
+import androidx.compose.material.Button
 import androidx.compose.material.ExperimentalMaterialApi
 import androidx.compose.material.ModalBottomSheetState
 import androidx.compose.material.ModalBottomSheetValue
@@ -34,10 +37,14 @@ import androidx.compose.ui.Modifier
 import androidx.compose.ui.platform.testTag
 import androidx.compose.ui.test.junit4.createComposeRule
 import androidx.compose.ui.test.onNodeWithTag
+import androidx.compose.ui.test.onNodeWithText
 import androidx.compose.ui.test.performClick
 import androidx.compose.ui.unit.dp
+import androidx.lifecycle.Lifecycle
+import androidx.navigation.NavBackStackEntry
 import androidx.navigation.NavHostController
 import androidx.navigation.compose.NavHost
+import androidx.navigation.compose.composable
 import androidx.navigation.compose.rememberNavController
 import androidx.navigation.plusAssign
 import androidx.navigation.testing.TestNavigatorState
@@ -305,6 +312,62 @@ internal class BottomSheetNavigatorTest {
             .containsExactly(entry)
     }
 
+    @Test
+    fun testBackPressedDestroysEntry() {
+        lateinit var onBackPressedDispatcher: OnBackPressedDispatcher
+        lateinit var navController: NavHostController
+
+        composeTestRule.setContent {
+            val bottomSheetNavigator = rememberBottomSheetNavigator()
+            navController = rememberNavController(bottomSheetNavigator)
+            onBackPressedDispatcher =
+                LocalOnBackPressedDispatcherOwner.current?.onBackPressedDispatcher!!
+
+            ModalBottomSheetLayout(bottomSheetNavigator) {
+                Box(modifier = Modifier.fillMaxSize()) {
+                    NavHost(
+                        navController = navController,
+                        startDestination = "mainScreen"
+                    ) {
+
+                        composable(
+                            route = "mainScreen",
+                            content = {
+                                Button(onClick = { navController.navigate("bottomSheet") }) {
+                                    Text(text = "open drawer")
+                                }
+                            }
+                        )
+
+                        bottomSheet(
+                            route = "bottomSheet",
+                            content = {
+                                Box(modifier = Modifier.fillMaxSize()) {
+                                    Text(
+                                        text = "bottomSheet"
+                                    )
+                                }
+                            }
+                        )
+                    }
+                }
+            }
+        }
+
+        composeTestRule.onNodeWithText("open drawer").performClick()
+
+        lateinit var bottomSheetEntry: NavBackStackEntry
+
+        composeTestRule.runOnIdle {
+            bottomSheetEntry = navController.currentBackStackEntry!!
+            onBackPressedDispatcher.onBackPressed()
+        }
+
+        composeTestRule.runOnIdle {
+            assertThat(bottomSheetEntry.lifecycle.currentState).isEqualTo(Lifecycle.State.DESTROYED)
+        }
+    }
+
     /**
      * Create a [BottomSheetNavigator.Destination] with some fake content
      * Having an empty Composable will result in the sheet's height being 0 which crashes

diff --git a/...material/src/main/java/com/google/accompanist/navigation/material/BottomSheetNavigator.kt b/...material/src/main/java/com/google/accompanist/navigation/material/BottomSheetNavigator.kt
@@ -150,6 +150,18 @@ public class BottomSheetNavigator(
             MutableStateFlow(emptyList())
         }
 
+    /**
+     * Get the transitionsInProgress from the [state]. In some cases, the [sheetContent] might be
+     * composed before the Navigator is attached, so we specifically return an empty flow if we
+     * aren't attached yet.
+     */
+    private val transitionsInProgress: StateFlow<Set<NavBackStackEntry>>
+        get() = if (attached) {
+            state.transitionsInProgress
+        } else {
+            MutableStateFlow(emptySet())
+        }
+
     /**
      * Access properties of the [ModalBottomSheetLayout]'s [ModalBottomSheetState]
      */
@@ -163,6 +175,7 @@ public class BottomSheetNavigator(
         val columnScope = this
         val saveableStateHolder = rememberSaveableStateHolder()
         val backStackEntries by backStack.collectAsState()
+        val transitionsInProgressEntries by transitionsInProgress.collectAsState()
 
         // We always replace the sheet's content instead of overlaying and nesting floating
         // window destinations. That means that only *one* concurrent destination is supported by
@@ -177,7 +190,7 @@ public class BottomSheetNavigator(
         // currently displaying because it will have its transition completed when the sheet's
         // animation has completed
         DisposableEffect(backStackEntries) {
-            backStackEntries.forEach {
+            transitionsInProgressEntries.forEach {
                 if (it != latestEntry) state.markTransitionComplete(it)
             }
             onDispose { }
@@ -194,7 +207,7 @@ public class BottomSheetNavigator(
             onSheetDismissed = { backStackEntry ->
                 // Sheet dismissal can be started through popBackStack in which case we have a
                 // transition that we'll want to complete
-                if (state.transitionsInProgress.value.contains(backStackEntry)) {
+                if (transitionsInProgressEntries.contains(backStackEntry)) {
                     state.markTransitionComplete(backStackEntry)
                 } else {
                     state.pop(popUpTo = backStackEntry, saveState = false)