Merge pull request #777 from android/bugfix/bookmarks_wrong_bg

Bookmarks screen has wrong background
android · Jun 1, 2023 · 376a715 · 376a715
2 parents caaa82b + 7b30720
commit 376a715
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 34 deletions.
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
@@ -39,6 +39,7 @@ import com.google.samples.apps.nowinandroid.ui.NiaAppState
 @Composable
 fun NiaNavHost(
     appState: NiaAppState,
+    onShowSnackbar: suspend (String, String?) -> Boolean,
     modifier: Modifier = Modifier,
     startDestination: String = forYouNavigationRoute,
 ) {
@@ -50,7 +51,10 @@ fun NiaNavHost(
     ) {
         // TODO: handle topic clicks from each top level destination
         forYouScreen(onTopicClick = {})
-        bookmarksScreen(onTopicClick = {})
+        bookmarksScreen(
+            onTopicClick = navController::navigateToTopic,
+            onShowSnackbar = onShowSnackbar,
+        )
         searchScreen(
             onBackClick = navController::popBackStack,
             onInterestsClick = { appState.navigateToTopLevelDestination(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
@@ -33,8 +33,10 @@ 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.SnackbarDuration.Short
 import androidx.compose.material3.SnackbarHost
 import androidx.compose.material3.SnackbarHostState
+import androidx.compose.material3.SnackbarResult.ActionPerformed
 import androidx.compose.material3.Text
 import androidx.compose.material3.TopAppBarDefaults
 import androidx.compose.material3.windowsizeclass.WindowSizeClass
@@ -195,7 +197,13 @@ fun NiaApp(
                             )
                         }
 
-                        NiaNavHost(appState)
+                        NiaNavHost(appState = appState, onShowSnackbar = { message, action ->
+                            snackbarHostState.showSnackbar(
+                                message = message,
+                                actionLabel = action,
+                                duration = Short,
+                            ) == ActionPerformed
+                        })
                     }
 
                     // TODO: We may want to add padding or spacer when the snackbar is shown so that

diff --git a/...idTest/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreenTest.kt b/...idTest/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreenTest.kt
@@ -50,6 +50,7 @@ class BookmarksScreenTest {
         composeTestRule.setContent {
             BookmarksScreen(
                 feedState = NewsFeedUiState.Loading,
+                onShowSnackbar = { _, _ -> false },
                 removeFromBookmarks = {},
                 onTopicClick = {},
                 onNewsResourceViewed = {},
@@ -70,6 +71,7 @@ class BookmarksScreenTest {
                 feedState = NewsFeedUiState.Success(
                     userNewsResourcesTestData.take(2),
                 ),
+                onShowSnackbar = { _, _ -> false },
                 removeFromBookmarks = {},
                 onTopicClick = {},
                 onNewsResourceViewed = {},
@@ -110,6 +112,7 @@ class BookmarksScreenTest {
                 feedState = NewsFeedUiState.Success(
                     userNewsResourcesTestData.take(2),
                 ),
+                onShowSnackbar = { _, _ -> false },
                 removeFromBookmarks = { newsResourceId ->
                     assertEquals(userNewsResourcesTestData[0].id, newsResourceId)
                     removeFromBookmarksCalled = true
@@ -144,6 +147,7 @@ class BookmarksScreenTest {
         composeTestRule.setContent {
             BookmarksScreen(
                 feedState = NewsFeedUiState.Success(emptyList()),
+                onShowSnackbar = { _, _ -> false },
                 removeFromBookmarks = {},
                 onTopicClick = {},
                 onNewsResourceViewed = {},

diff --git a/...s/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt b/...s/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreen.kt
@@ -19,7 +19,6 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks
 import androidx.annotation.VisibleForTesting
 import androidx.compose.foundation.Image
 import androidx.compose.foundation.layout.Arrangement
-import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.PaddingValues
 import androidx.compose.foundation.layout.Spacer
@@ -35,19 +34,12 @@ import androidx.compose.foundation.lazy.grid.GridCells.Adaptive
 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.MaterialTheme
-import androidx.compose.material3.Scaffold
-import androidx.compose.material3.SnackbarDuration.Short
-import androidx.compose.material3.SnackbarHost
-import androidx.compose.material3.SnackbarHostState
-import androidx.compose.material3.SnackbarResult.ActionPerformed
 import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
 import androidx.compose.runtime.DisposableEffect
 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.ColorFilter
@@ -79,12 +71,14 @@ import com.google.samples.apps.nowinandroid.core.ui.newsFeed
 @Composable
 internal fun BookmarksRoute(
     onTopicClick: (String) -> Unit,
+    onShowSnackbar: suspend (String, String?) -> Boolean,
     modifier: Modifier = Modifier,
     viewModel: BookmarksViewModel = hiltViewModel(),
 ) {
     val feedState by viewModel.feedUiState.collectAsStateWithLifecycle()
     BookmarksScreen(
         feedState = feedState,
+        onShowSnackbar = onShowSnackbar,
         removeFromBookmarks = viewModel::removeFromSavedResources,
         onNewsResourceViewed = { viewModel.setNewsResourceViewed(it, true) },
         onTopicClick = onTopicClick,
@@ -98,11 +92,11 @@ internal fun BookmarksRoute(
 /**
  * Displays the user's bookmarked articles. Includes support for loading and empty states.
  */
-@OptIn(ExperimentalMaterial3Api::class)
 @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE)
 @Composable
 internal fun BookmarksScreen(
     feedState: NewsFeedUiState,
+    onShowSnackbar: suspend (String, String?) -> Boolean,
     removeFromBookmarks: (String) -> Unit,
     onNewsResourceViewed: (String) -> Unit,
     onTopicClick: (String) -> Unit,
@@ -113,18 +107,14 @@ internal fun BookmarksScreen(
 ) {
     val bookmarkRemovedMessage = stringResource(id = R.string.bookmark_removed)
     val undoText = stringResource(id = R.string.undo)
-    val snackbarHostState = remember { SnackbarHostState() }
 
     LaunchedEffect(shouldDisplayUndoBookmark) {
         if (shouldDisplayUndoBookmark) {
-            val snackBarResult = snackbarHostState.showSnackbar(
-                message = bookmarkRemovedMessage,
-                actionLabel = undoText,
-                duration = Short,
-            )
-            when (snackBarResult) {
-                ActionPerformed -> { undoBookmarkRemoval() }
-                else -> { clearUndoState() }
+            val snackBarResult = onShowSnackbar(bookmarkRemovedMessage, undoText)
+            if (snackBarResult) {
+                undoBookmarkRemoval()
+            } else {
+                clearUndoState()
             }
         }
     }
@@ -140,20 +130,21 @@ internal fun BookmarksScreen(
         onDispose { lifecycleOwner.lifecycle.removeObserver(observer) }
     }
 
-    Scaffold(snackbarHost = { SnackbarHost(hostState = snackbarHostState) }) {
-        Box(
-            modifier = Modifier.padding(it).fillMaxSize(),
-        ) {
-            when (feedState) {
-                Loading -> LoadingState(modifier)
-                is Success -> if (feedState.feed.isNotEmpty()) {
-                    BookmarksGrid(feedState, removeFromBookmarks, onNewsResourceViewed, onTopicClick, modifier)
-                } else {
-                    EmptyState(modifier)
-                }
-            }
+    when (feedState) {
+        Loading -> LoadingState(modifier)
+        is Success -> if (feedState.feed.isNotEmpty()) {
+            BookmarksGrid(
+                feedState,
+                removeFromBookmarks,
+                onNewsResourceViewed,
+                onTopicClick,
+                modifier,
+            )
+        } else {
+            EmptyState(modifier)
         }
     }
+
     TrackScreenViewEvent(screenName = "Saved")
 }
 

diff --git a/.../com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt b/.../com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt
@@ -28,8 +28,11 @@ fun NavController.navigateToBookmarks(navOptions: NavOptions? = null) {
     this.navigate(bookmarksRoute, navOptions)
 }
 
-fun NavGraphBuilder.bookmarksScreen(onTopicClick: (String) -> Unit) {
+fun NavGraphBuilder.bookmarksScreen(
+    onTopicClick: (String) -> Unit,
+    onShowSnackbar: suspend (String, String?) -> Boolean,
+) {
     composable(route = bookmarksRoute) {
-        BookmarksRoute(onTopicClick)
+        BookmarksRoute(onTopicClick, onShowSnackbar)
     }
 }