From 40cf1b817198098c58235179e259f40f469086c4 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Fri, 15 Jul 2022 15:24:22 +0200 Subject: [PATCH] [NiA] Implement LazyVerticalGrid for Feed Change-Id: I0fd51bdd4c64a3d9ceaba05244d384bb9b463315 --- .../nowinandroid/navigation/NiaNavHost.kt | 8 +- .../samples/apps/nowinandroid/ui/NiaApp.kt | 1 - .../BaselineProfileGenerator.kt | 4 +- .../BookmarksActions.kt} | 12 +- .../apps/nowinandroid/core/ui/NewsFeed.kt | 117 +++----- .../feature/bookmarks/BookmarksScreenTest.kt | 69 ++--- .../feature/bookmarks/BookmarksScreen.kt | 55 ++-- .../navigation/BookmarksNavigation.kt | 7 +- feature-foryou/build.gradle.kts | 2 - .../feature/foryou/ForYouScreenTest.kt | 195 +++---------- .../feature/foryou/ForYouScreen.kt | 257 ++++++++---------- .../foryou/navigation/ForYouNavigation.kt | 7 +- 12 files changed, 239 insertions(+), 495 deletions(-) rename benchmark/src/main/java/com/google/samples/apps/nowinandroid/{saved/SavedActions.kt => bookmarks/BookmarksActions.kt} (70%) 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 92c0000e2..6d2206743 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 @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.navigation -import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.compose.runtime.Composable import androidx.compose.ui.Modifier import androidx.navigation.NavHostController @@ -43,7 +42,6 @@ fun NiaNavHost( navController: NavHostController, onNavigateToDestination: (NiaNavigationDestination, String) -> Unit, onBackClick: () -> Unit, - windowSizeClass: WindowSizeClass, modifier: Modifier = Modifier, startDestination: String = ForYouDestination.route ) { @@ -52,10 +50,8 @@ fun NiaNavHost( startDestination = startDestination, modifier = modifier, ) { - forYouGraph( - windowSizeClass = windowSizeClass - ) - bookmarksGraph(windowSizeClass) + forYouGraph() + bookmarksGraph() interestsGraph( navigateToTopic = { onNavigateToDestination( 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 29fc62b67..5bb603911 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 @@ -105,7 +105,6 @@ fun NiaApp( navController = appState.navController, onBackClick = appState::onBackClick, onNavigateToDestination = appState::navigate, - windowSizeClass = appState.windowSizeClass, modifier = Modifier .padding(padding) .consumedWindowInsets(padding) diff --git a/benchmark/src/main/java/com/google/samples/apps/nowinandroid/baselineprofile/BaselineProfileGenerator.kt b/benchmark/src/main/java/com/google/samples/apps/nowinandroid/baselineprofile/BaselineProfileGenerator.kt index 4c0cb34dd..159506bc8 100644 --- a/benchmark/src/main/java/com/google/samples/apps/nowinandroid/baselineprofile/BaselineProfileGenerator.kt +++ b/benchmark/src/main/java/com/google/samples/apps/nowinandroid/baselineprofile/BaselineProfileGenerator.kt @@ -20,12 +20,12 @@ import androidx.benchmark.macro.ExperimentalBaselineProfilesApi import androidx.benchmark.macro.junit4.BaselineProfileRule import androidx.test.uiautomator.By import com.google.samples.apps.nowinandroid.PACKAGE_NAME +import com.google.samples.apps.nowinandroid.bookmarks.bookmarksScrollFeedDownUp import com.google.samples.apps.nowinandroid.foryou.forYouScrollFeedDownUp import com.google.samples.apps.nowinandroid.foryou.forYouSelectAuthors import com.google.samples.apps.nowinandroid.foryou.forYouWaitForContent import com.google.samples.apps.nowinandroid.interests.interestsScrollPeopleDownUp import com.google.samples.apps.nowinandroid.interests.interestsScrollTopicsDownUp -import com.google.samples.apps.nowinandroid.saved.savedScrollFeedDownUp import org.junit.Rule import org.junit.Test @@ -55,7 +55,7 @@ class BaselineProfileGenerator { device.findObject(By.text("Saved")).click() device.waitForIdle() - savedScrollFeedDownUp() + bookmarksScrollFeedDownUp() // Navigate to interests screen device.findObject(By.text("Interests")).click() diff --git a/benchmark/src/main/java/com/google/samples/apps/nowinandroid/saved/SavedActions.kt b/benchmark/src/main/java/com/google/samples/apps/nowinandroid/bookmarks/BookmarksActions.kt similarity index 70% rename from benchmark/src/main/java/com/google/samples/apps/nowinandroid/saved/SavedActions.kt rename to benchmark/src/main/java/com/google/samples/apps/nowinandroid/bookmarks/BookmarksActions.kt index 39bd0d965..f5cde3795 100644 --- a/benchmark/src/main/java/com/google/samples/apps/nowinandroid/saved/SavedActions.kt +++ b/benchmark/src/main/java/com/google/samples/apps/nowinandroid/bookmarks/BookmarksActions.kt @@ -14,20 +14,14 @@ * limitations under the License. */ -package com.google.samples.apps.nowinandroid.saved +package com.google.samples.apps.nowinandroid.bookmarks import androidx.benchmark.macro.MacrobenchmarkScope import androidx.test.uiautomator.By import androidx.test.uiautomator.Direction -import androidx.test.uiautomator.Until -fun MacrobenchmarkScope.savedWaitForContent() { - // Wait until content is loaded - device.wait(Until.hasObject(By.res("saved:feed")), 30_000) -} - -fun MacrobenchmarkScope.savedScrollFeedDownUp() { - val feedList = device.findObject(By.res("saved:feed")) +fun MacrobenchmarkScope.bookmarksScrollFeedDownUp() { + val feedList = device.findObject(By.res("bookmarks:feed")) feedList.fling(Direction.DOWN) device.waitForIdle() feedList.fling(Direction.UP) diff --git a/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsFeed.kt b/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsFeed.kt index c36ba72f1..400cc1438 100644 --- a/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsFeed.kt +++ b/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsFeed.kt @@ -18,18 +18,19 @@ package com.google.samples.apps.nowinandroid.core.ui import android.content.Intent import android.net.Uri -import androidx.annotation.IntRange import androidx.annotation.StringRes -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.wrapContentSize -import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope -import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.lazy.grid.GridCells +import androidx.compose.foundation.lazy.grid.GridItemSpan +import androidx.compose.foundation.lazy.grid.LazyGridScope +import androidx.compose.foundation.lazy.grid.LazyVerticalGrid +import androidx.compose.foundation.lazy.grid.items import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource @@ -50,17 +51,16 @@ import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources * [feedState] is loading. This allows a caller to suppress a loading visual if one is already * present in the UI elsewhere. */ -fun LazyListScope.NewsFeed( +fun LazyGridScope.newsFeed( feedState: NewsFeedUiState, showLoadingUIIfLoading: Boolean, @StringRes loadingContentDescription: Int, - @IntRange(from = 1) numberOfColumns: Int, onNewsResourcesCheckedChanged: (String, Boolean) -> Unit ) { when (feedState) { NewsFeedUiState.Loading -> { if (showLoadingUIIfLoading) { - item { + item(span = { GridItemSpan(maxLineSpan) }) { NiaLoadingWheel( modifier = Modifier .fillMaxWidth() @@ -71,56 +71,24 @@ fun LazyListScope.NewsFeed( } } is NewsFeedUiState.Success -> { - items( - feedState.feed.chunked(numberOfColumns) - ) { saveableNewsResources -> - Row( - modifier = Modifier.padding( - top = 32.dp, - start = 16.dp, - end = 16.dp - ), - horizontalArrangement = Arrangement.spacedBy(32.dp) - ) { - // The last row may not be complete, but for a consistent grid - // structure we still want an element taking up the empty space. - // Therefore, the last row may have empty boxes. - repeat(numberOfColumns) { index -> - Box( - modifier = Modifier.weight(1f) - ) { - val saveableNewsResource = - saveableNewsResources.getOrNull(index) - - if (saveableNewsResource != null) { - val launchResourceIntent = - Intent( - Intent.ACTION_VIEW, - Uri.parse(saveableNewsResource.newsResource.url) - ) - val context = LocalContext.current + items(feedState.feed, key = { it.newsResource.id }) { saveableNewsResource -> + val resourceUrl by remember { + mutableStateOf(Uri.parse(saveableNewsResource.newsResource.url)) + } + val launchResourceIntent = Intent(Intent.ACTION_VIEW, resourceUrl) + val context = LocalContext.current - NewsResourceCardExpanded( - newsResource = saveableNewsResource.newsResource, - isBookmarked = saveableNewsResource.isSaved, - onClick = { - ContextCompat.startActivity( - context, - launchResourceIntent, - null - ) - }, - onToggleBookmark = { - onNewsResourcesCheckedChanged( - saveableNewsResource.newsResource.id, - !saveableNewsResource.isSaved - ) - } - ) - } - } + NewsResourceCardExpanded( + newsResource = saveableNewsResource.newsResource, + isBookmarked = saveableNewsResource.isSaved, + onClick = { ContextCompat.startActivity(context, launchResourceIntent, null) }, + onToggleBookmark = { + onNewsResourcesCheckedChanged( + saveableNewsResource.newsResource.id, + !saveableNewsResource.isSaved + ) } - } + ) } } } @@ -150,12 +118,11 @@ sealed interface NewsFeedUiState { @Composable fun NewsFeedLoadingPreview() { NiaTheme { - LazyColumn { - NewsFeed( + LazyVerticalGrid(columns = GridCells.Adaptive(300.dp)) { + newsFeed( feedState = NewsFeedUiState.Loading, showLoadingUIIfLoading = true, loadingContentDescription = 0, - numberOfColumns = 1, onNewsResourcesCheckedChanged = { _, _ -> } ) } @@ -163,39 +130,19 @@ fun NewsFeedLoadingPreview() { } @Preview -@Composable -fun NewsFeedSingleColumnPreview() { - NiaTheme { - LazyColumn { - NewsFeed( - feedState = NewsFeedUiState.Success( - previewNewsResources.map { - SaveableNewsResource(it, false) - } - ), - showLoadingUIIfLoading = true, - loadingContentDescription = 0, - numberOfColumns = 1, - onNewsResourcesCheckedChanged = { _, _ -> } - ) - } - } -} - @Preview(device = Devices.TABLET) @Composable -fun NewsFeedTwoColumnPreview() { +fun NewsFeedContentPreview() { NiaTheme { - LazyColumn { - NewsFeed( + LazyVerticalGrid(columns = GridCells.Adaptive(300.dp)) { + newsFeed( feedState = NewsFeedUiState.Success( - (previewNewsResources + previewNewsResources).map { + previewNewsResources.map { SaveableNewsResource(it, false) } ), showLoadingUIIfLoading = true, loadingContentDescription = 0, - numberOfColumns = 2, onNewsResourcesCheckedChanged = { _, _ -> } ) } 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 de79120cc..713741dee 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 @@ -17,8 +17,6 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks import androidx.activity.ComponentActivity -import androidx.compose.foundation.layout.BoxWithConstraints -import androidx.compose.material3.windowsizeclass.ExperimentalMaterial3WindowSizeClassApi import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.compose.ui.test.assertCountEquals import androidx.compose.ui.test.assertHasClickAction @@ -33,7 +31,6 @@ import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollToNode -import androidx.compose.ui.unit.DpSize import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState @@ -45,7 +42,6 @@ import org.junit.Test /** * UI tests for [BookmarksScreen] composable. */ -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) class BookmarksScreenTest { @get:Rule @@ -53,18 +49,11 @@ class BookmarksScreenTest { @Test fun loading_showsLoadingSpinner() { - lateinit var windowSizeClass: WindowSizeClass composeTestRule.setContent { - BoxWithConstraints { - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ) - BookmarksScreen( - windowSizeClass = windowSizeClass, - feedState = NewsFeedUiState.Loading, - removeFromBookmarks = { } - ) - } + BookmarksScreen( + feedState = NewsFeedUiState.Loading, + removeFromBookmarks = { } + ) } composeTestRule @@ -79,20 +68,13 @@ class BookmarksScreenTest { lateinit var windowSizeClass: WindowSizeClass composeTestRule.setContent { - BoxWithConstraints { - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ) - - BookmarksScreen( - windowSizeClass = windowSizeClass, - feedState = NewsFeedUiState.Success( - previewNewsResources.take(2) - .map { SaveableNewsResource(it, true) } - ), - removeFromBookmarks = { } - ) - } + BookmarksScreen( + feedState = NewsFeedUiState.Success( + previewNewsResources.take(2) + .map { SaveableNewsResource(it, true) } + ), + removeFromBookmarks = { } + ) } composeTestRule @@ -122,28 +104,19 @@ class BookmarksScreenTest { @Test fun feed_whenRemovingBookmark_removesBookmark() { - lateinit var windowSizeClass: WindowSizeClass - var removeFromBookmarksCalled = false composeTestRule.setContent { - BoxWithConstraints { - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ) - - BookmarksScreen( - windowSizeClass = windowSizeClass, - feedState = NewsFeedUiState.Success( - previewNewsResources.take(2) - .map { SaveableNewsResource(it, true) } - ), - removeFromBookmarks = { newsResourceId -> - assertEquals(previewNewsResources[0].id, newsResourceId) - removeFromBookmarksCalled = true - } - ) - } + BookmarksScreen( + feedState = NewsFeedUiState.Success( + previewNewsResources.take(2) + .map { SaveableNewsResource(it, true) } + ), + removeFromBookmarks = { newsResourceId -> + assertEquals(previewNewsResources[0].id, newsResourceId) + removeFromBookmarksCalled = true + } + ) } composeTestRule 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 082a7e833..06a9fdacf 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 @@ -16,7 +16,7 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks -import androidx.compose.foundation.layout.BoxWithConstraints +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.ExperimentalLayoutApi import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Spacer @@ -29,12 +29,12 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.safeDrawing import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.layout.windowInsetsPadding -import androidx.compose.foundation.lazy.LazyColumn +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.material3.ExperimentalMaterial3Api import androidx.compose.material3.Scaffold import androidx.compose.material3.TopAppBarDefaults -import androidx.compose.material3.windowsizeclass.WindowSizeClass -import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue @@ -47,19 +47,16 @@ import androidx.hilt.navigation.compose.hiltViewModel import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaGradientBackground import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaTopAppBar import com.google.samples.apps.nowinandroid.core.designsystem.icon.NiaIcons -import com.google.samples.apps.nowinandroid.core.ui.NewsFeed import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState -import kotlin.math.floor +import com.google.samples.apps.nowinandroid.core.ui.newsFeed @Composable fun BookmarksRoute( - windowSizeClass: WindowSizeClass, modifier: Modifier = Modifier, viewModel: BookmarksViewModel = hiltViewModel() ) { val feedState by viewModel.feedState.collectAsState() BookmarksScreen( - windowSizeClass = windowSizeClass, feedState = feedState, removeFromBookmarks = viewModel::removeFromSavedResources, modifier = modifier @@ -69,7 +66,6 @@ fun BookmarksRoute( @OptIn(ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class) @Composable fun BookmarksScreen( - windowSizeClass: WindowSizeClass, feedState: NewsFeedUiState, removeFromBookmarks: (String) -> Unit, modifier: Modifier = Modifier @@ -97,37 +93,26 @@ fun BookmarksScreen( }, containerColor = Color.Transparent ) { innerPadding -> - // TODO: Replace with `LazyVerticalGrid` when blocking bugs are fixed: - // https://issuetracker.google.com/issues/230514914 - // https://issuetracker.google.com/issues/231320714 - BoxWithConstraints( + LazyVerticalGrid( + columns = Adaptive(300.dp), + contentPadding = PaddingValues(16.dp), + horizontalArrangement = Arrangement.spacedBy(32.dp), + verticalArrangement = Arrangement.spacedBy(24.dp), modifier = modifier + .fillMaxSize() + .testTag("bookmarks:feed") .padding(innerPadding) .consumedWindowInsets(innerPadding) ) { - val numberOfColumns = when (windowSizeClass.widthSizeClass) { - WindowWidthSizeClass.Compact, WindowWidthSizeClass.Medium -> 1 - else -> floor(maxWidth / 300.dp).toInt().coerceAtLeast(1) - } - - LazyColumn( - modifier = Modifier - .fillMaxSize() - .testTag("saved:feed"), - contentPadding = PaddingValues(bottom = 16.dp) - ) { - - NewsFeed( - feedState = feedState, - numberOfColumns = numberOfColumns, - onNewsResourcesCheckedChanged = { id, _ -> removeFromBookmarks(id) }, - showLoadingUIIfLoading = true, - loadingContentDescription = R.string.saved_loading - ) + newsFeed( + feedState = feedState, + onNewsResourcesCheckedChanged = { id, _ -> removeFromBookmarks(id) }, + showLoadingUIIfLoading = true, + loadingContentDescription = R.string.saved_loading + ) - item { - Spacer(Modifier.windowInsetsBottomHeight(WindowInsets.safeDrawing)) - } + item(span = { GridItemSpan(maxLineSpan) }) { + Spacer(Modifier.windowInsetsBottomHeight(WindowInsets.safeDrawing)) } } } 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 a97562a74..bf64eb34c 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 @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.feature.bookmarks.navigation -import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.navigation.NavGraphBuilder import androidx.navigation.compose.composable import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination @@ -27,10 +26,8 @@ object BookmarksDestination : NiaNavigationDestination { override val destination = "bookmarks_destination" } -fun NavGraphBuilder.bookmarksGraph( - windowSizeClass: WindowSizeClass -) { +fun NavGraphBuilder.bookmarksGraph() { composable(route = BookmarksDestination.route) { - BookmarksRoute(windowSizeClass) + BookmarksRoute() } } diff --git a/feature-foryou/build.gradle.kts b/feature-foryou/build.gradle.kts index 5303a2782..e9fb9d0b0 100644 --- a/feature-foryou/build.gradle.kts +++ b/feature-foryou/build.gradle.kts @@ -25,7 +25,5 @@ plugins { dependencies { implementation(libs.kotlinx.datetime) - implementation(libs.androidx.compose.material3.windowSizeClass) - implementation(libs.accompanist.flowlayout) } diff --git a/feature-foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt b/feature-foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt index 554928f84..206c709d2 100644 --- a/feature-foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt +++ b/feature-foryou/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/foryou/ForYouScreenTest.kt @@ -18,11 +18,7 @@ package com.google.samples.apps.nowinandroid.feature.foryou import androidx.activity.ComponentActivity import androidx.compose.foundation.layout.BoxWithConstraints -import androidx.compose.material3.windowsizeclass.ExperimentalMaterial3WindowSizeClassApi -import androidx.compose.material3.windowsizeclass.WindowSizeClass -import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass import androidx.compose.ui.test.assertHasClickAction -import androidx.compose.ui.test.assertIsDisplayed import androidx.compose.ui.test.assertIsEnabled import androidx.compose.ui.test.assertIsNotEnabled import androidx.compose.ui.test.hasContentDescription @@ -33,21 +29,16 @@ import androidx.compose.ui.test.onFirst import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText import androidx.compose.ui.test.performScrollToNode -import androidx.compose.ui.unit.DpSize import com.google.samples.apps.nowinandroid.core.model.data.Author import com.google.samples.apps.nowinandroid.core.model.data.FollowableAuthor import com.google.samples.apps.nowinandroid.core.model.data.FollowableTopic -import com.google.samples.apps.nowinandroid.core.model.data.NewsResource -import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.Topic +import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState -import kotlinx.datetime.Instant -import org.junit.Assert import org.junit.Rule import org.junit.Test -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) class ForYouScreenTest { @get:Rule val composeTestRule = createAndroidComposeRule() @@ -63,13 +54,10 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.Loading, feedState = NewsFeedUiState.Loading, - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -88,9 +76,6 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( topics = testTopics, @@ -99,8 +84,8 @@ class ForYouScreenTest { feedState = NewsFeedUiState.Success( feed = emptyList() ), - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -110,14 +95,14 @@ class ForYouScreenTest { testAuthors.forEach { testAuthor -> composeTestRule .onNodeWithText(testAuthor.author.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } testTopics.forEach { testTopic -> composeTestRule .onNodeWithText(testTopic.topic.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } @@ -129,7 +114,7 @@ class ForYouScreenTest { composeTestRule .onNode(doneButtonMatcher) - .assertIsDisplayed() + .assertExists() .assertIsNotEnabled() .assertHasClickAction() } @@ -139,9 +124,6 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( // Follow one topic @@ -153,8 +135,8 @@ class ForYouScreenTest { feedState = NewsFeedUiState.Success( feed = emptyList() ), - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -164,14 +146,14 @@ class ForYouScreenTest { testAuthors.forEach { testAuthor -> composeTestRule .onNodeWithText(testAuthor.author.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } testTopics.forEach { testTopic -> composeTestRule .onNodeWithText(testTopic.topic.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } @@ -183,7 +165,7 @@ class ForYouScreenTest { composeTestRule .onNode(doneButtonMatcher) - .assertIsDisplayed() + .assertExists() .assertIsEnabled() .assertHasClickAction() } @@ -193,9 +175,6 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( // Follow one topic @@ -207,8 +186,8 @@ class ForYouScreenTest { feedState = NewsFeedUiState.Success( feed = emptyList() ), - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -218,14 +197,14 @@ class ForYouScreenTest { testAuthors.forEach { testAuthor -> composeTestRule .onNodeWithText(testAuthor.author.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } testTopics.forEach { testTopic -> composeTestRule .onNodeWithText(testTopic.topic.name) - .assertIsDisplayed() + .assertExists() .assertHasClickAction() } @@ -237,7 +216,7 @@ class ForYouScreenTest { composeTestRule .onNode(doneButtonMatcher) - .assertIsDisplayed() + .assertExists() .assertIsEnabled() .assertHasClickAction() } @@ -247,17 +226,14 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( topics = testTopics, authors = testAuthors ), feedState = NewsFeedUiState.Loading, - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -286,13 +262,10 @@ class ForYouScreenTest { composeTestRule.setContent { BoxWithConstraints { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ), interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, feedState = NewsFeedUiState.Loading, - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -318,60 +291,44 @@ class ForYouScreenTest { @Test fun feed_whenNoInterestsSelectionAndLoaded_showsFeed() { - lateinit var windowSizeClass: WindowSizeClass - composeTestRule.setContent { - BoxWithConstraints { - windowSizeClass = WindowSizeClass.calculateFromSize( - DpSize(maxWidth, maxHeight) - ) - - ForYouScreen( - windowSizeClass = windowSizeClass, - interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, - feedState = NewsFeedUiState.Success( - feed = testNewsResources - ), - onAuthorCheckedChanged = { _, _ -> }, - onTopicCheckedChanged = { _, _ -> }, - saveFollowedTopics = {}, - onNewsResourcesCheckedChanged = { _, _ -> } - ) - } + ForYouScreen( + interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, + feedState = NewsFeedUiState.Success( + feed = previewNewsResources.map { + SaveableNewsResource(it, false) + } + ), + onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, + saveFollowedTopics = {}, + onNewsResourcesCheckedChanged = { _, _ -> } + ) } - val firstFeedItem = composeTestRule + composeTestRule .onNodeWithText( - testNewsResources[0].newsResource.title, + previewNewsResources[0].title, substring = true ) + .assertExists() .assertHasClickAction() - .fetchSemanticsNode() - val secondFeedItem = composeTestRule + composeTestRule.onNode(hasScrollToNodeAction()) + .performScrollToNode( + hasText( + previewNewsResources[1].title, + substring = true + ) + ) + + composeTestRule .onNodeWithText( - testNewsResources[1].newsResource.title, + previewNewsResources[1].title, substring = true ) + .assertExists() .assertHasClickAction() - .fetchSemanticsNode() - - when (windowSizeClass.widthSizeClass) { - WindowWidthSizeClass.Compact, WindowWidthSizeClass.Medium -> { - // On smaller screen widths, the second feed item should be below the first because - // they are displayed in a single column - Assert.assertTrue( - firstFeedItem.positionInRoot.y < secondFeedItem.positionInRoot.y - ) - } - else -> { - // On larger screen widths, the second feed item should be inline with the first - // because they are displayed in more than one column - Assert.assertTrue( - firstFeedItem.positionInRoot.y == secondFeedItem.positionInRoot.y - ) - } - } } } @@ -415,71 +372,3 @@ private val testAuthors = listOf( isFollowed = false ), ) -private val testNewsResources = listOf( - SaveableNewsResource( - newsResource = NewsResource( - id = "1", - episodeId = "52", - title = "Small Title", - content = "small.", - url = "https://youtu.be/-fJ6poHQrjM", - headerImageUrl = null, - publishDate = Instant.parse("2021-11-09T00:00:00.000Z"), - type = Video, - topics = emptyList(), - authors = emptyList() - ), - isSaved = false - ), - SaveableNewsResource( - newsResource = NewsResource( - id = "2", - episodeId = "52", - title = "Transformations and customisations in the Paging Library", - content = "A demonstration of different operations that can be performed " + - "with Paging. Transformations like inserting separators, when to " + - "create a new pager, and customisation options for consuming " + - "PagingData.", - url = "https://youtu.be/ZARz0pjm5YM", - headerImageUrl = "https://i.ytimg.com/vi/ZARz0pjm5YM/maxresdefault.jpg", - publishDate = Instant.parse("2021-11-01T00:00:00.000Z"), - type = Video, - topics = listOf( - Topic( - id = "1", - name = "UI", - shortDescription = "", - longDescription = "", - url = "", - imageUrl = "" - ), - ), - authors = emptyList() - ), - isSaved = false - ), - SaveableNewsResource( - newsResource = NewsResource( - id = "3", - episodeId = "52", - title = "Community tip on Paging", - content = "Tips for using the Paging library from the developer community", - url = "https://youtu.be/r5JgIyS3t3s", - headerImageUrl = "https://i.ytimg.com/vi/r5JgIyS3t3s/maxresdefault.jpg", - publishDate = Instant.parse("2021-11-08T00:00:00.000Z"), - type = Video, - topics = listOf( - Topic( - id = "1", - name = "UI", - shortDescription = "", - longDescription = "", - url = "", - imageUrl = "" - ), - ), - authors = emptyList() - ), - isSaved = false - ), -) 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 8a97d2336..4e2c41810 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 @@ -19,6 +19,7 @@ package com.google.samples.apps.nowinandroid.feature.foryou import android.app.Activity import androidx.compose.foundation.layout.Arrangement 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 @@ -37,14 +38,15 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.layout.windowInsetsBottomHeight import androidx.compose.foundation.layout.windowInsetsPadding import androidx.compose.foundation.layout.wrapContentSize -import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.grid.GridCells +import androidx.compose.foundation.lazy.grid.GridCells.Adaptive +import androidx.compose.foundation.lazy.grid.GridItemSpan +import androidx.compose.foundation.lazy.grid.LazyGridScope import androidx.compose.foundation.lazy.grid.LazyHorizontalGrid +import androidx.compose.foundation.lazy.grid.LazyVerticalGrid import androidx.compose.foundation.lazy.grid.items import androidx.compose.foundation.lazy.grid.rememberLazyGridState -import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CornerSize import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material3.ExperimentalMaterial3Api @@ -54,9 +56,6 @@ import androidx.compose.material3.Scaffold import androidx.compose.material3.Surface import androidx.compose.material3.Text import androidx.compose.material3.TopAppBarDefaults -import androidx.compose.material3.windowsizeclass.ExperimentalMaterial3WindowSizeClassApi -import androidx.compose.material3.windowsizeclass.WindowSizeClass -import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue @@ -70,7 +69,6 @@ import androidx.compose.ui.res.painterResource import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview -import androidx.compose.ui.unit.DpSize import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.max import androidx.compose.ui.unit.sp @@ -93,36 +91,32 @@ import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.model.data.previewAuthors import com.google.samples.apps.nowinandroid.core.model.data.previewNewsResources import com.google.samples.apps.nowinandroid.core.model.data.previewTopics -import com.google.samples.apps.nowinandroid.core.ui.NewsFeed import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank -import kotlin.math.floor +import com.google.samples.apps.nowinandroid.core.ui.newsFeed @OptIn(ExperimentalLifecycleComposeApi::class) @Composable fun ForYouRoute( - windowSizeClass: WindowSizeClass, modifier: Modifier = Modifier, viewModel: ForYouViewModel = hiltViewModel() ) { val interestsSelectionState by viewModel.interestsSelectionState.collectAsStateWithLifecycle() val feedState by viewModel.feedState.collectAsStateWithLifecycle() ForYouScreen( - windowSizeClass = windowSizeClass, - modifier = modifier, interestsSelectionState = interestsSelectionState, feedState = feedState, onTopicCheckedChanged = viewModel::updateTopicSelection, onAuthorCheckedChanged = viewModel::updateAuthorSelection, saveFollowedTopics = viewModel::saveFollowedInterests, - onNewsResourcesCheckedChanged = viewModel::updateNewsResourceSaved + onNewsResourcesCheckedChanged = viewModel::updateNewsResourceSaved, + modifier = modifier ) } @OptIn(ExperimentalMaterial3Api::class, ExperimentalLayoutApi::class) @Composable fun ForYouScreen( - windowSizeClass: WindowSizeClass, interestsSelectionState: ForYouInterestsSelectionUiState, feedState: NewsFeedUiState, onTopicCheckedChanged: (String, Boolean) -> Unit, @@ -154,73 +148,63 @@ fun ForYouScreen( }, containerColor = Color.Transparent ) { innerPadding -> - // TODO: Replace with `LazyVerticalGrid` when blocking bugs are fixed: - // https://issuetracker.google.com/issues/230514914 - // https://issuetracker.google.com/issues/231320714 - BoxWithConstraints( - modifier = modifier - .padding(innerPadding) - .consumedWindowInsets(innerPadding) - ) { - val numberOfColumns = when (windowSizeClass.widthSizeClass) { - WindowWidthSizeClass.Compact, WindowWidthSizeClass.Medium -> 1 - else -> floor(maxWidth / 300.dp).toInt().coerceAtLeast(1) - } - - // 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 tag = "forYou:feed" + val tag = "forYou:feed" - val lazyListState = rememberLazyListState() - TrackScrollJank(scrollableState = lazyListState, stateName = tag) + val lazyGridState = rememberLazyGridState() + TrackScrollJank(scrollableState = lazyGridState, stateName = tag) - LazyColumn( - modifier = Modifier - .fillMaxSize() - .testTag(tag), - state = lazyListState, - ) { - InterestsSelection( - interestsSelectionState = interestsSelectionState, - showLoadingUIIfLoading = true, - onAuthorCheckedChanged = onAuthorCheckedChanged, - onTopicCheckedChanged = onTopicCheckedChanged, - saveFollowedTopics = saveFollowedTopics - ) + LazyVerticalGrid( + columns = Adaptive(300.dp), + contentPadding = PaddingValues(16.dp), + horizontalArrangement = Arrangement.spacedBy(32.dp), + verticalArrangement = Arrangement.spacedBy(24.dp), + modifier = modifier + .padding(innerPadding) + .consumedWindowInsets(innerPadding) + .fillMaxSize() + .testTag("forYou:feed"), + state = lazyGridState + ) { + interestsSelection( + interestsSelectionState = interestsSelectionState, + onAuthorCheckedChanged = onAuthorCheckedChanged, + onTopicCheckedChanged = onTopicCheckedChanged, + saveFollowedTopics = saveFollowedTopics + ) - NewsFeed( - feedState = feedState, - // Avoid showing a second loading wheel if we already are for the interests - // selection - showLoadingUIIfLoading = - interestsSelectionState !is ForYouInterestsSelectionUiState.Loading, - numberOfColumns = numberOfColumns, - onNewsResourcesCheckedChanged = onNewsResourcesCheckedChanged, - loadingContentDescription = R.string.for_you_loading - ) + newsFeed( + feedState = feedState, + // Avoid showing a second loading wheel if we already are for the interests + // selection + showLoadingUIIfLoading = + interestsSelectionState !is ForYouInterestsSelectionUiState.Loading, + onNewsResourcesCheckedChanged = onNewsResourcesCheckedChanged, + loadingContentDescription = R.string.for_you_loading + ) - item { - Spacer(Modifier.windowInsetsBottomHeight(WindowInsets.safeDrawing)) - } + item(span = { GridItemSpan(maxLineSpan) }) { + Spacer(Modifier.windowInsetsBottomHeight(WindowInsets.safeDrawing)) } } } @@ -231,85 +215,76 @@ fun ForYouScreen( * An extension on [LazyListScope] defining the interests selection portion of the for you screen. * Depending on the [interestsSelectionState], this might emit no items. * - * @param showLoadingUIIfLoading if true, show a visual indication of loading if the + * @param showLoaderWhenLoading if true, show a visual indication of loading if the * [interestsSelectionState] is loading. This is controllable to permit du-duplicating loading * states. */ -private fun LazyListScope.InterestsSelection( +private fun LazyGridScope.interestsSelection( interestsSelectionState: ForYouInterestsSelectionUiState, - showLoadingUIIfLoading: Boolean, onAuthorCheckedChanged: (String, Boolean) -> Unit, onTopicCheckedChanged: (String, Boolean) -> Unit, saveFollowedTopics: () -> Unit ) { when (interestsSelectionState) { ForYouInterestsSelectionUiState.Loading -> { - if (showLoadingUIIfLoading) { - item { - NiaLoadingWheel( - modifier = Modifier - .fillMaxWidth() - .wrapContentSize() - .testTag("forYou:loading"), - contentDesc = stringResource(id = R.string.for_you_loading), - ) - } - } - } - ForYouInterestsSelectionUiState.NoInterestsSelection -> Unit - is ForYouInterestsSelectionUiState.WithInterestsSelection -> { - item { - Text( - text = stringResource(R.string.onboarding_guidance_title), - textAlign = TextAlign.Center, + item(span = { GridItemSpan(maxLineSpan) }) { + NiaLoadingWheel( modifier = Modifier .fillMaxWidth() - .padding(top = 24.dp), - style = MaterialTheme.typography.titleMedium + .wrapContentSize() + .testTag("forYou:loading"), + contentDesc = stringResource(id = R.string.for_you_loading), ) } - item { - Text( - text = stringResource(R.string.onboarding_guidance_subtitle), - modifier = Modifier - .fillMaxWidth() - .padding(top = 8.dp, start = 16.dp, end = 16.dp), - textAlign = TextAlign.Center, - style = MaterialTheme.typography.bodyMedium - ) - } - item { - AuthorsCarousel( - authors = interestsSelectionState.authors, - onAuthorClick = onAuthorCheckedChanged, - modifier = Modifier - .fillMaxWidth() - .padding(vertical = 8.dp) - ) - } - item { - TopicSelection( - interestsSelectionState, - onTopicCheckedChanged, - Modifier.padding(bottom = 8.dp) - ) - } - item { - // Done button - Row( - horizontalArrangement = Arrangement.Center, - modifier = Modifier.fillMaxWidth() - ) { - NiaFilledButton( - onClick = saveFollowedTopics, - enabled = interestsSelectionState.canSaveInterests, + } + ForYouInterestsSelectionUiState.NoInterestsSelection -> Unit + is ForYouInterestsSelectionUiState.WithInterestsSelection -> { + item(span = { GridItemSpan(maxLineSpan) }) { + Column { + Text( + text = stringResource(R.string.onboarding_guidance_title), + textAlign = TextAlign.Center, + modifier = Modifier + .fillMaxWidth() + .padding(top = 24.dp), + style = MaterialTheme.typography.titleMedium + ) + Text( + text = stringResource(R.string.onboarding_guidance_subtitle), + modifier = Modifier + .fillMaxWidth() + .padding(top = 8.dp, start = 16.dp, end = 16.dp), + textAlign = TextAlign.Center, + style = MaterialTheme.typography.bodyMedium + ) + AuthorsCarousel( + authors = interestsSelectionState.authors, + onAuthorClick = onAuthorCheckedChanged, modifier = Modifier - .padding(horizontal = 40.dp) - .width(364.dp) + .fillMaxWidth() + .padding(vertical = 8.dp) + ) + TopicSelection( + interestsSelectionState, + onTopicCheckedChanged, + Modifier.padding(bottom = 8.dp) + ) + // Done button + Row( + horizontalArrangement = Arrangement.Center, + modifier = Modifier.fillMaxWidth() ) { - Text( - text = stringResource(R.string.done) - ) + NiaFilledButton( + onClick = saveFollowedTopics, + enabled = interestsSelectionState.canSaveInterests, + modifier = Modifier + .padding(horizontal = 40.dp) + .width(364.dp) + ) { + Text( + text = stringResource(R.string.done) + ) + } } } } @@ -428,7 +403,6 @@ fun TopicIcon( ) } -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) @Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480") @Preview(name = "landscape", device = "spec:shape=Normal,width=640,height=360,unit=dp,dpi=480") @Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480") @@ -438,7 +412,6 @@ fun ForYouScreenPopulatedFeed() { BoxWithConstraints { NiaTheme { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(maxWidth, maxHeight)), interestsSelectionState = ForYouInterestsSelectionUiState.NoInterestsSelection, feedState = NewsFeedUiState.Success( feed = previewNewsResources.map { @@ -454,7 +427,6 @@ fun ForYouScreenPopulatedFeed() { } } -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) @Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480") @Preview(name = "landscape", device = "spec:shape=Normal,width=640,height=360,unit=dp,dpi=480") @Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480") @@ -464,7 +436,6 @@ fun ForYouScreenTopicSelection() { BoxWithConstraints { NiaTheme { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(maxWidth, maxHeight)), interestsSelectionState = ForYouInterestsSelectionUiState.WithInterestsSelection( topics = previewTopics.map { FollowableTopic(it, false) }, authors = previewAuthors.map { FollowableAuthor(it, false) } @@ -474,8 +445,8 @@ fun ForYouScreenTopicSelection() { SaveableNewsResource(it, false) } ), - onAuthorCheckedChanged = { _, _ -> }, onTopicCheckedChanged = { _, _ -> }, + onAuthorCheckedChanged = { _, _ -> }, saveFollowedTopics = {}, onNewsResourcesCheckedChanged = { _, _ -> } ) @@ -483,7 +454,6 @@ fun ForYouScreenTopicSelection() { } } -@OptIn(ExperimentalMaterial3WindowSizeClassApi::class) @Preview(name = "phone", device = "spec:shape=Normal,width=360,height=640,unit=dp,dpi=480") @Preview(name = "landscape", device = "spec:shape=Normal,width=640,height=360,unit=dp,dpi=480") @Preview(name = "foldable", device = "spec:shape=Normal,width=673,height=841,unit=dp,dpi=480") @@ -493,7 +463,6 @@ fun ForYouScreenLoading() { BoxWithConstraints { NiaTheme { ForYouScreen( - windowSizeClass = WindowSizeClass.calculateFromSize(DpSize(maxWidth, maxHeight)), interestsSelectionState = ForYouInterestsSelectionUiState.Loading, feedState = NewsFeedUiState.Loading, onTopicCheckedChanged = { _, _ -> }, diff --git a/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt b/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt index c19b8016d..2c0dccccb 100644 --- a/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt +++ b/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/navigation/ForYouNavigation.kt @@ -16,7 +16,6 @@ package com.google.samples.apps.nowinandroid.feature.foryou.navigation -import androidx.compose.material3.windowsizeclass.WindowSizeClass import androidx.navigation.NavGraphBuilder import androidx.navigation.compose.composable import com.google.samples.apps.nowinandroid.core.navigation.NiaNavigationDestination @@ -27,10 +26,8 @@ object ForYouDestination : NiaNavigationDestination { override val destination = "for_you_destination" } -fun NavGraphBuilder.forYouGraph( - windowSizeClass: WindowSizeClass -) { +fun NavGraphBuilder.forYouGraph() { composable(route = ForYouDestination.route) { - ForYouRoute(windowSizeClass) + ForYouRoute() } }