From 828242dbaddf6078c08f13b74ebc0dd736c27a61 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 21 Oct 2022 00:13:28 +0100 Subject: [PATCH] Remove Scaffold from top level screens --- .../apps/nowinandroid/ui/NavigationTest.kt | 13 +- .../apps/nowinandroid/ui/NiaAppStateTest.kt | 89 +++++++-- .../samples/apps/nowinandroid/MainActivity.kt | 5 + .../samples/apps/nowinandroid/ui/NiaApp.kt | 52 +++++- .../apps/nowinandroid/ui/NiaAppState.kt | 39 +++- .../feature/bookmarks/BookmarksScreen.kt | 4 - .../feature/foryou/ForYouScreen.kt | 172 +++++++----------- .../feature/foryou/ForYouViewModel.kt | 8 - .../feature/foryou/ForYouViewModelTest.kt | 15 -- .../feature/settings/SettingsDialog.kt | 8 +- .../feature/settings/SettingsViewModel.kt | 7 +- 11 files changed, 246 insertions(+), 166 deletions(-) diff --git a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt index d7d092a26..dfc45f280 100644 --- a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt @@ -21,6 +21,7 @@ import androidx.compose.ui.test.assertIsOn import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onLast import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick @@ -168,13 +169,13 @@ class NavigationTest { // Verify that the top bar contains the app name on the first screen. onNodeWithText(appName).assertExists() - // Go to the bookmarks tab, verify that the top bar contains the app name. + // Go to the saved tab, verify that the top bar contains "saved". This means + // we'll have 2 elements with the text "saved" on screen. One in the top bar, and + // one in the bottom navigation. onNodeWithText(saved).performClick() - onNodeWithText(appName).assertExists() + onAllNodesWithText(saved).assertCountEquals(2) - // Go to the interests tab, verify that the top bar contains "Interests". This means - // we'll have 2 elements with the text "Interests" on screen. One in the top bar, and - // one in the bottom navigation. + // As above but for the interests tab. onNodeWithText(interests).performClick() onAllNodesWithText(interests).assertCountEquals(2) } @@ -214,7 +215,7 @@ class NavigationTest { onNodeWithText(ok).performClick() // Check that the saved screen is still visible and selected. - onNodeWithText(saved).assertIsSelected() + onAllNodesWithText(saved).onLast().assertIsSelected() } } 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 f6361d844..ce8f1aa00 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 @@ -30,9 +30,19 @@ 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.testing.util.MainDispatcherRule +import com.google.samples.apps.nowinandroid.core.testing.util.TestNetworkMonitor +import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue +import org.junit.Before import org.junit.Rule import org.junit.Test @@ -45,13 +55,33 @@ import org.junit.Test @OptIn(ExperimentalMaterial3WindowSizeClassApi::class) class NiaAppStateTest { + @get:Rule + val mainDispatcherRule = MainDispatcherRule() + @get:Rule val composeTestRule = createComposeRule() + // Create the test dependencies. + private lateinit var testScope: TestScope + private val networkMonitor = TestNetworkMonitor() + + // Subject under test. private lateinit var state: NiaAppState + @Before + fun setup() { + // We use the Unconfined dispatcher to ensure that coroutines are executed sequentially in + // tests. + testScope = TestScope(UnconfinedTestDispatcher()) + } + + @After + fun cleanup() { + testScope.cancel() + } + @Test - fun niaAppState_currentDestination() { + fun niaAppState_currentDestination() = runTest { var currentDestination: String? = null composeTestRule.setContent { @@ -59,7 +89,9 @@ class NiaAppStateTest { state = remember(navController) { NiaAppState( windowSizeClass = getCompactWindowClass(), - navController = navController + navController = navController, + networkMonitor = networkMonitor, + coroutineScope = testScope ) } @@ -76,9 +108,12 @@ class NiaAppStateTest { } @Test - fun niaAppState_destinations() { + fun niaAppState_destinations() = runTest { composeTestRule.setContent { - state = rememberNiaAppState(getCompactWindowClass()) + state = rememberNiaAppState( + windowSizeClass = getCompactWindowClass(), + networkMonitor = networkMonitor + ) } assertEquals(3, state.topLevelDestinations.size) @@ -93,7 +128,8 @@ class NiaAppStateTest { val navController = rememberTestNavController() state = rememberNiaAppState( windowSizeClass = getCompactWindowClass(), - navController = navController + navController = navController, + networkMonitor = networkMonitor ) // Do nothing - we should already be @@ -101,11 +137,13 @@ class NiaAppStateTest { } @Test - fun niaAppState_showBottomBar_compact() { + fun niaAppState_showBottomBar_compact() = runTest { composeTestRule.setContent { state = NiaAppState( windowSizeClass = getCompactWindowClass(), - navController = NavHostController(LocalContext.current) + navController = NavHostController(LocalContext.current), + networkMonitor = networkMonitor, + coroutineScope = testScope ) } @@ -114,11 +152,13 @@ class NiaAppStateTest { } @Test - fun niaAppState_showNavRail_medium() { + fun niaAppState_showNavRail_medium() = runTest { composeTestRule.setContent { state = NiaAppState( windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(800.dp, 800.dp)), - navController = NavHostController(LocalContext.current) + navController = NavHostController(LocalContext.current), + networkMonitor = networkMonitor, + coroutineScope = testScope ) } @@ -127,11 +167,14 @@ class NiaAppStateTest { } @Test - fun niaAppState_showNavRail_large() { + fun niaAppState_showNavRail_large() = runTest { + composeTestRule.setContent { state = NiaAppState( windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)), - navController = NavHostController(LocalContext.current) + navController = NavHostController(LocalContext.current), + networkMonitor = networkMonitor, + coroutineScope = testScope ) } @@ -139,6 +182,30 @@ class NiaAppStateTest { assertFalse(state.shouldShowBottomBar) } + @Test + fun stateIsOfflineWhenNetworkMonitorIsOffline() = runTest { + + composeTestRule.setContent { + state = NiaAppState( + windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(900.dp, 1200.dp)), + navController = NavHostController(LocalContext.current), + networkMonitor = networkMonitor, + coroutineScope = testScope + ) + } + + val collectJob = testScope.launch { state.isOffline.collect() } + + networkMonitor.setConnected(false) + + assertEquals( + true, + state.isOffline.value + ) + + collectJob.cancel() + } + private fun getCompactWindowClass() = WindowSizeClass.calculateFromSize(DpSize(500.dp, 300.dp)) } diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/MainActivity.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/MainActivity.kt index 2f69cdc37..60a865fbb 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/MainActivity.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/MainActivity.kt @@ -37,6 +37,7 @@ import androidx.metrics.performance.JankStats import com.google.accompanist.systemuicontroller.rememberSystemUiController import com.google.samples.apps.nowinandroid.MainActivityUiState.Loading import com.google.samples.apps.nowinandroid.MainActivityUiState.Success +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.model.data.DarkThemeConfig import com.google.samples.apps.nowinandroid.core.model.data.ThemeBrand @@ -57,6 +58,9 @@ class MainActivity : ComponentActivity() { @Inject lateinit var lazyStats: dagger.Lazy + @Inject + lateinit var networkMonitor: NetworkMonitor + val viewModel: MainActivityViewModel by viewModels() override fun onCreate(savedInstanceState: Bundle?) { @@ -105,6 +109,7 @@ class MainActivity : ComponentActivity() { androidTheme = shouldUseAndroidTheme(uiState) ) { NiaApp( + networkMonitor = networkMonitor, windowSizeClass = calculateWindowSizeClass(this), ) } 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 f28824c5a..ef545c70b 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 @@ -31,10 +31,16 @@ import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Scaffold +import androidx.compose.material3.SnackbarDuration.Indefinite +import androidx.compose.material3.SnackbarHost +import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Text import androidx.compose.material3.TopAppBarDefaults import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.compose.runtime.Composable +import androidx.compose.runtime.LaunchedEffect +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember import androidx.compose.ui.ExperimentalComposeUiApi import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.Color @@ -42,8 +48,11 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.testTagsAsResourceId +import androidx.lifecycle.compose.ExperimentalLifecycleComposeApi +import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.navigation.NavDestination import androidx.navigation.NavDestination.Companion.hierarchy +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 import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaNavigationBar @@ -61,12 +70,16 @@ import com.google.samples.apps.nowinandroid.navigation.TopLevelDestination @OptIn( ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class, - ExperimentalComposeUiApi::class + ExperimentalComposeUiApi::class, ExperimentalLifecycleComposeApi::class ) @Composable fun NiaApp( windowSizeClass: WindowSizeClass, - appState: NiaAppState = rememberNiaAppState(windowSizeClass) + networkMonitor: NetworkMonitor, + appState: NiaAppState = rememberNiaAppState( + networkMonitor = networkMonitor, + windowSizeClass = windowSizeClass + ), ) { val background: @Composable (@Composable () -> Unit) -> Unit = when (appState.currentDestination?.route) { @@ -75,6 +88,9 @@ fun NiaApp( } background { + + val snackbarHostState = remember { SnackbarHostState() } + Scaffold( modifier = Modifier.semantics { testTagsAsResourceId = true @@ -82,11 +98,14 @@ fun NiaApp( containerColor = Color.Transparent, contentColor = MaterialTheme.colorScheme.onBackground, contentWindowInsets = WindowInsets(0, 0, 0, 0), + snackbarHost = { SnackbarHost(snackbarHostState) }, topBar = { - val destination = appState.topLevelDestinations[appState.currentDestination?.route] - if (appState.shouldShowTopBar && destination != null) { + // Show the top app bar on top level destinations. + val topLevelDestination = + appState.topLevelDestinations[appState.currentDestination?.route] + if (topLevelDestination != null) { NiaTopAppBar( - titleRes = destination.titleTextId, + titleRes = topLevelDestination.titleTextId, actionIcon = NiaIcons.Settings, actionIconContentDescription = stringResource( id = settingsR.string.top_app_bar_action_icon_description @@ -94,7 +113,7 @@ fun NiaApp( colors = TopAppBarDefaults.centerAlignedTopAppBarColors( containerColor = Color.Transparent ), - onActionClick = { /*openAccountDialog = true*/ } + onActionClick = { appState.toggleSettingsDialog(true) } ) } }, @@ -108,6 +127,24 @@ fun NiaApp( } } ) { padding -> + + val isOffline by appState.isOffline.collectAsStateWithLifecycle() + + // If user is not connected to the internet show a snack bar to inform them. + val notConnected = stringResource(R.string.for_you_not_connected) + LaunchedEffect(isOffline) { + if (isOffline) snackbarHostState.showSnackbar( + message = notConnected, + duration = Indefinite + ) + } + + if (appState.shouldShowSettingsDialog) { + SettingsDialog( + onDismiss = { appState.toggleSettingsDialog(false) } + ) + } + Row( Modifier .fillMaxSize() @@ -134,6 +171,9 @@ fun NiaApp( .padding(padding) .consumedWindowInsets(padding) ) + + // TODO: We may want to add padding or spacer when the snackbar is shown so that + // content doesn't display behind it. } } } 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 250bdf5d1..19bc5366d 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 @@ -21,7 +21,11 @@ import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass import androidx.compose.runtime.Composable import androidx.compose.runtime.Stable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope +import androidx.compose.runtime.setValue import androidx.navigation.NavController import androidx.navigation.NavDestination import androidx.navigation.NavGraph.Companion.findStartDestination @@ -30,6 +34,11 @@ 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.util.NetworkMonitor +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.navigation.navigateToBookmarks import com.google.samples.apps.nowinandroid.feature.foryou.navigation.navigateToForYou @@ -38,29 +47,37 @@ 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 +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn @Composable fun rememberNiaAppState( windowSizeClass: WindowSizeClass, + networkMonitor: NetworkMonitor, + coroutineScope: CoroutineScope = rememberCoroutineScope(), navController: NavHostController = rememberNavController() ): NiaAppState { NavigationTrackingSideEffect(navController) - return remember(navController, windowSizeClass) { - NiaAppState(navController, windowSizeClass) + return remember(navController, coroutineScope, windowSizeClass, networkMonitor) { + NiaAppState(navController, coroutineScope, windowSizeClass, networkMonitor) } } @Stable class NiaAppState( val navController: NavHostController, - val windowSizeClass: WindowSizeClass + val coroutineScope: CoroutineScope, + val windowSizeClass: WindowSizeClass, + networkMonitor: NetworkMonitor, ) { val currentDestination: NavDestination? @Composable get() = navController .currentBackStackEntryAsState().value?.destination - val shouldShowTopBar: Boolean - @Composable get() = (currentDestination?.route in topLevelDestinations) + private var _shouldShowSettingsDialog by mutableStateOf(false) + val shouldShowSettingsDialog = _shouldShowSettingsDialog val shouldShowBottomBar: Boolean get() = windowSizeClass.widthSizeClass == WindowWidthSizeClass.Compact || @@ -69,6 +86,14 @@ class NiaAppState( val shouldShowNavRail: Boolean get() = !shouldShowBottomBar + val isOffline = networkMonitor.isOnline + .map(Boolean::not) + .stateIn( + scope = coroutineScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = false + ) + /** * Map of top level destinations to be used in the TopBar, BottomBar and NavRail. The key is the * route. @@ -109,6 +134,10 @@ class NiaAppState( fun onBackClick() { navController.popBackStack() } + + fun toggleSettingsDialog(shouldShow: Boolean) { + _shouldShowSettingsDialog = shouldShow + } } /** 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 672213492..6c45c4314 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 @@ -21,10 +21,8 @@ import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets -import androidx.compose.foundation.layout.consumedWindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawing import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.layout.wrapContentSize @@ -33,11 +31,9 @@ import androidx.compose.foundation.lazy.grid.GridItemSpan import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.rememberLazyGridState import androidx.compose.material3.ExperimentalMaterial3Api -import androidx.compose.material3.Scaffold import androidx.compose.runtime.Composable import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp 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 c15263d3b..75320caea 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 @@ -22,21 +22,14 @@ import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column -import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.WindowInsets -import androidx.compose.foundation.layout.consumedWindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.safeDrawing import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width -import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.grid.GridCells import androidx.compose.foundation.lazy.grid.GridCells.Adaptive @@ -51,19 +44,13 @@ import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.MaterialTheme -import androidx.compose.material3.Scaffold -import androidx.compose.material3.SnackbarDuration.Indefinite -import androidx.compose.material3.SnackbarHost -import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue -import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier -import androidx.compose.ui.graphics.Color import androidx.compose.ui.layout.layout import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalView @@ -104,11 +91,9 @@ internal fun ForYouRoute( ) { val interestsSelectionState by viewModel.interestsSelectionUiState.collectAsStateWithLifecycle() val feedState by viewModel.feedState.collectAsStateWithLifecycle() - val isOffline by viewModel.isOffline.collectAsStateWithLifecycle() val isSyncing by viewModel.isSyncing.collectAsStateWithLifecycle() ForYouScreen( - isOffline = isOffline, isSyncing = isSyncing, interestsSelectionState = interestsSelectionState, feedState = feedState, @@ -120,7 +105,6 @@ internal fun ForYouRoute( ) } -@OptIn(ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class) @Composable internal fun ForYouScreen( isOffline: Boolean, @@ -133,107 +117,88 @@ internal fun ForYouScreen( onNewsResourcesCheckedChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { - val snackbarHostState = remember { SnackbarHostState() } - Scaffold( - snackbarHost = { SnackbarHost(snackbarHostState) }, - containerColor = Color.Transparent, - contentWindowInsets = WindowInsets(0, 0, 0, 0) - ) { innerPadding -> - // Workaround to call Activity.reportFullyDrawn from Jetpack Compose. - // This code should be called when the UI is ready for use - // and relates to Time To Full Display. - val interestsLoaded = - interestsSelectionState !is ForYouInterestsSelectionUiState.Loading - val feedLoaded = feedState !is NewsFeedUiState.Loading + // Workaround to call Activity.reportFullyDrawn from Jetpack Compose. + // This code should be called when the UI is ready for use + // and relates to Time To Full Display. + val interestsLoaded = + interestsSelectionState !is ForYouInterestsSelectionUiState.Loading + val feedLoaded = feedState !is NewsFeedUiState.Loading - if (interestsLoaded && feedLoaded) { - val localView = LocalView.current - // We use Unit to call reportFullyDrawn only on the first recomposition, - // however it will be called again if this composable goes out of scope. - // Activity.reportFullyDrawn() has its own check for this - // and is safe to call multiple times though. - LaunchedEffect(Unit) { - // We're leveraging the fact, that the current view is directly set as content of Activity. - val activity = localView.context as? Activity ?: return@LaunchedEffect - // To be sure not to call in the middle of a frame draw. - localView.doOnPreDraw { activity.reportFullyDrawn() } - } + if (interestsLoaded && feedLoaded) { + val localView = LocalView.current + // We use Unit to call reportFullyDrawn only on the first recomposition, + // however it will be called again if this composable goes out of scope. + // Activity.reportFullyDrawn() has its own check for this + // and is safe to call multiple times though. + LaunchedEffect(Unit) { + // We're leveraging the fact, that the current view is directly set as content of Activity. + val activity = localView.context as? Activity ?: return@LaunchedEffect + // To be sure not to call in the middle of a frame draw. + localView.doOnPreDraw { activity.reportFullyDrawn() } } + } - val state = rememberLazyGridState() - TrackScrollJank(scrollableState = state, stateName = "forYou:feed") - - val notConnected = stringResource(R.string.for_you_not_connected) - LaunchedEffect(isOffline) { - if (isOffline) snackbarHostState.showSnackbar( - message = notConnected, - duration = Indefinite - ) - } + val state = rememberLazyGridState() + TrackScrollJank(scrollableState = state, stateName = "forYou:feed") - LazyVerticalGrid( - columns = Adaptive(300.dp), - contentPadding = PaddingValues(16.dp), - horizontalArrangement = Arrangement.spacedBy(16.dp), - verticalArrangement = Arrangement.spacedBy(24.dp), - modifier = modifier - .padding(innerPadding) - .consumedWindowInsets(innerPadding) - .fillMaxSize() - .testTag("forYou:feed"), - state = state - ) { - interestsSelection( - interestsSelectionState = interestsSelectionState, - onAuthorCheckedChanged = onAuthorCheckedChanged, - 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() - ) + LazyVerticalGrid( + columns = Adaptive(300.dp), + contentPadding = PaddingValues(16.dp), + horizontalArrangement = Arrangement.spacedBy(16.dp), + verticalArrangement = Arrangement.spacedBy(24.dp), + modifier = modifier + .fillMaxSize() + .testTag("forYou:feed"), + state = state + ) { + interestsSelection( + interestsSelectionState = interestsSelectionState, + onAuthorCheckedChanged = onAuthorCheckedChanged, + 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) - } + ) + layout(placeable.width, placeable.height) { + placeable.place(0, 0) } - ) + } + ) - newsFeed( - feedState = feedState, - onNewsResourcesCheckedChanged = onNewsResourcesCheckedChanged, - ) + newsFeed( + feedState = feedState, + onNewsResourcesCheckedChanged = onNewsResourcesCheckedChanged, + ) - item(span = { GridItemSpan(maxLineSpan) }) { + /*item(span = { GridItemSpan(maxLineSpan) }) { Column { Spacer(modifier = Modifier.height(8.dp)) // Add space for the content to clear the "offline" snackbar. + // TODO: Check that the Scaffold handles this correctly in NiaApp if (isOffline) Spacer(modifier = Modifier.height(48.dp)) Spacer(Modifier.windowInsetsBottomHeight(WindowInsets.safeDrawing)) } - } - } - AnimatedVisibility( - visible = isSyncing || - feedState is NewsFeedUiState.Loading || - interestsSelectionState is ForYouInterestsSelectionUiState.Loading + }*/ + } + AnimatedVisibility( + visible = isSyncing || + feedState is NewsFeedUiState.Loading || + interestsSelectionState is ForYouInterestsSelectionUiState.Loading + ) { + val loadingContentDescription = stringResource(id = R.string.for_you_loading) + Box( + modifier = Modifier.fillMaxWidth() ) { - val loadingContentDescription = stringResource(id = R.string.for_you_loading) - Box( - modifier = Modifier - .padding(innerPadding) - .consumedWindowInsets(innerPadding) - .fillMaxWidth() - ) { - NiaOverlayLoadingWheel( - modifier = Modifier.align(Alignment.Center), - contentDesc = loadingContentDescription - ) - } + NiaOverlayLoadingWheel( + modifier = Modifier.align(Alignment.Center), + contentDesc = loadingContentDescription + ) } } } @@ -426,7 +391,6 @@ fun ForYouScreenPopulatedFeed() { BoxWithConstraints { NiaTheme { ForYouScreen( - isOffline = false, isSyncing = false, interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, feedState = NewsFeedUiState.Success( @@ -449,7 +413,6 @@ fun ForYouScreenOfflinePopulatedFeed() { BoxWithConstraints { NiaTheme { ForYouScreen( - isOffline = true, isSyncing = false, interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, feedState = NewsFeedUiState.Success( @@ -472,7 +435,6 @@ fun ForYouScreenTopicSelection() { BoxWithConstraints { NiaTheme { ForYouScreen( - isOffline = false, isSyncing = false, interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( topics = previewTopics.map { FollowableTopic(it, false) }, @@ -498,7 +460,6 @@ fun ForYouScreenLoading() { BoxWithConstraints { NiaTheme { ForYouScreen( - isOffline = false, isSyncing = false, interestsSelectionState = ForYouInterestsSelectionUiState.Loading, feedState = NewsFeedUiState.Loading, @@ -517,7 +478,6 @@ fun ForYouScreenPopulatedAndLoading() { BoxWithConstraints { NiaTheme { ForYouScreen( - isOffline = false, isSyncing = true, interestsSelectionState = ForYouInterestsSelectionUiState.Loading, feedState = NewsFeedUiState.Success( diff --git a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt index 8cbf39c31..8eedf40f0 100644 --- a/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt +++ b/feature/foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModel.kt @@ -94,14 +94,6 @@ class ForYouViewModel @Inject constructor( mutableStateOf>(emptySet()) } - val isOffline = networkMonitor.isOnline - .map(Boolean::not) - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = false - ) - val isSyncing = syncStatusMonitor.isSyncing .stateIn( scope = viewModelScope, diff --git a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt index 7919dda9e..9549ecb90 100644 --- a/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt +++ b/feature/foryou/src/test/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouViewModelTest.kt @@ -1404,21 +1404,6 @@ class ForYouViewModelTest { collectJob1.cancel() collectJob2.cancel() } - - @Test - fun stateIsOfflineWhenNetworkMonitorIsOffline() = runTest { - val collectJob = - launch(UnconfinedTestDispatcher()) { viewModel.isOffline.collect() } - - networkMonitor.setConnected(false) - - assertEquals( - true, - viewModel.isOffline.value - ) - - collectJob.cancel() - } } private val sampleAuthors = listOf( diff --git a/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsDialog.kt b/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsDialog.kt index b51c5b6e7..11297d5c5 100644 --- a/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsDialog.kt +++ b/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsDialog.kt @@ -61,9 +61,9 @@ import com.google.samples.apps.nowinandroid.feature.settings.SettingsUiState.Suc @ExperimentalLifecycleComposeApi @Composable -internal fun SettingsDialog( - viewModel: SettingsViewModel = hiltViewModel(), - onDismiss: () -> Unit +fun SettingsDialog( + onDismiss: () -> Unit, + viewModel: SettingsViewModel = hiltViewModel() ) { val settingsUiState by viewModel.settingsUiState.collectAsStateWithLifecycle() SettingsDialog( @@ -132,7 +132,7 @@ private fun SettingsPanel( onChangeDarkThemeConfig: (darkThemeConfig: DarkThemeConfig) -> Unit ) { SettingsDialogSectionTitle(text = stringResource(R.string.theme)) - Column { + Column(Modifier.selectableGroup()) { SettingsDialogThemeChooserRow( text = stringResource(R.string.brand_default), selected = settings.brand == DEFAULT, diff --git a/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsViewModel.kt b/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsViewModel.kt index be2927546..da72f8beb 100644 --- a/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsViewModel.kt +++ b/feature/settings/src/main/java/com/google/samples/apps/nowinandroid/feature/settings/SettingsViewModel.kt @@ -47,7 +47,12 @@ class SettingsViewModel @Inject constructor( } .stateIn( scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), + // Starting eagerly means the user data is ready when the SettingsDialog is laid out + // for the first time. Without this the layout is done using the "Loading" text, + // then replaced with the user editable fields once loaded, however, the layout + // height doesn't change meaning all the fields are squashed into a small + // scrollable column. + started = SharingStarted.Eagerly, initialValue = Loading )