Skip to content

Commit

Permalink
Incorporate code review changes: Move UserNewsResourceRepository to data
Browse files Browse the repository at this point in the history
module; move UserNewsResource to model module. Implement unread dot for
bookmarked articles. Keep the flows cold in UserNewsResourceRepository.
  • Loading branch information
rosejr committed Mar 13, 2023
1 parent ebfbb5b commit 57c13d8
Show file tree
Hide file tree
Showing 48 changed files with 325 additions and 370 deletions.
2 changes: 0 additions & 2 deletions app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,9 @@ dependencies {
implementation(project(":feature:settings"))

implementation(project(":core:common"))
implementation(project(":core:domain"))
implementation(project(":core:ui"))
implementation(project(":core:designsystem"))
implementation(project(":core:data"))
implementation(project(":core:domain"))
implementation(project(":core:model"))
implementation(project(":core:analytics"))

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,14 @@ import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.unit.DpSize
import androidx.compose.ui.unit.dp
import com.google.accompanist.testharness.TestHarness
import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.domain.repository.CompositeUserNewsResourceRepository
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.uitesthiltmanifest.HiltComponentActivity
import dagger.hilt.android.testing.BindValue
import dagger.hilt.android.testing.HiltAndroidRule
import dagger.hilt.android.testing.HiltAndroidTest
import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.UnconfinedTestDispatcher
import org.junit.Before
import org.junit.Rule
import org.junit.Test
Expand Down Expand Up @@ -69,7 +67,6 @@ class NavigationUiTest {
val composeTestRule = createAndroidComposeRule<HiltComponentActivity>()

val userNewsResourceRepository = CompositeUserNewsResourceRepository(
coroutineScope = TestScope(UnconfinedTestDispatcher()),
newsRepository = TestNewsRepository(),
userDataRepository = TestUserDataRepository(),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import androidx.navigation.compose.ComposeNavigator
import androidx.navigation.compose.composable
import androidx.navigation.createGraph
import androidx.navigation.testing.TestNavHostController
import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository
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.TestNetworkMonitor
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.launch
Expand All @@ -56,6 +59,9 @@ class NiaAppStateTest {
// Create the test dependencies.
private val networkMonitor = TestNetworkMonitor()

private val userNewsResourceRepository =
CompositeUserNewsResourceRepository(TestNewsRepository(), TestUserDataRepository())

// Subject under test.
private lateinit var state: NiaAppState

Expand All @@ -67,10 +73,11 @@ class NiaAppStateTest {
val navController = rememberTestNavController()
state = remember(navController) {
NiaAppState(
windowSizeClass = getCompactWindowClass(),
navController = navController,
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -92,6 +99,7 @@ class NiaAppStateTest {
state = rememberNiaAppState(
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -105,10 +113,11 @@ class NiaAppStateTest {
fun niaAppState_showBottomBar_compact() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = getCompactWindowClass(),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = getCompactWindowClass(),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -120,10 +129,11 @@ class NiaAppStateTest {
fun niaAppState_showNavRail_medium() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(800.dp, 800.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(800.dp, 800.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -135,10 +145,11 @@ class NiaAppStateTest {
fun niaAppState_showNavRail_large() = runTest {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand All @@ -150,10 +161,11 @@ class NiaAppStateTest {
fun stateIsOfflineWhenNetworkMonitorIsOffline() = runTest(UnconfinedTestDispatcher()) {
composeTestRule.setContent {
state = NiaAppState(
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
navController = NavHostController(LocalContext.current),
networkMonitor = networkMonitor,
coroutineScope = backgroundScope,
windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)),
networkMonitor = networkMonitor,
userNewsResourceRepository = userNewsResourceRepository,
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import com.google.samples.apps.nowinandroid.MainActivityUiState.Loading
import com.google.samples.apps.nowinandroid.MainActivityUiState.Success
import com.google.samples.apps.nowinandroid.core.analytics.AnalyticsHelper
import com.google.samples.apps.nowinandroid.core.analytics.LocalAnalyticsHelper
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.model.data.DarkThemeConfig
import com.google.samples.apps.nowinandroid.core.model.data.ThemeBrand
import com.google.samples.apps.nowinandroid.ui.NiaApp
Expand Down
53 changes: 24 additions & 29 deletions app/src/main/java/com/google/samples/apps/nowinandroid/ui/NiaApp.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
import androidx.navigation.NavDestination
import androidx.navigation.NavDestination.Companion.hierarchy
import com.google.samples.apps.nowinandroid.R
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaBackground
import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaGradientBackground
Expand All @@ -70,7 +71,6 @@ import com.google.samples.apps.nowinandroid.core.designsystem.icon.Icon.ImageVec
import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons
import com.google.samples.apps.nowinandroid.core.designsystem.theme.GradientColors
import com.google.samples.apps.nowinandroid.core.designsystem.theme.LocalGradientColors
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.feature.settings.SettingsDialog
import com.google.samples.apps.nowinandroid.navigation.NiaNavHost
import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination
Expand All @@ -85,11 +85,12 @@ import com.google.samples.apps.nowinandroid.feature.settings.R as settingsR
fun NiaApp(
windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
appState: NiaAppState = rememberNiaAppState(
networkMonitor = networkMonitor,
windowSizeClass = windowSizeClass,
userNewsResourceRepository = userNewsResourceRepository,
),
userNewsResourceRepository: UserNewsResourceRepository,
) {
val shouldShowGradientBackground =
appState.currentTopLevelDestination == TopLevelDestination.FOR_YOU
Expand Down Expand Up @@ -133,14 +134,7 @@ fun NiaApp(
snackbarHost = { SnackbarHost(snackbarHostState) },
bottomBar = {
if (appState.shouldShowBottomBar) {
val forYouNewsResources by userNewsResourceRepository.getUserNewsResourcesForFollowedTopics()
.collectAsStateWithLifecycle(emptyList())
val unreadDestinations =
when {
forYouNewsResources.all { it.isViewed } -> emptySet()
else -> setOf(TopLevelDestination.FOR_YOU)
}

val unreadDestinations by appState.topLevelDestinationsWithUnreadResources.collectAsStateWithLifecycle()
NiaBottomBar(
destinations = appState.topLevelDestinations,
destinationsWithUnreadResources = unreadDestinations,
Expand Down Expand Up @@ -275,30 +269,31 @@ private fun NiaBottomBar(
}
},
label = { Text(stringResource(destination.iconTextId)) },
modifier = if (hasUnread) {
val tertiaryColor = MaterialTheme.colorScheme.tertiary
Modifier.drawWithContent {
drawContent()
drawCircle(
tertiaryColor,
radius = 5.dp.toPx(),
// This is based on the dimensions of the NavigationBar's "indicator pill";
// however, its parameters are private, so we must depend on them implicitly
// (NavigationBarTokens.ActiveIndicatorWidth = 64.dp)
center = center + Offset(
64.dp.toPx() * .45f,
32.dp.toPx() * -.45f - 6.dp.toPx(),
),
)
}
} else {
Modifier
},
modifier = if (hasUnread) notificationDot() else Modifier,
)
}
}
}

@Composable
private fun notificationDot(): Modifier {
val tertiaryColor = MaterialTheme.colorScheme.tertiary
return Modifier.drawWithContent {
drawContent()
drawCircle(
tertiaryColor,
radius = 5.dp.toPx(),
// This is based on the dimensions of the NavigationBar's "indicator pill";
// however, its parameters are private, so we must depend on them implicitly
// (NavigationBarTokens.ActiveIndicatorWidth = 64.dp)
center = center + Offset(
64.dp.toPx() * .45f,
32.dp.toPx() * -.45f - 6.dp.toPx(),
),
)
}
}

private fun NavDestination?.isTopLevelDestinationInHierarchy(destination: TopLevelDestination) =
this?.hierarchy?.any {
it.route?.contains(destination.name, true) ?: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ 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.data.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.util.NetworkMonitor
import com.google.samples.apps.nowinandroid.core.ui.TrackDisposableJank
import com.google.samples.apps.nowinandroid.feature.bookmarks.navigation.bookmarksRoute
Expand All @@ -47,19 +48,34 @@ import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.FOR_Y
import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination.INTERESTS
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.SharingStarted
import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.stateIn

@Composable
fun rememberNiaAppState(
windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
coroutineScope: CoroutineScope = rememberCoroutineScope(),
navController: NavHostController = rememberNavController(),
): NiaAppState {
NavigationTrackingSideEffect(navController)
return remember(navController, coroutineScope, windowSizeClass, networkMonitor) {
NiaAppState(navController, coroutineScope, windowSizeClass, networkMonitor)
return remember(
navController,
coroutineScope,
windowSizeClass,
networkMonitor,
userNewsResourceRepository,
) {
NiaAppState(
navController,
coroutineScope,
windowSizeClass,
networkMonitor,
userNewsResourceRepository,
)
}
}

Expand All @@ -69,6 +85,7 @@ class NiaAppState(
val coroutineScope: CoroutineScope,
val windowSizeClass: WindowSizeClass,
networkMonitor: NetworkMonitor,
userNewsResourceRepository: UserNewsResourceRepository,
) {
val currentDestination: NavDestination?
@Composable get() = navController
Expand Down Expand Up @@ -105,6 +122,22 @@ class NiaAppState(
*/
val topLevelDestinations: List<TopLevelDestination> = TopLevelDestination.values().asList()

/**
* The top level destinations that have unread news resources.
*/
val topLevelDestinationsWithUnreadResources: StateFlow<Set<TopLevelDestination>> =
userNewsResourceRepository.getUserNewsResourcesForFollowedTopics()
.combine(userNewsResourceRepository.getBookmarkedUserNewsResources()) { forYouNewsResources, bookmarkedNewsResources ->
setOfNotNull(
FOR_YOU.takeIf { forYouNewsResources.any { !it.hasBeenViewed } },
BOOKMARKS.takeIf { bookmarkedNewsResources.any { !it.hasBeenViewed } },
)
}.stateIn(
coroutineScope,
SharingStarted.WhileSubscribed(5_000),
initialValue = emptySet(),
)

/**
* 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
* limitations under the License.
*/

package com.google.samples.apps.nowinandroid.core.domain.di
package com.google.samples.apps.nowinandroid.core.data.di

import com.google.samples.apps.nowinandroid.core.domain.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.domain.repository.UserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.repository.CompositeUserNewsResourceRepository
import com.google.samples.apps.nowinandroid.core.data.repository.UserNewsResourceRepository
import dagger.Binds
import dagger.Module
import dagger.hilt.InstallIn
Expand Down
Loading

0 comments on commit 57c13d8

Please sign in to comment.