From a6250811ec8cd09f0a5785c5de5360190b744f32 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Fri, 22 Jul 2022 20:24:01 +0100 Subject: [PATCH 01/19] Adding local m2 repository for internal builds Change-Id: I83d05367b8225405e7256fde1b834447ba19f339 --- build.gradle.kts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 052e34df4..7543f4e20 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -18,6 +18,9 @@ buildscript { repositories { google() mavenCentral() + + // Android Build Server + maven { url = uri("../nowinandroid-prebuilts/m2repository") } } dependencies { From 40cf1b817198098c58235179e259f40f469086c4 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Fri, 15 Jul 2022 15:24:22 +0200 Subject: [PATCH 02/19] [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() } } From 5e7d395d2a2868e6c21aeef76880026057c7e6a7 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Tue, 26 Jul 2022 22:47:45 +0100 Subject: [PATCH 03/19] Adding execute permission to build script Change-Id: Icfcdc189bee0457593145f2c56880f46bfd6a022 --- build_android_release.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 build_android_release.sh diff --git a/build_android_release.sh b/build_android_release.sh old mode 100644 new mode 100755 From eecc000b4de280b7abdd5df7305621bbd5314ed4 Mon Sep 17 00:00:00 2001 From: Kaevon <45157667+KaevonD@users.noreply.github.com> Date: Tue, 26 Jul 2022 21:29:23 -0700 Subject: [PATCH 04/19] Fixed grammar errors --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index cb378e025..6a52bd9c2 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ The app also uses [product flavors](https://developer.android.com/studio/build/build-variants#product-flavors) to control where content for the app should be loaded from. -The `demo` flavor uses static local data to allow immediate building and exploring the UI. +The `demo` flavor uses static local data to allow immediate building and exploring of the UI. The `prod` flavor makes real network calls to a backend server, providing up-to-date content. At this time, there is not a public backend available. @@ -90,22 +90,22 @@ In tests, **Now in Android** notably does _not_ use any mocking libraries. Instead, the production implementations can be replaced with test doubles using Hilt's testing APIs (or via manual constructor injection for `ViewModel` tests). -These test doubles implement the same interface as the production implementations, and generally +These test doubles implement the same interface as the production implementations and generally provide a simplified (but still realistic) implementation with additional testing hooks. This results in less brittle tests that may exercise more production code, instead of just verifying specific calls against mocks. Examples: - In instrumentation tests, a temporary folder is used to store the user's preferences, which is - wiped after reach test. + wiped after the reach test. This allows using the real `DataStore` and exercising all related code, instead of mocking the flow of data updates. - There are `Test` implementations of each repository, which implement the normal, full repository interface and also provide test-only hooks. `ViewModel` tests use these `Test` repositories, and thus can use the test-only hooks to - manipulate the the state of the `Test` repository and verify the resulting behavior, instead of - checking that specific repository methods were called. + manipulate the state of the `Test` repository and verify the resulting behavior, instead of + checking if specific repository methods were called. # UI @@ -130,7 +130,7 @@ The baseline profile for this app is located at [`app/src/main/baseline-prof.txt It contains rules that enable AOT compilation of the critical user path taken during app launch. For more information on baseline profiles, read [this document](https://developer.android.com/studio/profile/baselineprofiles). -> Note: The baseline profile needs to be re-generated for release builds that touched code which changes app startup. +> Note: The baseline profile needs to be re-generated for release builds that touch code which changes app startup. To generate the baseline profile, select the `benchmark` build variant and run the `BaselineProfileGenerator` benchmark test on an AOSP Android Emulator. From 18ddfc845d4a31c3a8b31931b3a76f005aa3e111 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Wed, 27 Jul 2022 13:56:22 +0100 Subject: [PATCH 05/19] Add JAVA_HOME to build script Change-Id: I9e3bc35cc533f24be3aeca6dfd0a2e7775adbfc2 --- build_android_release.sh | 3 +++ 1 file changed, 3 insertions(+) mode change 100755 => 100644 build_android_release.sh diff --git a/build_android_release.sh b/build_android_release.sh old mode 100755 new mode 100644 index b0037c2bc..9895660a9 --- a/build_android_release.sh +++ b/build_android_release.sh @@ -21,6 +21,9 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" APP_OUT=$DIR/app/build/outputs +export JAVA_HOME="$(cd $DIR/../../../prebuilts/studio/jdk && pwd )" +echo "JAVA_HOME=$JAVA_HOME" + export ANDROID_HOME="$(cd $DIR/../../../prebuilts/fullsdk/linux && pwd )" echo "ANDROID_HOME=$ANDROID_HOME" From 9b698cff8295bc0f94b997ed268b488c7d613f8f Mon Sep 17 00:00:00 2001 From: Don Turner Date: Wed, 27 Jul 2022 14:04:13 +0100 Subject: [PATCH 06/19] Fix JDK path Change-Id: I4711d889071dba6f63189a7ce30b44302883d701 --- build_android_release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build_android_release.sh b/build_android_release.sh index 9895660a9..dfdf37500 100644 --- a/build_android_release.sh +++ b/build_android_release.sh @@ -21,7 +21,7 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" APP_OUT=$DIR/app/build/outputs -export JAVA_HOME="$(cd $DIR/../../../prebuilts/studio/jdk && pwd )" +export JAVA_HOME="$(cd $DIR/../../../prebuilts/studio/jdk/jdk11/linux && pwd )" echo "JAVA_HOME=$JAVA_HOME" export ANDROID_HOME="$(cd $DIR/../../../prebuilts/fullsdk/linux && pwd )" From c0a061ade3610bbf92693efbbb3eb297babd50e9 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Thu, 28 Jul 2022 10:26:06 +0000 Subject: [PATCH 07/19] Make build script executable in Git Change-Id: Ifdca594d75a709c46ccaa9a45c698a2eaac0d15f --- build_android_release.sh | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 build_android_release.sh diff --git a/build_android_release.sh b/build_android_release.sh old mode 100644 new mode 100755 From 7f5e32d8992bc979878795a746b10321a3f91b72 Mon Sep 17 00:00:00 2001 From: necati Date: Sat, 30 Jul 2022 22:08:24 +0300 Subject: [PATCH 08/19] Put some modifiers in correct order to respect official API guidelines --- .../samples/apps/nowinandroid/feature/foryou/ForYouScreen.kt | 4 ++-- .../apps/nowinandroid/feature/interests/InterestsScreen.kt | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) 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..af0fbd83f 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 @@ -414,8 +414,8 @@ private fun SingleTopicButton( @Composable fun TopicIcon( - modifier: Modifier = Modifier, - imageUrl: String + imageUrl: String, + modifier: Modifier = Modifier ) { AsyncImage( // TODO b/228077205, show loading image visual instead of static placeholder diff --git a/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt b/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt index c0961919e..a437a4d95 100644 --- a/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt +++ b/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt @@ -49,9 +49,9 @@ import com.google.samples.apps.nowinandroid.core.ui.JankMetricDisposableEffect @OptIn(ExperimentalLifecycleComposeApi::class) @Composable fun InterestsRoute( - modifier: Modifier = Modifier, navigateToAuthor: (String) -> Unit, navigateToTopic: (String) -> Unit, + modifier: Modifier = Modifier, viewModel: InterestsViewModel = hiltViewModel() ) { val uiState by viewModel.uiState.collectAsStateWithLifecycle() From 3fd18167f4ff63b41115082401e54d549b4bd17d Mon Sep 17 00:00:00 2001 From: Adetunji Dahunsi Date: Wed, 27 Jul 2022 12:44:17 -0400 Subject: [PATCH 09/19] Enable bookmarks on authors page Change-Id: I3ef1dd35472dd63718a9ab5dd10d9b4f67d7b9c4 --- .../feature/author/AuthorScreenTest.kt | 44 +++-- .../feature/author/AuthorScreen.kt | 60 ++++--- .../feature/author/AuthorViewModel.kt | 152 ++++++++++++------ .../feature/author/AuthorViewModelTest.kt | 81 +++++++--- 4 files changed, 238 insertions(+), 99 deletions(-) diff --git a/feature-author/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreenTest.kt b/feature-author/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreenTest.kt index 02e950de0..070226123 100644 --- a/feature-author/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreenTest.kt +++ b/feature-author/src/androidTest/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreenTest.kt @@ -24,6 +24,7 @@ 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.NewsResource import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video +import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import kotlinx.datetime.Instant import org.junit.Before import org.junit.Rule @@ -52,10 +53,11 @@ class AuthorScreenTest { fun niaLoadingWheel_whenScreenIsLoading_showLoading() { composeTestRule.setContent { AuthorScreen( - authorState = AuthorUiState.Loading, - newsState = NewsUiState.Loading, + authorUiState = AuthorUiState.Loading, + newsUiState = NewsUiState.Loading, onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -69,10 +71,11 @@ class AuthorScreenTest { val testAuthor = testAuthors.first() composeTestRule.setContent { AuthorScreen( - authorState = AuthorUiState.Success(testAuthor), - newsState = NewsUiState.Loading, + authorUiState = AuthorUiState.Success(testAuthor), + newsUiState = NewsUiState.Loading, onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -91,10 +94,18 @@ class AuthorScreenTest { fun news_whenAuthorIsLoading_isNotShown() { composeTestRule.setContent { AuthorScreen( - authorState = AuthorUiState.Loading, - newsState = NewsUiState.Success(sampleNewsResources), + authorUiState = AuthorUiState.Loading, + newsUiState = NewsUiState.Success( + sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + } + ), onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } @@ -103,15 +114,24 @@ class AuthorScreenTest { .onNodeWithContentDescription(authorLoading) .assertExists() } + @Test fun news_whenSuccessAndAuthorIsSuccess_isShown() { val testAuthor = testAuthors.first() composeTestRule.setContent { AuthorScreen( - authorState = AuthorUiState.Success(testAuthor), - newsState = NewsUiState.Success(sampleNewsResources), + authorUiState = AuthorUiState.Success(testAuthor), + newsUiState = NewsUiState.Success( + sampleNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + } + ), onBackClick = { }, - onFollowClick = { } + onFollowClick = { }, + onBookmarkChanged = { _, _ -> }, ) } diff --git a/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt b/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt index 5d5addf09..b1f45c9eb 100644 --- a/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt +++ b/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorScreen.kt @@ -56,6 +56,7 @@ import com.google.samples.apps.nowinandroid.core.designsystem.component.NiaLoadi import com.google.samples.apps.nowinandroid.core.designsystem.theme.NiaTheme 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.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.ui.newsResourceCardItems @@ -67,24 +68,27 @@ fun AuthorRoute( modifier: Modifier = Modifier, viewModel: AuthorViewModel = hiltViewModel(), ) { - val uiState: AuthorScreenUiState by viewModel.uiState.collectAsStateWithLifecycle() + val authorUiState: AuthorUiState by viewModel.authorUiState.collectAsStateWithLifecycle() + val newsUiState: NewsUiState by viewModel.newUiState.collectAsStateWithLifecycle() AuthorScreen( - authorState = uiState.authorState, - newsState = uiState.newsState, + authorUiState = authorUiState, + newsUiState = newsUiState, modifier = modifier, onBackClick = onBackClick, onFollowClick = viewModel::followAuthorToggle, + onBookmarkChanged = viewModel::bookmarkNews, ) } @VisibleForTesting @Composable internal fun AuthorScreen( - authorState: AuthorUiState, - newsState: NewsUiState, + authorUiState: AuthorUiState, + newsUiState: NewsUiState, onBackClick: () -> Unit, onFollowClick: (Boolean) -> Unit, + onBookmarkChanged: (String, Boolean) -> Unit, modifier: Modifier = Modifier, ) { LazyColumn( @@ -94,7 +98,7 @@ internal fun AuthorScreen( item { Spacer(Modifier.windowInsetsTopHeight(WindowInsets.safeDrawing)) } - when (authorState) { + when (authorUiState) { AuthorUiState.Loading -> { item { NiaLoadingWheel( @@ -111,12 +115,13 @@ internal fun AuthorScreen( AuthorToolbar( onBackClick = onBackClick, onFollowClick = onFollowClick, - uiState = authorState.followableAuthor, + uiState = authorUiState.followableAuthor, ) } authorBody( - author = authorState.followableAuthor.author, - news = newsState + author = authorUiState.followableAuthor.author, + news = newsUiState, + onBookmarkChanged = onBookmarkChanged, ) } } @@ -128,13 +133,14 @@ internal fun AuthorScreen( private fun LazyListScope.authorBody( author: Author, - news: NewsUiState + news: NewsUiState, + onBookmarkChanged: (String, Boolean) -> Unit ) { item { AuthorHeader(author) } - authorCards(news) + authorCards(news, onBookmarkChanged) } @Composable @@ -163,14 +169,17 @@ private fun AuthorHeader(author: Author) { } } -private fun LazyListScope.authorCards(news: NewsUiState) { +private fun LazyListScope.authorCards( + news: NewsUiState, + onBookmarkChanged: (String, Boolean) -> Unit +) { when (news) { is NewsUiState.Success -> { newsResourceCardItems( items = news.news, - newsResourceMapper = { it }, - isBookmarkedMapper = { /* TODO */ false }, - onToggleBookmark = { /* TODO */ }, + newsResourceMapper = { it.newsResource }, + isBookmarkedMapper = { it.isSaved }, + onToggleBookmark = { onBookmarkChanged(it.newsResource.id, !it.isSaved) }, itemModifier = Modifier.padding(24.dp) ) } @@ -227,10 +236,18 @@ fun AuthorScreenPopulated() { NiaTheme { NiaBackground { AuthorScreen( - authorState = AuthorUiState.Success(FollowableAuthor(previewAuthors[0], false)), - newsState = NewsUiState.Success(previewNewsResources), + authorUiState = AuthorUiState.Success(FollowableAuthor(previewAuthors[0], false)), + newsUiState = NewsUiState.Success( + previewNewsResources.mapIndexed { index, newsResource -> + SaveableNewsResource( + newsResource = newsResource, + isSaved = index % 2 == 0, + ) + } + ), onBackClick = {}, - onFollowClick = {} + onFollowClick = {}, + onBookmarkChanged = { _, _ -> }, ) } } @@ -245,10 +262,11 @@ fun AuthorScreenLoading() { NiaTheme { NiaBackground { AuthorScreen( - authorState = AuthorUiState.Loading, - newsState = NewsUiState.Loading, + authorUiState = AuthorUiState.Loading, + newsUiState = NewsUiState.Loading, onBackClick = {}, - onFollowClick = {} + onFollowClick = {}, + onBookmarkChanged = { _, _ -> }, ) } } diff --git a/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt b/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt index 3401c0529..889c3a326 100644 --- a/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt +++ b/feature-author/src/main/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModel.kt @@ -25,6 +25,7 @@ import com.google.samples.apps.nowinandroid.core.data.repository.UserDataReposit 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.NewsResource +import com.google.samples.apps.nowinandroid.core.model.data.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.result.Result import com.google.samples.apps.nowinandroid.core.result.asResult import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination @@ -50,66 +51,126 @@ class AuthorViewModel @Inject constructor( savedStateHandle[AuthorDestination.authorIdArg] ) + val authorUiState: StateFlow = authorUiStateStream( + authorId = authorId, + userDataRepository = userDataRepository, + authorsRepository = authorsRepository + ) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = AuthorUiState.Loading + ) + + val newUiState: StateFlow = newsUiStateStream( + authorId = authorId, + userDataRepository = userDataRepository, + newsRepository = newsRepository + ) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed(5_000), + initialValue = NewsUiState.Loading + ) + + fun followAuthorToggle(followed: Boolean) { + viewModelScope.launch { + userDataRepository.toggleFollowedAuthorId(authorId, followed) + } + } + + fun bookmarkNews(newsResourceId: String, bookmarked: Boolean) { + viewModelScope.launch { + userDataRepository.updateNewsResourceBookmark(newsResourceId, bookmarked) + } + } +} + +private fun authorUiStateStream( + authorId: String, + userDataRepository: UserDataRepository, + authorsRepository: AuthorsRepository, +): Flow { // Observe the followed authors, as they could change over time. - private val followedAuthorIdsStream: Flow>> = + val followedAuthorIdsStream: Flow> = userDataRepository.userDataStream .map { it.followedAuthors } - .asResult() // Observe author information - private val author: Flow> = authorsRepository.getAuthorStream( + val authorStream: Flow = authorsRepository.getAuthorStream( id = authorId - ).asResult() - - // Observe the News for this author - private val newsStream: Flow>> = - newsRepository.getNewsResourcesStream( - filterAuthorIds = setOf(element = authorId), - filterTopicIds = emptySet() - ).asResult() - - val uiState: StateFlow = - combine( - followedAuthorIdsStream, - author, - newsStream - ) { followedAuthorsResult, authorResult, newsResult -> - val author: AuthorUiState = - if (authorResult is Result.Success && followedAuthorsResult is Result.Success) { - val followed = followedAuthorsResult.data.contains(authorId) + ) + + return combine( + followedAuthorIdsStream, + authorStream, + ::Pair + ) + .asResult() + .map { followedAuthorToAuthorResult -> + when (followedAuthorToAuthorResult) { + is Result.Success -> { + val (followedAuthors, author) = followedAuthorToAuthorResult.data + val followed = followedAuthors.contains(authorId) AuthorUiState.Success( followableAuthor = FollowableAuthor( - author = authorResult.data, + author = author, isFollowed = followed ) ) - } else if ( - authorResult is Result.Loading || followedAuthorsResult is Result.Loading - ) { + } + is Result.Loading -> { AuthorUiState.Loading - } else { + } + is Result.Error -> { AuthorUiState.Error } - - val news: NewsUiState = when (newsResult) { - is Result.Success -> NewsUiState.Success(newsResult.data) - is Result.Loading -> NewsUiState.Loading - is Result.Error -> NewsUiState.Error } - - AuthorScreenUiState(author, news) } - .stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed(5_000), - initialValue = AuthorScreenUiState(AuthorUiState.Loading, NewsUiState.Loading) - ) +} - fun followAuthorToggle(followed: Boolean) { - viewModelScope.launch { - userDataRepository.toggleFollowedAuthorId(authorId, followed) +private fun newsUiStateStream( + authorId: String, + newsRepository: NewsRepository, + userDataRepository: UserDataRepository, +): Flow { + // Observe news + val newsStream: Flow> = newsRepository.getNewsResourcesStream( + filterAuthorIds = setOf(element = authorId), + filterTopicIds = emptySet() + ) + + // Observe bookmarks + val bookmarkStream: Flow> = userDataRepository.userDataStream + .map { it.bookmarkedNewsResources } + + return combine( + newsStream, + bookmarkStream, + ::Pair + ) + .asResult() + .map { newsToBookmarksResult -> + when (newsToBookmarksResult) { + is Result.Success -> { + val (news, bookmarks) = newsToBookmarksResult.data + NewsUiState.Success( + news.map { newsResource -> + SaveableNewsResource( + newsResource, + isSaved = bookmarks.contains(newsResource.id) + ) + } + ) + } + is Result.Loading -> { + NewsUiState.Loading + } + is Result.Error -> { + NewsUiState.Error + } + } } - } } sealed interface AuthorUiState { @@ -119,12 +180,7 @@ sealed interface AuthorUiState { } sealed interface NewsUiState { - data class Success(val news: List) : NewsUiState + data class Success(val news: List) : NewsUiState object Error : NewsUiState object Loading : NewsUiState } - -data class AuthorScreenUiState( - val authorState: AuthorUiState, - val newsState: NewsUiState -) diff --git a/feature-author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt b/feature-author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt index 0e1fc48d2..c5a803b7d 100644 --- a/feature-author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt +++ b/feature-author/src/test/java/com/google/samples/apps/nowinandroid/feature/author/AuthorViewModelTest.kt @@ -27,6 +27,7 @@ import com.google.samples.apps.nowinandroid.core.testing.repository.TestUserData import com.google.samples.apps.nowinandroid.core.testing.util.MainDispatcherRule import com.google.samples.apps.nowinandroid.feature.author.navigation.AuthorDestination import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.test.UnconfinedTestDispatcher @@ -68,16 +69,16 @@ class AuthorViewModelTest { @Test fun uiStateAuthor_whenSuccess_matchesAuthorFromRepository() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.authorUiState.collect() } // To make sure AuthorUiState is success authorsRepository.sendAuthors(testInputAuthors.map(FollowableAuthor::author)) userDataRepository.setFollowedAuthorIds(setOf(testInputAuthors[1].author.id)) - val item = viewModel.uiState.value - assertTrue(item.authorState is AuthorUiState.Success) + val item = viewModel.authorUiState.value + assertTrue(item is AuthorUiState.Success) - val successAuthorUiState = item.authorState as AuthorUiState.Success + val successAuthorUiState = item as AuthorUiState.Success val authorFromRepository = authorsRepository.getAuthorStream( id = testInputAuthors[0].author.id ).first() @@ -90,20 +91,20 @@ class AuthorViewModelTest { @Test fun uiStateNews_whenInitialized_thenShowLoading() = runTest { - assertEquals(NewsUiState.Loading, viewModel.uiState.value.newsState) + assertEquals(NewsUiState.Loading, viewModel.newUiState.value) } @Test fun uiStateAuthor_whenInitialized_thenShowLoading() = runTest { - assertEquals(AuthorUiState.Loading, viewModel.uiState.value.authorState) + assertEquals(AuthorUiState.Loading, viewModel.authorUiState.value) } @Test fun uiStateAuthor_whenFollowedIdsSuccessAndAuthorLoading_thenShowLoading() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.authorUiState.collect() } userDataRepository.setFollowedAuthorIds(setOf(testInputAuthors[1].author.id)) - assertEquals(AuthorUiState.Loading, viewModel.uiState.value.authorState) + assertEquals(AuthorUiState.Loading, viewModel.authorUiState.value) collectJob.cancel() } @@ -111,13 +112,21 @@ class AuthorViewModelTest { @Test fun uiStateAuthor_whenFollowedIdsSuccessAndAuthorSuccess_thenAuthorSuccessAndNewsLoading() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { + combine( + viewModel.authorUiState, + viewModel.newUiState, + ::Pair + ).collect() + } authorsRepository.sendAuthors(testInputAuthors.map { it.author }) userDataRepository.setFollowedAuthorIds(setOf(testInputAuthors[1].author.id)) - val item = viewModel.uiState.value - assertTrue(item.authorState is AuthorUiState.Success) - assertTrue(item.newsState is NewsUiState.Loading) + val authorState = viewModel.authorUiState.value + val newsUiState = viewModel.newUiState.value + + assertTrue(authorState is AuthorUiState.Success) + assertTrue(newsUiState is NewsUiState.Loading) collectJob.cancel() } @@ -125,21 +134,29 @@ class AuthorViewModelTest { @Test fun uiStateAuthor_whenFollowedIdsSuccessAndAuthorSuccessAndNewsIsSuccess_thenAllSuccess() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { + combine( + viewModel.authorUiState, + viewModel.newUiState, + ::Pair + ).collect() + } authorsRepository.sendAuthors(testInputAuthors.map { it.author }) userDataRepository.setFollowedAuthorIds(setOf(testInputAuthors[1].author.id)) newsRepository.sendNewsResources(sampleNewsResources) - val item = viewModel.uiState.value - assertTrue(item.authorState is AuthorUiState.Success) - assertTrue(item.newsState is NewsUiState.Success) + val authorState = viewModel.authorUiState.value + val newsUiState = viewModel.newUiState.value + + assertTrue(authorState is AuthorUiState.Success) + assertTrue(newsUiState is NewsUiState.Success) collectJob.cancel() } @Test fun uiStateAuthor_whenFollowingAuthor_thenShowUpdatedAuthor() = runTest { - val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.uiState.collect() } + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.authorUiState.collect() } authorsRepository.sendAuthors(testInputAuthors.map { it.author }) // Set which author IDs are followed, not including 0. @@ -149,7 +166,35 @@ class AuthorViewModelTest { assertEquals( AuthorUiState.Success(followableAuthor = testOutputAuthors[0]), - viewModel.uiState.value.authorState + viewModel.authorUiState.value + ) + + collectJob.cancel() + } + + @Test + fun uiStateAuthor_whenNewsBookmarked_thenShowBookmarkedNews() = runTest { + val collectJob = launch(UnconfinedTestDispatcher()) { viewModel.newUiState.collect() } + + authorsRepository.sendAuthors(testInputAuthors.map { it.author }) + newsRepository.sendNewsResources(sampleNewsResources) + + // Set initial bookmarked status to false + userDataRepository.updateNewsResourceBookmark( + newsResourceId = sampleNewsResources.first().id, + bookmarked = false + ) + + viewModel.bookmarkNews( + newsResourceId = sampleNewsResources.first().id, + bookmarked = true + ) + + assertTrue( + (viewModel.newUiState.value as NewsUiState.Success) + .news + .first { it.newsResource.id == sampleNewsResources.first().id } + .isSaved ) collectJob.cancel() From 1e001ad423e09ae95e857bd4cb6340c813952bcb Mon Sep 17 00:00:00 2001 From: Kaevon <45157667+KaevonD@users.noreply.github.com> Date: Mon, 1 Aug 2022 18:34:23 -0700 Subject: [PATCH 10/19] Fixing wording --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6a52bd9c2..92ac2fb05 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ specific calls against mocks. Examples: - In instrumentation tests, a temporary folder is used to store the user's preferences, which is - wiped after the reach test. + wiped after each test. This allows using the real `DataStore` and exercising all related code, instead of mocking the flow of data updates. @@ -105,7 +105,7 @@ Examples: interface and also provide test-only hooks. `ViewModel` tests use these `Test` repositories, and thus can use the test-only hooks to manipulate the state of the `Test` repository and verify the resulting behavior, instead of - checking if specific repository methods were called. + checking that specific repository methods were called. # UI From 6473f64d86fa5cee3c9e0de8963284cfd074a3bb Mon Sep 17 00:00:00 2001 From: wswon Date: Tue, 2 Aug 2022 21:41:21 +0900 Subject: [PATCH 11/19] Fix : class name Typo --- docs/ModularizationLearningJourney.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/ModularizationLearningJourney.md b/docs/ModularizationLearningJourney.md index 0a144ea2d..01e9b8750 100644 --- a/docs/ModularizationLearningJourney.md +++ b/docs/ModularizationLearningJourney.md @@ -183,7 +183,7 @@ Using the above modularization strategy, the Now in Android app has the followin Making network requests and handling responses from a remote data source. - RetrofitNiANetworkApi + RetrofitNiaNetworkApi @@ -209,7 +209,7 @@ Using the above modularization strategy, the Now in Android app has the followin Local database storage using Room. - NiADatabase
+ NiaDatabase
DatabaseMigrations
Dao classes From e701b9737abd215c362ccfc81a58eb1bdebc4ab2 Mon Sep 17 00:00:00 2001 From: Jolanda Verhoef Date: Wed, 3 Aug 2022 11:47:50 +0100 Subject: [PATCH 12/19] Remove gradle cache to check API23 test behavior --- .github/workflows/Build.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index e3e8768b3..ae5e708d8 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -79,6 +79,8 @@ jobs: - name: Setup Gradle uses: gradle/gradle-build-action@v2 + with: + cache-disabled: true - name: Run instrumentation tests uses: reactivecircus/android-emulator-runner@v2 From c37fdc7ccbd7788ef2a3557ad29e388d2e86ef3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jose=20Alc=C3=A9rreca?= Date: Wed, 3 Aug 2022 16:37:58 +0200 Subject: [PATCH 13/19] Enables cache, increases disk size --- .github/workflows/Build.yaml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/Build.yaml b/.github/workflows/Build.yaml index ae5e708d8..1a98c8a1b 100644 --- a/.github/workflows/Build.yaml +++ b/.github/workflows/Build.yaml @@ -79,8 +79,6 @@ jobs: - name: Setup Gradle uses: gradle/gradle-build-action@v2 - with: - cache-disabled: true - name: Run instrumentation tests uses: reactivecircus/android-emulator-runner@v2 @@ -88,8 +86,8 @@ jobs: api-level: ${{ matrix.api-level }} arch: x86_64 disable-animations: true - disk-size: 1500M - heap-size: 512M + disk-size: 2000M + heap-size: 600M script: ./gradlew connectedProdDebugAndroidTest -x :benchmark:connectedProdBenchmarkAndroidTest --stacktrace - name: Upload test reports From 4523129fd4a5c102338344e51c2a3e4ef2610ac3 Mon Sep 17 00:00:00 2001 From: Alex Vanyo Date: Mon, 8 Aug 2022 14:45:12 -0700 Subject: [PATCH 14/19] Remove non-Compose material Change-Id: I62a639b6c12e7d523c3dc5ad0766f180120d9e92 --- app/build.gradle.kts | 3 +-- app/src/main/res/values-night/themes.xml | 10 ++++---- app/src/main/res/values-v23/themes.xml | 3 +-- app/src/main/res/values-v27/themes.xml | 4 +--- app/src/main/res/values/themes.xml | 29 +++++++++--------------- gradle/libs.versions.toml | 2 -- 6 files changed, 18 insertions(+), 33 deletions(-) diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 7f9fb1b30..ed7d996b1 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -13,8 +13,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import com.google.samples.apps.nowinandroid.FlavorDimension import com.google.samples.apps.nowinandroid.Flavor +import com.google.samples.apps.nowinandroid.FlavorDimension plugins { id("nowinandroid.android.application") @@ -117,7 +117,6 @@ dependencies { implementation(libs.androidx.core.ktx) implementation(libs.androidx.compose.material3.windowSizeClass) implementation(libs.androidx.window.manager) - implementation(libs.material3) implementation(libs.androidx.profileinstaller) implementation(libs.coil.kt) diff --git a/app/src/main/res/values-night/themes.xml b/app/src/main/res/values-night/themes.xml index 78ed557f4..461c21cd5 100644 --- a/app/src/main/res/values-night/themes.xml +++ b/app/src/main/res/values-night/themes.xml @@ -14,13 +14,11 @@ See the License for the specific language governing permissions and limitations under the License. --> - + - - diff --git a/app/src/main/res/values-v23/themes.xml b/app/src/main/res/values-v23/themes.xml index 2a13f9c35..b637b1413 100644 --- a/app/src/main/res/values-v23/themes.xml +++ b/app/src/main/res/values-v23/themes.xml @@ -16,8 +16,7 @@ --> - diff --git a/app/src/main/res/values-v27/themes.xml b/app/src/main/res/values-v27/themes.xml index b322aee6b..969e51914 100644 --- a/app/src/main/res/values-v27/themes.xml +++ b/app/src/main/res/values-v27/themes.xml @@ -16,10 +16,8 @@ --> - diff --git a/app/src/main/res/values/themes.xml b/app/src/main/res/values/themes.xml index 9ace165b5..8c7705178 100644 --- a/app/src/main/res/values/themes.xml +++ b/app/src/main/res/values/themes.xml @@ -14,29 +14,22 @@ See the License for the specific language governing permissions and limitations under the License. --> - + + + + - - - - - - - - + + diff --git a/app/src/main/res/values/themes.xml b/app/src/main/res/values/themes.xml index 8c7705178..c304f8d16 100644 --- a/app/src/main/res/values/themes.xml +++ b/app/src/main/res/values/themes.xml @@ -32,4 +32,14 @@ + + + diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 0f399c7c1..ecae60313 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -8,6 +8,7 @@ androidxCompose = "1.2.0-rc03" androidxComposeCompiler = "1.2.0" androidxComposeMaterial3 = "1.0.0-alpha13" androidxCore = "1.8.0" +androidxCoreSplashscreen = "1.0.0" androidxCustomView = "1.0.0-rc01" androidxDataStore = "1.0.0" androidxEspresso = "3.4.0" @@ -67,6 +68,7 @@ androidx-compose-ui-tooling = { group = "androidx.compose.ui", name = "ui-toolin androidx-compose-ui-tooling-preview = { group = "androidx.compose.ui", name = "ui-tooling-preview", version.ref = "androidxCompose" } androidx-compose-ui-util = { group = "androidx.compose.ui", name = "ui-util", version.ref = "androidxCompose" } androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "androidxCore" } +androidx-core-splashscreen = { group = "androidx.core", name = "core-splashscreen", version.ref = "androidxCoreSplashscreen" } androidx-customview-poolingcontainer = { group = "androidx.customview", name = "customview-poolingcontainer", version.ref = "androidxCustomView"} androidx-dataStore-core = { group = "androidx.datastore", name = "datastore", version.ref = "androidxDataStore" } androidx-dataStore-preferences = { group = "androidx.datastore", name = "datastore-preferences", version.ref = "androidxDataStore" } From 6dc17126da6d4152183508e4b115eccdfa5f9905 Mon Sep 17 00:00:00 2001 From: Ben Weiss Date: Tue, 9 Aug 2022 13:50:25 +0100 Subject: [PATCH 16/19] Update JankStats to 1.0.0-alpha03 Accomodating API changes along the way. --- .../samples/apps/nowinandroid/di/JankStatsModule.kt | 11 +---------- .../samples/apps/nowinandroid/ui/NiaAppState.kt | 2 +- .../apps/nowinandroid/core/ui/JankStatsExtensions.kt | 12 ++++++------ .../feature/interests/InterestsScreen.kt | 2 +- gradle/libs.versions.toml | 2 +- 5 files changed, 10 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/google/samples/apps/nowinandroid/di/JankStatsModule.kt b/app/src/main/java/com/google/samples/apps/nowinandroid/di/JankStatsModule.kt index 6d11a9f68..90f844c1b 100644 --- a/app/src/main/java/com/google/samples/apps/nowinandroid/di/JankStatsModule.kt +++ b/app/src/main/java/com/google/samples/apps/nowinandroid/di/JankStatsModule.kt @@ -24,9 +24,6 @@ import dagger.Module import dagger.Provides import dagger.hilt.InstallIn import dagger.hilt.android.components.ActivityComponent -import java.util.concurrent.Executor -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.asExecutor @Module @InstallIn(ActivityComponent::class) @@ -47,17 +44,11 @@ object JankStatsModule { return activity.window } - @Provides - fun providesDefaultExecutor(): Executor { - return Dispatchers.Default.asExecutor() - } - @Provides fun providesJankStats( window: Window, - executor: Executor, frameListener: JankStats.OnFrameListener ): JankStats { - return JankStats.createAndTrack(window, executor, frameListener) + return JankStats.createAndTrack(window, frameListener) } } 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 dc620490a..bf80984fa 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 @@ -143,7 +143,7 @@ class NiaAppState( private fun NavigationTrackingSideEffect(navController: NavHostController) { JankMetricDisposableEffect(navController) { metricsHolder -> val listener = NavController.OnDestinationChangedListener { _, destination, _ -> - metricsHolder.state?.addState("Navigation", destination.route.toString()) + metricsHolder.state?.putState("Navigation", destination.route.toString()) } navController.addOnDestinationChangedListener(listener) diff --git a/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt b/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt index 6fdb948c7..ef7375f48 100644 --- a/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt +++ b/core-ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/JankStatsExtensions.kt @@ -26,7 +26,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.snapshotFlow import androidx.compose.ui.platform.LocalView import androidx.metrics.performance.PerformanceMetricsState -import androidx.metrics.performance.PerformanceMetricsState.MetricsStateHolder +import androidx.metrics.performance.PerformanceMetricsState.Holder import kotlinx.coroutines.CoroutineScope /** @@ -35,11 +35,11 @@ import kotlinx.coroutines.CoroutineScope * @see PerformanceMetricsState.getForHierarchy */ @Composable -fun rememberMetricsStateHolder(): MetricsStateHolder { +fun rememberMetricsStateHolder(): Holder { val localView = LocalView.current return remember(localView) { - PerformanceMetricsState.getForHierarchy(localView) + PerformanceMetricsState.getHolderForHierarchy(localView) } } @@ -51,7 +51,7 @@ fun rememberMetricsStateHolder(): MetricsStateHolder { @Composable fun JankMetricEffect( vararg keys: Any?, - reportMetric: suspend CoroutineScope.(state: MetricsStateHolder) -> Unit + reportMetric: suspend CoroutineScope.(state: Holder) -> Unit ) { val metrics = rememberMetricsStateHolder() LaunchedEffect(metrics, *keys) { @@ -66,7 +66,7 @@ fun JankMetricEffect( @Composable fun JankMetricDisposableEffect( vararg keys: Any?, - reportMetric: DisposableEffectScope.(state: MetricsStateHolder) -> DisposableEffectResult + reportMetric: DisposableEffectScope.(state: Holder) -> DisposableEffectResult ) { val metrics = rememberMetricsStateHolder() DisposableEffect(metrics, *keys) { @@ -80,7 +80,7 @@ fun TrackScrollJank(scrollableState: ScrollableState, stateName: String) { snapshotFlow { scrollableState.isScrollInProgress }.collect { isScrollInProgress -> metricsHolder.state?.apply { if (isScrollInProgress) { - addState(stateName, "Scrolling=true") + putState(stateName, "Scrolling=true") } else { removeState(stateName) } diff --git a/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt b/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt index c0961919e..31ca66db5 100644 --- a/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt +++ b/feature-interests/src/main/java/com/google/samples/apps/nowinandroid/feature/interests/InterestsScreen.kt @@ -69,7 +69,7 @@ fun InterestsRoute( ) JankMetricDisposableEffect(tabState) { metricsHolder -> - metricsHolder.state?.addState("Interests:TabState", "currentIndex:${tabState.currentIndex}") + metricsHolder.state?.putState("Interests:TabState", "currentIndex:${tabState.currentIndex}") onDispose { metricsHolder.state?.removeState("Interests:TabState") diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index a58115498..adc220d2e 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -15,7 +15,7 @@ androidxHiltNavigationCompose = "1.0.0" androidxLifecycle = "2.6.0-alpha01" androidxMacroBenchmark = "1.1.0" androidxNavigation = "2.5.0" -androidxMetrics = "1.0.0-alpha01" +androidxMetrics = "1.0.0-alpha03" androidxProfileinstaller = "1.2.0-rc01" androidxSavedState = "1.1.0" androidxStartup = "1.1.1" From 200aede1c7a0d5a030de9e1a2f34f414289ec579 Mon Sep 17 00:00:00 2001 From: Alex Vanyo Date: Wed, 10 Aug 2022 11:39:35 -0700 Subject: [PATCH 17/19] Rename NightAdjusted.Splash to NightAdjusted.Theme.Splash Change-Id: I2493c664945d3cfbe1c52397133fdc955265ac56 --- app/src/main/res/values-night/themes.xml | 2 +- app/src/main/res/values/themes.xml | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/res/values-night/themes.xml b/app/src/main/res/values-night/themes.xml index aacfc32bc..767663342 100644 --- a/app/src/main/res/values-night/themes.xml +++ b/app/src/main/res/values-night/themes.xml @@ -21,7 +21,7 @@ false - diff --git a/app/src/main/res/values/themes.xml b/app/src/main/res/values/themes.xml index c304f8d16..82456f53a 100644 --- a/app/src/main/res/values/themes.xml +++ b/app/src/main/res/values/themes.xml @@ -32,12 +32,12 @@ - From 35610457e0456e1e966841bd6fd81d7f07e3daca Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Thu, 11 Aug 2022 19:46:20 +0000 Subject: [PATCH 18/19] Update protobuf to v3.21.5 --- gradle/libs.versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 50f472382..514c08cb2 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -39,7 +39,7 @@ ksp = "1.7.0-1.0.6" ktlint = "0.43.0" lint = "30.2.1" okhttp = "4.10.0" -protobuf = "3.21.1" +protobuf = "3.21.5" protobufPlugin = "0.8.19" retrofit = "2.9.0" retrofitKotlinxSerializationJson = "0.8.0" From 63fd5a523b36ff4316b266b3f2f2b5e96798c947 Mon Sep 17 00:00:00 2001 From: madroid Date: Fri, 5 Aug 2022 17:41:58 +0800 Subject: [PATCH 19/19] Rename followedInterestsState to followedInterestsUiState --- ...InterestsState.kt => FollowedInterestsUiState.kt} | 8 ++++---- .../nowinandroid/feature/foryou/ForYouViewModel.kt | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) rename feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/{FollowedInterestsState.kt => FollowedInterestsUiState.kt} (86%) diff --git a/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsState.kt b/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt similarity index 86% rename from feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsState.kt rename to feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt index 61088d7c6..744cebbaa 100644 --- a/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsState.kt +++ b/feature-foryou/src/main/java/com/google/samples/apps/nowinandroid/feature/foryou/FollowedInterestsUiState.kt @@ -19,17 +19,17 @@ package com.google.samples.apps.nowinandroid.feature.foryou /** * A sealed hierarchy for the user's current followed interests state. */ -sealed interface FollowedInterestsState { +sealed interface FollowedInterestsUiState { /** * The current state is unknown (hasn't loaded yet) */ - object Unknown : FollowedInterestsState + object Unknown : FollowedInterestsUiState /** * The user hasn't followed any interests yet. */ - object None : FollowedInterestsState + object None : FollowedInterestsUiState /** * The user has followed the given (non-empty) set of [topicIds] or [authorIds]. @@ -37,5 +37,5 @@ sealed interface FollowedInterestsState { data class FollowedInterests( val topicIds: Set, val authorIds: Set - ) : FollowedInterestsState + ) : FollowedInterestsUiState } 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 6c8dc3470..a64e6b1c4 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 @@ -33,9 +33,9 @@ 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.SaveableNewsResource import com.google.samples.apps.nowinandroid.core.ui.NewsFeedUiState -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsState.FollowedInterests -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsState.None -import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsState.Unknown +import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.FollowedInterests +import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.None +import com.google.samples.apps.nowinandroid.feature.foryou.FollowedInterestsUiState.Unknown import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow @@ -60,7 +60,7 @@ class ForYouViewModel @Inject constructor( savedStateHandle: SavedStateHandle ) : ViewModel() { - private val followedInterestsState: StateFlow = + private val followedInterestsUiState: StateFlow = userDataRepository.userDataStream .map { userData -> if (userData.followedAuthors.isEmpty() && userData.followedTopics.isEmpty()) { @@ -107,7 +107,7 @@ class ForYouViewModel @Inject constructor( val feedState: StateFlow = combine( - followedInterestsState, + followedInterestsUiState, snapshotFlow { inProgressTopicSelection }, snapshotFlow { inProgressAuthorSelection } ) { followedInterestsUserState, inProgressTopicSelection, inProgressAuthorSelection -> @@ -148,7 +148,7 @@ class ForYouViewModel @Inject constructor( val interestsSelectionState: StateFlow = combine( - followedInterestsState, + followedInterestsUiState, topicsRepository.getTopicsStream(), authorsRepository.getAuthorsStream(), snapshotFlow { inProgressTopicSelection },