From 7a4eb0d9d329004bd92cfc5bd91d70812ad7e7c1 Mon Sep 17 00:00:00 2001 From: Ivan Date: Sun, 2 Feb 2025 21:42:18 +0200 Subject: [PATCH] NavHost replaced with if statement --- .../InterestsListDetailScreen.kt | 102 +++--------------- .../feature/foryou/ForYouScreen.kt | 32 +++--- .../nowinandroid/feature/topic/TopicScreen.kt | 10 +- .../feature/topic/TopicViewModel.kt | 76 ++++++++----- .../topic/navigation/TopicNavigation.kt | 4 +- .../feature/topic/TopicViewModelTest.kt | 9 +- 6 files changed, 95 insertions(+), 138 deletions(-) diff --git a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/interests2pane/InterestsListDetailScreen.kt b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/interests2pane/InterestsListDetailScreen.kt index 669c6300a8..0ad28f41f8 100644 --- a/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/interests2pane/InterestsListDetailScreen.kt +++ b/app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/interests2pane/InterestsListDetailScreen.kt @@ -17,46 +17,24 @@ package com.google.samples.apps.nowinandroid.ui.interests2pane import androidx.activity.compose.BackHandler -import androidx.annotation.Keep import androidx.compose.material3.adaptive.ExperimentalMaterial3AdaptiveApi -import androidx.compose.material3.adaptive.WindowAdaptiveInfo -import androidx.compose.material3.adaptive.currentWindowAdaptiveInfo import androidx.compose.material3.adaptive.layout.AnimatedPane import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffold import androidx.compose.material3.adaptive.layout.ListDetailPaneScaffoldRole import androidx.compose.material3.adaptive.layout.PaneAdaptedValue import androidx.compose.material3.adaptive.layout.ThreePaneScaffoldDestinationItem -import androidx.compose.material3.adaptive.layout.calculatePaneScaffoldDirective import androidx.compose.material3.adaptive.navigation.ThreePaneScaffoldNavigator import androidx.compose.material3.adaptive.navigation.rememberListDetailPaneScaffoldNavigator import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue -import androidx.compose.runtime.key -import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.remember -import androidx.compose.runtime.saveable.Saver -import androidx.compose.runtime.saveable.rememberSaveable -import androidx.compose.runtime.setValue import androidx.hilt.navigation.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavGraphBuilder -import androidx.navigation.compose.NavHost import androidx.navigation.compose.composable -import androidx.navigation.compose.rememberNavController import com.google.samples.apps.nowinandroid.feature.interests.InterestsRoute import com.google.samples.apps.nowinandroid.feature.interests.navigation.InterestsRoute import com.google.samples.apps.nowinandroid.feature.topic.TopicDetailPlaceholder -import com.google.samples.apps.nowinandroid.feature.topic.navigation.TopicRoute -import com.google.samples.apps.nowinandroid.feature.topic.navigation.navigateToTopic -import com.google.samples.apps.nowinandroid.feature.topic.navigation.topicScreen -import kotlinx.serialization.Serializable -import java.util.UUID - -@Serializable internal object TopicPlaceholderRoute - -// TODO: Remove @Keep when https://issuetracker.google.com/353898971 is fixed -@Keep -@Serializable internal object DetailPaneNavHostRoute +import com.google.samples.apps.nowinandroid.feature.topic.TopicScreen fun NavGraphBuilder.interestsListDetailScreen() { composable { @@ -64,31 +42,16 @@ fun NavGraphBuilder.interestsListDetailScreen() { } } -@Composable -internal fun InterestsListDetailScreen( - viewModel: Interests2PaneViewModel = hiltViewModel(), - windowAdaptiveInfo: WindowAdaptiveInfo = currentWindowAdaptiveInfo(), -) { - val selectedTopicId by viewModel.selectedTopicId.collectAsStateWithLifecycle() - InterestsListDetailScreen( - selectedTopicId = selectedTopicId, - onTopicClick = viewModel::onTopicClick, - windowAdaptiveInfo = windowAdaptiveInfo, - ) -} - @OptIn(ExperimentalMaterial3AdaptiveApi::class) @Composable internal fun InterestsListDetailScreen( - selectedTopicId: String?, - onTopicClick: (String) -> Unit, - windowAdaptiveInfo: WindowAdaptiveInfo, + viewModel: Interests2PaneViewModel = hiltViewModel() ) { - val listDetailNavigator = rememberListDetailPaneScaffoldNavigator( - scaffoldDirective = calculatePaneScaffoldDirective(windowAdaptiveInfo), + val selectedTopicId by viewModel.selectedTopicId.collectAsStateWithLifecycle() + val listDetailNavigator = rememberListDetailPaneScaffoldNavigator( initialDestinationHistory = listOfNotNull( ThreePaneScaffoldDestinationItem(ListDetailPaneScaffoldRole.List), - ThreePaneScaffoldDestinationItem(ListDetailPaneScaffoldRole.Detail).takeIf { + ThreePaneScaffoldDestinationItem(ListDetailPaneScaffoldRole.Detail).takeIf { selectedTopicId != null }, ), @@ -97,33 +60,9 @@ internal fun InterestsListDetailScreen( listDetailNavigator.navigateBack() } - var nestedNavHostStartRoute by remember { - val route = selectedTopicId?.let { TopicRoute(id = it) } ?: TopicPlaceholderRoute - mutableStateOf(route) - } - var nestedNavKey by rememberSaveable( - stateSaver = Saver({ it.toString() }, UUID::fromString), - ) { - mutableStateOf(UUID.randomUUID()) - } - val nestedNavController = key(nestedNavKey) { - rememberNavController() - } - - fun onTopicClickShowDetailPane(topicId: String) { - onTopicClick(topicId) - if (listDetailNavigator.isDetailPaneVisible()) { - // If the detail pane was visible, then use the nestedNavController navigate call - // directly - nestedNavController.navigateToTopic(topicId) { - popUpTo() - } - } else { - // Otherwise, recreate the NavHost entirely, and start at the new destination - nestedNavHostStartRoute = TopicRoute(id = topicId) - nestedNavKey = UUID.randomUUID() - } - listDetailNavigator.navigateTo(ListDetailPaneScaffoldRole.Detail) + fun onTopicClickShowDetailPane(selectedTopicId: String) { + viewModel.onTopicClick(selectedTopicId) + listDetailNavigator.navigateTo(ListDetailPaneScaffoldRole.Detail, selectedTopicId) } ListDetailPaneScaffold( @@ -139,22 +78,15 @@ internal fun InterestsListDetailScreen( }, detailPane = { AnimatedPane { - key(nestedNavKey) { - NavHost( - navController = nestedNavController, - startDestination = nestedNavHostStartRoute, - route = DetailPaneNavHostRoute::class, - ) { - topicScreen( - showBackButton = !listDetailNavigator.isListPaneVisible(), - onBackClick = listDetailNavigator::navigateBack, - onTopicClick = ::onTopicClickShowDetailPane, - ) - composable { - TopicDetailPlaceholder() - } - } - } + if (selectedTopicId != null) { + TopicScreen( + topicId = selectedTopicId!!, + showBackButton = !listDetailNavigator.isListPaneVisible(), + onBackClick = listDetailNavigator::navigateBack, + onTopicClick = ::onTopicClickShowDetailPane, + ) + } else + TopicDetailPlaceholder() } }, ) diff --git a/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt b/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt index 1a3325996c..9e902ead84 100644 --- a/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt +++ b/feature/foryou/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt @@ -175,18 +175,6 @@ internal fun ForYouScreen( onboardingUiState = onboardingUiState, onTopicCheckedChanged = onTopicCheckedChanged, saveFollowedTopics = saveFollowedTopics, - // Custom LayoutModifier to remove the enforced parent 16.dp contentPadding - // from the LazyVerticalGrid and enable edge-to-edge scrolling for this section - interestsItemModifier = Modifier.layout { measurable, constraints -> - val placeable = measurable.measure( - constraints.copy( - maxWidth = constraints.maxWidth + 32.dp.roundToPx(), - ), - ) - layout(placeable.width, placeable.height) { - placeable.place(0, 0) - } - }, ) newsFeed( @@ -258,17 +246,29 @@ private fun LazyStaggeredGridScope.onboarding( onboardingUiState: OnboardingUiState, onTopicCheckedChanged: (String, Boolean) -> Unit, saveFollowedTopics: () -> Unit, - interestsItemModifier: Modifier = Modifier, ) { when (onboardingUiState) { OnboardingUiState.Loading, OnboardingUiState.LoadFailed, OnboardingUiState.NotShown, - -> Unit + -> Unit is OnboardingUiState.Shown -> { item(span = StaggeredGridItemSpan.FullLine, contentType = "onboarding") { - Column(modifier = interestsItemModifier) { + // Custom LayoutModifier to remove the enforced parent 16.dp contentPadding + // from the LazyVerticalGrid and enable edge-to-edge scrolling for this section + Column( + modifier = Modifier.layout { measurable, constraints -> + val placeable = measurable.measure( + constraints.copy( + maxWidth = constraints.maxWidth + 32.dp.roundToPx(), + ), + ) + layout(placeable.width, placeable.height) { + placeable.place(0, 0) + } + }, + ) { Text( text = stringResource(R.string.feature_foryou_onboarding_guidance_title), textAlign = TextAlign.Center, @@ -493,7 +493,7 @@ private fun feedItemsSize( OnboardingUiState.Loading, OnboardingUiState.LoadFailed, OnboardingUiState.NotShown, - -> 0 + -> 0 is OnboardingUiState.Shown -> 1 } diff --git a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index 8ef0d786d6..b79c9a6010 100644 --- a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -72,20 +72,24 @@ import com.google.samples.apps.nowinandroid.feature.topic.R.string @Composable fun TopicScreen( + topicId: String, showBackButton: Boolean, onBackClick: () -> Unit, onTopicClick: (String) -> Unit, modifier: Modifier = Modifier, - viewModel: TopicViewModel = hiltViewModel(), + viewModel: TopicViewModel = hiltViewModel() ) { + viewModel.updateTopic(topicId) + val topicUiState: TopicUiState by viewModel.topicUiState.collectAsStateWithLifecycle() val newsUiState: NewsUiState by viewModel.newsUiState.collectAsStateWithLifecycle() + val selectedTopicId by viewModel.topicId.collectAsStateWithLifecycle() - TrackScreenViewEvent(screenName = "Topic: ${viewModel.topicId}") + TrackScreenViewEvent(screenName = "Topic: $selectedTopicId") TopicScreen( topicUiState = topicUiState, newsUiState = newsUiState, - modifier = modifier.testTag("topic:${viewModel.topicId}"), + modifier = modifier.testTag("topic:${selectedTopicId}"), showBackButton = showBackButton, onBackClick = onBackClick, onFollowClick = viewModel::followTopicToggle, diff --git a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt index ba8baad14f..942bb4caed 100644 --- a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt +++ b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModel.kt @@ -16,10 +16,8 @@ package com.google.samples.apps.nowinandroid.feature.topic -import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import androidx.navigation.toRoute import com.google.samples.apps.nowinandroid.core.data.repository.NewsResourceQuery import com.google.samples.apps.nowinandroid.core.data.repository.TopicsRepository import com.google.samples.apps.nowinandroid.core.data.repository.UserDataRepository @@ -29,52 +27,74 @@ import com.google.samples.apps.nowinandroid.core.model.data.Topic import com.google.samples.apps.nowinandroid.core.model.data.UserNewsResource 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.TopicRoute import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.flow.update import kotlinx.coroutines.launch import javax.inject.Inject @HiltViewModel class TopicViewModel @Inject constructor( - savedStateHandle: SavedStateHandle, private val userDataRepository: UserDataRepository, topicsRepository: TopicsRepository, - userNewsResourceRepository: UserNewsResourceRepository, + userNewsResourceRepository: UserNewsResourceRepository ) : ViewModel() { - val topicId = savedStateHandle.toRoute().id + private val _topicId = MutableStateFlow(null) + val topicId = _topicId.asStateFlow() + + private val _topicUIState = MutableStateFlow(TopicUiState.Loading) + + private val _newsUIState = MutableStateFlow(NewsUiState.Loading) + - val topicUiState: StateFlow = topicUiState( - topicId = topicId, - userDataRepository = userDataRepository, - topicsRepository = topicsRepository, + init { + viewModelScope.launch { + _topicId.filterNotNull().collect { topicId -> + combine( + topicUiState( + topicId = topicId, + userDataRepository = userDataRepository, + topicsRepository = topicsRepository, + ), + newsUiState( + topicId = topicId, + userDataRepository = userDataRepository, + userNewsResourceRepository = userNewsResourceRepository, + ), + ) { topicIUState, newsUIState -> + _topicUIState.update { topicIUState } + _newsUIState.update { newsUIState } + }.stateIn(viewModelScope) + } + } + } + + val topicUiState: StateFlow = _topicUIState.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = TopicUiState.Loading, ) - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = TopicUiState.Loading, - ) - - val newsUiState: StateFlow = newsUiState( - topicId = topicId, - userDataRepository = userDataRepository, - userNewsResourceRepository = userNewsResourceRepository, + + val newsUiState: StateFlow = _newsUIState.stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = NewsUiState.Loading, ) - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = NewsUiState.Loading, - ) fun followTopicToggle(followed: Boolean) { viewModelScope.launch { - userDataRepository.setTopicIdFollowed(topicId, followed) + _topicId.value?.let { + userDataRepository.setTopicIdFollowed(it, followed) + } } } @@ -89,6 +109,10 @@ class TopicViewModel @Inject constructor( userDataRepository.setNewsResourceViewed(newsResourceId, viewed) } } + + fun updateTopic(id: String) { + this._topicId.value = id + } } private fun topicUiState( diff --git a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt index fabb82b10d..3a5629383e 100644 --- a/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt +++ b/feature/topic/src/main/kotlin/com/google/samples/apps/nowinandroid/feature/topic/navigation/TopicNavigation.kt @@ -36,8 +36,10 @@ fun NavGraphBuilder.topicScreen( onBackClick: () -> Unit, onTopicClick: (String) -> Unit, ) { - composable { + composable { entry -> + val id = entry.arguments?.getString("id")!! TopicScreen( + topicId = id, showBackButton = showBackButton, onBackClick = onBackClick, onTopicClick = onTopicClick, diff --git a/feature/topic/src/test/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt b/feature/topic/src/test/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt index 34f21a59a3..7a98356d8a 100644 --- a/feature/topic/src/test/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt +++ b/feature/topic/src/test/kotlin/com/google/samples/apps/nowinandroid/feature/topic/TopicViewModelTest.kt @@ -16,8 +16,6 @@ package com.google.samples.apps.nowinandroid.feature.topic -import androidx.lifecycle.SavedStateHandle -import androidx.navigation.testing.invoke import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic import com.google.samples.apps.nowinandroid.core.model.data.NewsResource @@ -26,7 +24,6 @@ import com.google.samples.apps.nowinandroid.core.testing.repository.TestNewsRepo 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.TopicRoute import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first @@ -70,18 +67,16 @@ class TopicViewModelTest { @Before fun setup() { viewModel = TopicViewModel( - savedStateHandle = SavedStateHandle( - route = TopicRoute(id = testInputTopics[0].topic.id), - ), userDataRepository = userDataRepository, topicsRepository = topicsRepository, userNewsResourceRepository = userNewsResourceRepository, ) + viewModel.updateTopic(testInputTopics[0].topic.id) } @Test fun topicId_matchesTopicIdFromSavedStateHandle() = - assertEquals(testInputTopics[0].topic.id, viewModel.topicId) + assertEquals(testInputTopics[0].topic.id, viewModel.topicId.value) @Test fun uiStateTopic_whenSuccess_matchesTopicFromRepository() = runTest {