From 859e856e33945bc90b60b234d1c70224c7cbe755 Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 8 Sep 2026 11:58:24 -0400 Subject: [PATCH] fix(chat): report a long press from the bubble that consumed it Long-pressing a cash bubble did nothing. The bubble installs a tap target of its own for token info, and a press a child clickable takes never reaches the row's combinedClickable behind it, so the transcript's selection gesture resolved only on the row padding around the bubble. The capability model was already right: cash resolves to Reply alone, and any capability makes a bubble selectable. The bar a cash message puts up therefore already offered reply and nothing else, on a bubble no one could select. Bubble takes the tap and the long press through one combinedClickable, and MessageRow hands the same haptic-and-ToggleSelection lambda to the row and to the bubble, so the two cannot report different things. Both stay gated on `interactive`: with the backdrop up the bubble drops its gestures, as the row already drops its own. The reply citation panel had the same defect one bubble over, since it carries the tap that jumps to the quoted message, and is fixed the same way. BubbleGestureTest drives both presses and both taps; each long-press assertion fails against the clickable it replaces. ChatSelectionBarTest pins what a selected cash bubble is offered, which is where reply-only lives. --- .../internal/screens/components/MessageRow.kt | 25 ++- .../internal/ChatSelectionBarTest.kt | 77 +++++++++ .../flipcash/shared/chat/ui/ChatQuotePanel.kt | 12 +- .../flipcash/shared/chat/ui/MessageBubble.kt | 27 +++- .../shared/chat/ui/BubbleGestureTest.kt | 147 ++++++++++++++++++ 5 files changed, 275 insertions(+), 13 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt create mode 100644 apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubbleGestureTest.kt 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 bdeb8aac6..38ac14633 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 @@ -107,6 +107,15 @@ internal fun MessageRow( val bubble = item as? ChatListItem.ContentBubble val interactionSource = remember { MutableInteractionSource() } + // Hoisted because two targets report the same gesture: the row, and any bubble that installs a + // tap target of its own and would otherwise consume the press before the row sees it. + val select = bubble?.takeIf { it.isSelectable }?.let { target -> + { + vibrator.tick() + onAction(ChatAction.ToggleSelection(target)) + } + } + val dimAlpha by animateFloatAsState( targetValue = if (focused) 1f else 0.4f, label = "messageDim", @@ -158,18 +167,14 @@ internal fun MessageRow( // press there would move the selection out from under the message the bar — // or the composer — is already acting on. .addIf(bubble != null && !selecting) { - // Long-press is the whole row's gesture, not the bubble's: a + // Long-press is the whole row's gesture, not just the bubble's: a // bubble-sized target is harder to hit, and the top bar is what reports - // the selection, so nothing about the row has to change. + // the selection, so nothing about the row has to change. A bubble that + // takes the press for its own tap target reports the same gesture back. Modifier.combinedClickable( interactionSource = interactionSource, indication = null, - onLongClick = bubble?.takeIf { it.isSelectable }?.let { target -> - { - vibrator.tick() - onAction(ChatAction.ToggleSelection(target)) - } - }, + onLongClick = select, // Only reachable with the backdrop down, so the tap has nothing to // dismiss but the keyboard. onClick = { keyboard.hide() }, @@ -206,6 +211,10 @@ internal fun MessageRow( // bubble behind the backdrop would otherwise open token // info from under the bar. interactive = !selecting, + // A bubble with a tap target of its own consumes the press, + // so the row's long-press never reaches it. Handing it the + // same gesture is what makes a cash bubble selectable. + onLongClick = select, position = bubblePositionOf( index, item, diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt new file mode 100644 index 000000000..e118ec2a8 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatSelectionBarTest.kt @@ -0,0 +1,77 @@ +package com.flipcash.app.messenger.internal + +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertCountEquals +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.test.onAllNodesWithTag +import androidx.compose.ui.test.onNodeWithTag +import com.flipcash.app.messenger.internal.screens.components.ChatTopBar +import com.flipcash.app.theme.FlipcashPreview +import com.flipcash.services.models.chat.ChatType +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.MessageCapability +import com.flipcash.shared.chat.models.ChatListItem +import com.getcode.navigation.core.CodeNavigator +import com.getcode.opencode.model.core.RandomId +import com.getcode.opencode.model.financial.toFiat +import com.getcode.solana.keys.Mint +import io.mockk.mockk +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import kotlin.time.Instant + +/** + * What the selection bar offers is a function of the selected bubble's capabilities alone. A cash + * bubble is the narrowest case and the one worth pinning: a payment cannot be edited or deleted, + * and it carries no text to copy, so reply is the whole bar. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], qualifiers = "w400dp-h800dp-xhdpi") +class ChatSelectionBarTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + private val cash = ChatListItem.ContentBubble( + messageId = 1, + contentIndex = 0, + content = MessageContent.Cash( + intentId = RandomId, + amount = 25.toFiat(), + mint = Mint.usdf, + ), + isFromSelf = true, + timestamp = Instant.fromEpochSeconds(1_000), + capabilities = setOf(MessageCapability.Reply), + ) + + @Test + fun `a selected cash bubble offers reply and nothing else`() { + composeTestRule.setContent { + FlipcashPreview { + ChatTopBar( + navigator = mockk(relaxed = true), + state = ChatViewModel.State( + chatType = ChatType.CONTACT_DM, + selection = cash, + ), + chatActionHandler = {}, + dispatch = {}, + ) + } + } + + composeTestRule.onNodeWithTag("action_reply_message").assertIsDisplayed() + // Including the overflow: one action fits, so nothing is a menu away either. + listOf( + "action_delete_message", + "action_copy_message", + "action_edit_message", + "action_message_overflow", + ).forEach { composeTestRule.onAllNodesWithTag(it).assertCountEquals(0) } + } +} diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatQuotePanel.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatQuotePanel.kt index 032c697f2..2c1d82aa1 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatQuotePanel.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/ChatQuotePanel.kt @@ -1,7 +1,7 @@ package com.flipcash.shared.chat.ui 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.Box import androidx.compose.foundation.layout.Column @@ -43,6 +43,7 @@ fun ChatQuotePanel( quote: ChatQuote, modifier: Modifier = Modifier, onClick: (() -> Unit)? = null, + onLongClick: (() -> Unit)? = null, ) { val accent = quote.accent ?: CodeTheme.colors.tertiary val name = quote.nameAccent ?: accent @@ -55,7 +56,14 @@ fun ChatQuotePanel( // The author's own colour at low alpha rather than a neutral scrim: the panel sits on a // filled bubble, and tinting it to match the rule is what separates the two surfaces. .background(accent.copy(alpha = QuotePanelDefaults.groundAlpha)) - .addIf(onClick != null) { Modifier.clickable { onClick?.invoke() } } + // Long-press comes down with the tap: the panel sits inside the bubble, so a press + // it takes for its own target is a press the row behind it never sees. + .addIf(onClick != null || onLongClick != null) { + Modifier.combinedClickable( + onLongClick = onLongClick, + onClick = { onClick?.invoke() }, + ) + } .height(IntrinsicSize.Min), horizontalArrangement = Arrangement.spacedBy(QuotePanelDefaults.gap), verticalAlignment = Alignment.CenterVertically, diff --git a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/MessageBubble.kt b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/MessageBubble.kt index 0baec2b47..74716ee75 100644 --- a/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/MessageBubble.kt +++ b/apps/flipcash/shared/chat-ui/src/main/kotlin/com/flipcash/shared/chat/ui/MessageBubble.kt @@ -4,7 +4,7 @@ import androidx.compose.animation.core.animateDpAsState import androidx.compose.animation.core.spring import androidx.compose.foundation.background import androidx.compose.foundation.border -import androidx.compose.foundation.clickable +import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxScope @@ -73,12 +73,19 @@ private const val BUBBLE_MAX_WIDTH_FRACTION = 0.78f private val EDITED_MARKER_GAP = 6.dp private const val CASH_BUBBLE_MAX_WIDTH_FRACTION = 0.64f +/** + * @param onLongClick what a long-press on this bubble reports, or `null` where the row behind it + * has nothing to select. Only bubbles that install a tap target of their own need it: a gesture the + * bubble handles is consumed there, so a cash bubble without this swallows the transcript's + * selection gesture and answers a long press with nothing. + */ @Composable fun ContentBubble( item: ChatListItem.ContentBubble, position: BubblePosition, modifier: Modifier = Modifier, interactive: Boolean = true, + onLongClick: (() -> Unit)? = null, ) { val actionHandler = LocalChatActionHandler.current BoxWithConstraints(modifier = Modifier.fillMaxWidth()) { @@ -139,6 +146,9 @@ fun ContentBubble( } else { null }, + // Gated with the tap, and for the same reason: behind the backdrop the bar is + // already acting on a message, and the row drops its own gestures there too. + onLongClick = onLongClick?.takeIf { interactive }, ) // A reply is a text bubble with a citation above the body. Routing it through @@ -158,6 +168,7 @@ fun ContentBubble( onQuoteClick = item.quote?.takeIf { interactive }?.let { quote -> { actionHandler(ChatAction.JumpToMessage(quote.messageId)) } }, + onQuoteLongClick = onLongClick?.takeIf { interactive }, ) // TODO @@ -184,6 +195,7 @@ private fun TextBubble( isTombstone: Boolean = false, quote: ChatQuote? = null, onQuoteClick: (() -> Unit)? = null, + onQuoteLongClick: (() -> Unit)? = null, ) { Bubble(isFromSelf, position, maxWidth, modifier) { val linkStyle = SpanStyle( @@ -257,6 +269,7 @@ private fun TextBubble( ChatQuotePanel( quote = quote, onClick = onQuoteClick, + onLongClick = onQuoteLongClick, // Tagged because the citation repeats the quoted message's own text, so a // UI test matching on that text cannot tell the two apart. modifier = Modifier.testTag("bubble_reply_quote"), @@ -292,6 +305,7 @@ private fun CashBubble( maxWidth: Dp, action: MessageContent.Cash.Action = MessageContent.Cash.Action.SENT, onClick: (() -> Unit)? = null, + onLongClick: (() -> Unit)? = null, modifier: Modifier = Modifier, ) { Bubble( @@ -300,6 +314,7 @@ private fun CashBubble( minWidth = maxWidth, maxWidth = maxWidth, onClick = onClick, + onLongClick = onLongClick, modifier = modifier ) { val exchange = LocalExchange.current @@ -394,6 +409,7 @@ private fun Bubble( modifier: Modifier = Modifier, minWidth: Dp = 0.dp, onClick: (() -> Unit)? = null, + onLongClick: (() -> Unit)? = null, content: @Composable BoxScope.() -> Unit, ) { val bubble = if (isFromSelf) { @@ -410,8 +426,13 @@ private fun Bubble( Modifier.border(1.dp, bubble.border, shape) } .background(bubble.background) - .addIf(onClick != null) { - Modifier.clip(shape).clickable { onClick?.invoke() } + .addIf(onClick != null || onLongClick != null) { + // combinedClickable rather than two modifiers: a bubble that takes the tap takes + // the long press with it, so both gestures are reported from the same target. + Modifier.clip(shape).combinedClickable( + onLongClick = onLongClick, + onClick = { onClick?.invoke() }, + ) } .padding(horizontal = 16.dp, vertical = 10.dp), ) { diff --git a/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubbleGestureTest.kt b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubbleGestureTest.kt new file mode 100644 index 000000000..28d65dfe8 --- /dev/null +++ b/apps/flipcash/shared/chat-ui/src/test/kotlin/com/flipcash/shared/chat/ui/BubbleGestureTest.kt @@ -0,0 +1,147 @@ +package com.flipcash.shared.chat.ui + +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.longClick +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performClick +import androidx.compose.ui.test.performTouchInput +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.MessageCapability +import com.flipcash.shared.chat.models.ChatAction +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.models.ChatQuote +import com.flipcash.shared.chat.models.ChatQuoteSnippet +import com.flipcash.shared.chat.models.LocalChatActionHandler +import com.getcode.opencode.model.core.RandomId +import com.getcode.opencode.model.financial.toFiat +import com.getcode.solana.keys.Mint +import com.getcode.theme.DesignSystem +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import kotlin.test.assertEquals +import kotlin.time.Instant + +/** + * Two bubbles install a tap target of their own — the cash bubble, and the citation panel inside a + * reply — and that is why their long-press needs asserting: a gesture a bubble handles never + * reaches the row behind it, so the transcript's selection gesture is the bubble's to report or to + * lose. + */ +@RunWith(RobolectricTestRunner::class) +class BubbleGestureTest { + + @get:Rule + val composeTestRule = createComposeRule() + + private val cash = ChatListItem.ContentBubble( + messageId = 1, + contentIndex = 0, + // A blank token name leaves the header off, so the bubble renders without loading an icon. + content = MessageContent.Cash( + intentId = RandomId, + amount = 25.toFiat(), + mint = Mint.usdf, + ), + isFromSelf = true, + timestamp = Instant.fromEpochSeconds(1_000), + capabilities = setOf(MessageCapability.Reply), + ) + + private val reply = cash.copy( + messageId = 2, + content = MessageContent.Reply( + repliedMessageId = 1, + content = listOf(MessageContent.Text("on its way")), + ), + quote = ChatQuote( + messageId = 1, + authorName = "Ada", + snippet = ChatQuoteSnippet.Text("did you send it?"), + accent = null, + nameAccent = null, + ), + ) + + private fun setBubble( + item: ChatListItem.ContentBubble = cash, + interactive: Boolean = true, + onLongClick: (() -> Unit)? = null, + onAction: (ChatAction) -> Unit = {}, + ) { + composeTestRule.setContent { + CompositionLocalProvider(LocalChatActionHandler provides onAction) { + DesignSystem { + ContentBubble( + item = item, + position = BubblePosition.Solo, + interactive = interactive, + onLongClick = onLongClick, + ) + } + } + } + } + + @Test + fun `long-pressing a cash bubble reports the press`() { + var longPresses = 0 + setBubble(onLongClick = { longPresses++ }) + + composeTestRule.onNodeWithText("You sent").performTouchInput { longClick() } + + composeTestRule.runOnIdle { assertEquals(1, longPresses) } + } + + @Test + fun `tapping a cash bubble still opens its token`() { + val actions = mutableListOf() + setBubble(onLongClick = {}, onAction = { actions += it }) + + composeTestRule.onNodeWithText("You sent").performClick() + + composeTestRule.runOnIdle { + assertEquals(listOf(ChatAction.ViewToken(Mint.usdf)), actions.toList()) + } + } + + @Test + fun `long-pressing a reply's citation reports the press`() { + var longPresses = 0 + setBubble(item = reply, onLongClick = { longPresses++ }) + + composeTestRule.onNodeWithTag("bubble_reply_quote").performTouchInput { longClick() } + + composeTestRule.runOnIdle { assertEquals(1, longPresses) } + } + + @Test + fun `tapping a reply's citation still jumps to the quoted message`() { + val actions = mutableListOf() + setBubble(item = reply, onLongClick = {}, onAction = { actions += it }) + + composeTestRule.onNodeWithTag("bubble_reply_quote").performClick() + + composeTestRule.runOnIdle { + assertEquals(listOf(ChatAction.JumpToMessage(1)), actions.toList()) + } + } + + @Test + fun `a cash bubble behind the selection backdrop reports nothing`() { + var longPresses = 0 + val actions = mutableListOf() + setBubble(interactive = false, onLongClick = { longPresses++ }, onAction = { actions += it }) + + composeTestRule.onNodeWithText("You sent").performTouchInput { longClick() } + composeTestRule.onNodeWithText("You sent").performClick() + + composeTestRule.runOnIdle { + assertEquals(0, longPresses) + assertEquals(emptyList(), actions.toList()) + } + } +}