[Navigation] Fix back handling issue in nested navs on Android

gradle/libs.versions.toml

@@ -13,16 +13,20 @@ kotlinx-coroutines = "1.8.0"
 mirego-publish = "1.5"
 coil = "2.5.0"
 dokka = "1.9.20"
+assertk = "0.28.1"
 
 [libraries]
 kotlin-stdlib-jdk8 = { module = "org.jetbrains.kotlin:kotlin-stdlib-jdk8", version.ref = "kotlin" }
+kotlin-test = { module = "org.jetbrains.kotlin:kotlin-test", version.ref = "kotlin" }
+kotlin-test-junit = { module = "org.jetbrains.kotlin:kotlin-test-junit", version.ref = "kotlin" }
 kotlinx-coroutines-core = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinx-coroutines" }
+kotlinx-coroutines-test = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-test", version.ref = "kotlinx-coroutines" }
+assertk = { module = "com.willowtreeapps.assertk:assertk", version.ref = "assertk"  }
 
 androidx-compose-foundation = { module = "androidx.compose.foundation:foundation", version.ref = "androidx-compose-runtime" }
 androidx-compose-ui = { module = "androidx.compose.ui:ui", version.ref = "androidx-compose-runtime" }
 androidx-compose-material3 = { module = "androidx.compose.material3:material3", version.ref = "androidx-compose-material3" }
 
-
 androidx-lifecycle-common = { module = "androidx.lifecycle:lifecycle-common", version.ref = "androidx-lifecycle" }
 androidx-lifecycle-viewmodel = { module = "androidx.lifecycle:lifecycle-viewmodel", version.ref = "androidx-lifecycle" }
 androidx-lifecycle-viewmodel-ktx = { module = "androidx.lifecycle:lifecycle-viewmodel-ktx", version.ref = "androidx-lifecycle" }

navigation/common/build.gradle.kts

@@ -32,6 +32,14 @@ kotlin {
                 implementation(libs.kotlinx.coroutines.core)
             }
         }
