Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,78 +17,41 @@
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<InterestsRoute> {
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<String>(
initialDestinationHistory = listOfNotNull(
ThreePaneScaffoldDestinationItem(ListDetailPaneScaffoldRole.List),
ThreePaneScaffoldDestinationItem<Nothing>(ListDetailPaneScaffoldRole.Detail).takeIf {
ThreePaneScaffoldDestinationItem<String>(ListDetailPaneScaffoldRole.Detail).takeIf {
selectedTopicId != null
},
),
Expand All @@ -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<DetailPaneNavHostRoute>()
}
} 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(
Expand All @@ -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<TopicPlaceholderRoute> {
TopicDetailPlaceholder()
}
}
}
if (selectedTopicId != null) {
TopicScreen(
topicId = selectedTopicId!!,
showBackButton = !listDetailNavigator.isListPaneVisible(),
onBackClick = listDetailNavigator::navigateBack,
onTopicClick = ::onTopicClickShowDetailPane,
)
} else
TopicDetailPlaceholder()
}
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -493,7 +493,7 @@ private fun feedItemsSize(
OnboardingUiState.Loading,
OnboardingUiState.LoadFailed,
OnboardingUiState.NotShown,
-> 0
-> 0

is OnboardingUiState.Shown -> 1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line feels the strangest with this usage, since we are reusing a single instance of the ViewModel scoped to the overall navigation entry with many different topicIds that we are swapping between.

One side-effect is that we can't display two different versions of the TopicScreen at the same time, which might be ideal to animate between two different selected topics.


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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<TopicRoute>().id
private val _topicId = MutableStateFlow<String?>(null)
val topicId = _topicId.asStateFlow()

private val _topicUIState = MutableStateFlow<TopicUiState>(TopicUiState.Loading)

private val _newsUIState = MutableStateFlow<NewsUiState>(NewsUiState.Loading)


val topicUiState: StateFlow<TopicUiState> = 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The combines for old topicIds will continue to run right now, since these won't be cancelled if the topic id changes.

}
}
}

val topicUiState: StateFlow<TopicUiState> = _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> = newsUiState(
topicId = topicId,
userDataRepository = userDataRepository,
userNewsResourceRepository = userNewsResourceRepository,

val newsUiState: StateFlow<NewsUiState> = _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)
}
}
}

Expand All @@ -89,6 +109,10 @@ class TopicViewModel @Inject constructor(
userDataRepository.setNewsResourceViewed(newsResourceId, viewed)
}
}

fun updateTopic(id: String) {
this._topicId.value = id
}
}

private fun topicUiState(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ fun NavGraphBuilder.topicScreen(
onBackClick: () -> Unit,
onTopicClick: (String) -> Unit,
) {
composable<TopicRoute> {
composable<TopicRoute> { entry ->
val id = entry.arguments?.getString("id")!!
TopicScreen(
topicId = id,
showBackButton = showBackButton,
onBackClick = onBackClick,
onTopicClick = onTopicClick,
Expand Down
Loading