diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/MainActivity.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/MainActivity.kt index f5fd1f93c..cc03c0058 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/MainActivity.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/MainActivity.kt @@ -85,6 +85,8 @@ class MainActivity : ComponentActivity() { private val viewModel: MainActivityViewModel by viewModels() + + // TODO: This isn't used private val backStackViewModel: NiaBackStackViewModel by viewModels() override fun onCreate(savedInstanceState: Bundle?) { diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NiaNavigatorProvider.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NavigationStateProvider.kt similarity index 83% rename from app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NiaNavigatorProvider.kt rename to app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NavigationStateProvider.kt index aa0aa48f7..0bb3886c8 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NiaNavigatorProvider.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/di/NavigationStateProvider.kt @@ -17,7 +17,7 @@ package com.google.samples.apps.nowinandroid.di import com.google.samples.apps.nowinandroid.core.navigation.NiaNavKey -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigatorState +import com.google.samples.apps.nowinandroid.core.navigation.NavigationState import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination import dagger.Module import dagger.Provides @@ -28,15 +28,19 @@ import kotlinx.serialization.modules.SerializersModule import kotlinx.serialization.modules.polymorphic import javax.inject.Singleton +// TODO: Rename to `NiaNavigationStateProvider` +// Does this even need to be injected? Can't we just instantiate it directly using `rememberNavigationState`? @Module @InstallIn(SingletonComponent::class) -object NiaNavigatorProvider { +object NavigationStateProvider { @Provides @Singleton - fun providerNiaNavigatorState(): NiaNavigatorState = - NiaNavigatorState( + fun provideNavigationState(): NavigationState = + NavigationState( startKey = TopLevelDestination.FOR_YOU.key, ) + +// TODO: Remove commented out code // // @Provides // @Singleton diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/navigation/NiaNavDisplay.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/navigation/NiaNavDisplay.kt index de9e70a41..0d5a3b5e1 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/navigation/NiaNavDisplay.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/navigation/NiaNavDisplay.kt @@ -23,8 +23,7 @@ import androidx.navigation3.runtime.EntryProviderScope import androidx.navigation3.ui.NavDisplay import com.google.samples.apps.nowinandroid.core.navigation.NiaNavKey import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigator -import com.google.samples.apps.nowinandroid.core.navigation.getEntries -import kotlin.collections.plus +import com.google.samples.apps.nowinandroid.core.navigation.toEntries @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable @@ -33,7 +32,7 @@ fun NiaNavDisplay( entryProviderBuilders: Set.() -> Unit>, ) { val listDetailStrategy = rememberListDetailSceneStrategy() - val entries = niaNavigator.navigatorState.getEntries(entryProviderBuilders) + val entries = niaNavigator.navigationState.toEntries(entryProviderBuilders) NavDisplay( entries = entries, sceneStrategy = listDetailStrategy, diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaApp.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaApp.kt index 7949496d4..50af71980 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaApp.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaApp.kt @@ -241,6 +241,11 @@ internal fun NiaApp( }, ), ) { + + // Instantiate the NavigationState here + + + NiaNavDisplay( niaNavigator = appState.niaNavigator, entryProviderBuilders = entryProviderBuilders, diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt index ec4413304..c6595f0ac 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt @@ -71,7 +71,7 @@ class NiaAppState( timeZoneMonitor: TimeZoneMonitor, ) { val currentTopLevelDestination: TopLevelDestination? - @Composable get() = TopLevelDestinations[niaNavigator.navigatorState.currentTopLevelKey] + @Composable get() = TopLevelDestinations[niaNavigator.navigationState.currentTopLevelKey] val isOffline = networkMonitor.isOnline .map(Boolean::not) diff --git a/core/navigation/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModelTest.kt b/core/navigation/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModelTest.kt index e1e27be2c..1704a6151 100644 --- a/core/navigation/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModelTest.kt +++ b/core/navigation/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModelTest.kt @@ -48,7 +48,7 @@ class NiaBackStackViewModelTest { private fun createViewModel() = NiaBackStackViewModel( savedStateHandle = SavedStateHandle(), - niaNavigatorState = NiaNavigatorState(TestStartKey), + navigationState = NavigationState(TestStartKey), serializersModules = serializersModules, ) @@ -67,7 +67,7 @@ class NiaBackStackViewModelTest { fun testNonTopLevelKeySaved() { val viewModel = createViewModel() rule.setContent { - val navigator = remember { NiaNavigator(viewModel.niaNavigatorState) } + val navigator = remember { NiaNavigator(viewModel.navigationState) } navigator.navigate(TestKeyFirst) } assertThat(viewModel.backStackMap.size).isEqualTo(1) @@ -80,7 +80,7 @@ class NiaBackStackViewModelTest { fun testTopLevelKeySaved() { val viewModel = createViewModel() rule.setContent { - val navigator = remember { NiaNavigator(viewModel.niaNavigatorState) } + val navigator = remember { NiaNavigator(viewModel.navigationState) } navigator.navigate(TestKeyFirst) navigator.navigate(TestTopLevelKeyFirst) @@ -101,7 +101,7 @@ class NiaBackStackViewModelTest { fun testMultiStacksSaved() { val viewModel = createViewModel() rule.setContent { - val navigator = remember { NiaNavigator(viewModel.niaNavigatorState) } + val navigator = remember { NiaNavigator(viewModel.navigationState) } navigator.navigate(TestKeyFirst) navigator.navigate(TestTopLevelKeyFirst) navigator.navigate(TestKeySecond) @@ -122,7 +122,7 @@ class NiaBackStackViewModelTest { fun testPopSaved() { val viewModel = createViewModel() rule.setContent { - val navigator = remember { NiaNavigator(viewModel.niaNavigatorState) } + val navigator = remember { NiaNavigator(viewModel.navigationState) } navigator.navigate(TestKeyFirst) @@ -144,14 +144,14 @@ class NiaBackStackViewModelTest { fun testRestore() { lateinit var scenario: ViewModelScenario lateinit var navigator: NiaNavigator - lateinit var navigatorState: NiaNavigatorState + lateinit var navigatorState: NavigationState rule.setContent { - navigatorState = remember { NiaNavigatorState(TestStartKey) } + navigatorState = remember { NavigationState(TestStartKey) } navigator = remember { NiaNavigator(navigatorState) } scenario = viewModelScenario { NiaBackStackViewModel( savedStateHandle = createSavedStateHandle(), - niaNavigatorState = navigatorState, + navigationState = navigatorState, serializersModules = serializersModules, ) } @@ -181,14 +181,14 @@ class NiaBackStackViewModelTest { fun testRestoreMultiStacks() { lateinit var scenario: ViewModelScenario lateinit var navigator: NiaNavigator - lateinit var navigatorState: NiaNavigatorState + lateinit var navigatorState: NavigationState rule.setContent { - navigatorState = remember { NiaNavigatorState(TestStartKey) } + navigatorState = remember { NavigationState(TestStartKey) } navigator = remember { NiaNavigator(navigatorState) } scenario = viewModelScenario { NiaBackStackViewModel( savedStateHandle = createSavedStateHandle(), - niaNavigatorState = navigatorState, + navigationState = navigatorState, serializersModules = serializersModules, ) } diff --git a/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModel.kt b/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModel.kt index 3d61dc3ff..3c4b721c8 100644 --- a/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModel.kt +++ b/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaBackStackViewModel.kt @@ -33,10 +33,16 @@ import kotlinx.serialization.modules.SerializersModule import kotlinx.serialization.serializer import javax.inject.Inject +/** + * TODO: I'm not sure why this needs to be a ViewModel - why can't it be a plain state holder that + * is scoped to `NiaAppState`? + * https://github.com/android/nav3-recipes/blob/main/app/src/main/java/com/example/nav3recipes/multiplestacks/NavigationState.kt#L71 + * + */ @HiltViewModel class NiaBackStackViewModel @Inject constructor( savedStateHandle: SavedStateHandle, - val niaNavigatorState: NiaNavigatorState, + val navigationState: NavigationState, serializersModules: SerializersModule, ) : ViewModel() { @@ -63,9 +69,9 @@ class NiaBackStackViewModel @Inject constructor( init { if (backStackMap.isNotEmpty()) { - // Restore backstack from saved state handle if not emtpy + // Restore backstack from saved state handle if not empty @Suppress("UNCHECKED_CAST") - niaNavigatorState.restore( + navigationState.restore( activeTopLeveLKeys, backStackMap as LinkedHashMap>, ) @@ -74,8 +80,8 @@ class NiaBackStackViewModel @Inject constructor( // Start observing changes to the backStack and save backStack whenever it updates viewModelScope.launch { snapshotFlow { - activeTopLeveLKeys = niaNavigatorState.activeTopLeveLKeys.toList() - backStackMap = niaNavigatorState.backStacks + activeTopLeveLKeys = navigationState.activeTopLeveLKeys.toList() + backStackMap = navigationState.backStacks }.collect() } } diff --git a/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigator.kt b/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigator.kt index 8152d0e91..27f7bda13 100644 --- a/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigator.kt +++ b/core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigator.kt @@ -34,7 +34,8 @@ import org.jetbrains.annotations.VisibleForTesting import javax.inject.Inject import kotlin.collections.plus -class NiaNavigatorState( +// TODO: Consider changing this to `NiaNavigationState` +class NavigationState( internal val startKey: NiaNavKey, ) { internal var backStacks: MutableMap> = @@ -51,7 +52,7 @@ class NiaNavigatorState( val currentBackStack: List get() = activeTopLeveLKeys.fold(mutableListOf()) { list, topLevelKey -> list.apply { - addAll(backStacks[topLevelKey]!!) + addAll(backStacks[topLevelKey] ?: error("No back stack found for $topLevelKey")) } } @@ -76,13 +77,18 @@ class NiaNavigatorState( } } -// https://github.com/android/nowinandroid/issues/1934 +/** + * TODO: Document this + */ class NiaNavigator @Inject constructor( - val navigatorState: NiaNavigatorState, + val navigationState: NavigationState, ) { + // TODO: I wonder if it'd be simpler to have separate methods + // for navigating to a graph and navigating to a key. If the key is on a separate graph then + // navigate to that graph first. fun navigate(key: NiaNavKey) { val currentActiveSubStacks = linkedSetOf() - navigatorState.apply { + navigationState.apply { currentActiveSubStacks.addAll(activeTopLeveLKeys) when { // top level singleTop -> clear substack @@ -120,7 +126,7 @@ class NiaNavigator @Inject constructor( } fun pop() { - navigatorState.apply { + navigationState.apply { val currentSubstack = backStacks[currentTopLevelKey]!! if (currentSubstack.size == 1) { // if current sub-stack only has one key, remove the sub-stack from the map @@ -134,12 +140,17 @@ class NiaNavigator @Inject constructor( } } +// TODO: I wonder if removing this would remove the need for the serializers modules interface NiaNavKey { val isTopLevel: Boolean } +/** + * Convert the navigation state to `NavEntry`s that can be displayed by a `NavDisplay` + */ @Composable -public fun NiaNavigatorState.getEntries( +fun NavigationState.toEntries( + // TODO: Might be better to pass this in fully constructed entryProviderBuilders: Set.() -> Unit>, ): List> = activeTopLeveLKeys.fold(emptyList()) { entries, topLevelKey -> diff --git a/core/navigation/src/test/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigatorStateTest.kt b/core/navigation/src/test/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigatorStateTest.kt index eeef1b803..2bb4db7b8 100644 --- a/core/navigation/src/test/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigatorStateTest.kt +++ b/core/navigation/src/test/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigatorStateTest.kt @@ -24,49 +24,49 @@ import kotlin.test.assertFailsWith class NiaNavigatorStateTest { - private lateinit var niaNavigatorState: NiaNavigatorState + private lateinit var navigationState: NavigationState private lateinit var niaNavigator: NiaNavigator @Before fun setup() { - niaNavigatorState = NiaNavigatorState(TestStartKey) - niaNavigator = NiaNavigator(niaNavigatorState) + navigationState = NavigationState(TestStartKey) + niaNavigator = NiaNavigator(navigationState) } @Test fun testStartKey() { - assertThat(niaNavigatorState.currentKey).isEqualTo(TestStartKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test fun testNavigate() { niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test fun testNavigateTopLevel() { niaNavigator.navigate(TestTopLevelKey) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestTopLevelKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestTopLevelKey) } @Test fun testNavigateSingleTop() { niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, ).inOrder() niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, ).inOrder() @@ -77,7 +77,7 @@ class NiaNavigatorStateTest { niaNavigator.navigate(TestTopLevelKey) niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestTopLevelKey, TestKeyFirst, @@ -85,7 +85,7 @@ class NiaNavigatorStateTest { niaNavigator.navigate(TestTopLevelKey) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestTopLevelKey, ).inOrder() @@ -95,13 +95,13 @@ class NiaNavigatorStateTest { fun testSubStack() { niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) niaNavigator.navigate(TestKeySecond) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeySecond) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeySecond) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test @@ -109,33 +109,33 @@ class NiaNavigatorStateTest { // add to start stack niaNavigator.navigate(TestKeyFirst) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) // navigate to new top level niaNavigator.navigate(TestTopLevelKey) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestTopLevelKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestTopLevelKey) // add to new stack niaNavigator.navigate(TestKeySecond) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeySecond) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeySecond) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestTopLevelKey) // go back to start stack niaNavigator.navigate(TestStartKey) - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test fun testRestore() { - assertThat(niaNavigatorState.currentBackStack).containsExactly(TestStartKey) + assertThat(navigationState.currentBackStack).containsExactly(TestStartKey) - niaNavigatorState.restore( + navigationState.restore( listOf(TestStartKey, TestTopLevelKey), linkedMapOf( TestStartKey to mutableStateListOf(TestStartKey, TestKeyFirst), @@ -143,15 +143,15 @@ class NiaNavigatorStateTest { ), ) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, TestTopLevelKey, TestKeySecond, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeySecond) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeySecond) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestTopLevelKey) } @Test @@ -159,7 +159,7 @@ class NiaNavigatorStateTest { niaNavigator.navigate(TestKeyFirst) niaNavigator.navigate(TestKeySecond) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, TestKeySecond, @@ -167,13 +167,13 @@ class NiaNavigatorStateTest { niaNavigator.pop() - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test @@ -181,25 +181,25 @@ class NiaNavigatorStateTest { niaNavigator.navigate(TestKeyFirst) niaNavigator.navigate(TestTopLevelKey) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, TestTopLevelKey, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestTopLevelKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentKey).isEqualTo(TestTopLevelKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestTopLevelKey) // remove TopLevel niaNavigator.pop() - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestKeyFirst) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test @@ -207,7 +207,7 @@ class NiaNavigatorStateTest { niaNavigator.navigate(TestKeyFirst) niaNavigator.navigate(TestKeySecond) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestKeyFirst, TestKeySecond, @@ -216,12 +216,12 @@ class NiaNavigatorStateTest { niaNavigator.pop() niaNavigator.pop() - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestStartKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test @@ -238,7 +238,7 @@ class NiaNavigatorStateTest { niaNavigator.navigate(testTopLevelKeyTwo) niaNavigator.navigate(TestKeySecond) - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, TestTopLevelKey, TestKeyFirst, @@ -250,12 +250,12 @@ class NiaNavigatorStateTest { niaNavigator.pop() } - assertThat(niaNavigatorState.currentBackStack).containsExactly( + assertThat(navigationState.currentBackStack).containsExactly( TestStartKey, ).inOrder() - assertThat(niaNavigatorState.currentKey).isEqualTo(TestStartKey) - assertThat(niaNavigatorState.currentTopLevelKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentKey).isEqualTo(TestStartKey) + assertThat(navigationState.currentTopLevelKey).isEqualTo(TestStartKey) } @Test diff --git a/feature/foryou/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/impl/navigation/ForYouEntryProvider.kt b/feature/foryou/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/impl/navigation/ForYouEntryProvider.kt index 7e942f261..f4fd9ab81 100644 --- a/feature/foryou/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/impl/navigation/ForYouEntryProvider.kt +++ b/feature/foryou/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/impl/navigation/ForYouEntryProvider.kt @@ -39,11 +39,16 @@ object ForYouEntryProvider { @IntoSet fun provideForYouEntryProviderBuilder( navigator: NiaNavigator, - ): EntryProviderScope.() -> Unit = { - entry { - ForYouScreen( - onTopicClick = navigator::navigateToTopic, - ) - } + ): EntryProviderScope.() -> Unit = forYouEntry(navigator) +} + +fun forYouEntry(navigator: NiaNavigator): EntryProviderScope.() -> Unit = { + entry { + ForYouScreen( + onTopicClick = navigator::navigateToTopic, + ) } } + + + diff --git a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsScreen.kt b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsScreen.kt index 000c1f429..689b5bbf0 100644 --- a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsScreen.kt +++ b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsScreen.kt @@ -47,6 +47,7 @@ fun InterestsScreen( uiState = uiState, followTopic = viewModel::followTopic, onTopicClick = { + // TODO: this violates SSOT, events should go through the ViewModel viewModel.onTopicClick(it) onTopicClick(it) }, diff --git a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsViewModel.kt b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsViewModel.kt index 8f30fbe95..8f203d6fb 100644 --- a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsViewModel.kt +++ b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/InterestsViewModel.kt @@ -39,9 +39,12 @@ class InterestsViewModel @AssistedInject constructor( private val savedStateHandle: SavedStateHandle, val userDataRepository: UserDataRepository, getFollowableTopics: GetFollowableTopicsUseCase, + // TODO: see comment below @Assisted val key: InterestsRoute, ) : ViewModel() { + // TODO: this should no longer be necessary, the currently selected topic should be + // available through the navigation state // Key used to save and retrieve the currently selected topic id from saved state. private val selectedTopicIdKey = "selectedTopicIdKey" @@ -67,6 +70,8 @@ class InterestsViewModel @AssistedInject constructor( } fun onTopicClick(topicId: String?) { + // TODO: This should modify the navigation state directly rather than just updating the + // savedStateHandle savedStateHandle[selectedTopicIdKey] = topicId } diff --git a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/navigation/InterestsEntryProvider.kt b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/navigation/InterestsEntryProvider.kt index ba8691473..60eb4c4ce 100644 --- a/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/navigation/InterestsEntryProvider.kt +++ b/feature/interests/impl/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/interests/impl/navigation/InterestsEntryProvider.kt @@ -52,7 +52,11 @@ object InterestsEntryProvider { it.create(key) } InterestsScreen( + // TODO: This event should be provided by the ViewModel onTopicClick = navigator::navigateToTopic, + + // TODO: This should be dynamically calculated based on the rendering scene + // See https://github.com/android/nav3-recipes/commit/488f4811791ca3ed7192f4fe3c86e7371b32ebdc#diff-374e02026cdd2f68057dd940f203dc4ba7319930b33e9555c61af7e072211cabR89 shouldHighlightSelectedTopic = false, viewModel = viewModel, ) diff --git a/feature/interests/impl/src/test/kotlin/com/google/samples/apps/nowinandroid/interests/impl/InterestsListDetailScreenTest.kt b/feature/interests/impl/src/test/kotlin/com/google/samples/apps/nowinandroid/interests/impl/InterestsListDetailScreenTest.kt index 0f251322f..da3abaa68 100644 --- a/feature/interests/impl/src/test/kotlin/com/google/samples/apps/nowinandroid/interests/impl/InterestsListDetailScreenTest.kt +++ b/feature/interests/impl/src/test/kotlin/com/google/samples/apps/nowinandroid/interests/impl/InterestsListDetailScreenTest.kt @@ -162,6 +162,7 @@ class InterestsListDetailScreenTest { composeTestRule.apply { setContent { val backStackViewModel by composeTestRule.activity.viewModels() + // TODO: This is broken val backStack = backStackViewModel.niaNavigator.backStack NiaTheme { NavDisplay(