Fix android navigation crash (#21)

* Fix internalRouteList not being updated in time * Pr comments * ktlint and fix test * api dump
mirego · Nov 26, 2024 · dda45db · dda45db
1 parent 128e8a9
commit dda45db
Show file tree

Hide file tree

Showing 7 changed files with 42 additions and 23 deletions.
diff --git a/navigation/common/api/navigation.api b/navigation/common/api/navigation.api
@@ -30,9 +30,10 @@ public abstract class com/mirego/pilot/navigation/PilotActionNavigationListener
 public abstract class com/mirego/pilot/navigation/PilotNavigationListener {
 	public static final field $stable I
 	public fun <init> ()V
+	public abstract fun canNavigate (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
 	public abstract fun pop ()V
 	public abstract fun popTo (Lcom/mirego/pilot/navigation/PilotNavigationRoute;Z)V
-	public abstract fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
+	public abstract fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)V
 }
 
 public abstract class com/mirego/pilot/navigation/PilotNavigationManager {
@@ -68,10 +69,11 @@ public final class com/mirego/pilot/navigation/compose/PilotBackHandlerKt {
 public class com/mirego/pilot/navigation/compose/PilotNavControllerNavigationListener : com/mirego/pilot/navigation/PilotNavigationListener {
 	public static final field $stable I
 	public fun <init> (Landroidx/navigation/NavController;)V
+	public fun canNavigate (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
 	public fun onRootPop ()V
 	public fun pop ()V
 	public fun popTo (Lcom/mirego/pilot/navigation/PilotNavigationRoute;Z)V
-	public fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)Z
+	public fun push (Lcom/mirego/pilot/navigation/PilotNavigationRoute;)V
 }
 
 public final class com/mirego/pilot/navigation/compose/PilotNavigationHelpersKt {

diff --git a/...idMain/kotlin/com/mirego/pilot/navigation/compose/PilotNavControllerNavigationListener.kt b/...idMain/kotlin/com/mirego/pilot/navigation/compose/PilotNavControllerNavigationListener.kt
@@ -8,9 +8,13 @@ public open class PilotNavControllerNavigationListener<ROUTE : PilotNavigationRo
     private val navController: NavController,
 ) : PilotNavigationListener<ROUTE>() {
 
-    override fun push(route: ROUTE): Boolean {
+    // Override this method if you want to prevent navigation in specific situations.
+    // This will be verified before calling push
+    override fun canNavigate(route: ROUTE): Boolean =
+        true
+
+    override fun push(route: ROUTE) {
         navController.navigate(route.navRoute)
-        return true
     }
 
     override fun pop() {

diff --git a/...common/src/commonMain/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManager.kt b/...common/src/commonMain/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManager.kt
@@ -24,8 +24,9 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
                 return@launch
             }
             listener?.let {
-                if (it.push(route)) {
+                if (it.canNavigate(route)) {
                     internalRouteList.add(route)
+                    it.push(route)
                 }
             }
         }

diff --git a/...gation/common/src/commonMain/kotlin/com/mirego/pilot/navigation/PilotNavigationManager.kt b/...gation/common/src/commonMain/kotlin/com/mirego/pilot/navigation/PilotNavigationManager.kt
@@ -21,7 +21,8 @@ public abstract class PilotNavigationManager<ROUTE : PilotNavigationRoute, ACTIO
 }
 
 public abstract class PilotNavigationListener<ROUTE : PilotNavigationRoute> {
-    public abstract fun push(route: ROUTE): Boolean
+    public abstract fun canNavigate(route: ROUTE): Boolean
+    public abstract fun push(route: ROUTE)
     public abstract fun pop()
     public abstract fun popTo(route: ROUTE, inclusive: Boolean)
 }

diff --git a/...on/src/commonTest/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManagerTest.kt b/...on/src/commonTest/kotlin/com/mirego/pilot/navigation/DefaultPilotNavigationManagerTest.kt
@@ -94,11 +94,14 @@ class DefaultPilotNavigationManagerTest {
     private class TestNavigationListener : PilotNavigationListener<TestNavigationRoute>() {
         val routes = mutableListOf<TestNavigationRoute>()
 
-        override fun push(route: TestNavigationRoute): Boolean {
-            routes.add(route)
+        override fun canNavigate(route: TestNavigationRoute): Boolean {
             return true
         }
 
+        override fun push(route: TestNavigationRoute) {
+            routes.add(route)
+        }
+
         override fun pop() {
             routes.removeLastOrNull()
         }

diff --git a/navigation/ios/Navigation.swift b/navigation/ios/Navigation.swift
@@ -5,17 +5,19 @@ public extension View {
     func pilotNavigation<ScreenData, Route: PilotNavigationRoute, Action: AnyObject, ScreenView: View, NavModifier: ViewModifier>(
         navigationManager: PilotNavigationManager<Route, Action>,
         @ViewBuilder buildView: @escaping (ScreenData) -> ScreenView,
-        buildNavigation: @escaping ([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>?,
+        buildNavigation: @escaping ([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>,
         handleAction: ((Action) -> Void)? = nil,
-        handlePopRoot: (() -> Void)? = nil
+        handlePopRoot: (() -> Void)? = nil,
+        canNavigateToRoute: ((Route) -> Bool)? = nil
     ) -> some View {
         modifier(
             NavigationModifier<ScreenData, Route, Action, ScreenView, NavModifier>(
                 buildView: buildView,
                 buildNavigation: buildNavigation,
                 handleAction: handleAction,
                 navigationManager: navigationManager,
-                handlePopRoot: handlePopRoot
+                handlePopRoot: handlePopRoot,
+                canNavigateToRoute: canNavigateToRoute
             )
         )
     }
@@ -28,10 +30,11 @@ private struct NavigationModifier<ScreenData, Route: PilotNavigationRoute, Actio
 
     init(
         buildView: @escaping (ScreenData) -> ScreenView,
-        buildNavigation: @escaping ([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>?,
+        buildNavigation: @escaping ([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>,
         handleAction: ((Action) -> Void)? = nil,
         navigationManager: PilotNavigationManager<Route, Action>? = nil,
-        handlePopRoot: (() -> Void)?
+        handlePopRoot: (() -> Void)? = nil,
+        canNavigateToRoute: ((Route) -> Bool)? = nil
     ) {
         self.buildView = buildView
         let rootNavigationState = NavigationState<ScreenData, Route, Action, NavModifier>(
@@ -40,7 +43,8 @@ private struct NavigationModifier<ScreenData, Route: PilotNavigationRoute, Actio
             buildNavigation: buildNavigation,
             handleAction: handleAction,
             navigationManager: navigationManager,
-            handlePopRoot: handlePopRoot
+            handlePopRoot: handlePopRoot,
+            canNavigateToRoute: canNavigateToRoute
         )
         _rootState = StateObject(wrappedValue: rootNavigationState)
     }

diff --git a/navigation/ios/NavigationState.swift b/navigation/ios/NavigationState.swift
@@ -20,25 +20,28 @@ class NavigationState<
     @Published var child: NavigationStateTyped?
     @Published var navigationDismissTriggered = false
 
-    private let buildNavigation: (([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>?)?
+    private let buildNavigation: (([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>)?
     private let navigationManager: PilotNavigationManager<Route, Action>?
     private let actionListener: ActionListener<Route, Action>
     private let handlePopRoot: (() -> Void)?
+    private let canNavigateToRoute: ((_ route: Route) -> Bool)
     private var lastNavigationDate: Foundation.Date?
 
     init(
         navigation: PilotNavigationType<ScreenData, NavModifier>,
         route: Route?,
-        buildNavigation: (([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>?)? = nil,
+        buildNavigation: (([Route], Route) -> PilotNavigationType<ScreenData, NavModifier>)? = nil,
         handleAction: ((Action) -> Void)? = nil,
         navigationManager: PilotNavigationManager<Route, Action>? = nil,
-        handlePopRoot: (() -> Void)? = nil
+        handlePopRoot: (() -> Void)? = nil,
+        canNavigateToRoute: ((_ route: Route) -> Bool)? = nil
     ) {
         self.navigation = navigation
         self.route = route
         self.buildNavigation = buildNavigation
         self.navigationManager = navigationManager
         self.handlePopRoot = handlePopRoot
+        self.canNavigateToRoute = canNavigateToRoute ?? { _ in return true }
         actionListener = ActionListener(navigationManager: navigationManager, handleAction: handleAction)
 
         super.init()
@@ -49,18 +52,19 @@ class NavigationState<
         actionListener.startListening()
     }
 
-    override func push(route: PilotNavigationRoute) -> Bool {
+    override func canNavigate(route: PilotNavigationRoute) -> Bool {
+        guard let route = route as? Route else { return false }
+        return canNavigateToRoute(route)
+    }
+
+    override func push(route: PilotNavigationRoute) {
         guard let buildNavigation else { fatalError("buildNavigation not set")}
         guard let route = route as? Route else { fatalError("Invalid route type")}
-        guard let navigation = buildNavigation(currentStack(), route) else {
-            return false
-        }
 
         debounceNavigation { [weak self] in
             guard let self else { return }
-            top().child = NavigationState(navigation: navigation, route: route)
+            top().child = NavigationState(navigation: buildNavigation(currentStack(), route), route: route)
         }
-        return true
     }
 
     override func popTo(route: PilotNavigationRoute, inclusive: Bool) {