fix(nav): restore broken traceroute map navigation (#5104)

This commit is contained in:
James Rich 2026-04-13 07:25:21 -05:00 committed by GitHub
parent 35bf1fded5
commit 39620d063b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 71 additions and 6 deletions

View file

@ -111,4 +111,35 @@ class MultiBackstackTest {
assertEquals(2, multiBackstack.activeBackStack.size)
assertEquals(SettingsRoute.About, multiBackstack.activeBackStack.last())
}
@Test
fun `handleDeepLink from different tab switches tab and sets stack`() {
// Start on Connections tab
val startTab = TopLevelDestination.Connections.route
val multiBackstack = MultiBackstack(startTab)
val connectionsStack = NavBackStack<NavKey>().apply { addAll(listOf(TopLevelDestination.Connections.route)) }
val nodesStack = NavBackStack<NavKey>().apply { addAll(listOf(TopLevelDestination.Nodes.route)) }
multiBackstack.backStacks =
mapOf(
TopLevelDestination.Connections.route to connectionsStack,
TopLevelDestination.Nodes.route to nodesStack,
)
// Verify we start on Connections
assertEquals(TopLevelDestination.Connections.route, multiBackstack.currentTabRoute)
// Deep-link to a TracerouteMap on the Nodes tab (this is the exact pattern
// MeshtasticAppShell uses for traceroute alert "View on Map")
val tracerouteMap = NodeDetailRoute.TracerouteMap(destNum = 100, requestId = 42, logUuid = "abc")
multiBackstack.handleDeepLink(listOf(NodesRoute.NodesGraph, tracerouteMap))
// Should have switched to the Nodes tab
assertEquals(TopLevelDestination.Nodes.route, multiBackstack.currentTabRoute)
// Stack should contain the graph root + the traceroute map route
assertEquals(2, multiBackstack.activeBackStack.size)
assertEquals(NodesRoute.NodesGraph, multiBackstack.activeBackStack.first())
assertEquals(tracerouteMap, multiBackstack.activeBackStack.last())
}
}

View file

@ -21,6 +21,7 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.ui.Modifier
import org.meshtastic.core.navigation.MultiBackstack
import org.meshtastic.core.navigation.NodeDetailRoute
import org.meshtastic.core.navigation.NodesRoute
import org.meshtastic.core.ui.viewmodel.UIViewModel
/**
@ -43,8 +44,11 @@ fun MeshtasticAppShell(
MeshtasticCommonAppSetup(
uiViewModel = uiViewModel,
onNavigateToTracerouteMap = { destNum, requestId, logUuid ->
multiBackstack.activeBackStack.add(
NodeDetailRoute.TracerouteMap(destNum = destNum, requestId = requestId, logUuid = logUuid),
multiBackstack.handleDeepLink(
listOf(
NodesRoute.NodesGraph,
NodeDetailRoute.TracerouteMap(destNum = destNum, requestId = requestId, logUuid = logUuid),
),
)
},
)

View file

@ -26,9 +26,11 @@ import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.lifecycle.compose.collectAsStateWithLifecycle
import kotlinx.coroutines.launch
import org.meshtastic.core.resources.Res
import org.meshtastic.core.resources.okay
import org.meshtastic.core.resources.traceroute
@ -52,6 +54,7 @@ fun TracerouteAlertHandler(
val traceRouteResponse by uiViewModel.tracerouteResponse.collectAsStateWithLifecycle(null)
var dismissedTracerouteRequestId by remember { mutableStateOf<Int?>(null) }
val colorScheme = MaterialTheme.colorScheme
val scope = rememberCoroutineScope()
LaunchedEffect(traceRouteResponse, dismissedTracerouteRequestId) {
val response = traceRouteResponse
@ -83,8 +86,10 @@ fun TracerouteAlertHandler(
dismissedTracerouteRequestId = response.requestId
onNavigateToMap(response.destinationNodeNum, response.requestId, response.logUuid)
} else {
uiViewModel.showAlert(titleRes = Res.string.traceroute, messageRes = errorRes)
uiViewModel.clearTracerouteResponse()
// Post the error alert after the current alert is dismissed to avoid
// the wrapping dismissAlert() in AlertManager immediately clearing it.
scope.launch { uiViewModel.showAlert(titleRes = Res.string.traceroute, messageRes = errorRes) }
}
},
dismissTextRes = Res.string.okay,

View file

@ -68,4 +68,27 @@ class AlertManagerTest {
assertEquals(true, dismissClicked)
assertNull(alertManager.currentAlert.value)
}
@Test
fun showAlert_inside_onConfirm_is_dismissed_by_wrapping_dismissAlert() {
// Documents the known race condition: AlertManager wraps onConfirm to call
// dismissAlert() AFTER the user callback, so a showAlert() inside onConfirm
// gets immediately cleared. Callers must defer via launch {} to work around this.
alertManager.showAlert(
title = "First",
onConfirm = {
// This simulates an error path where onConfirm shows a follow-up alert
alertManager.showAlert(title = "Second", message = "Error details")
},
)
// Trigger the wrapped onConfirm (user callback + dismissAlert)
alertManager.currentAlert.value?.onConfirm?.invoke()
// The second alert is wiped by dismissAlert() — currentAlert is null
assertNull(
alertManager.currentAlert.value,
"showAlert inside onConfirm is cleared by the wrapping dismissAlert; callers must defer via launch {}",
)
}
}

View file

@ -277,7 +277,6 @@ open class MetricsViewModel(
responseLogUuid: String,
overlay: TracerouteOverlay?,
onViewOnMap: (Int, String) -> Unit,
onShowError: (StringResource) -> Unit,
) {
viewModelScope.launch {
val snapshotPositions = tracerouteSnapshotRepository.getSnapshotPositions(responseLogUuid).first()
@ -300,7 +299,11 @@ open class MetricsViewModel(
)
val errorRes = availability.toMessageRes()
if (errorRes != null) {
onShowError(errorRes)
// Post the error alert after the current alert is dismissed to avoid
// the wrapping dismissAlert() in AlertManager immediately clearing it.
viewModelScope.launch {
alertManager.showAlert(titleRes = Res.string.traceroute, messageRes = errorRes)
}
} else {
onViewOnMap(requestId, responseLogUuid)
}

View file

@ -361,7 +361,6 @@ private fun showTracerouteDetail(
responseLogUuid = result.uuid,
overlay = overlay,
onViewOnMap = onViewOnMap,
onShowError = {},
)
}