From 75864b0113d8f32faf03ae9470714c6a3587cfe9 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Mon, 28 Sep 2026 11:23:46 -0400 Subject: [PATCH] fix(chat): show one reaction strip per link-card message A message split around its link card is two or three rows, and every row of the selected message is focused, so each composed its own quick reaction strip. Only the pressed row hosts the strip now, anchored to the union of the rows' bounds that MessageList collects. The stored reaction pills were also copied onto every row by splitAroundLinkCard (the live overlay already went to the last row only), so a reacted-to message showed its pills under each row. The split now leaves them on the last row. --- .../screens/components/MessageList.kt | 34 +++++++++++++++++-- .../internal/screens/components/MessageRow.kt | 29 ++++++++++------ .../shared/chat/models/ChatListItem.kt | 6 ++-- .../shared/chat/ui/QuickReactionStrip.kt | 14 ++++++++ .../chat/models/SplitAroundLinkCardTest.kt | 17 +++++++++- .../ui/QuickReactionStripPlacementTest.kt | 28 +++++++++++++++ 6 files changed, 111 insertions(+), 17 deletions(-) diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt index 0b1a1ba6e3..a735d350a7 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageList.kt @@ -19,10 +19,12 @@ import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.foundation.shape.CircleShape import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.runtime.DisposableEffect import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableIntStateOf +import androidx.compose.runtime.mutableStateMapOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState @@ -36,6 +38,7 @@ import androidx.compose.ui.input.pointer.pointerInput import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.IntRect import androidx.compose.ui.unit.dp import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.LocalLifecycleOwner @@ -61,6 +64,7 @@ import com.flipcash.shared.chat.reactions.ReactionRefreshPlanner import com.flipcash.shared.chat.ui.ChatAnimations import com.flipcash.shared.chat.ui.GroupInviteCardDefaults import com.flipcash.shared.chat.ui.GroupInviteLinkCard +import com.flipcash.shared.chat.ui.QuickReactionStripPlacement import com.flipcash.shared.chat.ui.transcriptCardWidth import com.getcode.theme.CodeTheme import com.getcode.ui.utils.rememberKeyboardController @@ -296,6 +300,12 @@ internal fun MessageList( } } + // The window bounds of each row of the selected message, by row key, so the quick strip + // can sit over all of them. Emptied with every new selection. + val selectedRowBounds = remember(state.selection?.messageKey) { + mutableStateMapOf() + } + LazyColumn( // NB: no sheetResignmentBehavior, unlike every other scrolling list in the app. The // conversation is a full-screen destination, not a sheet, so there is no dismiss drag @@ -358,6 +368,18 @@ internal fun MessageList( else -> true } + // Every row of a message split around its card is focused, but only the pressed + // one hosts the quick strip — one strip per message, as iOS gives one per context + // menu — and it spans the rows the others measure. + val inSelection = focused && bubble != null && + bubble.messageKey == state.selection?.messageKey + if (inSelection) { + DisposableEffect(selectedRowBounds, item.itemKey) { + onDispose { selectedRowBounds.remove(item.itemKey) } + } + } + val hostsStrip = inSelection && item.itemKey == state.selection?.itemKey + MessageRow( index = index, item = item, @@ -369,10 +391,16 @@ internal fun MessageList( animateInsertion = animateInsertion, showsSenderGutter = isGroup, topBarBottom = topBarBottom, - quickReactionStrip = if (focused && bubble?.messageId == state.selection?.messageId) { - state.quickReactionStrip + quickReactionStrip = if (hostsStrip) state.quickReactionStrip else emptyList(), + stripAnchor = if (hostsStrip) { + QuickReactionStripPlacement.messageBounds(selectedRowBounds.values) + } else { + null + }, + onBubbleBounds = if (inSelection) { + { bounds -> selectedRowBounds[item.itemKey] = bounds } } else { - emptyList() + null }, attention = if (bubble != null && bubble.messageId == attentionId) { readAttention diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt index a8ebcefbdd..befb41a531 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/components/MessageRow.kt @@ -100,7 +100,15 @@ internal fun MessageRow( focused: Boolean, animateInsertion: Boolean, showsSenderGutter: Boolean, + /** The quick strip, for the one row that shows it; empty on every other row. */ quickReactionStrip: List = emptyList(), + /** + * What the strip lines up with, in window coordinates: every measured row of the message, so a + * message split around its card gets one strip over the whole of it. + */ + stripAnchor: IntRect? = null, + /** Receives this row's bubble bounds in the window while its message is selected. */ + onBubbleBounds: ((IntRect) -> Unit)? = null, /** Where the top bar ends, from the window's top; the strip stays below it. */ topBarBottom: Dp = 0.dp, attention: () -> Float = { 0f }, @@ -322,14 +330,13 @@ internal fun MessageRow( Box(insertionModifier.then(liftModifier)) { val stripShown = selecting && focused && item.canReact && quickReactionStrip.isNotEmpty() - // The strip lines up with the bubble as drawn, which sits inside - // a full-width layout, so it's measured here rather than taken - // from the popup's anchor. - var bubbleBounds by remember { mutableStateOf(null) } ContentBubble( - modifier = Modifier.addIf(stripShown) { + // The strip lines up with the bubble as drawn, which sits + // inside a full-width layout, so it's measured here rather than + // taken from the popup's anchor. + modifier = Modifier.addIf(onBubbleBounds != null) { Modifier.onGloballyPositioned { - bubbleBounds = it.boundsInWindow().roundToIntRect() + onBubbleBounds?.invoke(it.boundsInWindow().roundToIntRect()) } }, item = item, @@ -350,14 +357,14 @@ internal fun MessageRow( attention = attention, ) - // The quick strip lives above the bubble, only while this exact - // message is the selected one — it replaces the backdrop's own - // reach for a reaction with something faster than opening the - // picker, and disappears the moment selection moves off. + // The quick strip lives above the message, only while it is the + // selected one — it replaces the backdrop's own reach for a + // reaction with something faster than opening the picker, and + // disappears the moment selection moves off. if (stripShown) { QuickReactionStripPopup( entries = quickReactionStrip, - bubbleBounds = bubbleBounds, + bubbleBounds = stripAnchor, hugsTrailing = item.isFromSelf, onToggle = { emoji -> onAction( diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt index 54e6b8991f..3d5b54bdd4 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/models/ChatListItem.kt @@ -206,8 +206,9 @@ enum class MessagePart(val id: String) { * gap and the brackets around a word that now has a row of its own. Punctuation past a space * belongs to the words beside it. Other links stay in whichever text row they fell in, underlined. * - * The reply citation goes on the first row and the "Edited" marker and receipt on the last, so a - * split message reads as one message stacked in three pieces rather than as three messages. + * The reply citation goes on the first row and the "Edited" marker, receipt and reaction pills on + * the last, so a split message reads as one message stacked in three pieces rather than as three + * messages. * * A card whose span does not fit the text is dropped rather than split, and the message renders as * text with its links underlined. @@ -241,6 +242,7 @@ fun ChatListItem.ContentBubble.splitAroundLinkCard(): List= minTop) above else anchor.bottom + gap return IntOffset(x, y.coerceIn(minTop, maxOf(minTop, window.height - strip.height))) } + + /** + * The anchor for a message drawn as several rows -- one split around its link card -- so the + * strip sits above the top row, or below the bottom one, rather than between two of them. + * `null` until a row has been measured. + */ + fun messageBounds(rows: Collection): IntRect? = rows.reduceOrNull { union, row -> + IntRect( + left = minOf(union.left, row.left), + top = minOf(union.top, row.top), + right = maxOf(union.right, row.right), + bottom = maxOf(union.bottom, row.bottom), + ) + } } diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SplitAroundLinkCardTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SplitAroundLinkCardTest.kt index 311ee025bc..ca1265751e 100644 --- a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SplitAroundLinkCardTest.kt +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/models/SplitAroundLinkCardTest.kt @@ -1,6 +1,7 @@ package com.flipcash.shared.chat.models import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.reactions.ReactionPill import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNull @@ -14,7 +15,11 @@ class SplitAroundLinkCardTest { private val link = "https://send.flipcash.com/c/#/e=KNi8pQr1n5hRU65vKJGge3" - private fun split(text: String, content: MessageContent = MessageContent.Text(text)) = + private fun split( + text: String, + content: MessageContent = MessageContent.Text(text), + reactionPills: List = emptyList(), + ) = ChatListItem.ContentBubble( messageId = 42, contentIndex = 0, @@ -29,6 +34,7 @@ class SplitAroundLinkCardTest { entropy = "KNi8pQr1n5hRU65vKJGge3", state = LinkCard.Cash.State.Unresolved, ), + reactionPills = reactionPills, ).splitAroundLinkCard() @Test @@ -60,6 +66,15 @@ class SplitAroundLinkCardTest { assertEquals(1, rows.map { it.messageKey }.distinct().size) } + @Test + fun `text and a link carry the reaction pills on the last row only`() { + val pills = listOf(ReactionPill(emoji = "👍", count = 2, selfReacted = true, pending = false)) + val rows = split("Join my group $link", reactionPills = pills) + + assertEquals(listOf(MessagePart.Leading, MessagePart.Card), rows.map { it.part }) + assertEquals(listOf(emptyList(), pills), rows.map { it.reactionPills }) + } + @Test fun `row ids are the message id plus the part`() { val rows = split("before $link after") diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/QuickReactionStripPlacementTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/QuickReactionStripPlacementTest.kt index 9562c85b6e..d075accbe1 100644 --- a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/QuickReactionStripPlacementTest.kt +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/QuickReactionStripPlacementTest.kt @@ -66,4 +66,32 @@ class QuickReactionStripPlacementTest { place(IntRect(left = 120, top = 350, right = 500, bottom = 600), hugsTrailing = false), ) } + + @Test + fun `a message split around its card is one anchor spanning its rows`() { + // "Join my group ": a text bubble over a wider card, both outgoing. + val text = IntRect(left = 700, top = 1000, right = 960, bottom = 1100) + val card = IntRect(left = 400, top = 1120, right = 960, bottom = 1600) + + val anchor = QuickReactionStripPlacement.messageBounds(listOf(card, text)) + + assertEquals(IntRect(left = 400, top = 1000, right = 960, bottom = 1600), anchor) + // Above the text row, not between the text and the card. + assertEquals(IntOffset(960 - 600, 1000 - 40 - 100), place(anchor!!, hugsTrailing = true)) + } + + @Test + fun `a split message with no room above puts the strip below its last row`() { + val text = IntRect(left = 120, top = 320, right = 400, bottom = 420) + val card = IntRect(left = 120, top = 440, right = 680, bottom = 900) + + val anchor = QuickReactionStripPlacement.messageBounds(listOf(text, card))!! + + assertEquals(IntOffset(120, 900 + 40), place(anchor, hugsTrailing = false)) + } + + @Test + fun `no rows measured yet is no anchor`() { + assertEquals(null, QuickReactionStripPlacement.messageBounds(emptyList())) + } }