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()) + } + } +}