From 7b30720b2526903b74b5fd0a689d5810409f60be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=82osz=20Moczkowski?= Date: Wed, 31 May 2023 17:38:08 +0200 Subject: [PATCH] Remove nested scaffold from the bookmarks screen Change-Id: Ie8b6f160d341156a6f9c02c0ca7f530095fb2950 --- .../nowinandroid/navigation/NiaNavHost.kt | 6 ++- .../samples/apps/nowinandroid/ui/NiaApp.kt | 10 +++- .../feature/bookmarks/BookmarksScreenTest.kt | 4 ++ .../feature/bookmarks/BookmarksScreen.kt | 51 ++++++++----------- .../navigation/BookmarksNavigation.kt | 7 ++- 5 files changed, 44 insertions(+), 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 index e43dfaba7..1d600b53d 100644 --- 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 index 01726c909..aa85afebd 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 @@ -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/feature/bookmarks/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreenTest.kt b/feature/bookmarks/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreenTest.kt index 680c6dcf7..6e432f2ab 100644 --- a/feature/bookmarks/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/bookmarks/BookmarksScreenTest.kt +++ b/feature/bookmarks/src/androidTest/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/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 25412e851..0f15e29b0 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 @@ -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/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt b/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt index eeb7f1576..ebcde4ab1 100644 --- a/feature/bookmarks/src/main/java/com/google/samples/apps/nowinandroid/feature/bookmarks/navigation/BookmarksNavigation.kt +++ b/feature/bookmarks/src/main/java/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) } }