+        val commonTest by getting {
+            dependencies {
+                implementation(libs.kotlin.test)
+                implementation(libs.kotlin.test.junit)
+                implementation(libs.kotlinx.coroutines.test)
+                implementation(libs.assertk)
+            }
+        }
         val androidMain by getting {
             dependencies {
                 implementation(libs.kotlin.stdlib.jdk8)

navigation/common/src/androidMain/kotlin/com/mirego/pilot/navigation/compose/PilotBackHandler.kt

@@ -11,6 +11,6 @@ import com.mirego.pilot.navigation.PilotNavigationManager
 public fun PilotBackHandler(navController: NavHostController, navigationManager: PilotNavigationManager<*, *>, rootName: String) {
     val backStackEntry by navController.currentBackStackEntryFlow.collectAsState(initial = null)
     BackHandler(enabled = backStackEntry?.destination?.route != rootName) {
-        navigationManager.pop()
+        navigationManager.pop(locally = true)
     }
 }

navigation/common/src/commonMain/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManager.kt

@@ -32,21 +32,19 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
     }
 
     override fun pop(locally: Boolean) {
-        if (!locally && parentNavigationManager != null) {
-            parentNavigationManager.pop(locally = locally)
-            return
+        if ((!locally && parentNavigationManager != null) || !internalPop(callListener = true)) {
+            parentNavigationManager?.pop(locally = locally)
         }
-        internalPop(callListener = true)
     }
 
-    private fun internalPop(callListener: Boolean) {
-        coroutineScope.launch {
-            internalRouteList.removeLastOrNull()
-
-            if (callListener) {
+    private fun internalPop(callListener: Boolean): Boolean {
+        val last = internalRouteList.removeLastOrNull()
+        if (callListener) {
+            coroutineScope.launch {
                 listener?.pop()
             }
         }
+        return last != null
     }
 
     override fun popToId(uniqueId: String, inclusive: Boolean) {

navigation/common/src/commonTest/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManagerTest.kt

@@ -0,0 +1,115 @@
+package com.mirego.pilot.navigation
+
+import assertk.assertThat
+import assertk.assertions.containsExactly
+import assertk.assertions.isEmpty
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlin.test.Test
+
+@OptIn(ExperimentalCoroutinesApi::class)
+class DefaultPilotNavigationManagerTest {
+
+    private val testScope = CoroutineScope(UnconfinedTestDispatcher())
+    private val parentListener = TestNavigationListener()
+    private val childListener = TestNavigationListener()
+    private val parentNavigationManager = DefaultPilotNavigationManager<TestNavigationRoute, Any>(testScope).apply {
+        listener = parentListener
+    }
+    private val childNavigationManager = DefaultPilotNavigationManager(testScope, parentNavigationManager = parentNavigationManager).apply {
+        listener = childListener
+    }
+
+    @Test
+    fun `when pushing and popping on the child not locally it should push and pop to the parent`() {
+        childNavigationManager.push(TestNavigationRoute.Route1, locally = false)
+        childNavigationManager.push(TestNavigationRoute.Route2, locally = false)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1, TestNavigationRoute.Route2)
+        assertThat(childListener.routes).isEmpty()
+
+        childNavigationManager.pop(locally = false)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1)
+        assertThat(childListener.routes).isEmpty()
+
+        childNavigationManager.pop(locally = false)
+        assertThat(parentListener.routes).isEmpty()
+        assertThat(childListener.routes).isEmpty()
+    }
+
+    @Test
+    fun `when popping locally and there is nothing to pop it should pop the parent`() {
+        parentNavigationManager.push(TestNavigationRoute.Route1, locally = true)
+        childNavigationManager.push(TestNavigationRoute.Route2, locally = true)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1)
+        assertThat(childListener.routes).containsExactly(TestNavigationRoute.Route2)
+
+        childNavigationManager.pop(locally = true)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1)
+        assertThat(childListener.routes).isEmpty()
+
+        childNavigationManager.pop(locally = true)
+        assertThat(parentListener.routes).isEmpty()
+        assertThat(childListener.routes).isEmpty()
+    }
+
+    @Test
+    fun `when pop inclusive it should pop the given route`() {
+        parentNavigationManager.push(TestNavigationRoute.Route1)
+        parentNavigationManager.push(TestNavigationRoute.Route2)
+        parentNavigationManager.push(TestNavigationRoute.Route3)
+
+        parentNavigationManager.popToId(TestNavigationRoute.Route2.uniqueId, inclusive = true)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1)
+
+        parentNavigationManager.popToName(TestNavigationRouteName.ROUTE1.name, inclusive = true)
+        assertThat(parentListener.routes).isEmpty()
+    }
+
+    @Test
+    fun `when pop exclusive it should not pop the given route`() {
+        parentNavigationManager.push(TestNavigationRoute.Route1)
+        parentNavigationManager.push(TestNavigationRoute.Route2)
+        parentNavigationManager.push(TestNavigationRoute.Route3)
+
+        parentNavigationManager.popToId(TestNavigationRoute.Route2.uniqueId, inclusive = false)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1, TestNavigationRoute.Route2)
+
+        parentNavigationManager.popToName(TestNavigationRoute.Route1.name, inclusive = false)
+        assertThat(parentListener.routes).containsExactly(TestNavigationRoute.Route1)
+    }
+
+    private enum class TestNavigationRouteName {
+        ROUTE1,
+        ROUTE2,
+        ROUTE3,
+    }
+
+    private sealed class TestNavigationRoute(routeName: TestNavigationRouteName) : EnumPilotNavigationRoute(routeName) {
+        data object Route1 : TestNavigationRoute(TestNavigationRouteName.ROUTE1)
+        data object Route2 : TestNavigationRoute(TestNavigationRouteName.ROUTE2)
+        data object Route3 : TestNavigationRoute(TestNavigationRouteName.ROUTE3)
+    }
+
+    private class TestNavigationListener : PilotNavigationListener<TestNavigationRoute>() {
+        val routes = mutableListOf<TestNavigationRoute>()
+
+        override fun push(route: TestNavigationRoute): Boolean {
+            routes.add(route)
+            return true
+        }
+
+        override fun pop() {
+            routes.removeLastOrNull()
+        }
+
+        override fun popTo(route: TestNavigationRoute, inclusive: Boolean) {
+            while (routes.last() != route) {
+                routes.removeLastOrNull()
+            }
+            if (inclusive) {
+                routes.removeLastOrNull()
+            }
+        }
+    }
+}