From 5ba9fe47166b5591e72245fe28ed8f9581163b85 Mon Sep 17 00:00:00 2001 From: James Rich <2199651+jamesarich@users.noreply.github.com> Date: Mon, 16 Jun 2025 22:55:06 +0000 Subject: [PATCH] Refactor: Improve MessageItem layout and display of reactions (#2141) --- .../geeksville/mesh/ui/message/MessageList.kt | 18 +- .../mesh/ui/message/components/MessageItem.kt | 159 ++++++++++-------- .../mesh/ui/message/components/Reaction.kt | 70 ++++---- 3 files changed, 136 insertions(+), 111 deletions(-) diff --git a/app/src/main/java/com/geeksville/mesh/ui/message/MessageList.kt b/app/src/main/java/com/geeksville/mesh/ui/message/MessageList.kt index 2ddfe8428..dc44c03da 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/message/MessageList.kt +++ b/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 + } ) } } diff --git a/app/src/main/java/com/geeksville/mesh/ui/message/components/MessageItem.kt b/app/src/main/java/com/geeksville/mesh/ui/message/components/MessageItem.kt index 007978590..7b0a3ad66 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/message/components/MessageItem.kt +++ b/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 = 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 + ), + ) ) } } diff --git a/app/src/main/java/com/geeksville/mesh/ui/message/components/Reaction.kt b/app/src/main/java/com/geeksville/mesh/ui/message/components/Reaction.kt index 283152c8f..6945278aa 100644 --- a/app/src/main/java/com/geeksville/mesh/ui/message/components/Reaction.kt +++ b/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 = 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,