Skip to content

Commit

Permalink
Temporarily keep routes after they have been popped. (#23)
Browse files Browse the repository at this point in the history
Temporarily keep routes after they have been popped.

This fix a crash that can occur when navigating and pressing back almost at the same time on android. When popping, the route was immediately removed from the routes but android did not have time to render it. findRoute would be called after and find nothing.

This keeps the popped routes in a separate list for 5 seconds to make sure that if android needs them to compose the view, they are still accessible. Hooking on the nav controller listeners when the destinations changed was still too early.

This was the stack trace

java.lang.IllegalStateException: Cannot find route with id ad54c5e1-1f82-4010-9011-15ba06d3992a
    at com.mirego.pilot.navigation.compose.PilotNavigationHelpersKt.findRoute(PilotNavigationHelpers.kt:24)
   ...
  • Loading branch information
npresseault authored Jan 6, 2025
1 parent faf2a08 commit 7783917
Showing 1 changed file with 24 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.mirego.pilot.navigation

import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.delay
import kotlinx.coroutines.launch

public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, ACTION : Any>(
Expand All @@ -9,13 +10,15 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
) : PilotNavigationManager<ROUTE, ACTION>() {

private val internalRouteList: MutableList<ROUTE> = mutableListOf()
private val internalTemporaryPoppedRouteList: MutableList<ROUTE> = mutableListOf()

override fun currentRoutes(): List<ROUTE> =
internalRouteList

@Suppress("UNCHECKED_CAST")
override fun <T : ROUTE> findRoute(uniqueId: String): T? =
internalRouteList.firstOrNull { it.uniqueId == uniqueId } as? T
?: internalTemporaryPoppedRouteList.firstOrNull { it.uniqueId == uniqueId } as? T

override fun push(route: ROUTE, locally: Boolean) {
coroutineScope.launch {
Expand All @@ -40,6 +43,9 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC

private fun internalPop(callListener: Boolean): Boolean {
val last = internalRouteList.removeLastOrNull()
if (last != null) {
addToPoppingList(last)
}
if (callListener) {
coroutineScope.launch {
listener?.pop()
Expand All @@ -48,6 +54,22 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
return last != null
}

private fun addToPoppingList(route: ROUTE) {
addToPoppingList(listOf(route))
}

private fun addToPoppingList(routes: List<ROUTE>) {
// We keep the routes for a little while in the popping list to allow animations to complete
// Otherwise, on android when pushing and pressing back quickly, the route would be removed
// from the list before the composable would have time to animate out and findRoute could not
// find the route anymore
internalTemporaryPoppedRouteList.addAll(routes)
coroutineScope.launch {
delay(5000)
internalTemporaryPoppedRouteList.removeAll(routes)
}
}

override fun popToId(uniqueId: String, inclusive: Boolean) {
internalPopTo(popType = PopType.ById(uniqueId), inclusive = inclusive, callListener = true)
}
Expand Down Expand Up @@ -80,10 +102,12 @@ public open class DefaultPilotNavigationManager<ROUTE : PilotNavigationRoute, AC
if (callListener) {
listener?.let {
internalRouteList.removeAll(elementsToRemove)
addToPoppingList(elementsToRemove)
listener?.popTo(navigationItem, inclusive = inclusive)
}
} else {
internalRouteList.removeAll(elementsToRemove)
addToPoppingList(elementsToRemove)
}
}
}
Expand Down

0 comments on commit 7783917

Please sign in to comment.