Refactor Topics news feed, tidy a few other bits up

Change-Id: Id4c9d1bce484137b363ec4cd21a45ca9863a2e7f

Author: dturner

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()
         )
     }

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<String?>(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
                         }
                     )
                 }

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 <T> LazyListScope.newsResourceCardItems(
-    items: List<T>,
-    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<UserNewsResource>,
+    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

core/ui/src/main/res/values/strings.xml

@@ -22,6 +22,6 @@
     <string name="card_tap_action">Open Resource Link</string>
     <string name="card_meta_data_text">%1$s • %2$s</string>
 
-    <string name="topic_is_followed">is followed</string>
-    <string name="topic_is_not_followed">is not followed</string>
+    <string name="topic_chip_content_description_when_followed">%1$s is followed</string>
+    <string name="topic_chip_content_description_when_not_followed">%1$s is not followed</string>
 </resources>

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)
             )