Skip to content

Commit 263f86e

Browse files
committed
Save navigator names by type to be able to use it later. (#1982)
There was a bug where two different navigators with the same base type and the name were saved in the provider. In that case we should save only the latest. ## Release Notes ### Fixes - Navigation - _(prerelease fix)_ Fix of the custom navigation animation in nested graphs in non-android targets.
1 parent 6c63971 commit 263f86e

File tree

2 files changed

+20
-14
lines changed

2 files changed

+20
-14
lines changed

navigation/navigation-common/src/commonTest/kotlin/androidx/navigation/NavigatorProviderTest.kt

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,23 @@ class NavigatorProviderTest {
8888
assertThat(provider.getNavigator<EmptyNavigator>(EmptyNavigator.NAME)).isEqualTo(navigatorB)
8989
}
9090

91+
@Test
92+
fun replaceNavigatorOfCommonTypeWhenGetByType() {
93+
val provider = NavigatorProvider()
94+
val navigatorA = EmptyNavigator()
95+
96+
class EmptyNavigator2 : EmptyNavigator()
97+
val navigatorB = EmptyNavigator2()
98+
99+
assertThat(navigatorA).isNotEqualTo(navigatorB)
100+
101+
provider.addNavigator(navigatorA)
102+
assertThat(provider[EmptyNavigator::class]).isEqualTo(navigatorA)
103+
104+
provider.addNavigator(navigatorB)
105+
assertThat(provider[EmptyNavigator::class]).isEqualTo(navigatorB)
106+
}
107+
91108
private val provider = NavigatorProvider()
92109

93110
@Test

navigation/navigation-common/src/nonAndroidMain/kotlin/androidx/navigation/NavigatorProvider.nonAndroid.kt

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import kotlin.jvm.JvmStatic
2727
import kotlin.reflect.KClass
2828

2929
public actual open class NavigatorProvider actual constructor() {
30-
private val _typeNavigators = mutableMapOf<KClass<*>, Navigator<out NavDestination>>()
30+
private val _typeToNavigatorName = mutableMapOf<KClass<*>, String>()
3131
private val _namedNavigators = mutableMapOf<String, Navigator<out NavDestination>>()
3232

3333
@get:RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
@@ -45,7 +45,7 @@ public actual open class NavigatorProvider actual constructor() {
4545
@Suppress("UNCHECKED_CAST")
4646
public fun <T : Navigator<*>> getNavigator(navigatorClass: KClass<T>): T {
4747
val navigator =
48-
_typeNavigators[navigatorClass]
48+
_typeToNavigatorName[navigatorClass]?.let { name -> _namedNavigators[name] }
4949
?: throw IllegalStateException(
5050
"Could not find Navigator with class \"$navigatorClass\". You must call " +
5151
"NavController.addNavigator() for each navigation type."
@@ -69,18 +69,6 @@ public actual open class NavigatorProvider actual constructor() {
6969
public actual fun addNavigator(
7070
navigator: Navigator<out NavDestination>
7171
): Navigator<out NavDestination>? {
72-
val previousNavigator = _typeNavigators[navigator::class]
73-
if (previousNavigator == navigator) {
74-
return navigator
75-
}
76-
check(previousNavigator?.isAttached != true) {
77-
"Navigator $navigator is replacing an already attached $previousNavigator"
78-
}
79-
check(!navigator.isAttached) {
80-
"Navigator $navigator is already attached to another NavController"
81-
}
82-
83-
_typeNavigators[navigator::class] = navigator
8472
return addNavigator(navigator.name, navigator)
8573
}
8674

@@ -90,6 +78,7 @@ public actual open class NavigatorProvider actual constructor() {
9078
navigator: Navigator<out NavDestination>
9179
): Navigator<out NavDestination>? {
9280
require(validateName(name)) { "Navigator name cannot be an empty string" }
81+
_typeToNavigatorName[navigator::class] = navigator.name
9382
val previousNavigator = _namedNavigators[name]
9483
if (previousNavigator == navigator) {
9584
return navigator

0 commit comments

Comments
 (0)