refactor(#619): kill DRAWER_GESTURE_SCREENS — screen-owned drawer gestures via HermesScaffold (#624)

Replaces the fragile DRAWER_GESTURE_SCREENS global set + defensive
LaunchedEffect(snapTo(Closed)) + closeDrawer callback workaround with a
single source of truth: each screen declares its drawer-gesture preference
via HermesScaffold's new drawerGesturesEnabled parameter (default true).

Architecture:
- DrawerGestureController holds the active gesture state. HermesScaffold
  reconciles each screen's preference into it via SideEffect; when a screen
  opts out while the drawer is open, the controller closes the drawer itself.
- LocalDrawerGestureController CompositionLocal flows the controller from
  MainNavigation down to ModalNavigationDrawer, which reads .enabled and
  passes it to Material's gesturesEnabled parameter.
- DisableDrawerGestures() helper for entry screens that don't use
  HermesScaffold (Landing, AuthLogin, PairingCodeEntry).
- All 6 Settings drill-down sub-pages pass drawerGesturesEnabled = false.
- NavigationController.closeDrawer removed; navigateTo no longer needs a
  synchronous drawer-dismiss hook.

Removes: DRAWER_GESTURE_SCREENS set, both LaunchedEffect(gesturesEnabled)
and LaunchedEffect(drawerGesturesEnabled) blocks, drawerGesturesEnabled
local state, closeDrawer callback on NavigationController.

Fixes the recurring drawer-scrim stuck-open bug class (PRs #445, #454, #455).

Verified: ktlintCheck, assembleDebug, testDebugUnitTest all green locally.

Co-authored-by: Hy4ri <hy4ri@users.noreply.github.com>
This commit is contained in:
M57andHy4ri authored and GitHub committed 2026-07-18 14:25:23 +03:00
1 parent 0c2b9679b7
commit 0a0b612d3a
5 files changed
+145 -52

No files matched your search

+19 -11
View File
@@ -77,17 +77,25 @@ Uses `androidx.navigation3` (`NavKey`, `NavBackStack`, `NavDisplay`, `entry<T>`)
deduplication guard. Bypassing it stacks duplicate screen entries that compete for deduplication guard. Bypassing it stacks duplicate screen entries that compete for
touch events and become unresponsive. touch events and become unresponsive.
**⚠ Drawer scrim gets stuck when navigating to a non-gesture screen.** Screens **Drawer gesture state is screen-owned (issue #619).** Each screen declares whether
not in `DRAWER_GESTURE_SCREENS` disable drawer gestures. If the drawer is open the modal drawer's swipe gestures should be active while it is visible, via a single
when this happens, the scrim stays visible and blocks touch. Always pair gesture source of truth — no global gesture set, no defensive `LaunchedEffect(snapTo(Closed))`,
transitions with a `LaunchedEffect`: no `closeDrawer` callback on `NavigationController`.
```kotlin
LaunchedEffect(gesturesEnabled) { - `HermesScaffold(drawerGesturesEnabled = true)` — default; primary screens and
if (!gesturesEnabled && drawerState.isOpen) { most list screens. The scaffold reconciles this preference into the
drawerState.snapTo(DrawerValue.Closed) `DrawerGestureController` via a `SideEffect`.
} - `HermesScaffold(drawerGesturesEnabled = false)` — drill-down sub-pages (e.g.
} `SettingsConnectionPage`, `SettingsAppearancePage`, …). The controller closes
``` the drawer itself if it was open when the screen composed, so the scrim can't
stick around and intercept the next tap.
- `DisableDrawerGestures()` — for entry screens that don't use `HermesScaffold`
(Landing, AuthLogin, PairingCodeEntry).
`ModalNavigationDrawer` in `Navigation.kt` reads `gestureController.enabled`
(provided via `LocalDrawerGestureController`) and passes it to Material's
`gesturesEnabled` parameter. To change a screen's gesture behavior, edit the
screen — not `Navigation.kt`.
### Activity-Scoped ViewModels ### Activity-Scoped ViewModels
@@ -30,13 +30,12 @@ import androidx.compose.material3.Scaffold
import androidx.compose.material3.Text import androidx.compose.material3.Text
import androidx.compose.material3.rememberDrawerState import androidx.compose.material3.rememberDrawerState
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.collectAsState import androidx.compose.runtime.collectAsState
import androidx.compose.runtime.getValue import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.ui.Alignment import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Color
@@ -54,6 +53,9 @@ import com.m57.hermescontrol.data.ws.ConnectionStatus
import com.m57.hermescontrol.data.ws.HermesWsClient import com.m57.hermescontrol.data.ws.HermesWsClient
import com.m57.hermescontrol.theme.BottomNavDisplayMode import com.m57.hermescontrol.theme.BottomNavDisplayMode
import com.m57.hermescontrol.theme.LocalHermesStatusColors import com.m57.hermescontrol.theme.LocalHermesStatusColors
import com.m57.hermescontrol.ui.common.DisableDrawerGestures
import com.m57.hermescontrol.ui.common.DrawerGestureController
import com.m57.hermescontrol.ui.common.LocalDrawerGestureController
import com.m57.hermescontrol.ui.settings.SettingsAboutPage import com.m57.hermescontrol.ui.settings.SettingsAboutPage
import com.m57.hermescontrol.ui.settings.SettingsAppearancePage import com.m57.hermescontrol.ui.settings.SettingsAppearancePage
import com.m57.hermescontrol.ui.settings.SettingsBehaviorPage import com.m57.hermescontrol.ui.settings.SettingsBehaviorPage
@@ -69,8 +71,6 @@ import com.m57.hermescontrol.ui.pairing.PairingCodeEntryScreen as PairingCodeEnt
private fun resolveBottomNavItems(names: List<String>): List<ScreenDefinition> = private fun resolveBottomNavItems(names: List<String>): List<ScreenDefinition> =
names.mapNotNull { name -> ScreenRegistry.ALL_SCREENS.firstOrNull { it.key::class.simpleName == name } } names.mapNotNull { name -> ScreenRegistry.ALL_SCREENS.firstOrNull { it.key::class.simpleName == name } }
private val DRAWER_GESTURE_SCREENS: Set<NavKey> = ScreenRegistry.ALL_SCREENS.mapTo(mutableSetOf()) { it.key }
private fun appEntryProvider( private fun appEntryProvider(
sessionId: String?, sessionId: String?,
openDrawer: () -> Unit, openDrawer: () -> Unit,
@@ -85,6 +85,8 @@ private fun appEntryProvider(
NavigationController.navigateTo(PairingCodeEntryScreen) NavigationController.navigateTo(PairingCodeEntryScreen)
}, },
) )
// Landing doesn't use HermesScaffold — opt out of drawer gestures explicitly (issue #619).
DisableDrawerGestures()
} }
entry<AuthLoginScreen> { entry<AuthLoginScreen> {
@@ -96,6 +98,7 @@ private fun appEntryProvider(
NavigationController.goBack() NavigationController.goBack()
}, },
) )
DisableDrawerGestures()
} }
entry<PairingCodeEntryScreen> { entry<PairingCodeEntryScreen> {
@@ -107,6 +110,7 @@ private fun appEntryProvider(
NavigationController.goBack() NavigationController.goBack()
}, },
) )
DisableDrawerGestures()
} }
ScreenRegistry.ALL_SCREENS.forEach { screen -> ScreenRegistry.ALL_SCREENS.forEach { screen ->
@@ -116,6 +120,10 @@ private fun appEntryProvider(
} }
// ── Settings drill-down sub-pages ─────────────────────────────────── // ── Settings drill-down sub-pages ───────────────────────────────────
// Each passes drawerGesturesEnabled = false to HermesScaffold — this is the
// single source of truth that prevents the drawer-scrim stuck-open bug
// (issue #619). No global gesture set, no closeDrawer callback, no
// LaunchedEffect(snapTo(Closed)) — the scaffold reconciles automatically.
entry<SettingsConnection> { entry<SettingsConnection> {
SettingsConnectionPage( SettingsConnectionPage(
onBack = { NavigationController.goBack() }, onBack = { NavigationController.goBack() },
@@ -169,9 +177,16 @@ fun MainNavigation(sessionId: String? = null) {
val drawerState = rememberDrawerState(initialValue = DrawerValue.Closed) val drawerState = rememberDrawerState(initialValue = DrawerValue.Closed)
val scope = rememberCoroutineScope() val scope = rememberCoroutineScope()
// Synchronous drawer dismiss hook used by NavigationController.navigateTo // Single source of truth for drawer gestures (issue #619).
// when drilling into a non-gesture sub-page (see NavigationController.closeDrawer). // HermesScaffold / DisableDrawerGestures reconcile each screen's preference
NavigationController.closeDrawer = { scope.launch { drawerState.close() } } // into this controller via SideEffect; ModalNavigationDrawer reads .enabled
// below. No DRAWER_GESTURE_SCREENS set, no LaunchedEffect(snapTo(Closed)),
// no closeDrawer callback — the controller closes the drawer itself when a
// screen opts out.
val gestureController =
remember(drawerState, scope) {
DrawerGestureController(drawerState, scope)
}
val bottomNavItemsState by AuthManager.bottomNavItemsFlow.collectAsState() val bottomNavItemsState by AuthManager.bottomNavItemsFlow.collectAsState()
val bottomNavDisplayMode by AuthManager.bottomNavDisplayModeFlow.collectAsState() val bottomNavDisplayMode by AuthManager.bottomNavDisplayModeFlow.collectAsState()
@@ -186,35 +201,13 @@ fun MainNavigation(sessionId: String? = null) {
currentScreen != LandingScreen && currentScreen != LandingScreen &&
currentScreen != AuthLoginScreen && currentScreen != AuthLoginScreen &&
currentScreen != PairingCodeEntryScreen currentScreen != PairingCodeEntryScreen
val gesturesEnabled = currentScreen in DRAWER_GESTURE_SCREENS
var drawerGesturesEnabled by remember { mutableStateOf(false) }
LaunchedEffect(gesturesEnabled) {
if (gesturesEnabled) {
// Delay enabling gestures slightly to prevent residual swipe-to-open from back button taps
kotlinx.coroutines.delay(200)
drawerGesturesEnabled = true
} else {
drawerGesturesEnabled = false
}
}
val openDrawer: () -> Unit = { scope.launch { drawerState.open() } } val openDrawer: () -> Unit = { scope.launch { drawerState.open() } }
// B7 (Jun 30 2026, kanban t_424): close drawer if gestures are disabled to dismiss scrim. CompositionLocalProvider(LocalDrawerGestureController provides gestureController) {
// Also force-close on transition into a non-gesture screen so the drawer's scrim
// surface cannot intercept the first back-button tap (e.g. settings sub-pages drilled
// down from the gesture-enabled Settings root). snapTo(Closed) is a safe no-op when
// already closed.
LaunchedEffect(drawerGesturesEnabled) {
if (!drawerGesturesEnabled) {
drawerState.snapTo(DrawerValue.Closed)
}
}
ModalNavigationDrawer( ModalNavigationDrawer(
drawerState = drawerState, drawerState = drawerState,
gesturesEnabled = drawerGesturesEnabled, gesturesEnabled = gestureController.enabled,
drawerContent = { drawerContent = {
ModalDrawerSheet( ModalDrawerSheet(
modifier = Modifier.verticalScroll(rememberScrollState()), modifier = Modifier.verticalScroll(rememberScrollState()),
@@ -355,7 +348,9 @@ fun MainNavigation(sessionId: String? = null) {
}, },
modifier = modifier =
Modifier.testTag( Modifier.testTag(
"nav_${item.key::class.simpleName?.lowercase()?.removeSuffix("screen") ?: ""}", "nav_${item.key::class.simpleName?.lowercase()?.removeSuffix(
"screen",
) ?: ""}",
), ),
) )
} }
@@ -377,3 +372,4 @@ fun MainNavigation(sessionId: String? = null) {
} }
} }
} }
}
@@ -18,11 +18,6 @@ object NavigationController {
var backStack: NavBackStack<NavKey>? = null var backStack: NavBackStack<NavKey>? = null
var pendingSessionId: String? = null var pendingSessionId: String? = null
// Set by MainNavigation so navigation can synchronously dismiss the drawer
// (e.g. when drilling into a non-gesture sub-page) before a recomposition,
// preventing the drawer's gesture surface from intercepting the first tap.
var closeDrawer: (() -> Unit)? = null
// Bottom-nav primary screens — dynamic, updated by Navigation.kt via // Bottom-nav primary screens — dynamic, updated by Navigation.kt via
// updatePrimaryScreens() when the user customises the bottom nav bar. // updatePrimaryScreens() when the user customises the bottom nav bar.
// Default matches the default 5 bottom-nav items. // Default matches the default 5 bottom-nav items.
@@ -51,11 +46,11 @@ object NavigationController {
if (isPrimaryScreen(key)) { if (isPrimaryScreen(key)) {
stack.clear() stack.clear()
} else {
// Drilling into a sub-page (non-primary): dismiss the drawer
// synchronously so its gesture surface can't eat the next tap.
closeDrawer?.invoke()
} }
// Drawer dismissal is handled by DrawerGestureController (issue #619):
// when a non-gesture sub-page composes, its HermesScaffold reconciles
// drawerGesturesEnabled=false and the controller closes the drawer
// itself via SideEffect. No synchronous closeDrawer callback here.
stack.add(key) stack.add(key)
} }
@@ -11,6 +11,7 @@ import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.automirrored.filled.ArrowBack import androidx.compose.material.icons.automirrored.filled.ArrowBack
import androidx.compose.material.icons.filled.Menu import androidx.compose.material.icons.filled.Menu
import androidx.compose.material.icons.filled.Refresh import androidx.compose.material.icons.filled.Refresh
import androidx.compose.material3.DrawerState
import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.ExperimentalMaterial3Api
import androidx.compose.material3.Icon import androidx.compose.material3.Icon
import androidx.compose.material3.IconButton import androidx.compose.material3.IconButton
@@ -21,6 +22,11 @@ import androidx.compose.material3.TopAppBarDefaults
import androidx.compose.material3.TopAppBarScrollBehavior import androidx.compose.material3.TopAppBarScrollBehavior
import androidx.compose.material3.pulltorefresh.PullToRefreshBox import androidx.compose.material3.pulltorefresh.PullToRefreshBox
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.runtime.SideEffect
import androidx.compose.runtime.compositionLocalOf
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.input.nestedscroll.nestedScroll import androidx.compose.ui.input.nestedscroll.nestedScroll
import androidx.compose.ui.layout.layout import androidx.compose.ui.layout.layout
@@ -29,6 +35,8 @@ import androidx.compose.ui.platform.testTag
import androidx.compose.ui.res.stringResource import androidx.compose.ui.res.stringResource
import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.Dp
import com.m57.hermescontrol.R import com.m57.hermescontrol.R
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import kotlin.math.roundToInt import kotlin.math.roundToInt
/** Sealed interface for navigation icon types — eliminates boolean-flag anti-pattern. */ /** Sealed interface for navigation icon types — eliminates boolean-flag anti-pattern. */
@@ -44,6 +52,66 @@ sealed interface NavIcon {
) : NavIcon ) : NavIcon
} }
/**
* Single source of truth for whether the modal navigation drawer's swipe gestures
* should be enabled for the currently-composed screen.
*
* Each [HermesScaffold] (or [DisableDrawerGestures]) reconciles its preference into
* the singleton instance via a [SideEffect]; `ModalNavigationDrawer` in `Navigation.kt`
* reads [enabled] and passes it straight to Material's `gesturesEnabled` parameter.
*
* This replaces the old `DRAWER_GESTURE_SCREENS` set + `LaunchedEffect(snapTo(Closed))`
* workaround documented in issue #619: gesture state now flows from the rendered
* screen automatically, and the drawer controller closes the drawer itself when a
* screen opts out — no global allow-list, no defensive close callbacks.
*/
class DrawerGestureController(
private val drawerState: DrawerState,
private val scope: CoroutineScope,
) {
var enabled by mutableStateOf(false)
private set
/**
* Reconcile a screen's desired gesture state.
*
* Called from a [SideEffect] on every screen composition. If a screen opts out
* of gestures while the drawer is open, we close it synchronously so the scrim
* cannot intercept the next tap (the bug class from PRs #445 / #454 / #455).
*/
fun reconcile(desired: Boolean) {
if (desired == enabled) return
enabled = desired
if (!desired && drawerState.isOpen) {
scope.launch { drawerState.close() }
}
}
}
/**
* CompositionLocal that exposes the active [DrawerGestureController].
*
* Provided by `MainNavigation` and read both by [HermesScaffold] (to reconcile a
* screen's preference) and by `ModalNavigationDrawer` (to gate gestures).
*/
val LocalDrawerGestureController =
compositionLocalOf<DrawerGestureController?> {
null
}
/**
* Opt out of drawer gestures for a screen that does NOT use [HermesScaffold].
*
* Used by full-bleed entry screens (Landing, AuthLogin, PairingCodeEntry) that
* render their own layout. Scaffold-based screens should instead pass
* `drawerGesturesEnabled = false` to [HermesScaffold].
*/
@Composable
fun DisableDrawerGestures() {
val controller = LocalDrawerGestureController.current ?: return
SideEffect { controller.reconcile(false) }
}
/** /**
* Shared Scaffold wrapper that standardizes the TopAppBar. * Shared Scaffold wrapper that standardizes the TopAppBar.
* *
@@ -52,6 +120,7 @@ sealed interface NavIcon {
* - Uses Spacing tokens internally * - Uses Spacing tokens internally
* - Simplified API: [navigationIcon] replaces the old showBack/onBack/onOpenDrawer trio * - Simplified API: [navigationIcon] replaces the old showBack/onBack/onOpenDrawer trio
* - Safe-area aware via Scaffold insets * - Safe-area aware via Scaffold insets
* - Drawer-gesture aware via [drawerGesturesEnabled] (issue #619)
* *
* Screens that still need a Back arrow instead of a Menu icon can set * Screens that still need a Back arrow instead of a Menu icon can set
* `navigationIcon = NavIcon.Back { … }` instead. * `navigationIcon = NavIcon.Back { … }` instead.
@@ -62,6 +131,16 @@ fun HermesScaffold(
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
title: @Composable () -> Unit, title: @Composable () -> Unit,
navigationIcon: NavIcon? = null, navigationIcon: NavIcon? = null,
/**
* Whether the modal navigation drawer's swipe-open/swipe-close gestures should
* be active while this screen is visible. Defaults to `true` (primary screens).
*
* Pass `false` for drill-down sub-pages (e.g. Settings sub-pages) where the
* drawer should be closed and locked. The [DrawerGestureController] reconciles
* this preference and closes the drawer automatically if a screen opts out
* while the drawer is open — see issue #619.
*/
drawerGesturesEnabled: Boolean = true,
onRefresh: (() -> Unit)? = null, onRefresh: (() -> Unit)? = null,
isRefreshing: Boolean = false, isRefreshing: Boolean = false,
pinTopBar: Boolean = false, pinTopBar: Boolean = false,
@@ -69,6 +148,9 @@ fun HermesScaffold(
actions: @Composable () -> Unit = {}, actions: @Composable () -> Unit = {},
content: @Composable (PaddingValues) -> Unit, content: @Composable (PaddingValues) -> Unit,
) { ) {
val gestureController = LocalDrawerGestureController.current
SideEffect { gestureController?.reconcile(drawerGesturesEnabled) }
val scrollBehavior = val scrollBehavior =
if (pinTopBar) { if (pinTopBar) {
TopAppBarDefaults.pinnedScrollBehavior() TopAppBarDefaults.pinnedScrollBehavior()
@@ -39,6 +39,10 @@ import com.m57.hermescontrol.ui.settings.components.TestResultCard
* Drill-down sub-pages for Settings. Each is its own NavKey destination * Drill-down sub-pages for Settings. Each is its own NavKey destination
* (see Navigation.kt) so the native back stack handles navigation — no * (see Navigation.kt) so the native back stack handles navigation — no
* manual routing. The SettingsViewModel stays the single source of truth. * manual routing. The SettingsViewModel stays the single source of truth.
*
* All sub-pages pass `drawerGesturesEnabled = false` to [HermesScaffold] so the
* modal drawer's swipe gestures are disabled and the drawer auto-closes when a
* sub-page is entered — the single source of truth introduced in issue #619.
*/ */
@Composable @Composable
@@ -53,6 +57,9 @@ internal fun SettingsConnectionPage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_connection)) }, title = { Text(stringResource(R.string.settings_sec_connection)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
// Non-primary drill-down: opt out of drawer gestures so the scrim can't
// get stuck open (issue #619). DrawerGestureController handles the close.
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =
@@ -104,6 +111,7 @@ internal fun SettingsAppearancePage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_appearance)) }, title = { Text(stringResource(R.string.settings_sec_appearance)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =
@@ -137,6 +145,7 @@ internal fun SettingsChatPage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_chat)) }, title = { Text(stringResource(R.string.settings_sec_chat)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =
@@ -166,6 +175,7 @@ internal fun SettingsNavBarPage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_nav_bar)) }, title = { Text(stringResource(R.string.settings_sec_nav_bar)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =
@@ -198,6 +208,7 @@ internal fun SettingsBehaviorPage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_behavior)) }, title = { Text(stringResource(R.string.settings_sec_behavior)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =
@@ -224,6 +235,7 @@ internal fun SettingsAboutPage(
HermesScaffold( HermesScaffold(
title = { Text(stringResource(R.string.settings_sec_about)) }, title = { Text(stringResource(R.string.settings_sec_about)) },
navigationIcon = NavIcon.Back(onBack), navigationIcon = NavIcon.Back(onBack),
drawerGesturesEnabled = false,
) { ) {
Column( Column(
modifier = modifier =