From 2cef47c271dbc5781a86ae699a0753cc191bfaea Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Mon, 26 Jun 2023 18:31:50 +0200 Subject: [PATCH] Iterate on reactions (#668) --- .../impl/timeline/TimelineStateProvider.kt | 6 +-- .../components/MessagesReactionButton.kt | 42 ++++++++++++------- .../event/TimelineItemEventFactory.kt | 8 ++-- .../impl/timeline/model/AggregatedReaction.kt | 8 ++-- .../model/AggregatedReactionProvider.kt | 10 ++--- .../messages/fixtures/aMessageEvent.kt | 2 +- .../api/timeline/item/event/EventReaction.kt | 5 ++- .../item/event/EventTimelineItemMapper.kt | 3 +- 8 files changed, 51 insertions(+), 33 deletions(-) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt index a15a5ffd6f..1c2b53bc56 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/TimelineStateProvider.kt @@ -100,7 +100,7 @@ internal fun aTimelineItemEvent( sendState: EventSendState = EventSendState.Sent(eventId), inReplyTo: InReplyTo? = null, debugInfo: TimelineItemDebugInfo = aTimelineItemDebugInfo(), - timelineItemReactions: TimelineItemReactions = aTimelineItemReactions(isMine = isMine), + timelineItemReactions: TimelineItemReactions = aTimelineItemReactions(), ): TimelineItem.Event { return TimelineItem.Event( id = eventId.value, @@ -122,12 +122,12 @@ internal fun aTimelineItemEvent( fun aTimelineItemReactions( count: Int = 1, - isMine: Boolean = true, + isHighlighted: Boolean = false, ): TimelineItemReactions { return TimelineItemReactions( reactions = buildList { repeat(count) { - add(AggregatedReaction(key = "👍", count = (it + 1).toString(), isOnMyMessage = isMine)) + add(AggregatedReaction(key = "👍", count = 1 + it, isHighlighted = isHighlighted)) } }.toPersistentList() ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/MessagesReactionButton.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/MessagesReactionButton.kt index 7a5e02efe5..f20f666a95 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/MessagesReactionButton.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/MessagesReactionButton.kt @@ -17,6 +17,8 @@ package io.element.android.features.messages.impl.timeline.components import androidx.compose.foundation.BorderStroke +import androidx.compose.foundation.border +import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.padding @@ -27,6 +29,7 @@ import androidx.compose.material3.MaterialTheme import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.graphics.Color import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp @@ -41,25 +44,36 @@ import io.element.android.libraries.designsystem.theme.components.Text @Composable fun MessagesReactionButton(reaction: AggregatedReaction, modifier: Modifier = Modifier) { - val backgroundColor = if (reaction.isOnMyMessage) { - ElementTheme.colors.messageFromMeBackground - } else { - ElementTheme.colors.messageFromOtherBackground - } + // First Surface is to render a border with the same background color as the background Surface( modifier = modifier, - color = backgroundColor, + // TODO Should use compound.bgSubtlePrimary + color = ElementTheme.colors.gray300, border = BorderStroke(2.dp, MaterialTheme.colorScheme.background), shape = RoundedCornerShape(corner = CornerSize(14.dp)), ) { - Row( - modifier = Modifier.padding(vertical = 6.dp, horizontal = 12.dp), - verticalAlignment = Alignment.CenterVertically - ) { - // TODO `reaction.isHighlighted` is not used. - Text(text = reaction.key, fontSize = 14.sp) - Spacer(modifier = Modifier.width(4.dp)) - Text(text = reaction.count, color = MaterialTheme.colorScheme.secondary, fontSize = 12.sp) + Box(modifier = Modifier.padding(2.dp)) { + val reactionModifier = if (reaction.isHighlighted) { + Modifier + // TODO Check the color, should use compound.borderInteractivePrimary + .border(BorderStroke(1.dp, Color(0xFF808994)), RoundedCornerShape(corner = CornerSize(12.dp))) + } else { + Modifier + } + Row( + modifier = reactionModifier.padding(vertical = 4.dp, horizontal = 12.dp), + verticalAlignment = Alignment.CenterVertically + ) { + Text(text = reaction.key, fontSize = 15.sp) + if (reaction.count > 1) { + Spacer(modifier = Modifier.width(4.dp)) + Text( + text = reaction.count.toString(), + color = if (reaction.isHighlighted) MaterialTheme.colorScheme.primary else MaterialTheme.colorScheme.secondary, + fontSize = 14.sp + ) + } + } } } } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt index 5a46b22855..7663c14517 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt @@ -24,6 +24,7 @@ import io.element.android.features.messages.impl.timeline.model.TimelineItemReac import io.element.android.libraries.core.bool.orTrue import io.element.android.libraries.designsystem.components.avatar.AvatarData import io.element.android.libraries.designsystem.components.avatar.AvatarSize +import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.event.EventSendState import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails @@ -34,6 +35,7 @@ import javax.inject.Inject class TimelineItemEventFactory @Inject constructor( private val contentFactory: TimelineItemContentFactory, + private val matrixClient: MatrixClient, ) { fun create( @@ -91,11 +93,11 @@ class TimelineItemEventFactory @Inject constructor( val aggregatedReactions = event.reactions.map { AggregatedReaction( key = it.key, - count = it.count.toString(), - isHighlighted = false, - isOnMyMessage = event.isOwn, + count = it.count.toInt(), + isHighlighted = it.senderIds.contains(matrixClient.sessionId), ) } + aggregatedReactions.sortedByDescending { it.count } return TimelineItemReactions(aggregatedReactions.toImmutableList()) } diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReaction.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReaction.kt index 78b7e48a0a..a435189dfe 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReaction.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReaction.kt @@ -16,9 +16,11 @@ package io.element.android.features.messages.impl.timeline.model -data class AggregatedReaction constructor( +/** + * @property isHighlighted true if the reaction has (also) been sent by the current user. + */ +data class AggregatedReaction( val key: String, - val count: String, - val isOnMyMessage: Boolean, + val count: Int, val isHighlighted: Boolean = false ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReactionProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReactionProvider.kt index 501bfc8c6b..148f565911 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReactionProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/model/AggregatedReactionProvider.kt @@ -22,22 +22,18 @@ open class AggregatedReactionProvider : PreviewParameterProvider get() = sequenceOf(false, true).flatMap { sequenceOf( - anAggregatedReaction(isOnMyMessage = it), - anAggregatedReaction(isOnMyMessage = it, count = "88"), - anAggregatedReaction(isOnMyMessage = it, isHighlighted = true), - anAggregatedReaction(isOnMyMessage = it, count = "88", isHighlighted = true), + anAggregatedReaction(isHighlighted = it), + anAggregatedReaction(isHighlighted = it, count = 88), ) } } fun anAggregatedReaction( key: String = "👍", - count: String = "1", // TODO Why is it a String? - isOnMyMessage: Boolean = false, + count: Int = 1, isHighlighted: Boolean = false, ) = AggregatedReaction( key = key, count = count, - isOnMyMessage = isOnMyMessage, isHighlighted = isHighlighted, ) diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/aMessageEvent.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/aMessageEvent.kt index e208daf6d5..6298042e89 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/aMessageEvent.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/fixtures/aMessageEvent.kt @@ -47,7 +47,7 @@ internal fun aMessageEvent( content = content, sentTime = "", isMine = isMine, - reactionsState = aTimelineItemReactions(count = 0, isMine = isMine), + reactionsState = aTimelineItemReactions(count = 0), sendState = EventSendState.Sent(AN_EVENT_ID), inReplyTo = inReplyTo, debugInfo = debugInfo, diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/EventReaction.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/EventReaction.kt index 7d20c727c7..8bea4b5330 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/EventReaction.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/EventReaction.kt @@ -16,7 +16,10 @@ package io.element.android.libraries.matrix.api.timeline.item.event +import io.element.android.libraries.matrix.api.core.UserId + data class EventReaction( val key: String, - val count: Long + val count: Long, + val senderIds: List ) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventTimelineItemMapper.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventTimelineItemMapper.kt index d8bb928d8f..549b0096d0 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventTimelineItemMapper.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/item/event/EventTimelineItemMapper.kt @@ -78,7 +78,8 @@ private fun List?.map(): List { return this?.map { EventReaction( key = it.key, - count = it.count.toLong() + count = it.count.toLong(), + senderIds = it.senders.map { sender -> UserId(sender) } ) } ?: emptyList() }