From a0b22d8ed289869d51073518fc2cec0ea8239f67 Mon Sep 17 00:00:00 2001 From: Manuel Vivo Date: Thu, 20 Oct 2022 16:16:24 -0700 Subject: [PATCH] Updates Navigation approach with new guidance (#347) --- app/build.gradle.kts | 5 +- .../apps/nowinandroid/ui/NiaAppStateTest.kt | 6 +- .../nowinandroid/navigation/NiaNavHost.kt | 38 ++++---- .../navigation/TopLevelDestination.kt | 29 +++++- .../samples/apps/nowinandroid/ui/NiaApp.kt | 21 +++-- .../apps/nowinandroid/ui/NiaAppState.kt | 91 +++++++------------ .../kotlin/AndroidFeatureConventionPlugin.kt | 3 +- .../core/decoder/StringDecoder.kt} | 18 +--- .../nowinandroid/core/decoder/UriDecoder.kt | 24 +++++ .../core/decoder/di/StringDecoderModule.kt | 31 +++++++ core/navigation/.gitignore | 1 - core/navigation/README.md | 3 - core/navigation/src/main/AndroidManifest.xml | 19 ---- .../navigation/NiaNavigationDestination.kt | 38 -------- .../core/testing/decoder/FakeStringDecoder.kt | 24 +++++ .../testing/di/TestStringDecoderModule.kt | 35 +++++++ docs/ModularizationLearningJourney.md | 8 -- .../feature/author/AuthorScreen.kt | 2 +- .../feature/author/AuthorViewModel.kt | 14 +-- .../author/navigation/AuthorNavigation.kt | 39 ++++---- .../feature/author/AuthorViewModelTest.kt | 6 +- .../feature/bookmarks/BookmarksScreen.kt | 2 +- .../navigation/BookmarksNavigation.kt | 14 +-- .../feature/foryou/ForYouScreen.kt | 4 +- .../foryou/navigation/ForYouNavigation.kt | 14 +-- .../feature/interests/InterestsScreen.kt | 4 +- .../navigation/InterestsNavigation.kt | 18 ++-- .../nowinandroid/feature/topic/TopicScreen.kt | 2 +- .../feature/topic/TopicViewModel.kt | 12 ++- .../topic/navigation/TopicNavigation.kt | 39 ++++---- .../feature/topic/TopicViewModelTest.kt | 7 +- settings.gradle.kts | 3 +- 32 files changed, 296 insertions(+), 278 deletions(-) rename core/{navigation/build.gradle.kts => common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt} (59%) create mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt create mode 100644 core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt delete mode 100644 core/navigation/.gitignore delete mode 100644 core/navigation/README.md delete mode 100644 core/navigation/src/main/AndroidManifest.xml delete mode 100644 core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt create mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt create mode 100644 core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 10bfe9c22..fe1f1cb64 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -85,7 +85,6 @@ dependencies { implementation(project(":core:ui")) implementation(project(":core:designsystem")) - implementation(project(":core:navigation")) implementation(project(":sync:work")) implementation(project(":sync:sync-test")) @@ -104,6 +103,8 @@ dependencies { implementation(libs.androidx.compose.runtime) implementation(libs.androidx.compose.runtime.tracing) implementation(libs.androidx.compose.material3.windowSizeClass) + implementation(libs.androidx.hilt.navigation.compose) + implementation(libs.androidx.navigation.compose) implementation(libs.androidx.window.manager) implementation(libs.androidx.profileinstaller) @@ -118,4 +119,4 @@ configurations.configureEach { // Temporary workaround for https://issuetracker.google.com/174733673 force("org.objenesis:objenesis:2.6") } -} \ No newline at end of file +} diff --git a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt index 2814fdbc9..c2e17700f 100644 --- a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NiaAppStateTest.kt @@ -82,9 +82,9 @@ class NiaAppStateTest { } assertEquals(3, state.topLevelDestinations.size) - assertTrue(state.topLevelDestinations[0].destination.contains("for_you")) - assertTrue(state.topLevelDestinations[1].destination.contains("bookmarks")) - assertTrue(state.topLevelDestinations[2].destination.contains("interests")) + assertTrue(state.topLevelDestinations[0].name.contains("for_you", true)) + assertTrue(state.topLevelDestinations[1].name.contains("bookmarks", true)) + assertTrue(state.topLevelDestinations[2].name.contains("interests", true)) } @Test diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt index 6d2206743..10fafddd9 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/NiaNavHost.kt @@ -20,15 +20,14 @@ import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.navigation.NavHostController import androidx.navigation.compose.NavHost -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination -import com.google.samples.apps.nowinandroid.feature.author.navigation.authorGraph -import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksGraph -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouGraph +import com.google.samples.apps.nowinandroid.feature.author.navigation.authorScreen +import com.google.samples.apps.nowinandroid.feature.author.navigation.navigateToAuthor +import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksScreen +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouNavigationRoute +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.forYouScreen import com.google.samples.apps.nowinandroid.feature.interests.navigation.interestsGraph -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination -import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicGraph +import com.google.samples.apps.nowinandroid.feature.topic.navigation.navigateToTopic +import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicScreen /** * Top-level navigation graph. Navigation is organized as explained at @@ -40,32 +39,27 @@ import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicGraph @Composable fun NiaNavHost( navController: NavHostController, - onNavigateToDestination: (NiaNavigationDestination, String) -> Unit, onBackClick: () -> Unit, modifier: Modifier = Modifier, - startDestination: String = ForYouDestination.route + startDestination: String = forYouNavigationRoute ) { NavHost( navController = navController, startDestination = startDestination, modifier = modifier, ) { - forYouGraph() - bookmarksGraph() + forYouScreen() + bookmarksScreen() interestsGraph( - navigateToTopic = { - onNavigateToDestination( - TopicDestination, TopicDestination.createNavigationRoute(it) - ) + navigateToTopic = { topicId -> + navController.navigateToTopic(topicId) }, - navigateToAuthor = { - onNavigateToDestination( - AuthorDestination, AuthorDestination.createNavigationRoute(it) - ) + navigateToAuthor = { authorId -> + navController.navigateToAuthor(authorId) }, nestedGraphs = { - topicGraph(onBackClick) - authorGraph(onBackClick) + topicScreen(onBackClick) + authorScreen(onBackClick) } ) } diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt index 4a2523bb5..566997920 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/navigation/TopLevelDestination.kt @@ -17,17 +17,36 @@ package com.google.samples.apps.nowinandroid.navigation import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon +import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon +import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons +import com.google.samples.apps.nowinandroid.feature.bookmarks.R as bookmarksR +import com.google.samples.apps.nowinandroid.feature.foryou.R as forYouR +import com.google.samples.apps.nowinandroid.feature.interests.R as interestsR /** * Type for the top level destinations in the application. Each of these destinations * can contain one or more screens (based on the window size). Navigation from one screen to the * next within a single destination will be handled directly in composables. */ -data class TopLevelDestination( - override val route: String, - override val destination: String, +enum class TopLevelDestination( val selectedIcon: Icon, val unselectedIcon: Icon, val iconTextId: Int -) : NiaNavigationDestination +) { + FOR_YOU( + selectedIcon = DrawableResourceIcon(NiaIcons.Upcoming), + unselectedIcon = DrawableResourceIcon(NiaIcons.UpcomingBorder), + iconTextId = forYouR.string.for_you + ), + BOOKMARKS( + selectedIcon = DrawableResourceIcon(NiaIcons.Bookmarks), + unselectedIcon = DrawableResourceIcon(NiaIcons.BookmarksBorder), + iconTextId = bookmarksR.string.saved + ), + INTERESTS( + selectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), + unselectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), + iconTextId = interestsR.string.interests + ) +} diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt index 1d5ec6e98..213374100 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt @@ -52,7 +52,6 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaNavig import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination import com.google.samples.apps.nowinandroid.navigation.NiaNavHost import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination @@ -69,7 +68,9 @@ fun NiaApp( NiaTheme { val background: @Composable (@Composable () -> Unit) -> Unit = when (appState.currentDestination?.route) { - ForYouDestination.route -> { content -> NiaGradientBackground(content = content) } + TopLevelDestination.FOR_YOU.name -> { content -> + NiaGradientBackground(content = content) + } else -> { content -> NiaBackground(content = content) } } @@ -85,7 +86,7 @@ fun NiaApp( if (appState.shouldShowBottomBar) { NiaBottomBar( destinations = appState.topLevelDestinations, - onNavigateToDestination = appState::navigate, + onNavigateToDestination = appState::navigateToTopLevelDestination, currentDestination = appState.currentDestination ) } @@ -103,7 +104,7 @@ fun NiaApp( if (appState.shouldShowNavRail) { NiaNavRail( destinations = appState.topLevelDestinations, - onNavigateToDestination = appState::navigate, + onNavigateToDestination = appState::navigateToTopLevelDestination, currentDestination = appState.currentDestination, modifier = Modifier.safeDrawingPadding() ) @@ -112,7 +113,6 @@ fun NiaApp( NiaNavHost( navController = appState.navController, onBackClick = appState::onBackClick, - onNavigateToDestination = appState::navigate, modifier = Modifier .padding(padding) .consumedWindowInsets(padding) @@ -132,8 +132,7 @@ private fun NiaNavRail( ) { NiaNavigationRail(modifier = modifier) { destinations.forEach { destination -> - val selected = - currentDestination?.hierarchy?.any { it.route == destination.route } == true + val selected = currentDestination.isTopLevelDestinationInHierarchy(destination) NiaNavigationRailItem( selected = selected, onClick = { onNavigateToDestination(destination) }, @@ -168,8 +167,7 @@ private fun NiaBottomBar( ) { NiaNavigationBar { destinations.forEach { destination -> - val selected = - currentDestination?.hierarchy?.any { it.route == destination.route } == true + val selected = currentDestination.isTopLevelDestinationInHierarchy(destination) NiaNavigationBarItem( selected = selected, onClick = { onNavigateToDestination(destination) }, @@ -195,3 +193,8 @@ private fun NiaBottomBar( } } } + +private fun NavDestination?.isTopLevelDestinationInHierarchy(destination: TopLevelDestination) = + this?.hierarchy?.any { + it.route?.contains(destination.name, true) ?: false + } ?: false diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt index 127c3e206..04e2f395f 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaAppState.kt @@ -28,19 +28,16 @@ import androidx.navigation.NavGraph.Companion.findStartDestination import androidx.navigation.NavHostController import androidx.navigation.compose.currentBackStackEntryAsState import androidx.navigation.compose.rememberNavController +import androidx.navigation.navOptions import androidx.tracing.trace -import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.DrawableResourceIcon -import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVectorIcon -import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank -import com.google.samples.apps.nowinandroid.feature.bookmarks.R as bookmarksR -import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.BookmarksDestination -import com.google.samples.apps.nowinandroid.feature.foryou.R as forYouR -import com.google.samples.apps.nowinandroid.feature.foryou.navigation.ForYouDestination -import com.google.samples.apps.nowinandroid.feature.interests.R as interestsR -import com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsDestination +import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.navigateToBookmarks +import com.google.samples.apps.nowinandroid.feature.foryou.navigation.navigateToForYou +import com.google.samples.apps.nowinandroid.feature.interests.navigation.navigateToInterestsGraph import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.BOOKMARKS +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.FOR_YOU +import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.INTERESTS @Composable fun rememberNiaAppState( @@ -72,61 +69,35 @@ class NiaAppState( /** * Top level destinations to be used in the BottomBar and NavRail */ - val topLevelDestinations: List = listOf( - TopLevelDestination( - route = ForYouDestination.route, - destination = ForYouDestination.destination, - selectedIcon = DrawableResourceIcon(NiaIcons.Upcoming), - unselectedIcon = DrawableResourceIcon(NiaIcons.UpcomingBorder), - iconTextId = forYouR.string.for_you - ), - TopLevelDestination( - route = BookmarksDestination.route, - destination = BookmarksDestination.destination, - selectedIcon = DrawableResourceIcon(NiaIcons.Bookmarks), - unselectedIcon = DrawableResourceIcon(NiaIcons.BookmarksBorder), - iconTextId = bookmarksR.string.saved - ), - TopLevelDestination( - route = InterestsDestination.route, - destination = InterestsDestination.destination, - selectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), - unselectedIcon = ImageVectorIcon(NiaIcons.Grid3x3), - iconTextId = interestsR.string.interests - ) - ) + val topLevelDestinations: List = TopLevelDestination.values().asList() /** - * UI logic for navigating to a particular destination in the app. The NavigationOptions to - * navigate with are based on the type of destination, which could be a top level destination or - * just a regular destination. + * UI logic for navigating to a top level destination in the app. Top level destinations have + * only one copy of the destination of the back stack, and save and restore state whenever you + * navigate to and from it. * - * Top level destinations have only one copy of the destination of the back stack, and save and - * restore state whenever you navigate to and from it. - * Regular destinations can have multiple copies in the back stack and state isn't saved nor - * restored. - * - * @param destination: The [NiaNavigationDestination] the app needs to navigate to. - * @param route: Optional route to navigate to in case the destination contains arguments. + * @param topLevelDestination: The destination the app needs to navigate to. */ - fun navigate(destination: NiaNavigationDestination, route: String? = null) { - trace("Navigation: ${destination.route}") { - if (destination is TopLevelDestination) { - navController.navigate(route ?: destination.route) { - // Pop up to the start destination of the graph to - // avoid building up a large stack of destinations - // on the back stack as users select items - popUpTo(navController.graph.findStartDestination().id) { - saveState = true - } - // Avoid multiple copies of the same destination when - // reselecting the same item - launchSingleTop = true - // Restore state when reselecting a previously selected item - restoreState = true + fun navigateToTopLevelDestination(topLevelDestination: TopLevelDestination) { + trace("Navigation: ${topLevelDestination.name}") { + val topLevelNavOptions = navOptions { + // Pop up to the start destination of the graph to + // avoid building up a large stack of destinations + // on the back stack as users select items + popUpTo(navController.graph.findStartDestination().id) { + saveState = true } - } else { - navController.navigate(route ?: destination.route) + // Avoid multiple copies of the same destination when + // reselecting the same item + launchSingleTop = true + // Restore state when reselecting a previously selected item + restoreState = true + } + + when (topLevelDestination) { + FOR_YOU -> navController.navigateToForYou(topLevelNavOptions) + BOOKMARKS -> navController.navigateToBookmarks(topLevelNavOptions) + INTERESTS -> navController.navigateToInterestsGraph(topLevelNavOptions) } } } diff --git a/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt b/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt index e6ad9c031..7dd9b8972 100644 --- a/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt +++ b/build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt @@ -44,7 +44,6 @@ class AndroidFeatureConventionPlugin : Plugin { add("implementation", project(":core:designsystem")) add("implementation", project(":core:data")) add("implementation", project(":core:common")) - add("implementation", project(":core:navigation")) add("implementation", project(":core:domain")) add("testImplementation", project(":core:testing")) @@ -68,4 +67,4 @@ class AndroidFeatureConventionPlugin : Plugin { } } } -} \ No newline at end of file +} diff --git a/core/navigation/build.gradle.kts b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt similarity index 59% rename from core/navigation/build.gradle.kts rename to core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt index f762d5b7d..29437cc71 100644 --- a/core/navigation/build.gradle.kts +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/StringDecoder.kt @@ -13,19 +13,9 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -// TODO: Remove once https://youtrack.jetbrains.com/issue/KTIJ-19369 is fixed -@Suppress("DSL_SCOPE_VIOLATION") -plugins { - id("nowinandroid.android.library") - id("nowinandroid.android.library.jacoco") - id("nowinandroid.android.hilt") -} -android { - namespace = "com.google.samples.apps.nowinandroid.core.navigation" -} +package com.google.samples.apps.nowinandroid.core.decoder -dependencies { - api(libs.androidx.hilt.navigation.compose) - api(libs.androidx.navigation.compose) -} \ No newline at end of file +interface StringDecoder { + fun decodeString(encodedString: String): String +} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt new file mode 100644 index 000000000..b114bdab6 --- /dev/null +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/UriDecoder.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.decoder + +import android.net.Uri +import javax.inject.Inject + +class UriDecoder @Inject constructor() : StringDecoder { + override fun decodeString(encodedString: String): String = Uri.decode(encodedString) +} diff --git a/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt new file mode 100644 index 000000000..e39b0e1f8 --- /dev/null +++ b/core/common/src/main/java/com/google/samples/apps/nowinandroid/core/decoder/di/StringDecoderModule.kt @@ -0,0 +1,31 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.decoder.di + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import com.google.samples.apps.nowinandroid.core.decoder.UriDecoder +import dagger.Binds +import dagger.Module +import dagger.hilt.InstallIn +import dagger.hilt.components.SingletonComponent + +@Module +@InstallIn(SingletonComponent::class) +abstract class StringDecoderModule { + @Binds + abstract fun bindStringDecoder(uriDecoder: UriDecoder): StringDecoder +} diff --git a/core/navigation/.gitignore b/core/navigation/.gitignore deleted file mode 100644 index 42afabfd2..000000000 --- a/core/navigation/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/build \ No newline at end of file diff --git a/core/navigation/README.md b/core/navigation/README.md deleted file mode 100644 index a61f10aee..000000000 --- a/core/navigation/README.md +++ /dev/null @@ -1,3 +0,0 @@ -# :core:navigation module - -![Dependency graph](../../docs/images/graphs/dep_graph_core_navigation.png) diff --git a/core/navigation/src/main/AndroidManifest.xml b/core/navigation/src/main/AndroidManifest.xml deleted file mode 100644 index ec921f928..000000000 --- a/core/navigation/src/main/AndroidManifest.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - \ No newline at end of file diff --git a/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt b/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt deleted file mode 100644 index af3303f18..000000000 --- a/core/navigation/src/main/java/com/google/samples/apps/nowinandroid/core/navigation/NiaNavigationDestination.kt +++ /dev/null @@ -1,38 +0,0 @@ -/* - * Copyright 2022 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.google.samples.apps.nowinandroid.core.navigation - -/** - * Interface for describing the Now in Android navigation destinations - */ - -interface NiaNavigationDestination { - /** - * Defines a specific route this destination belongs to. - * Route is a String that defines the path to your composable. - * You can think of it as an implicit deep link that leads to a specific destination. - * Each destination should have a unique route. - */ - val route: String - - /** - * Defines a specific destination ID. - * This is needed when using nested graphs via the navigation DLS, to differentiate a specific - * destination's route from the route of the entire nested graph it belongs to. - */ - val destination: String -} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt new file mode 100644 index 000000000..7282661cc --- /dev/null +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/decoder/FakeStringDecoder.kt @@ -0,0 +1,24 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.testing.decoder + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import javax.inject.Inject + +class FakeStringDecoder @Inject constructor() : StringDecoder { + override fun decodeString(encodedString: String): String = encodedString +} diff --git a/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt new file mode 100644 index 000000000..0873ee546 --- /dev/null +++ b/core/testing/src/main/java/com/google/samples/apps/nowinandroid/core/testing/di/TestStringDecoderModule.kt @@ -0,0 +1,35 @@ +/* + * Copyright 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.samples.apps.nowinandroid.core.testing.di + +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder +import com.google.samples.apps.nowinandroid.core.decoder.di.StringDecoderModule +import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder +import dagger.Binds +import dagger.Module +import dagger.hilt.components.SingletonComponent +import dagger.hilt.testing.TestInstallIn + +@Module +@TestInstallIn( + components = [SingletonComponent::class], + replaces = [StringDecoderModule::class], +) +abstract class TestStringDecoderModule { + @Binds + abstract fun bindsStringDecoder(fakeStringDecoder: FakeStringDecoder): StringDecoder +} diff --git a/docs/ModularizationLearningJourney.md b/docs/ModularizationLearningJourney.md index 56461ce73..fbb0ac791 100644 --- a/docs/ModularizationLearningJourney.md +++ b/docs/ModularizationLearningJourney.md @@ -224,14 +224,6 @@ Using the above modularization strategy, the Now in Android app has the followin NewsResource - - core:navigation - - Navigation dependencies and shared navigation classes. - - NiaNavigationDestination - - diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt index ecebb2ac5..114b4c438 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt @@ -65,7 +65,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun AuthorRoute( +internal fun AuthorRoute( onBackClick: () -> Unit, modifier: Modifier = Modifier, viewModel: AuthorViewModel = hiltViewModel(), diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt index 137285193..2f63dbc11 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt @@ -21,13 +21,14 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.google.samples.apps.nowinandroid.core.data.repository.AuthorsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResourcesStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.domain.model.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Author import com.google.samples.apps.nowinandroid.core.result.Result import com.google.samples.apps.nowinandroid.core.result.asResult -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination +import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorArgs import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -41,17 +42,16 @@ import kotlinx.coroutines.launch @HiltViewModel class AuthorViewModel @Inject constructor( savedStateHandle: SavedStateHandle, + stringDecoder: StringDecoder, private val userDataRepository: UserDataRepository, authorsRepository: AuthorsRepository, getSaveableNewsResourcesStream: GetSaveableNewsResourcesStreamUseCase ) : ViewModel() { - private val authorId: String = checkNotNull( - savedStateHandle[AuthorDestination.authorIdArg] - ) + private val authorArgs: AuthorArgs = AuthorArgs(savedStateHandle, stringDecoder) val authorUiState: StateFlow = authorUiStateStream( - authorId = authorId, + authorId = authorArgs.authorId, userDataRepository = userDataRepository, authorsRepository = authorsRepository ) @@ -62,7 +62,7 @@ class AuthorViewModel @Inject constructor( ) val newsUiState: StateFlow = - getSaveableNewsResourcesStream.newsUiStateStream(authorId = authorId) + getSaveableNewsResourcesStream.newsUiStateStream(authorId = authorArgs.authorId) .stateIn( scope = viewModelScope, started = SharingStarted.WhileSubscribed(5_000), @@ -71,7 +71,7 @@ class AuthorViewModel @Inject constructor( fun followAuthorToggle(followed: Boolean) { viewModelScope.launch { - userDataRepository.toggleFollowedAuthorId(authorId, followed) + userDataRepository.toggleFollowedAuthorId(authorArgs.authorId, followed) } } diff --git a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt index ee37bab95..59b68ce91 100644 --- a/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt +++ b/feature/author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/navigation/AuthorNavigation.kt @@ -17,43 +17,36 @@ package com.google.samples.apps.nowinandroid.feature.author.navigation import android.net.Uri -import androidx.navigation.NavBackStackEntry +import androidx.annotation.VisibleForTesting +import androidx.lifecycle.SavedStateHandle +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder import androidx.navigation.NavType import androidx.navigation.compose.composable import androidx.navigation.navArgument -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.feature.author.AuthorRoute -object AuthorDestination : NiaNavigationDestination { - const val authorIdArg = "authorId" - override val route = "author_route/{$authorIdArg}" - override val destination = "author_destination" +@VisibleForTesting +internal const val authorIdArg = "authorId" - /** - * Creates destination route for an authorId that could include special characters - */ - fun createNavigationRoute(authorIdArg: String): String { - val encodedId = Uri.encode(authorIdArg) - return "author_route/$encodedId" - } +internal class AuthorArgs(val authorId: String) { + constructor(savedStateHandle: SavedStateHandle, stringDecoder: StringDecoder) : + this(stringDecoder.decodeString(checkNotNull(savedStateHandle[authorIdArg]))) +} - /** - * Returns the authorId from a [NavBackStackEntry] after an author destination navigation call - */ - fun fromNavArgs(entry: NavBackStackEntry): String { - val encodedId = entry.arguments?.getString(authorIdArg)!! - return Uri.decode(encodedId) - } +fun NavController.navigateToAuthor(authorId: String) { + val encodedString = Uri.encode(authorId) + this.navigate("author_route/$encodedString") } -fun NavGraphBuilder.authorGraph( +fun NavGraphBuilder.authorScreen( onBackClick: () -> Unit ) { composable( - route = AuthorDestination.route, + route = "author_route/{$authorIdArg}", arguments = listOf( - navArgument(AuthorDestination.authorIdArg) { type = NavType.StringType } + navArgument(authorIdArg) { type = NavType.StringType } ) ) { AuthorRoute(onBackClick = onBackClick) diff --git a/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt b/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt index cb6c17076..aac9617a4 100644 --- a/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt +++ b/feature/author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt @@ -22,11 +22,12 @@ import com.google.samples.apps.nowinandroid.core.domain.model.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.Author import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video +import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder import com.google.samples.apps.nowinandroid.core.testing.repository.TestAuthorsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule -import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination +import com.google.samples.apps.nowinandroid.feature.author.navigation.authorIdArg import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first @@ -63,9 +64,10 @@ class AuthorViewModelTest { viewModel = AuthorViewModel( savedStateHandle = SavedStateHandle( mapOf( - AuthorDestination.authorIdArg to testInputAuthors[0].author.id + authorIdArg to testInputAuthors[0].author.id ) ), + stringDecoder = FakeStringDecoder(), userDataRepository = userDataRepository, authorsRepository = authorsRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStreamUseCase diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt index dba490a73..849291df3 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt @@ -54,7 +54,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun BookmarksRoute( +internal fun BookmarksRoute( modifier: Modifier = Modifier, viewModel: BookmarksViewModel = hiltViewModel() ) { diff --git a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt index bf64eb34c..0d530019d 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt +++ b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt @@ -16,18 +16,20 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.bookmarks.BookmarksRoute -object BookmarksDestination : NiaNavigationDestination { - override val route = "bookmarks_route" - override val destination = "bookmarks_destination" +private const val bookmarksRoute = "bookmarks_route" + +fun NavController.navigateToBookmarks(navOptions: NavOptions? = null) { + this.navigate(bookmarksRoute, navOptions) } -fun NavGraphBuilder.bookmarksGraph() { - composable(route = BookmarksDestination.route) { +fun NavGraphBuilder.bookmarksScreen() { + composable(route = bookmarksRoute) { BookmarksRoute() } } diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index 24de3548f..252afb1ce 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -100,7 +100,7 @@ import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun ForYouRoute( +internal fun ForYouRoute( modifier: Modifier = Modifier, viewModel: ForYouViewModel = hiltViewModel() ) { @@ -124,7 +124,7 @@ fun ForYouRoute( @OptIn(ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class) @Composable -fun ForYouScreen( +internal fun ForYouScreen( isOffline: Boolean, isSyncing: Boolean, interestsSelectionState: ForYouInterestsSelectionUiState, diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt index 2c0dccccb..f57deab90 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt @@ -16,18 +16,20 @@ package com.google.samples.apps.nowinandroid.feature.foryou.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.foryou.ForYouRoute -object ForYouDestination : NiaNavigationDestination { - override val route = "for_you_route" - override val destination = "for_you_destination" +const val forYouNavigationRoute = "for_you_route" + +fun NavController.navigateToForYou(navOptions: NavOptions? = null) { + this.navigate(forYouNavigationRoute, navOptions) } -fun NavGraphBuilder.forYouGraph() { - composable(route = ForYouDestination.route) { +fun NavGraphBuilder.forYouScreen() { + composable(route = forYouNavigationRoute) { ForYouRoute() } } diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt index 843a45e49..d92579723 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt @@ -45,7 +45,7 @@ import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun InterestsRoute( +internal fun InterestsRoute( navigateToAuthor: (String) -> Unit, navigateToTopic: (String) -> Unit, modifier: Modifier = Modifier, @@ -76,7 +76,7 @@ fun InterestsRoute( @OptIn(ExperimentalMaterial3Api::class) @Composable -fun InterestsScreen( +internal fun InterestsScreen( uiState: InterestsUiState, tabState: InterestsTabState, followAuthor: (String, Boolean) -> Unit, diff --git a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt index 87612beeb..19e04b8b5 100644 --- a/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt +++ b/feature/interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/navigation/InterestsNavigation.kt @@ -16,28 +16,30 @@ package com.google.samples.apps.nowinandroid.feature.interests.navigation +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder +import androidx.navigation.NavOptions import androidx.navigation.compose.composable import androidx.navigation.navigation -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination import com.google.samples.apps.nowinandroid.feature.interests.InterestsRoute -object InterestsDestination : NiaNavigationDestination { - override val route = "interests_route" - override val destination = "interests_destination" +private const val interestsGraphRoutePattern = "interests_graph" +private const val interestsRoute = "interests_route" + +fun NavController.navigateToInterestsGraph(navOptions: NavOptions? = null) { + this.navigate(interestsGraphRoutePattern, navOptions) } fun NavGraphBuilder.interestsGraph( navigateToTopic: (String) -> Unit, navigateToAuthor: (String) -> Unit, nestedGraphs: NavGraphBuilder.() -> Unit - ) { navigation( - route = InterestsDestination.route, - startDestination = InterestsDestination.destination + route = interestsGraphRoutePattern, + startDestination = interestsRoute ) { - composable(route = InterestsDestination.destination) { + composable(route = interestsRoute) { InterestsRoute( navigateToTopic = navigateToTopic, navigateToAuthor = navigateToAuthor, diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index 0adc66b40..58c572ade 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -64,7 +64,7 @@ import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading @OptIn(ExperimentalLifecycleComposeApi::class) @Composable -fun TopicRoute( +internal fun TopicRoute( onBackClick: () -> Unit, modifier: Modifier = Modifier, viewModel: TopicViewModel = hiltViewModel(), diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt index 862e130f1..160bcfb3b 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt @@ -21,13 +21,14 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.google.samples.apps.nowinandroid.core.data.repository.TopicsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.core.domain.GetSaveableNewsResourcesStreamUseCase import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.domain.model.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Topic import com.google.samples.apps.nowinandroid.core.result.Result import com.google.samples.apps.nowinandroid.core.result.asResult -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination +import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicArgs import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -41,16 +42,17 @@ import kotlinx.coroutines.launch @HiltViewModel class TopicViewModel @Inject constructor( savedStateHandle: SavedStateHandle, + stringDecoder: StringDecoder, private val userDataRepository: UserDataRepository, topicsRepository: TopicsRepository, // newsRepository: NewsRepository, getSaveableNewsResourcesStream: GetSaveableNewsResourcesStreamUseCase ) : ViewModel() { - private val topicId: String = checkNotNull(savedStateHandle[TopicDestination.topicIdArg]) + private val topicArgs: TopicArgs = TopicArgs(savedStateHandle, stringDecoder) val topicUiState: StateFlow = topicUiStateStream( - topicId = topicId, + topicId = topicArgs.topicId, userDataRepository = userDataRepository, topicsRepository = topicsRepository ) @@ -61,7 +63,7 @@ class TopicViewModel @Inject constructor( ) val newUiState: StateFlow = newsUiStateStream( - topicId = topicId, + topicId = topicArgs.topicId, userDataRepository = userDataRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStream ) @@ -73,7 +75,7 @@ class TopicViewModel @Inject constructor( fun followTopicToggle(followed: Boolean) { viewModelScope.launch { - userDataRepository.toggleFollowedTopicId(topicId, followed) + userDataRepository.toggleFollowedTopicId(topicArgs.topicId, followed) } } diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt index f3d4d021b..808143275 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt @@ -17,43 +17,36 @@ package com.google.samples.apps.nowinandroid.feature.topic.navigation import android.net.Uri -import androidx.navigation.NavBackStackEntry +import androidx.annotation.VisibleForTesting +import androidx.lifecycle.SavedStateHandle +import androidx.navigation.NavController import androidx.navigation.NavGraphBuilder import androidx.navigation.NavType import androidx.navigation.compose.composable import androidx.navigation.navArgument -import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination +import com.google.samples.apps.nowinandroid.core.decoder.StringDecoder import com.google.samples.apps.nowinandroid.feature.topic.TopicRoute -object TopicDestination : NiaNavigationDestination { - const val topicIdArg = "topicId" - override val route = "topic_route/{$topicIdArg}" - override val destination = "topic_destination" +@VisibleForTesting +internal const val topicIdArg = "topicId" - /** - * Creates destination route for a topicId that could include special characters - */ - fun createNavigationRoute(topicIdArg: String): String { - val encodedId = Uri.encode(topicIdArg) - return "topic_route/$encodedId" - } +internal class TopicArgs(val topicId: String) { + constructor(savedStateHandle: SavedStateHandle, stringDecoder: StringDecoder) : + this(stringDecoder.decodeString(checkNotNull(savedStateHandle[topicIdArg]))) +} - /** - * Returns the topicId from a [NavBackStackEntry] after a topic destination navigation call - */ - fun fromNavArgs(entry: NavBackStackEntry): String { - val encodedId = entry.arguments?.getString(topicIdArg)!! - return Uri.decode(encodedId) - } +fun NavController.navigateToTopic(topicId: String) { + val encodedId = Uri.encode(topicId) + this.navigate("topic_route/$encodedId") } -fun NavGraphBuilder.topicGraph( +fun NavGraphBuilder.topicScreen( onBackClick: () -> Unit ) { composable( - route = TopicDestination.route, + route = "topic_route/{$topicIdArg}", arguments = listOf( - navArgument(TopicDestination.topicIdArg) { type = NavType.StringType } + navArgument(topicIdArg) { type = NavType.StringType } ) ) { TopicRoute(onBackClick = onBackClick) diff --git a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt index 94e0e9337..b914856be 100644 --- a/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt +++ b/feature/topic/src/test/java/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt @@ -22,11 +22,12 @@ import com.google.samples.apps.nowinandroid.core.domain.model.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video import com.google.samples.apps.nowinandroid.core.model.data.Topic +import com.google.samples.apps.nowinandroid.core.testing.decoder.FakeStringDecoder import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestTopicsRepository import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserDataRepository import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicDestination +import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicIdArg import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first @@ -61,8 +62,8 @@ class TopicViewModelTest { @Before fun setup() { viewModel = TopicViewModel( - savedStateHandle = - SavedStateHandle(mapOf(TopicDestination.topicIdArg to testInputTopics[0].topic.id)), + savedStateHandle = SavedStateHandle(mapOf(topicIdArg to testInputTopics[0].topic.id)), + stringDecoder = FakeStringDecoder(), userDataRepository = userDataRepository, topicsRepository = topicsRepository, getSaveableNewsResourcesStream = getSaveableNewsResourcesStreamUseCase diff --git a/settings.gradle.kts b/settings.gradle.kts index 253fa23d4..c6e2d3bf5 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -43,7 +43,6 @@ include(":core:datastore-test") include(":core:designsystem") include(":core:domain") include(":core:model") -include(":core:navigation") include(":core:network") include(":core:ui") include(":core:testing") @@ -54,4 +53,4 @@ include(":feature:bookmarks") include(":feature:topic") include(":lint") include(":sync:work") -include(":sync:sync-test") \ No newline at end of file +include(":sync:sync-test")