From 5700d8626d26858cb5befedd7f53837c27c3b2c1 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Thu, 20 Aug 2026 12:55:11 -0400 Subject: [PATCH] fix(ui/navigation): pop the sheet entry when a drag dismisses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UnstyledBottomSheet has no dismissal callback, so nothing tied "the sheet settled back at Hidden" to popping its nav entry — that wiring was lost when the scene moved off Material3's ModalBottomSheet (6e49a91db), whose onDismissRequest did the pop. A drag-to-dismiss therefore left the (now invisible) sheet entry on the backstack: currentRouteKey still resolved to the sheet, so chrome that hides for sheets — the v2 tab bar — stayed hidden, and the full-size alpha-0 scrim kept swallowing touches until the next tap hit it and popped the entry. Hence "the tab bar doesn't come back until you interact with the screen" after dismissing region selection. Observe the sheet's state rather than the gesture, so every dismissal route is covered: once the sheet has actually been presented, settling back at Hidden pops the entry. All paths funnel through an idempotent finishDismiss() that re-arms per sheetGeneration, so the scrim-tap and pendingSheetDismiss (dismiss-then-replace) paths can't double-pop while a same-route replace still works. --- ui/navigation/build.gradle.kts | 1 + .../scenes/ModalBottomSheetSceneStrategy.kt | 42 ++++++-- .../scenes/ModalBottomSheetSceneTest.kt | 95 +++++++++++++++++++ 3 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 ui/navigation/src/test/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneTest.kt diff --git a/ui/navigation/build.gradle.kts b/ui/navigation/build.gradle.kts index cab531aac7..f06c3bd5a4 100644 --- a/ui/navigation/build.gradle.kts +++ b/ui/navigation/build.gradle.kts @@ -35,4 +35,5 @@ dependencies { testImplementation(kotlin("test")) testImplementation(libs.bundles.unit.testing) + testImplementation(libs.bundles.compose.ui.testing) } diff --git a/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt b/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt index bdd60e3208..f7d65d72f5 100644 --- a/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt +++ b/ui/navigation/src/main/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneStrategy.kt @@ -22,6 +22,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue +import androidx.compose.runtime.snapshotFlow import androidx.compose.runtime.snapshots.Snapshot import androidx.compose.runtime.staticCompositionLocalOf import androidx.compose.ui.Modifier @@ -53,6 +54,7 @@ import com.getcode.theme.CodeTheme import com.getcode.ui.core.noRippleClickable import com.getcode.ui.utils.LocalSheetExpansionState import com.getcode.ui.utils.LocalSheetGesturesState +import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch data class BottomSheetProperties( @@ -173,12 +175,43 @@ internal class ModalBottomSheetScene constructor( } } + // One presentation of the sheet must pop its entry exactly once. Every dismissal ends + // with the sheet settled at Hidden, and the settle observer below fires for all of + // them — including the explicit paths here and in [pendingSheetDismiss] — so the pop is + // funnelled through this guard. Reset when the sheet is (re-)opened. + var dismissHandled by remember { mutableStateOf(false) } + val finishDismiss = { + if (!dismissHandled) { + dismissHandled = true + handleBackResult() + onBack() + } + } + // Animate the sheet in on first composition and after // same-route dismiss-replace (sheetGeneration increments). LaunchedEffect(navigator.sheetGeneration) { sheetState.animateTo(restingDetent) } + // [UnstyledBottomSheet] is a plain sheet with no dismissal callback of its own, so + // dragging it closed hides it without telling anyone: the entry stays on the backstack + // as an invisible sheet that still swallows every touch through its full-size scrim, + // and chrome that hides while a sheet is up (the v2 tab bar) stays hidden until the + // next tap lands on that scrim and pops it. Watch the sheet's state rather than the + // gesture, so a drag-dismiss pops the entry the same way a scrim tap does. + LaunchedEffect(navigator.sheetGeneration) { + dismissHandled = false + // The sheet starts out Hidden, so a return to Hidden only means "dismissed" once + // it has actually been presented. + snapshotFlow { sheetState.targetDetent }.first { it != SheetDetent.Hidden } + // Settled, not merely targeted: mid-drag the target flips to Hidden and back as the + // finger crosses the dismiss threshold. + snapshotFlow { sheetState.isIdle && sheetState.currentDetent == SheetDetent.Hidden } + .first { it } + finishDismiss() + } + val composeScope = rememberCoroutineScope() val dismiss = { hide: Boolean -> @@ -186,12 +219,10 @@ internal class ModalBottomSheetScene constructor( composeScope.launch { sheetState.animateTo(SheetDetent.Hidden) }.invokeOnCompletion { - handleBackResult() - onBack() + finishDismiss() } } else { - handleBackResult() - onBack() + finishDismiss() } } @@ -203,8 +234,7 @@ internal class ModalBottomSheetScene constructor( sheetState.animateTo(SheetDetent.Hidden) } finally { Snapshot.withMutableSnapshot { - handleBackResult() - onBack() + finishDismiss() navigator.pendingSheetDismiss = null pendingDismiss() } diff --git a/ui/navigation/src/test/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneTest.kt b/ui/navigation/src/test/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneTest.kt new file mode 100644 index 0000000000..5e0fbcbb85 --- /dev/null +++ b/ui/navigation/src/test/kotlin/com/getcode/navigation/scenes/ModalBottomSheetSceneTest.kt @@ -0,0 +1,95 @@ +package com.getcode.navigation.scenes + +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.runtime.CompositionLocalProvider +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.performTouchInput +import androidx.compose.ui.test.swipeDown +import androidx.navigation3.runtime.NavEntry +import androidx.navigation3.runtime.NavKey +import com.getcode.navigation.AppHome +import com.getcode.navigation.DemoSheet +import com.getcode.navigation.core.EmptyCodeNavigator +import com.getcode.navigation.core.LocalCodeNavigator +import com.getcode.navigation.scrim.LocalScrimController +import com.getcode.navigation.scrim.ScrimController +import com.getcode.navigation.testNavigator +import com.getcode.theme.DesignSystem +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals + +private const val SheetContentTag = "sheet-content" +private const val BaseContentTag = "base-content" + +/** + * [UnstyledBottomSheet] has no dismissal callback, so the scene has to notice the sheet settling + * back at [SheetDetent.Hidden] itself. If it doesn't, a drag-dismiss leaves the (now invisible) + * sheet entry on the backstack — swallowing touches through its full-screen scrim and keeping + * app chrome that hides for sheets (the v2 tab bar) hidden until the next tap pops it. + */ +@RunWith(RobolectricTestRunner::class) +class ModalBottomSheetSceneTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private var backCount = 0 + + private fun showSheet() { + backCount = 0 + val navigator = testNavigator(AppHome, DemoSheet) + val baseEntry = NavEntry(key = AppHome) { + Box(Modifier.fillMaxSize().testTag(BaseContentTag)) + } + val sheetEntry = NavEntry(key = DemoSheet) { + Box(Modifier.fillMaxSize().testTag(SheetContentTag)) + } + val scene = ModalBottomSheetScene( + key = DemoSheet, + previousEntries = listOf(baseEntry), + overlaidEntries = listOf(baseEntry), + entry = sheetEntry, + sheetProperties = BottomSheetProperties(), + onBack = { backCount++ }, + metadata = emptyMap(), + navResultStore = EmptyCodeNavigator.resultStore, + lastNavKey = { AppHome }, + ) + + composeTestRule.setContent { + DesignSystem { + CompositionLocalProvider( + LocalCodeNavigator provides navigator, + LocalScrimController provides ScrimController(), + ) { + scene.content() + } + } + } + composeTestRule.waitForIdle() + } + + @Test + fun `an open sheet does not pop its entry`() { + showSheet() + + assertEquals(0, backCount, "sheet popped its entry without being dismissed") + } + + @Test + fun `dragging the sheet closed pops its entry`() { + showSheet() + + composeTestRule.onNodeWithTag(SheetContentTag).performTouchInput { swipeDown() } + composeTestRule.waitForIdle() + + assertEquals(1, backCount, "drag-to-dismiss did not pop the sheet entry") + } +}