From 5ef7084baa9e638340ecc55f873ecd732d26cc27 Mon Sep 17 00:00:00 2001 From: Don Turner Date: Thu, 5 Jan 2023 20:54:15 +0000 Subject: [PATCH] Refactor Topics news feed, tidy a few other bits up Change-Id: Id4c9d1bce484137b363ec4cd21a45ca9863a2e7f --- .../domain/GetUserNewsResourcesUseCaseTest.kt | 12 ++----- .../nowinandroid/core/ui/NewsResourceCard.kt | 18 +++++++---- .../core/ui/NewsResourceCardList.kt | 32 ++++++++----------- core/ui/src/main/res/values/strings.xml | 4 +-- .../nowinandroid/feature/topic/TopicScreen.kt | 11 +++---- 5 files changed, 35 insertions(+), 42 deletions(-) diff --git a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt index 11e46e41d..fe50ecf4d 100644 --- a/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt +++ b/core/domain/src/test/java/com/google/samples/apps/nowinandroid/core/domain/GetUserNewsResourcesUseCaseTest.kt @@ -16,7 +16,7 @@ package com.google.samples.apps.nowinandroid.core.domain -import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource +import com.google.samples.apps.nowinandroid.core.domain.model.mapToUserNewsResources 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.Topic @@ -60,11 +60,7 @@ class GetUserNewsResourcesUseCaseTest { // Check that the correct news resources are returned with their bookmarked state. assertEquals( - listOf( - UserNewsResource(sampleNewsResources[0], userData), - UserNewsResource(sampleNewsResources[1], userData), - UserNewsResource(sampleNewsResources[2], userData), - ), + sampleNewsResources.mapToUserNewsResources(userData), userNewsResources.first() ) } @@ -83,9 +79,7 @@ class GetUserNewsResourcesUseCaseTest { assertEquals( sampleNewsResources .filter { it.topics.contains(sampleTopic1) } - .map { - UserNewsResource(it, emptyUserData) - }, + .mapToUserNewsResources(emptyUserData), userNewsResources.first() ) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt index 59b18ca9c..76657067e 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCard.kt @@ -236,8 +236,6 @@ fun NewsResourceTopics( // Store the ID of the Topic which has its "following" menu expanded, if any. // To avoid UI confusion, only one topic can have an expanded menu at a time. var expandedTopicId by remember { mutableStateOf(null) } - val isFollowed = stringResource(R.string.topic_is_followed) - val isNotFollowed = stringResource(R.string.topic_is_not_followed) Row( modifier = modifier.horizontalScroll(rememberScrollState()), // causes narrow chips @@ -254,13 +252,21 @@ fun NewsResourceTopics( onUnfollowClick = { }, // ToDo onBrowseClick = { }, // ToDo text = { + val contentDescription = if (followableTopic.isFollowed) { + stringResource( + R.string.topic_chip_content_description_when_followed, + followableTopic.topic.name + ) + } else { + stringResource( + R.string.topic_chip_content_description_when_not_followed, + followableTopic.topic.name + ) + } Text( text = followableTopic.topic.name.uppercase(Locale.getDefault()), modifier = Modifier.semantics { - - contentDescription = followableTopic.topic.name + " " + - if (followableTopic.isFollowed) isFollowed - else isNotFollowed + this.contentDescription = contentDescription } ) } diff --git a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCardList.kt b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCardList.kt index c7441c182..e0bd22785 100644 --- a/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCardList.kt +++ b/core/ui/src/main/java/com/google/samples/apps/nowinandroid/core/ui/NewsResourceCardList.kt @@ -24,42 +24,36 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.graphics.toArgb import androidx.compose.ui.platform.LocalContext import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource -import com.google.samples.apps.nowinandroid.core.model.data.NewsResource /** - * Extension function for displaying a [List] of [NewsResourceCardExpanded] backed by a generic - * [List] [T]. + * Extension function for displaying a [List] of [NewsResourceCardExpanded] backed by a list of + * [UserNewsResource]s. * - * [newsResourceMapper] maps type [T] to a [NewsResource] - * [isBookmarkedMapper] maps type [T] to whether the [NewsResource] is bookmarked * [onToggleBookmark] defines the action invoked when a user wishes to bookmark an item * [onItemClick] optional parameter for action to be performed when the card is clicked. The * default action launches an intent matching the card. */ -fun LazyListScope.newsResourceCardItems( - items: List, - newsResourceMapper: (item: T) -> UserNewsResource, // TODO remove this? - isBookmarkedMapper: (item: T) -> Boolean, - onToggleBookmark: (item: T) -> Unit, - onItemClick: ((item: T) -> Unit)? = null, +fun LazyListScope.userNewsResourceCardItems( + items: List, + onToggleBookmark: (item: UserNewsResource) -> Unit, + onItemClick: ((item: UserNewsResource) -> Unit)? = null, itemModifier: Modifier = Modifier, ) = items( items = items, - key = { newsResourceMapper(it).id }, - itemContent = { item -> - val newsResource = newsResourceMapper(item) - val resourceUrl = Uri.parse(newsResource.url) + key = { it.id }, + itemContent = { userNewsResource -> + val resourceUrl = Uri.parse(userNewsResource.url) val backgroundColor = MaterialTheme.colorScheme.background.toArgb() val context = LocalContext.current NewsResourceCardExpanded( - userNewsResource = newsResource, - isBookmarked = isBookmarkedMapper(item), - onToggleBookmark = { onToggleBookmark(item) }, + userNewsResource = userNewsResource, + isBookmarked = userNewsResource.isSaved, + onToggleBookmark = { onToggleBookmark(userNewsResource) }, onClick = { when (onItemClick) { null -> launchCustomChromeTab(context, resourceUrl, backgroundColor) - else -> onItemClick(item) + else -> onItemClick(userNewsResource) } }, modifier = itemModifier diff --git a/core/ui/src/main/res/values/strings.xml b/core/ui/src/main/res/values/strings.xml index fff33bf3a..bfb1d38de 100644 --- a/core/ui/src/main/res/values/strings.xml +++ b/core/ui/src/main/res/values/strings.xml @@ -22,6 +22,6 @@ Open Resource Link %1$s • %2$s - is followed - is not followed + %1$s is followed + %1$s is not followed diff --git a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt index 263c04d78..b3263839f 100644 --- a/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt +++ b/feature/topic/src/main/java/com/google/samples/apps/nowinandroid/feature/topic/TopicScreen.kt @@ -57,7 +57,7 @@ import com.google.samples.apps.nowinandroid.core.domain.model.previewUserNewsRes import com.google.samples.apps.nowinandroid.core.model.data.previewTopics import com.google.samples.apps.nowinandroid.core.ui.DevicePreviews import com.google.samples.apps.nowinandroid.core.ui.TrackScrollJank -import com.google.samples.apps.nowinandroid.core.ui.newsResourceCardItems +import com.google.samples.apps.nowinandroid.core.ui.userNewsResourceCardItems import com.google.samples.apps.nowinandroid.feature.topic.R.string import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading @@ -145,7 +145,7 @@ private fun LazyListScope.TopicBody( TopicHeader(name, description, imageUrl) } - TopicCards(news, onBookmarkChanged) + userNewsResourceCards(news, onBookmarkChanged) } @Composable @@ -173,16 +173,15 @@ private fun TopicHeader(name: String, description: String, imageUrl: String) { } } -private fun LazyListScope.TopicCards( +// TODO: Could/should this be replaced with [LazyGridScope.newsFeed]? +private fun LazyListScope.userNewsResourceCards( news: NewsUiState, onBookmarkChanged: (String, Boolean) -> Unit ) { when (news) { is NewsUiState.Success -> { - newsResourceCardItems( + userNewsResourceCardItems( items = news.news, - newsResourceMapper = { it }, - isBookmarkedMapper = { it.isSaved }, onToggleBookmark = { onBookmarkChanged(it.id, !it.isSaved) }, itemModifier = Modifier.padding(24.dp) )