From 71cef7951b903ccc2dfa16e76df3d76d797ade4d Mon Sep 17 00:00:00 2001 From: Ran Nachmany Date: Fri, 25 Mar 2022 18:02:50 +0300 Subject: [PATCH] Improving For You onboaring [http://b/223204846] Test: updated tests that were failing in firebase testlab due to the UI change. Change-Id: If892a7a81eecc69bf71f41d75c11af363903f962 --- .../apps/nowinandroid/ui/NavigationTest.kt | 17 +- .../feature/foryou/ForYouScreenTest.kt | 16 +- .../feature/foryou/ForYouScreen.kt | 226 +++++++++++------- .../src/main/res/values/strings.xml | 2 + gradle/libs.versions.toml | 2 +- settings.gradle | 2 +- 6 files changed, 163 insertions(+), 102 deletions(-) diff --git a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt index 6e3dab8b7..6a8312c67 100644 --- a/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt +++ b/app/src/androidTest/java/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt @@ -18,6 +18,7 @@ package com.google.samples.apps.nowinandroid.ui import androidx.compose.material.window.ExperimentalMaterialWindowApi import androidx.compose.ui.test.assertIsOn +import androidx.compose.ui.test.assertIsSelected import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText @@ -68,6 +69,7 @@ class NavigationTest { private lateinit var episodes: String private lateinit var saved: String private lateinit var topics: String + private lateinit var sampleTopic: String @Before fun setup() { @@ -79,14 +81,15 @@ class NavigationTest { episodes = getString(R.string.episodes) saved = getString(R.string.saved) topics = getString(R.string.following) + sampleTopic = "Headlines" } } @Test fun firstScreen_isForYou() { composeTestRule.apply { - // VERIFY first topic is displayed - onNodeWithText("HEADLINES").assertExists() + // VERIFY for you is selected + onNodeWithText(forYou).assertIsSelected() } } @@ -101,13 +104,13 @@ class NavigationTest { fun navigationBar_navigateToPreviouslySelectedTab_restoresContent() { composeTestRule.apply { // GIVEN the user follows a topic - onNodeWithText("HEADLINES").performClick() + onNodeWithText(sampleTopic).performClick() // WHEN the user navigates to the Topics destination onNodeWithText(topics).performClick() // AND the user navigates to the For You destination onNodeWithText(forYou).performClick() // THEN the state of the For You destination is restored - onNodeWithText("HEADLINES").assertIsOn() + onNodeWithContentDescription(sampleTopic).assertIsOn() } } @@ -118,11 +121,11 @@ class NavigationTest { fun navigationBar_reselectTab_keepsState() { composeTestRule.apply { // GIVEN the user follows a topic - onNodeWithText("HEADLINES").performClick() + onNodeWithText(sampleTopic).performClick() // WHEN the user taps the For You navigation bar item onNodeWithText(forYou).performClick() // THEN the state of the For You destination is restored - onNodeWithText("HEADLINES").assertIsOn() + onNodeWithContentDescription(sampleTopic).assertIsOn() } } @@ -177,7 +180,7 @@ class NavigationTest { // WHEN the user uses the system button/gesture to go back, Espresso.pressBack() // THEN the app shows the For You destination - onNodeWithText("HEADLINES").assertExists() + onNodeWithText(forYou).assertExists() } } } 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 225d96169..b4213c847 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 @@ -21,8 +21,6 @@ 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.assertIsOff -import androidx.compose.ui.test.assertIsOn import androidx.compose.ui.test.junit4.createAndroidComposeRule import androidx.compose.ui.test.onNodeWithContentDescription import androidx.compose.ui.test.onNodeWithText @@ -93,21 +91,18 @@ class ForYouScreenTest { } composeTestRule - .onNodeWithText("HEADLINES") + .onNodeWithText("Headlines") .assertIsDisplayed() - .assertIsOff() .assertHasClickAction() composeTestRule .onNodeWithText("UI") .assertIsDisplayed() - .assertIsOff() .assertHasClickAction() composeTestRule - .onNodeWithText("TOOLS") + .onNodeWithText("Tools") .assertIsDisplayed() - .assertIsOff() .assertHasClickAction() composeTestRule @@ -157,21 +152,18 @@ class ForYouScreenTest { } composeTestRule - .onNodeWithText("HEADLINES") + .onNodeWithText("Headlines") .assertIsDisplayed() - .assertIsOff() .assertHasClickAction() composeTestRule .onNodeWithText("UI") .assertIsDisplayed() - .assertIsOn() .assertHasClickAction() composeTestRule - .onNodeWithText("TOOLS") + .onNodeWithText("Tools") .assertIsDisplayed() - .assertIsOff() .assertHasClickAction() composeTestRule 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 93d793b57..fd4974f41 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 @@ -18,32 +18,40 @@ package com.google.samples.apps.nowinandroid.feature.foryou import android.content.Intent import android.net.Uri +import androidx.compose.foundation.background +import androidx.compose.foundation.clickable +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn -import androidx.compose.foundation.lazy.LazyListScope +import androidx.compose.foundation.lazy.grid.GridCells.Fixed +import androidx.compose.foundation.lazy.grid.LazyHorizontalGrid +import androidx.compose.foundation.lazy.grid.items import androidx.compose.foundation.lazy.items -import androidx.compose.foundation.selection.toggleable +import androidx.compose.foundation.shape.CornerSize import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.material.MaterialTheme import androidx.compose.material3.Button -import androidx.compose.material3.ButtonDefaults -import androidx.compose.material3.OutlinedButton +import androidx.compose.material3.Icon import androidx.compose.material3.Text import androidx.compose.runtime.Composable import androidx.compose.runtime.collectAsState import androidx.compose.runtime.getValue -import androidx.compose.runtime.key +import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.res.stringResource -import androidx.compose.ui.semantics.Role +import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.unit.dp import androidx.core.content.ContextCompat.startActivity import androidx.hilt.navigation.compose.hiltViewModel -import com.google.accompanist.flowlayout.FlowRow 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 @@ -51,6 +59,12 @@ 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.ui.NewsResourceCardExpanded import com.google.samples.apps.nowinandroid.core.ui.NiaLoadingIndicator +import com.google.samples.apps.nowinandroid.core.ui.component.NiaToggleButton +import com.google.samples.apps.nowinandroid.core.ui.icon.NiaIcons +import com.google.samples.apps.nowinandroid.core.ui.theme.NiaTypography +import com.google.samples.apps.nowinandroid.feature.foryou.ForYouFeedUiState.PopulatedFeed +import com.google.samples.apps.nowinandroid.feature.foryou.ForYouFeedUiState.PopulatedFeed.FeedWithTopicSelection +import com.google.samples.apps.nowinandroid.feature.foryou.ForYouFeedUiState.PopulatedFeed.FeedWithoutTopicSelection import kotlinx.datetime.Instant @Composable @@ -59,7 +73,6 @@ fun ForYouRoute( viewModel: ForYouViewModel = hiltViewModel() ) { val uiState by viewModel.uiState.collectAsState() - ForYouScreen( modifier = modifier, uiState = uiState, @@ -77,98 +90,149 @@ fun ForYouScreen( onNewsResourcesCheckedChanged: (Int, Boolean) -> Unit, modifier: Modifier = Modifier, ) { - Box(modifier = modifier.fillMaxSize()) { + LazyColumn( + modifier = modifier.fillMaxSize() + ) { when (uiState) { - ForYouFeedUiState.Loading -> { - NiaLoadingIndicator( - modifier = modifier, - contentDesc = stringResource(id = R.string.for_you_loading), - ) + is ForYouFeedUiState.Loading -> { + item { + NiaLoadingIndicator( + modifier = modifier, + contentDesc = stringResource(id = R.string.for_you_loading), + ) + } } - is ForYouFeedUiState.PopulatedFeed -> { - LazyColumn { - when (uiState) { - is ForYouFeedUiState.PopulatedFeed.FeedWithTopicSelection -> { - TopicSelection(uiState, onTopicCheckedChanged, saveFollowedTopics) + is PopulatedFeed -> { + when (uiState) { + is FeedWithTopicSelection -> { + item { + TopicSelection(uiState, onTopicCheckedChanged) + } + item { + // Done button + Row( + horizontalArrangement = Arrangement.Center, + modifier = Modifier.fillMaxWidth() + ) { + Button( + onClick = saveFollowedTopics, + enabled = uiState.canSaveSelectedTopics, + modifier = Modifier + .padding(horizontal = 40.dp) + .width(364.dp) + ) { + Text(text = stringResource(R.string.done)) + } + } } - is ForYouFeedUiState.PopulatedFeed.FeedWithoutTopicSelection -> Unit } + is FeedWithoutTopicSelection -> Unit + } - items(uiState.feed) { (newsResource: NewsResource, isBookmarked: Boolean) -> - val launchResourceIntent = - Intent(Intent.ACTION_VIEW, Uri.parse(newsResource.url)) - val context = LocalContext.current + items(uiState.feed) { (newsResource: NewsResource, isBookmarked: Boolean) -> + val launchResourceIntent = + Intent(Intent.ACTION_VIEW, Uri.parse(newsResource.url)) + val context = LocalContext.current - NewsResourceCardExpanded( - newsResource = newsResource, - isBookmarked = isBookmarked, - onToggleBookmark = { - onNewsResourcesCheckedChanged(newsResource.id, !isBookmarked) - }, - onClick = { - startActivity(context, launchResourceIntent, null) - }, - ) - } + NewsResourceCardExpanded( + newsResource = newsResource, + isBookmarked = isBookmarked, + onClick = { startActivity(context, launchResourceIntent, null) }, + onToggleBookmark = { + onNewsResourcesCheckedChanged(newsResource.id, !isBookmarked) + } + ) } } } } } -/** - * The topic selection items - */ -private fun LazyListScope.TopicSelection( - uiState: ForYouFeedUiState.PopulatedFeed.FeedWithTopicSelection, - onTopicCheckedChanged: (Int, Boolean) -> Unit, - saveFollowedTopics: () -> Unit +@Composable +private fun TopicSelection( + uiState: ForYouFeedUiState, + onTopicCheckedChanged: (Int, Boolean) -> Unit ) { - item { - FlowRow( - mainAxisSpacing = 8.dp, - crossAxisSpacing = 8.dp, - modifier = Modifier.padding(horizontal = 40.dp) - ) { - uiState.topics.forEach { (topic, isSelected) -> - key(topic.id) { - // TODO: Add toggleable semantics - OutlinedButton( - onClick = { - onTopicCheckedChanged(topic.id, !isSelected) - }, - shape = RoundedCornerShape(50), - colors = if (isSelected) { - ButtonDefaults.buttonColors() - } else { - ButtonDefaults.outlinedButtonColors() - }, - modifier = Modifier.toggleable( - value = isSelected, role = Role.Button, onValueChange = {} - ) - ) { - Text( - text = topic.name.uppercase(), - ) - } - } - } - } - } + Column(Modifier.padding(top = 24.dp)) { + + Text( + text = stringResource(R.string.onboarding_guidance_title), + textAlign = TextAlign.Center, + modifier = Modifier.fillMaxWidth(), + style = NiaTypography.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 = NiaTypography.bodyMedium + ) - item { - Button( - onClick = saveFollowedTopics, - enabled = uiState.canSaveSelectedTopics, + LazyHorizontalGrid( + rows = Fixed(3), + horizontalArrangement = Arrangement.spacedBy(12.dp), + verticalArrangement = Arrangement.spacedBy(12.dp), modifier = Modifier - .padding(horizontal = 40.dp) + .height(192.dp) + .padding(top = 24.dp, bottom = 24.dp) .fillMaxWidth() ) { - Text(text = stringResource(R.string.done)) + val state: FeedWithTopicSelection = uiState as FeedWithTopicSelection + items(state.topics) { + SingleTopicButton( + name = it.topic.name, + topicId = it.topic.id, + isSelected = it.isFollowed, + onClick = onTopicCheckedChanged + ) + } } } } +@Composable +private fun SingleTopicButton( + name: String, + topicId: Int, + isSelected: Boolean, + onClick: (Int, Boolean) -> Unit +) { + Box( + modifier = Modifier + .width(264.dp) + .height(56.dp) + .padding(start = 12.dp, end = 8.dp) + .background( + MaterialTheme.colors.surface, + shape = RoundedCornerShape(corner = CornerSize(8.dp)) + ) + .clickable(onClick = { onClick(topicId, !isSelected) }), + contentAlignment = Alignment.CenterStart + ) { + Text( + text = name, + style = NiaTypography.titleSmall, + modifier = Modifier.padding(12.dp), + color = MaterialTheme.colors.onSurface + + ) + NiaToggleButton( + checked = isSelected, + modifier = Modifier.align(alignment = Alignment.CenterEnd), + onCheckedChange = { checked -> onClick(topicId, !isSelected) }, + icon = { + Icon(imageVector = NiaIcons.Add, contentDescription = name) + }, + checkedIcon = { + Icon(imageVector = NiaIcons.Check, contentDescription = name) + } + ) + } +} + @Preview @Composable fun ForYouScreenLoading() { @@ -184,7 +248,7 @@ fun ForYouScreenLoading() { @Composable fun ForYouScreenTopicSelection() { ForYouScreen( - uiState = ForYouFeedUiState.PopulatedFeed.FeedWithTopicSelection( + uiState = FeedWithTopicSelection( topics = listOf( FollowableTopic( topic = Topic( @@ -311,7 +375,7 @@ fun ForYouScreenTopicSelection() { @Composable fun PopulatedFeed() { ForYouScreen( - uiState = ForYouFeedUiState.PopulatedFeed.FeedWithoutTopicSelection( + uiState = FeedWithoutTopicSelection( feed = emptyList() ), onTopicCheckedChanged = { _, _ -> }, diff --git a/feature-foryou/src/main/res/values/strings.xml b/feature-foryou/src/main/res/values/strings.xml index fba7ed7c2..586ee170a 100644 --- a/feature-foryou/src/main/res/values/strings.xml +++ b/feature-foryou/src/main/res/values/strings.xml @@ -20,4 +20,6 @@ Done Loading for you… Navigate up + What are you interested in? + Updates from topics you follow will appear here. Follow some things to get started. diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 4719c1005..5eec1efa1 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -4,7 +4,7 @@ androidDesugarJdkLibs = "1.1.5" androidGradlePlugin = "7.1.2" androidxActivity = "1.4.0" androidxAppCompat = "1.3.0" -androidxCompose = "1.2.0-alpha03" +androidxCompose = "1.2.0-alpha06" androidxMaterialWindow = "1.2.0-SNAPSHOT" androidxComposeMaterial3 = "1.0.0-alpha07" androidxCore = "1.7.0" diff --git a/settings.gradle b/settings.gradle index 1c5bbab06..a76543a9f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -20,7 +20,7 @@ dependencyResolutionManagement { // Register the AndroidX snapshot repository first so snapshots don't attempt (and fail) // to download from the non-snapshot repositories maven { - url 'https://androidx.dev/snapshots/builds/8273139/artifacts/repository' + url 'https://androidx.dev/snapshots/builds/8350530/artifacts/repository' content { // The AndroidX snapshot repository will only have androidx artifacts, don't // bother trying to find other ones