From ebab2ee9add2aec31de94592780dae2757600a4e Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Sat, 29 Nov 2025 19:28:44 -0600 Subject: [PATCH] refactor(navigation): Simplify adaptive back nav and state (#3860) Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com> --- .../mesh/ui/contact/AdaptiveContactsScreen.kt | 40 +++++--------- .../mesh/ui/node/AdaptiveNodeListScreen.kt | 55 +++++-------------- .../meshtastic/feature/messaging/Message.kt | 4 -- .../feature/messaging/MessageScreenEvent.kt | 3 - .../feature/node/detail/NodeDetailScreen.kt | 8 +-- 5 files changed, 30 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/com/geeksville/mesh/ui/contact/AdaptiveContactsScreen.kt b/app/src/main/java/com/geeksville/mesh/ui/contact/AdaptiveContactsScreen.kt index fb3dfaa08..8b499311b 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/contact/AdaptiveContactsScreen.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/contact/AdaptiveContactsScreen.kt @@ -17,7 +17,6 @@ package com.geeksville.mesh.ui.contact -import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -32,9 +31,11 @@ import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi import androidx.compose.material3.adaptive.layout.AnimatedPane import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffold import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffoldRole +import androidx.compose.material3.adaptive.navigation.BackNavigationBehavior import androidx.compose.material3.adaptive.navigation.rememberListDetailPaneScaffoldNavigator import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.key import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -64,15 +65,13 @@ fun AdaptiveContactsScreen( ) { val navigator = rememberListDetailPaneScaffoldNavigator() val scope = rememberCoroutineScope() - - BackHandler(navigator.canNavigateBack()) { scope.launch { navigator.navigateBack() } } + val backNavigationBehavior = BackNavigationBehavior.PopUntilScaffoldValueChange LaunchedEffect(initialContactKey) { if (initialContactKey != null) { navigator.navigateTo(ListDetailPaneScaffoldRole.Detail, initialContactKey) } } - ListDetailPaneScaffold( directive = navigator.scaffoldDirective, value = navigator.scaffoldValue, @@ -97,28 +96,17 @@ fun AdaptiveContactsScreen( }, detailPane = { AnimatedPane { - val contactKey = navigator.currentDestination?.contentKey - - if (contactKey != null) { - MessageScreen( - contactKey = contactKey, - message = if (contactKey == initialContactKey) initialMessage else "", - navigateToMessages = { newContactKey -> - scope.launch { navigator.navigateTo(ListDetailPaneScaffoldRole.Detail, newContactKey) } - }, - navigateToNodeDetails = { navController.navigate(NodesRoutes.NodeDetailGraph(it)) }, - navigateToQuickChatOptions = { navController.navigate(ContactsRoutes.QuickChat) }, - onNavigateBack = { - if (navigator.canNavigateBack()) { - scope.launch { navigator.navigateBack() } - } else { - navController.navigateUp() - } - }, - ) - } else { - PlaceholderScreen() - } + navigator.currentDestination?.contentKey?.let { contactKey -> + key(contactKey) { + MessageScreen( + contactKey = contactKey, + message = if (contactKey == initialContactKey) initialMessage else "", + navigateToNodeDetails = { navController.navigate(NodesRoutes.NodeDetailGraph(it)) }, + navigateToQuickChatOptions = { navController.navigate(ContactsRoutes.QuickChat) }, + onNavigateBack = { scope.launch { navigator.navigateBack(backNavigationBehavior) } }, + ) + } + } ?: PlaceholderScreen() } }, ) diff --git a/app/src/main/java/com/geeksville/mesh/ui/node/AdaptiveNodeListScreen.kt b/app/src/main/java/com/geeksville/mesh/ui/node/AdaptiveNodeListScreen.kt index 8c8932f1e..b74937832 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/node/AdaptiveNodeListScreen.kt +++ b/app/src/main/java/com/geeksville/mesh/ui/node/AdaptiveNodeListScreen.kt @@ -17,7 +17,6 @@ package com.geeksville.mesh.ui.node -import androidx.activity.compose.BackHandler import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column @@ -32,9 +31,11 @@ import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi import androidx.compose.material3.adaptive.layout.AnimatedPane import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffold import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffoldRole +import androidx.compose.material3.adaptive.navigation.BackNavigationBehavior import androidx.compose.material3.adaptive.navigation.rememberListDetailPaneScaffoldNavigator import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.key import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -44,7 +45,6 @@ import androidx.navigation.NavHostController import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.launch import org.jetbrains.compose.resources.stringResource -import org.meshtastic.core.navigation.Route import org.meshtastic.core.strings.Res import org.meshtastic.core.strings.nodes import org.meshtastic.core.ui.component.ScrollToTopEvent @@ -64,18 +64,7 @@ fun AdaptiveNodeListScreen( ) { val navigator = rememberListDetailPaneScaffoldNavigator() val scope = rememberCoroutineScope() - - val isDetailActive = navigator.currentDestination?.contentKey != null - - BackHandler(enabled = isDetailActive) { - scope.launch { - if (navigator.canNavigateBack()) { - navigator.navigateBack() - } else { - navigator.navigateTo(ListDetailPaneScaffoldRole.List, null) - } - } - } + val backNavigationBehavior = BackNavigationBehavior.PopUntilScaffoldValueChange LaunchedEffect(initialNodeId) { if (initialNodeId != null) { @@ -103,39 +92,23 @@ fun AdaptiveNodeListScreen( detailPane = { AnimatedPane { val focusManager = LocalFocusManager.current - val nodeId = navigator.currentDestination?.contentKey // Prevent TextFields from auto-focusing when pane animates in - LaunchedEffect(nodeId) { focusManager.clearFocus() } - if (nodeId != null) { - NodeDetailScreenWrapper( - nodeId = nodeId, - navigateToMessages = onNavigateToMessages, - onNavigate = { route -> navController.navigate(route) }, - onNavigateUp = { scope.launch { navigator.navigateTo(ListDetailPaneScaffoldRole.List, null) } }, - ) - } else { - PlaceholderScreen() - } + navigator.currentDestination?.contentKey?.let { nodeId -> + key(nodeId) { + LaunchedEffect(nodeId) { focusManager.clearFocus() } + NodeDetailScreen( + nodeId = nodeId, + navigateToMessages = onNavigateToMessages, + onNavigate = { route -> navController.navigate(route) }, + onNavigateUp = { scope.launch { navigator.navigateBack(backNavigationBehavior) } }, + ) + } + } ?: PlaceholderScreen() } }, ) } -@Composable -private fun NodeDetailScreenWrapper( - nodeId: Int, - navigateToMessages: (String) -> Unit, - onNavigate: (Route) -> Unit, - onNavigateUp: () -> Unit, -) { - NodeDetailScreen( - onNavigateUp = onNavigateUp, - navigateToMessages = navigateToMessages, - onNavigate = onNavigate, - overrideNodeId = nodeId, - ) -} - @Composable private fun PlaceholderScreen() { Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) { diff --git a/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/Message.kt b/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/Message.kt index 2bff55e33..30c86942e 100644 --- a/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/Message.kt +++ b/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/Message.kt @@ -85,7 +85,6 @@ import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.platform.LocalClipboard -import androidx.compose.ui.res.pluralStringResource import androidx.compose.ui.text.input.KeyboardCapitalization import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow @@ -144,7 +143,6 @@ private const val ROUNDED_CORNER_PERCENT = 100 * @param contactKey A unique key identifying the contact or channel. * @param message An optional message to pre-fill in the input field. * @param viewModel The [MessageViewModel] instance for handling business logic and state. - * @param navigateToMessages Callback to navigate to a different message thread. * @param navigateToNodeDetails Callback to navigate to a node's detail screen. * @param onNavigateBack Callback to navigate back from this screen. */ @@ -154,7 +152,6 @@ fun MessageScreen( contactKey: String, message: String, viewModel: MessageViewModel = hiltViewModel(), - navigateToMessages: (String) -> Unit, navigateToNodeDetails: (Int) -> Unit, navigateToQuickChatOptions: () -> Unit, onNavigateBack: () -> Unit, @@ -270,7 +267,6 @@ fun MessageScreen( is MessageScreenEvent.NodeDetails -> navigateToNodeDetails(event.node.num) is MessageScreenEvent.SetTitle -> viewModel.setTitle(event.title) - is MessageScreenEvent.NavigateToMessages -> navigateToMessages(event.contactKey) is MessageScreenEvent.NavigateToNodeDetails -> navigateToNodeDetails(event.nodeNum) MessageScreenEvent.NavigateBack -> onNavigateBack() is MessageScreenEvent.CopyToClipboard -> { diff --git a/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/MessageScreenEvent.kt b/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/MessageScreenEvent.kt index 14e3d886c..ee69b3547 100644 --- a/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/MessageScreenEvent.kt +++ b/feature/messaging/src/main/kotlin/org/meshtastic/feature/messaging/MessageScreenEvent.kt @@ -39,9 +39,6 @@ internal sealed interface MessageScreenEvent { /** Set the title of the screen (typically the contact or channel name). */ data class SetTitle(val title: String) : MessageScreenEvent - /** Navigate to a different message thread. */ - data class NavigateToMessages(val contactKey: String) : MessageScreenEvent - /** Navigate to the details screen for a specific node. */ data class NavigateToNodeDetails(val nodeNum: Int) : MessageScreenEvent diff --git a/feature/node/src/main/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreen.kt b/feature/node/src/main/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreen.kt index 3c09f7d08..863a2fbc8 100644 --- a/feature/node/src/main/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreen.kt +++ b/feature/node/src/main/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreen.kt @@ -43,19 +43,15 @@ import org.meshtastic.feature.node.model.NodeDetailAction @Suppress("LongMethod") @Composable fun NodeDetailScreen( + nodeId: Int, modifier: Modifier = Modifier, viewModel: MetricsViewModel = hiltViewModel(), nodeDetailViewModel: NodeDetailViewModel = hiltViewModel(), navigateToMessages: (String) -> Unit = {}, onNavigate: (Route) -> Unit = {}, onNavigateUp: () -> Unit = {}, - overrideNodeId: Int? = null, ) { - LaunchedEffect(overrideNodeId) { - if (overrideNodeId != null) { - viewModel.setNodeId(overrideNodeId) - } - } + LaunchedEffect(nodeId) { viewModel.setNodeId(nodeId) } val state by viewModel.state.collectAsStateWithLifecycle() val environmentState by viewModel.environmentState.collectAsStateWithLifecycle()