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 d89a8d5866..c52f4419f6 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 @@ -36,6 +36,7 @@ import androidx.paging.compose.LazyPagingItems import androidx.paging.compose.itemKey import com.flipcash.app.messenger.internal.ChatViewModel import com.flipcash.app.messenger.internal.screens.ChatAnimations +import com.flipcash.services.models.chat.ChatType import com.flipcash.services.models.chat.MessagePointer import com.flipcash.shared.chat.models.ChatAction import com.flipcash.shared.chat.models.ChatActionHandler @@ -69,6 +70,9 @@ internal fun MessageList( val keyboard = rememberKeyboardController() val listState = rememberLazyListState() val vibrator = LocalVibrator.current + // Whether messages are attributed at all, which is a property of the chat and not of any one + // row: a DM needs no avatar column, a group reserves one on every incoming row. + val isGroup = state.chatType == ChatType.GROUP CompositionLocalProvider(LocalChatActionHandler provides onAction) { HandleMessageReads(listState, messages) @@ -280,6 +284,7 @@ internal fun MessageList( selecting = selecting, focused = focused, animateInsertion = animateInsertion, + showsSenderGutter = isGroup, attention = if (bubble != null && bubble.messageId == attentionId) { readAttention } else { 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 baf28b438a..d4e2062233 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 @@ -67,7 +67,9 @@ import com.getcode.util.vibration.LocalVibrator * the row below — and takes the rest as flags, because they are decided across the whole list: * [selecting] is true for every row while the backdrop is up, [focused] for the single row it leaves * sharp, [attention] carries the flash the list points at a jumped-to message, and - * [animateInsertion] is granted once per message and never again. + * [animateInsertion] is granted once per message and never again, and [showsSenderGutter] says + * whether incoming rows reserve the avatar column — true for a group, where every message is + * attributed, and false for a DM, where none is. */ @OptIn(ExperimentalFoundationApi::class) @Composable @@ -80,6 +82,7 @@ internal fun MessageRow( selecting: Boolean, focused: Boolean, animateInsertion: Boolean, + showsSenderGutter: Boolean, attention: () -> Float = { 0f }, ) { val onAction = LocalChatActionHandler.current @@ -224,12 +227,15 @@ internal fun MessageRow( verticalAlignment = Alignment.Bottom, horizontalArrangement = Arrangement.spacedBy(CodeTheme.dimens.grid.x1), ) { - // The gutter is reserved on every row of a sender run, not only the labelled - // one, so the bubbles above and below line up on the same left edge instead of - // stepping in and out as the run starts. - if (sender != null) { + // The gutter is reserved on every incoming row of a group, not only the + // labelled ones, so bubbles line up on the same left edge instead of stepping + // in and out as runs start. Keyed off the transcript rather than off this row's + // `sender`, which is also null while a member's profile is still resolving — a + // row that reserved no gutter would sit out at the inset and then jump inward + // when the name arrived. + if (showsSenderGutter && !item.isFromSelf) { Box(modifier = Modifier.requiredSize(CodeTheme.dimens.staticGrid.x6)) { - if (runStart) { + if (sender != null && runStart) { ContactAvatar( image = sender.picture, displayName = sender.displayName, @@ -368,27 +374,47 @@ private fun bottomSpacingFor( messages: LazyPagingItems, config: SeparatorConfig, ): Dp { - val tight = CodeTheme.dimens.grid.x1 - val normal = CodeTheme.dimens.grid.x2 - val wide = CodeTheme.dimens.grid.x3 // index-1 is the item below (newer) in reverseLayout - val itemBelow = (if (index > 0) messages.peek(index - 1) else null) ?: return tight + val gap = rowGapBelow(item, if (index > 0) messages.peek(index - 1) else null, config) + return when (gap) { + RowGap.Tight -> CodeTheme.dimens.grid.x1 + RowGap.Normal -> CodeTheme.dimens.grid.x2 + RowGap.Wide -> CodeTheme.dimens.grid.x3 + } +} + +/** The three gaps the transcript puts between rows, resolved to `grid.x1`/`x2`/`x3`. */ +internal enum class RowGap { Tight, Normal, Wide } + +/** + * The gap under [item], where [below] is the row drawn beneath it — the newer message, `peek(index + * - 1)` under `reverseLayout`. + * + * A change of author opens the widest gap, because that is where the next run's name and picture + * go. `isFromSelf` alone used to stand in for the author, which is right in a DM and wrong in a + * group: two members' messages are both incoming, so a whole conversation between them ran at the + * tight same-sender gap. + * + * Pure so it can be tested without a `PagingData`, like [startsSenderRun]: the messenger module has + * no `paging-testing` dependency, and the peek belongs to the caller. + */ +internal fun rowGapBelow(item: ChatListItem, below: ChatListItem?, config: SeparatorConfig): RowGap { + below ?: return RowGap.Tight // Separator adjacent → normal gap - if (item is ChatListItem.DateSeparator || itemBelow is ChatListItem.DateSeparator) { - return normal + if (item is ChatListItem.DateSeparator || below is ChatListItem.DateSeparator) { + return RowGap.Normal } - val current = item as? ChatListItem.ContentBubble ?: return tight - val below = itemBelow as? ChatListItem.ContentBubble ?: return tight + val current = item as? ChatListItem.ContentBubble ?: return RowGap.Tight + val newer = below as? ChatListItem.ContentBubble ?: return RowGap.Tight return when { - // Different sender → wide - current.isFromSelf != below.isFromSelf -> wide + !current.isSameAuthorAs(newer) -> RowGap.Wide // Same sender, outside grouping window → normal - !config.isGrouped(current.timestamp, below.timestamp) -> normal + !config.isGrouped(current.timestamp, newer.timestamp) -> RowGap.Normal // Same sender, close together → tight - else -> tight + else -> RowGap.Tight } } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt new file mode 100644 index 0000000000..f466f51583 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/GroupTranscriptScreenshotTest.kt @@ -0,0 +1,154 @@ +package com.flipcash.app.messenger.internal + +import android.graphics.Bitmap +import android.graphics.Canvas +import android.view.View +import androidx.activity.ComponentActivity +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.width +import androidx.compose.runtime.Composable +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.unit.dp +import androidx.paging.PagingData +import androidx.paging.compose.LazyPagingItems +import androidx.paging.compose.collectAsLazyPagingItems +import com.flipcash.app.messenger.internal.screens.components.MessageRow +import com.flipcash.app.theme.FlipcashThemeWrapper +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.models.SenderIdentity +import com.flipcash.shared.chat.models.SeparatorConfig +import kotlinx.coroutines.flow.flowOf +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode +import java.io.File +import kotlin.time.Duration.Companion.seconds +import kotlin.time.Instant + +/** + * Renders a group transcript to a PNG in `build/screenshots/` so the run spacing and the avatar + * gutter can be eyeballed without an emulator. Not an assertion test. + * + * Rows are drawn in a plain `Column` rather than the real `LazyColumn`, oldest first, because the + * transcript's list is `reverseLayout` and index 0 is the newest message. What the rows compute + * from their neighbours — the gap below, whether they start a run — comes off the paging snapshot + * either way. + * + * Same mechanics as [GroupChromeScreenshotTest]: pump a fixed number of frames and draw the Android + * view directly, so a composable that keeps scheduling frames can't hang `captureToImage()`. + */ +@RunWith(RobolectricTestRunner::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +@Config(sdk = [34], qualifiers = "w400dp-h1100dp-xhdpi") +class GroupTranscriptScreenshotTest { + + @get:Rule + val composeRule = createAndroidComposeRule() + + private val noah = + SenderIdentity(userId = listOf(1), displayName = "Noah Parker", picture = null) + private val chloe = + SenderIdentity(userId = listOf(2), displayName = "Chloe Anderson", picture = null) + + private val start = Instant.fromEpochMilliseconds(1_700_000_000_000) + + private fun bubble( + id: Long, + text: String, + sender: SenderIdentity?, + isFromSelf: Boolean = false, + secondsIn: Long = 0, + ) = ChatListItem.ContentBubble( + messageId = id, + contentIndex = 0, + content = MessageContent.Text(text), + isFromSelf = isFromSelf, + timestamp = start + secondsIn.seconds, + sender = sender, + ) + + @Test + fun rendersGroupTranscript() { + // Newest first, as the paging snapshot holds them. + val items = listOf( + bubble(6, "Fine, Saturday then", sender = null, isFromSelf = true, secondsIn = 50), + bubble(5, "Watch the tape", chloe, secondsIn = 40), + bubble(4, "What are you talking about. Did you see him last night?", chloe, secondsIn = 30), + bubble(3, "Not even close", noah, secondsIn = 20), + bubble(2, "I disagree. He isn't the best at all", noah, secondsIn = 10), + ChatListItem.DateSeparator(start), + ) + + composeRule.setContent { + FlipcashThemeWrapper().Wrap { + val messages = flowOf(PagingData.from(items)).collectAsLazyPagingItems() + Transcript(messages) + } + } + repeat(20) { composeRule.mainClock.advanceTimeByFrame() } + + capture("group_transcript.png") + } + + @Composable + private fun Transcript(messages: LazyPagingItems) { + Column(modifier = Modifier.width(360.dp).padding(16.dp)) { + // Oldest first: the list this stands in for is reverseLayout. + for (index in messages.itemCount - 1 downTo 0) { + val item = messages[index] ?: continue + MessageRow( + index = index, + item = item, + messages = messages, + separatorConfig = SeparatorConfig.DayOnly, + otherReadPointer = null, + selecting = false, + focused = true, + animateInsertion = false, + showsSenderGutter = true, + ) + } + } + } + + private fun capture(name: String) { + val root: View = composeRule.activity.findViewById(android.R.id.content) + val width = root.width.takeIf { it > 0 } ?: 1080 + val height = root.height.takeIf { it > 0 } ?: 1920 + val bitmap = Bitmap.createBitmap(width, height, Bitmap.Config.ARGB_8888) + root.draw(Canvas(bitmap)) + val cropped = bitmap.trimmedToDrawnArea() + + val outDir = File("build/screenshots").apply { mkdirs() } + val file = File(outDir, name) + file.outputStream().use { cropped.compress(Bitmap.CompressFormat.PNG, 100, it) } + println("SCREENSHOT_WRITTEN: ${file.absolutePath} (${cropped.width}x${cropped.height})") + } + + /** Crop away the untouched (fully transparent) margin, so the PNG is just what was composed. */ + private fun Bitmap.trimmedToDrawnArea(): Bitmap { + val pixels = IntArray(width * height) + getPixels(pixels, 0, width, 0, 0, width, height) + var left = width + var top = height + var right = -1 + var bottom = -1 + for (y in 0 until height) { + for (x in 0 until width) { + if (pixels[y * width + x] ushr 24 == 0) continue + if (x < left) left = x + if (x > right) right = x + if (y < top) top = y + if (y > bottom) bottom = y + } + } + if (right < left || bottom < top) return this + return Bitmap.createBitmap(this, left, top, right - left + 1, bottom - top + 1) + } +} diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RowGapTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RowGapTest.kt new file mode 100644 index 0000000000..897e2fb9cb --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/RowGapTest.kt @@ -0,0 +1,83 @@ +package com.flipcash.app.messenger.internal + +import com.flipcash.app.messenger.internal.screens.components.RowGap +import com.flipcash.app.messenger.internal.screens.components.rowGapBelow +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.models.SenderIdentity +import com.flipcash.shared.chat.models.SeparatorConfig +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.time.Duration.Companion.minutes +import kotlin.time.Instant + +/** + * How far apart two rows sit. + * + * The list is `reverseLayout`, so the gap a row owns is the one under it — to the *newer* message, + * `peek(index - 1)`. A change of author opens the widest gap, which is the space the next run's + * name and picture go in. + */ +class RowGapTest { + + private val config = SeparatorConfig.DayOnly + private val noon = Instant.fromEpochMilliseconds(1_700_000_000_000) + + private val alice = + SenderIdentity(userId = listOf(1), displayName = "Alice", picture = null) + private val bob = SenderIdentity(userId = listOf(2), displayName = "Bob", picture = null) + + private fun bubble( + sender: SenderIdentity?, + isFromSelf: Boolean = false, + at: Instant = noon, + ) = ChatListItem.ContentBubble( + messageId = 1L, + contentIndex = 0, + content = MessageContent.Text("hi"), + isFromSelf = isFromSelf, + timestamp = at, + sender = sender, + ) + + @Test + fun `two members of a group are a run apart`() { + assertEquals(RowGap.Wide, rowGapBelow(bubble(alice), bubble(bob), config)) + } + + @Test + fun `the viewer and a member are a run apart`() { + val own = bubble(sender = null, isFromSelf = true) + assertEquals(RowGap.Wide, rowGapBelow(bubble(alice), own, config)) + assertEquals(RowGap.Wide, rowGapBelow(own, bubble(alice), config)) + } + + @Test + fun `one member's consecutive messages stay tight`() { + assertEquals(RowGap.Tight, rowGapBelow(bubble(alice), bubble(alice), config)) + } + + @Test + fun `one member's messages open up outside the grouping window`() { + val later = bubble(alice, at = noon + 5.minutes) + assertEquals(RowGap.Normal, rowGapBelow(bubble(alice), later, config)) + } + + @Test + fun `the viewer's own consecutive messages stay tight`() { + val own = bubble(sender = null, isFromSelf = true) + assertEquals(RowGap.Tight, rowGapBelow(own, own, config)) + } + + @Test + fun `a separator takes the normal gap on either side`() { + val separator = ChatListItem.DateSeparator(noon) + assertEquals(RowGap.Normal, rowGapBelow(bubble(alice), separator, config)) + assertEquals(RowGap.Normal, rowGapBelow(separator, bubble(alice), config)) + } + + @Test + fun `the newest row has nothing under it`() { + assertEquals(RowGap.Tight, rowGapBelow(bubble(alice), below = null, config = config)) + } +} 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 010090e869..b29a07660e 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 @@ -49,6 +49,21 @@ sealed interface ChatListItem { */ val sender: SenderIdentity? = null, ) : ChatListItem { + /** + * Whether [other] was sent by the same person as this bubble. + * + * `isFromSelf` cannot answer this in a group, where every other member's message is + * incoming alike: two members' bubbles would read as one run. [sender] is what tells them + * apart, and it is null only for the viewer's own messages and for DMs — cases + * `isFromSelf` already decides. Two bubbles whose sender profiles have not resolved yet do + * read as one author until they do. + */ + fun isSameAuthorAs(other: ContentBubble): Boolean = when { + isFromSelf != other.isFromSelf -> false + isFromSelf -> true + else -> sender?.userId == other.sender?.userId + } + /** The body a Copy or an Edit acts on, or `null` for a bubble that carries no text. */ val plainText: String? get() = when (val content = content) { 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 3e89a7f8f4..39a8d82bf9 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 @@ -627,11 +627,11 @@ fun bubblePositionOf( } else null val groupedAbove = above != null && - above.isFromSelf == item.isFromSelf && + item.isSameAuthorAs(above) && config.isGrouped(item.timestamp, above.timestamp) val groupedBelow = below != null && - below.isFromSelf == item.isFromSelf && + item.isSameAuthorAs(below) && config.isGrouped(item.timestamp, below.timestamp) return when { @@ -656,11 +656,11 @@ fun bubblePositionOf( } else null val groupedAbove = above != null && - above.isFromSelf == item.isFromSelf && + item.isSameAuthorAs(above) && config.isGrouped(item.timestamp, above.timestamp) val groupedBelow = below != null && - below.isFromSelf == item.isFromSelf && + item.isSameAuthorAs(below) && config.isGrouped(item.timestamp, below.timestamp) return when {