Refactor Topics news feed, tidy a few other bits up

Change-Id: Id4c9d1bce484137b363ec4cd21a45ca9863a2e7f
pull/508/head
Don Turner 3 years ago
parent 45e3cfd989
commit bad3ea60f5

@ -16,7 +16,7 @@
package com.google.samples.apps.nowinandroid.core.domain 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.NewsResource
import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video import com.google.samples.apps.nowinandroid.core.model.data.NewsResourceType.Video
import com.google.samples.apps.nowinandroid.core.model.data.Topic 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. // Check that the correct news resources are returned with their bookmarked state.
assertEquals( assertEquals(
listOf( sampleNewsResources.mapToUserNewsResources(userData),
UserNewsResource(sampleNewsResources[0], userData),
UserNewsResource(sampleNewsResources[1], userData),
UserNewsResource(sampleNewsResources[2], userData),
),
userNewsResources.first() userNewsResources.first()
) )
} }
@ -83,9 +79,7 @@ class GetUserNewsResourcesUseCaseTest {
assertEquals( assertEquals(
sampleNewsResources sampleNewsResources
.filter { it.topics.contains(sampleTopic1) } .filter { it.topics.contains(sampleTopic1) }
.map { .mapToUserNewsResources(emptyUserData),
UserNewsResource(it, emptyUserData)
},
userNewsResources.first() userNewsResources.first()
) )
} }

@ -236,8 +236,6 @@ fun NewsResourceTopics(
// Store the ID of the Topic which has its "following" menu expanded, if any. // 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. // To avoid UI confusion, only one topic can have an expanded menu at a time.
var expandedTopicId by remember { mutableStateOf<String?>(null) } 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( Row(
modifier = modifier.horizontalScroll(rememberScrollState()), // causes narrow chips modifier = modifier.horizontalScroll(rememberScrollState()), // causes narrow chips
@ -254,13 +252,21 @@ fun NewsResourceTopics(
onUnfollowClick = { }, // ToDo onUnfollowClick = { }, // ToDo
onBrowseClick = { }, // ToDo onBrowseClick = { }, // ToDo
text = { 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(
text = followableTopic.topic.name.uppercase(Locale.getDefault()), text = followableTopic.topic.name.uppercase(Locale.getDefault()),
modifier = Modifier.semantics { modifier = Modifier.semantics {
this.contentDescription = contentDescription
contentDescription = followableTopic.topic.name + " " +
if (followableTopic.isFollowed) isFollowed
else isNotFollowed
} }
) )
} }

@ -24,42 +24,36 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.toArgb import androidx.compose.ui.graphics.toArgb
import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.platform.LocalContext
import com.google.samples.apps.nowinandroid.core.domain.model.UserNewsResource 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 * Extension function for displaying a [List] of [NewsResourceCardExpanded] backed by a list of
* [List] [T]. * [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 * [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 * [onItemClick] optional parameter for action to be performed when the card is clicked. The
* default action launches an intent matching the card. * default action launches an intent matching the card.
*/ */
fun <T> LazyListScope.newsResourceCardItems( fun LazyListScope.userNewsResourceCardItems(
items: List<T>, items: List<UserNewsResource>,
newsResourceMapper: (item: T) -> UserNewsResource, // TODO remove this? onToggleBookmark: (item: UserNewsResource) -> Unit,
isBookmarkedMapper: (item: T) -> Boolean, onItemClick: ((item: UserNewsResource) -> Unit)? = null,
onToggleBookmark: (item: T) -> Unit,
onItemClick: ((item: T) -> Unit)? = null,
itemModifier: Modifier = Modifier, itemModifier: Modifier = Modifier,
) = items( ) = items(
items = items, items = items,
key = { newsResourceMapper(it).id }, key = { it.id },
itemContent = { item -> itemContent = { userNewsResource ->
val newsResource = newsResourceMapper(item) val resourceUrl = Uri.parse(userNewsResource.url)
val resourceUrl = Uri.parse(newsResource.url)
val backgroundColor = MaterialTheme.colorScheme.background.toArgb() val backgroundColor = MaterialTheme.colorScheme.background.toArgb()
val context = LocalContext.current val context = LocalContext.current
NewsResourceCardExpanded( NewsResourceCardExpanded(
userNewsResource = newsResource, userNewsResource = userNewsResource,
isBookmarked = isBookmarkedMapper(item), isBookmarked = userNewsResource.isSaved,
onToggleBookmark = { onToggleBookmark(item) }, onToggleBookmark = { onToggleBookmark(userNewsResource) },
onClick = { onClick = {
when (onItemClick) { when (onItemClick) {
null -> launchCustomChromeTab(context, resourceUrl, backgroundColor) null -> launchCustomChromeTab(context, resourceUrl, backgroundColor)
else -> onItemClick(item) else -> onItemClick(userNewsResource)
} }
}, },
modifier = itemModifier modifier = itemModifier

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

@ -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.model.data.previewTopics
import com.google.samples.apps.nowinandroid.core.ui.DevicePreviews 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.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.R.string
import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading import com.google.samples.apps.nowinandroid.feature.topic.TopicUiState.Loading
@ -145,7 +145,7 @@ private fun LazyListScope.TopicBody(
TopicHeader(name, description, imageUrl) TopicHeader(name, description, imageUrl)
} }
TopicCards(news, onBookmarkChanged) userNewsResourceCards(news, onBookmarkChanged)
} }
@Composable @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, news: NewsUiState,
onBookmarkChanged: (String, Boolean) -> Unit onBookmarkChanged: (String, Boolean) -> Unit
) { ) {
when (news) { when (news) {
is NewsUiState.Success -> { is NewsUiState.Success -> {
newsResourceCardItems( userNewsResourceCardItems(
items = news.news, items = news.news,
newsResourceMapper = { it },
isBookmarkedMapper = { it.isSaved },
onToggleBookmark = { onBookmarkChanged(it.id, !it.isSaved) }, onToggleBookmark = { onBookmarkChanged(it.id, !it.isSaved) },
itemModifier = Modifier.padding(24.dp) itemModifier = Modifier.padding(24.dp)
) )

Loading…
Cancel
Save