Refactor: Improve MessageItem layout and display of reactions (#2141)

app/src/main/java/com/geeksville/mesh/ui/message/MessageList.kt

@@ -49,7 +49,6 @@ import androidx.compose.ui.res.stringResource
 import androidx.compose.ui.text.style.TextAlign
 import androidx.compose.ui.unit.dp
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
-import com.geeksville.mesh.DataPacket
 import com.geeksville.mesh.MessageStatus
 import com.geeksville.mesh.R
 import com.geeksville.mesh.database.entity.Reaction
@@ -57,7 +56,6 @@ import com.geeksville.mesh.model.Message
 import com.geeksville.mesh.model.UIViewModel
 import com.geeksville.mesh.ui.message.components.MessageItem
 import com.geeksville.mesh.ui.message.components.ReactionDialog
-import com.geeksville.mesh.ui.message.components.ReactionRow
 import com.geeksville.mesh.ui.node.components.NodeMenuAction
 import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.flow.collectLatest
@@ -167,7 +165,6 @@ internal fun MessageList(
         reverseLayout = true,
     ) {
         items(messages, key = { it.uuid }) { msg ->
-            val fromLocal = msg.node.user.id == DataPacket.ID_LOCAL
             val selected by remember { derivedStateOf { selectedIds.value.contains(msg.uuid) } }
             var node by remember {
                 mutableStateOf(nodes.find { it.num == msg.node.num } ?: msg.node)
@@ -175,7 +172,6 @@ internal fun MessageList(
             LaunchedEffect(nodes) {
                 node = nodes.find { it.num == msg.node.num } ?: msg.node
             }
-            ReactionRow(fromLocal, msg.emojis) { showReactionDialog = msg.emojis }
             Box(Modifier.wrapContentSize(Alignment.TopStart)) {
                 MessageItem(
                     node = node,
@@ -190,16 +186,20 @@ internal fun MessageList(
                     },
                     onAction = onNodeMenuAction,
                     onStatusClick = { showStatusDialog = msg },
-                    onSendReaction = { onSendReaction(it, msg.packetId) },
+                    emojis = msg.emojis,
+                    sendReaction = { onSendReaction(it, msg.packetId) },
+                    onShowReactions = { showReactionDialog = msg.emojis },
                     isConnected = isConnected,
                     snr = msg.snr,
                     rssi = msg.rssi,
-                    hopsAway = if (msg.hopsAway > 0) { "%s: %d".format(
+                    hopsAway = if (msg.hopsAway > 0) {
+                        "%s: %d".format(
                             stringResource(id = R.string.hops_away),
                             msg.hopsAway
-                        ) } else {
-                            null
-                        }
+                        )
+                    } else {
+                        null
+                    }
                 )
             }
         }

app/src/main/java/com/geeksville/mesh/ui/message/components/MessageItem.kt

@@ -28,7 +28,7 @@ import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.Spacer
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
-import androidx.compose.foundation.shape.RoundedCornerShape
+import androidx.compose.foundation.layout.width
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.twotone.Cloud
 import androidx.compose.material.icons.twotone.CloudDone
@@ -38,9 +38,11 @@ import androidx.compose.material.icons.twotone.HowToReg
 import androidx.compose.material.icons.twotone.Warning
 import androidx.compose.material3.Card
 import androidx.compose.material3.CardDefaults
+import androidx.compose.material3.ExperimentalMaterial3ExpressiveApi
 import androidx.compose.material3.Icon
 import androidx.compose.material3.MaterialTheme
 import androidx.compose.material3.Text
+import androidx.compose.material3.contentColorFor
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Alignment
 import androidx.compose.ui.Modifier
@@ -52,6 +54,7 @@ import androidx.compose.ui.unit.dp
 import com.geeksville.mesh.DataPacket
 import com.geeksville.mesh.MessageStatus
 import com.geeksville.mesh.R
+import com.geeksville.mesh.database.entity.Reaction
 import com.geeksville.mesh.model.Node
 import com.geeksville.mesh.ui.common.components.AutoLinkText
 import com.geeksville.mesh.ui.common.components.Rssi
@@ -62,75 +65,69 @@ import com.geeksville.mesh.ui.node.components.NodeChip
 import com.geeksville.mesh.ui.node.components.NodeMenuAction
 
 @Suppress("LongMethod", "CyclomaticComplexMethod")
-@OptIn(ExperimentalFoundationApi::class)
+@OptIn(ExperimentalFoundationApi::class, ExperimentalMaterial3ExpressiveApi::class)
 @Composable
 internal fun MessageItem(
     node: Node,
     messageText: String?,
     messageTime: String,
     messageStatus: MessageStatus?,
+    emojis: List<Reaction> = emptyList(),
+    sendReaction: (String) -> Unit = {},
+    onShowReactions: () -> Unit = {},
     selected: Boolean,
     modifier: Modifier = Modifier,
     onClick: () -> Unit = {},
     onLongClick: () -> Unit = {},
     onAction: (NodeMenuAction) -> Unit = {},
     onStatusClick: () -> Unit = {},
-    onSendReaction: (String) -> Unit = {},
     isConnected: Boolean,
     snr: Float,
     rssi: Int,
     hopsAway: String?,
-) = Row(
+) = Column(
     modifier = modifier
         .fillMaxWidth()
         .background(color = if (selected) Color.Gray else MaterialTheme.colorScheme.background),
-    verticalAlignment = Alignment.CenterVertically,
 ) {
     val fromLocal = node.user.id == DataPacket.ID_LOCAL
     val messageColor = if (fromLocal) {
         MaterialTheme.colorScheme.secondaryContainer
     } else {
-        MaterialTheme.colorScheme.tertiaryContainer
-    }
-    val (topStart, topEnd) = if (fromLocal) 12.dp to 4.dp else 4.dp to 12.dp
-    val messageModifier = if (fromLocal) {
-        Modifier.padding(start = 48.dp, top = 8.dp, end = 8.dp, bottom = 6.dp)
-    } else {
-        Modifier.padding(start = 8.dp, top = 8.dp, end = 0.dp, bottom = 6.dp)
-    }
-    if (!fromLocal) {
-        NodeChip(
-            node = node,
-            modifier = Modifier
-                .padding(start = 8.dp, end = 4.dp),
-            onAction = onAction,
-            isConnected = isConnected,
-            isThisNode = false,
-        )
+        Color(node.colors.second).copy(alpha = 0.25f)
     }
+    val messageModifier = Modifier.padding(start = 8.dp, top = 8.dp, end = 8.dp)
+
     Card(
         modifier = Modifier
-            .weight(1f)
             .combinedClickable(
                 onClick = onClick,
                 onLongClick = onLongClick,
             )
             .then(messageModifier),
         colors = CardDefaults.cardColors(
-            containerColor = messageColor
+            containerColor = messageColor,
+            contentColor = contentColorFor(messageColor),
         ),
-        shape = RoundedCornerShape(topStart, topEnd, bottomStart = 12.dp, bottomEnd = 12.dp)
     ) {
-        Row(
+        Column(
             modifier = Modifier
                 .fillMaxWidth()
-                .padding(horizontal = 8.dp),
-            verticalAlignment = Alignment.CenterVertically,
+                .padding(8.dp),
         ) {
-            Column(
-                modifier = Modifier.padding(top = 8.dp),
+            Row(
+                modifier = Modifier
+                    .fillMaxWidth(),
+                verticalAlignment = Alignment.CenterVertically,
             ) {
                 if (!fromLocal) {
+                    NodeChip(
+                        node = node,
+                        onAction = onAction,
+                        isConnected = isConnected,
+                        isThisNode = false,
+                    )
+                    Spacer(Modifier.width(4.dp))
                     Text(
                         text = with(node.user) { "$longName ($id)" },
                         modifier = Modifier.padding(bottom = 4.dp),
@@ -139,52 +136,66 @@ internal fun MessageItem(
                         style = MaterialTheme.typography.labelLarge
                     )
                 }
-                AutoLinkText(
-                    text = messageText.orEmpty(),
-                )
-                Row(
-                    modifier = Modifier.fillMaxWidth(),
-                    horizontalArrangement = Arrangement.End,
-                    verticalAlignment = Alignment.CenterVertically,
-                ) {
-                    if (!fromLocal) {
-                        if (hopsAway == null) {
-                            Snr(snr, fontSize = MaterialTheme.typography.bodySmall.fontSize)
-                            Spacer(Modifier.weight(1f))
-                            Rssi(rssi, fontSize = MaterialTheme.typography.bodySmall.fontSize)
-                        } else { Text(
+            }
+            AutoLinkText(
+                modifier = Modifier
+                    .fillMaxWidth()
+                    .padding(4.dp),
+                text = messageText.orEmpty(),
+                style = MaterialTheme.typography.bodyMedium,
+            )
+            Row(
+                modifier = Modifier
+                    .fillMaxWidth()
+                    .padding(horizontal = 4.dp),
+                horizontalArrangement = Arrangement.SpaceBetween,
+                verticalAlignment = Alignment.CenterVertically,
+            ) {
+                if (!fromLocal) {
+                    if (hopsAway == null) {
+                        Row(
+                            horizontalArrangement = Arrangement.spacedBy(8.dp),
+                        ) {
+                            Snr(snr, fontSize = MaterialTheme.typography.labelSmall.fontSize)
+                            Rssi(rssi, fontSize = MaterialTheme.typography.labelSmall.fontSize)
+                        }
+                    } else {
+                        Text(
                             text = hopsAway,
-                            fontSize = MaterialTheme.typography.bodySmall.fontSize,
-                        ) }
-                        Spacer(Modifier.weight(1f))
-                    }
-                    Text(
-                        text = messageTime,
-                        fontSize = MaterialTheme.typography.bodySmall.fontSize,
-                    )
-                    AnimatedVisibility(visible = fromLocal) {
-                        Icon(
-                            imageVector = when (messageStatus) {
-                                MessageStatus.RECEIVED -> Icons.TwoTone.HowToReg
-                                MessageStatus.QUEUED -> Icons.TwoTone.CloudUpload
-                                MessageStatus.DELIVERED -> Icons.TwoTone.CloudDone
-                                MessageStatus.ENROUTE -> Icons.TwoTone.Cloud
-                                MessageStatus.ERROR -> Icons.TwoTone.CloudOff
-                                else -> Icons.TwoTone.Warning
-                            },
-                            contentDescription = stringResource(R.string.message_delivery_status),
-                            modifier = Modifier
-                                .padding(start = 8.dp)
-                                .clickable { onStatusClick() },
+                            style = MaterialTheme.typography.labelSmall,
                         )
                     }
                 }
+                Text(
+                    text = messageTime,
+                    style = MaterialTheme.typography.labelSmall,
+                )
+                AnimatedVisibility(visible = fromLocal) {
+                    Icon(
+                        imageVector = when (messageStatus) {
+                            MessageStatus.RECEIVED -> Icons.TwoTone.HowToReg
+                            MessageStatus.QUEUED -> Icons.TwoTone.CloudUpload
+                            MessageStatus.DELIVERED -> Icons.TwoTone.CloudDone
+                            MessageStatus.ENROUTE -> Icons.TwoTone.Cloud
+                            MessageStatus.ERROR -> Icons.TwoTone.CloudOff
+                            else -> Icons.TwoTone.Warning
+                        },
+                        contentDescription = stringResource(R.string.message_delivery_status),
+                        modifier = Modifier
+                            .padding(start = 8.dp)
+                            .clickable { onStatusClick() },
+                    )
+                }
             }
         }
     }
-    if (!fromLocal) {
-        ReactionButton(Modifier.padding(4.dp), onSendReaction)
-    }
+    ReactionRow(
+        modifier = Modifier
+            .fillMaxWidth(),
+        reactions = emojis,
+        onSendReaction = sendReaction,
+        onShowReactions = onShowReactions
+    )
 }
 
 @PreviewLightDark
@@ -200,7 +211,15 @@ private fun MessageItemPreview() {
             isConnected = true,
             snr = 20.5f,
             rssi = 90,
-            hopsAway = null
+            hopsAway = null,
+            emojis = listOf(
+                Reaction(
+                    emoji = "\uD83D\uDE42",
+                    user = NodePreviewParameterProvider().values.first().user,
+                    replyId = 0,
+                    timestamp = 0L
+                ),
+            )
         )
     }
 }

app/src/main/java/com/geeksville/mesh/ui/message/components/Reaction.kt

@@ -19,20 +19,20 @@ package com.geeksville.mesh.ui.message.components
 
 import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
+import androidx.compose.foundation.combinedClickable
 import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.ExperimentalLayoutApi
-import androidx.compose.foundation.layout.FlowRow
 import androidx.compose.foundation.layout.Row
 import androidx.compose.foundation.layout.fillMaxHeight
 import androidx.compose.foundation.layout.fillMaxWidth
+import androidx.compose.foundation.layout.height
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.layout.size
 import androidx.compose.foundation.lazy.LazyColumn
 import androidx.compose.foundation.lazy.LazyRow
 import androidx.compose.foundation.lazy.items
 import androidx.compose.foundation.shape.CircleShape
-import androidx.compose.foundation.shape.RoundedCornerShape
 import androidx.compose.material.icons.Icons
 import androidx.compose.material.icons.filled.EmojiEmotions
 import androidx.compose.material3.Badge
@@ -65,21 +65,22 @@ import com.geeksville.mesh.ui.common.theme.AppTheme
 @Composable
 fun ReactionButton(
     modifier: Modifier = Modifier,
-    onClick: (String) -> Unit = {}
+    onSendReaction: (String) -> Unit = {},
 ) {
     var showEmojiPickerDialog by remember { mutableStateOf(false) }
     if (showEmojiPickerDialog) {
         EmojiPickerDialog(
-            onConfirm = {
+            onConfirm = { selectedEmoji ->
                 showEmojiPickerDialog = false
-                onClick(it)
+                onSendReaction(selectedEmoji)
             },
             onDismiss = { showEmojiPickerDialog = false }
         )
     }
     IconButton(
-        modifier = modifier.size(48.dp),
-        onClick = { showEmojiPickerDialog = true }
+        modifier = modifier
+            .size(48.dp),
+        onClick = { showEmojiPickerDialog = true },
     ) {
         Icon(
             imageVector = Icons.Default.EmojiEmotions,
@@ -93,9 +94,9 @@ private fun ReactionItem(
     emoji: String,
     emojiCount: Int = 1,
     onClick: () -> Unit = {},
+    onLongClick: () -> Unit = {},
 ) {
     BadgedBox(
-        modifier = Modifier.padding(start = 2.dp, top = 2.dp, end = 2.dp, bottom = 4.dp),
         badge = {
             if (emojiCount > 1) {
                 Badge {
@@ -109,14 +110,17 @@ private fun ReactionItem(
     ) {
         Surface(
             modifier = Modifier
-                .clickable { onClick() },
+                .combinedClickable(
+                    onClick = onClick,
+                    onLongClick = onLongClick
+                ),
             color = MaterialTheme.colorScheme.primaryContainer,
-            shape = RoundedCornerShape(32.dp),
+            shape = CircleShape,
         ) {
             Text(
                 text = emoji,
                 modifier = Modifier
-                    .padding(8.dp)
+                    .padding(4.dp)
                     .clip(CircleShape),
             )
         }
@@ -126,35 +130,38 @@ private fun ReactionItem(
 @OptIn(ExperimentalLayoutApi::class)
 @Composable
 fun ReactionRow(
-    fromLocal: Boolean,
+    modifier: Modifier = Modifier,
     reactions: List<Reaction> = emptyList(),
-    onSendReaction: (String) -> Unit = {}
+    onSendReaction: (String) -> Unit = {},
+    onShowReactions: () -> Unit = {}
 ) {
-    val emojiList by remember(reactions) {
-        mutableStateOf(
-            reduceEmojis(
-                if (fromLocal) {
-                    reactions.map { it.emoji }
-                } else {
-                    reactions.map { it.emoji }.reversed()
-                }
-            ).entries
-        )
-    }
+    val emojiList =
+        reduceEmojis(
+            reactions.reversed().map { it.emoji }
+        ).entries
 
-    FlowRow(
-        modifier = Modifier
-            .fillMaxWidth()
-            .padding(horizontal = 16.dp),
-        horizontalArrangement = if (fromLocal) Arrangement.End else Arrangement.Start
+    LazyRow(
+        modifier = modifier.height(48.dp).padding(bottom = 8.dp),
+        horizontalArrangement = Arrangement.End,
+        verticalAlignment = Alignment.CenterVertically,
+        reverseLayout = true
     ) {
-        emojiList.forEach { entry ->
+        item {
+            ReactionButton(
+                onSendReaction = onSendReaction,
+            )
+        }
+        items(
+            emojiList.size
+        ) { index ->
+            val entry = emojiList.elementAt(index)
             ReactionItem(
                 emoji = entry.key,
                 emojiCount = entry.value,
                 onClick = {
                     onSendReaction(entry.key)
-                }
+                },
+                onLongClick = onShowReactions,
             )
         }
     }
@@ -237,7 +244,6 @@ fun ReactionItemPreview() {
 fun ReactionRowPreview() {
     AppTheme {
         ReactionRow(
-            fromLocal = true,
             reactions = listOf(
                 Reaction(
                     replyId = 1,