From 3753eac85862cbd02d7d246b5c57994af273239d Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 8 Sep 2026 14:01:49 -0400 Subject: [PATCH 1/2] feat(chat): mark the message a reply-quote jump lands on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping a quote scrolled the transcript to the quoted message and left it looking like every other message, at the bottom edge of the screen. Two changes. The target now lands in the middle of the transcript, which is where iOS puts it (ChatViewController.scrollToRow, .centeredVertically), in two scrolls: one to park its leading edge mid-band and measure it, one to correct by where it came to rest. Correcting from the measured position is what makes the ends behave — a target the list cannot scroll to the middle stops short on the first pass, and the correction follows it there. Then it flashes: white at 14% over the bubble, held and faded off. Drawn in Bubble, so every variant gets it from one place and the scrim is clipped by the shape the bubble already has. The strength arrives as a lambda read in the draw phase, so the fade repaints the one bubble and recomposes nothing. --- .../internal/screens/ChatAnimations.kt | 12 +++ .../screens/components/MessageList.kt | 89 +++++++++++++++++- .../internal/screens/components/MessageRow.kt | 5 +- .../ChatJumpAttentionScreenshotTest.kt | 94 +++++++++++++++++++ .../screens/components/JumpCenteringTest.kt | 46 +++++++++ .../flipcash/shared/chat/ui/MessageBubble.kt | 34 ++++++- 6 files changed, 276 insertions(+), 4 deletions(-) create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatJumpAttentionScreenshotTest.kt create mode 100644 apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt diff --git a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ChatAnimations.kt b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ChatAnimations.kt index be635b43df..7652a2e713 100644 --- a/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ChatAnimations.kt +++ b/apps/flipcash/features/messenger/src/main/kotlin/com/flipcash/app/messenger/internal/screens/ChatAnimations.kt @@ -3,8 +3,11 @@ package com.flipcash.app.messenger.internal.screens import androidx.compose.animation.EnterTransition import androidx.compose.animation.ExitTransition import androidx.compose.animation.core.Spring +import androidx.compose.animation.core.LinearEasing import androidx.compose.animation.core.SpringSpec +import androidx.compose.animation.core.TweenSpec import androidx.compose.animation.core.spring +import androidx.compose.animation.core.tween import androidx.compose.animation.expandVertically import androidx.compose.animation.fadeOut import androidx.compose.animation.shrinkVertically @@ -53,6 +56,15 @@ internal object ChatAnimations { val replySurfaceExit: ExitTransition = shrinkVertically(replySurfaceIntSize, shrinkTowards = Alignment.Top) + fadeOut(replySurface) + // The flash a jump leaves on the message it landed on: white at full, held long enough to be + // caught by an eye still following the scroll, then faded off. + // + // A tween rather than a spring, unlike everything above it: this one runs from full to nothing + // with no competing target to be interrupted by, so there is no settling for a spring to + // express — only a rate, and a linear one is what reads as a light going out. + const val attentionHoldMs = 250L + val attentionFade: TweenSpec = tween(durationMillis = 750, easing = LinearEasing) + // Receipt label exit when a new message is sent — fade out + collapse. private val deliveredIntSize: SpringSpec = spring(dampingRatio = 0.88f, stiffness = 250f) val receiptExit: ExitTransition = shrinkVertically(deliveredIntSize) + fadeOut(delivered) 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 dd556a7c5e..64dfd20b1f 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 @@ -1,5 +1,6 @@ package com.flipcash.app.messenger.internal.screens.components +import androidx.compose.animation.core.Animatable import androidx.compose.foundation.gestures.animateScrollBy import androidx.compose.foundation.gestures.detectTapGestures import androidx.compose.foundation.layout.Arrangement @@ -10,12 +11,15 @@ import androidx.compose.foundation.layout.ime import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListLayoutInfo +import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.rememberLazyListState import androidx.compose.runtime.Composable import androidx.compose.runtime.CompositionLocalProvider import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.rememberUpdatedState @@ -31,6 +35,7 @@ import androidx.paging.LoadState 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.MessagePointer import com.flipcash.shared.chat.models.ChatAction import com.flipcash.shared.chat.models.ChatActionHandler @@ -40,6 +45,7 @@ import com.flipcash.shared.chat.models.SeparatorConfig import com.getcode.theme.CodeTheme import com.getcode.ui.utils.rememberKeyboardController import com.getcode.util.vibration.LocalVibrator +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.collectLatest import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.dropWhile @@ -116,6 +122,25 @@ internal fun MessageList( if (buried > 0) listState.animateScrollBy(buried.toFloat()) } + // The mark a jump leaves on the message it landed on, so the scroll answers which message + // it was for. Held here rather than in ChatViewModel.State: it is a property of this list + // having moved, and it has to outlive the jump — jumpTarget is cleared the moment the + // scroll settles, and the flash is only starting there. + val attention = remember { Animatable(0f) } + var attentionId by remember { mutableStateOf(null) } + // Counted rather than keyed on the id alone: tapping the same quote twice is two jumps, and + // an id that did not change would leave the second one dark. + var attentionRequest by remember { mutableIntStateOf(0) } + LaunchedEffect(attentionRequest) { + if (attentionRequest == 0) return@LaunchedEffect + attention.snapTo(1f) + delay(ChatAnimations.attentionHoldMs) + attention.animateTo(0f, ChatAnimations.attentionFade) + } + // Read in the draw phase, so the fade repaints the one bubble without recomposing the list. + // Remembered so the row keeps being handed the same reference and stays skippable. + val readAttention: () -> Float = remember { { attention.value } } + // Walking the append path, rather than PagingConfig.jumpThreshold: a jump there routes // through PageFetcher::refresh with triggerRemoteRefresh set, so every tap would fire a // RemoteMediator refresh — token = null and a fetch of the newest page — to reach a message @@ -150,7 +175,14 @@ internal fun MessageList( index = indexOf(messages, target) } - index?.let { listState.animateScrollToItem(it) } + index?.let { + listState.centerItem(it) + attentionId = target + attentionRequest++ + } + // Last, and it has to be: this clears jumpTarget, which is what this effect is keyed + // on, so the coroutine is cancelled here. The flash runs in its own effect for the + // same reason. onJumpConsumed() } @@ -248,6 +280,11 @@ internal fun MessageList( selecting = selecting, focused = focused, animateInsertion = animateInsertion, + attention = if (bubble != null && bubble.messageId == attentionId) { + readAttention + } else { + NoAttention + }, ) } @@ -384,6 +421,56 @@ private fun indexOf(messages: LazyPagingItems, messageId: Long): I (messages.peek(i) as? ChatListItem.ContentBubble)?.messageId == messageId } +/** + * Brings [index] to the middle of the transcript, which is where iOS lands a jump too + * (`ChatViewController.scrollToRow`, `.centeredVertically`). + * + * Two scrolls, because the height of the target is not known until something has laid it out. The + * first parks its leading edge — visually its bottom, this list is reverseLayout — at the middle, + * which is also what measures it; the second corrects by where it actually came to rest. Measuring + * rather than assuming is what makes the ends behave: a target close enough to the newest message + * that the list cannot scroll it to the middle stops short on the first pass, and the correction is + * then computed from where it stopped instead of pushing it further off. + * + * The correction is small whenever the first pass was free to land where it was asked, so what it + * reads as is the scroll settling rather than a second move. + */ +private suspend fun LazyListState.centerItem(index: Int) { + // Content padding is not transcript a message can sit in: the top of it is under the bar's fade, + // and the bottom is behind the composer. + val band = layoutInfo.centeringBand() + if (band.isEmpty()) { + animateScrollToItem(index) + return + } + + animateScrollToItem(index, scrollOffset = -(band.last - band.first) / 2) + + val landed = layoutInfo.visibleItemsInfo.firstOrNull { it.index == index } ?: return + val delta = centeringDelta(landed.offset, landed.size, layoutInfo.centeringBand()) + if (delta != 0f) animateScrollBy(delta) +} + +/** The part of the viewport a message can actually be centred in, in item-offset coordinates. */ +private fun LazyListLayoutInfo.centeringBand(): IntRange = + (viewportStartOffset + beforeContentPadding)..(viewportEndOffset - afterContentPadding) + +/** + * How far to scroll to bring an item of [itemHeight] currently at [itemOffset] to the middle of + * [band]. Positive is on toward the newest message. + * + * An item taller than the band cannot be centred, so it is aligned to the band's start instead — + * the edge the jump arrives from, since the reply that quoted it sits below it. + */ +internal fun centeringDelta(itemOffset: Int, itemHeight: Int, band: IntRange): Float { + val height = band.last - band.first + if (itemHeight >= height) return (itemOffset - band.first).toFloat() + return (itemOffset + itemHeight / 2f) - (band.first + height / 2f) +} + +/** Handed to every row that is not the one a jump just landed on. */ +private val NoAttention: () -> Float = { 0f } + private const val JUMP_PAGE_SIZE = 50 private const val MAX_JUMP_ITEMS = 5_000 private const val JUMP_STEP_TIMEOUT_MS = 2_000L 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 38ac14633a..3befb5439c 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 @@ -58,7 +58,8 @@ import com.getcode.util.vibration.LocalVibrator * The row owns what is a function of itself — its insertion animation, its gestures, its spacing to * 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, and [animateInsertion] is granted once per message and never again. + * sharp, [attention] carries the flash the list points at a jumped-to message, and + * [animateInsertion] is granted once per message and never again. */ @OptIn(ExperimentalFoundationApi::class) @Composable @@ -71,6 +72,7 @@ internal fun MessageRow( selecting: Boolean, focused: Boolean, animateInsertion: Boolean, + attention: () -> Float = { 0f }, ) { val onAction = LocalChatActionHandler.current val vibrator = LocalVibrator.current @@ -221,6 +223,7 @@ internal fun MessageRow( messages, separatorConfig ), + attention = attention, ) } val showReceipt = diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatJumpAttentionScreenshotTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatJumpAttentionScreenshotTest.kt new file mode 100644 index 0000000000..f930abf333 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/ChatJumpAttentionScreenshotTest.kt @@ -0,0 +1,94 @@ +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.Arrangement +import androidx.compose.foundation.layout.Column +import androidx.compose.foundation.layout.width +import androidx.compose.ui.Modifier +import androidx.compose.ui.test.junit4.createAndroidComposeRule +import androidx.compose.ui.unit.dp +import com.flipcash.app.theme.FlipcashPreview +import com.flipcash.services.models.chat.MessageContent +import com.flipcash.shared.chat.models.ChatListItem +import com.flipcash.shared.chat.ui.BubblePosition +import com.flipcash.shared.chat.ui.ContentBubble +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.Instant + +/** + * Renders the flash a jump leaves on the message it landed on, at three points through its fade and + * on both bubble colours, so the scrim can be judged without an emulator. Not an assertion test — + * it writes to `build/screenshots/`. + * + * Same mechanics as `ChatMessageActionScreenshotTest`: pause the clock, pump frames, draw the view. + * The strength is passed as a constant rather than animated, because what is being looked at is the + * scrim at a given strength, not the timing that walks through them. + */ +@RunWith(RobolectricTestRunner::class) +@GraphicsMode(GraphicsMode.Mode.NATIVE) +@Config(sdk = [34], qualifiers = "w400dp-h1100dp-xhdpi") +class ChatJumpAttentionScreenshotTest { + + @get:Rule + val composeRule = createAndroidComposeRule() + + private fun bubble(isFromSelf: Boolean, text: String) = ChatListItem.ContentBubble( + messageId = 1, + contentIndex = 0, + content = MessageContent.Text(text), + isFromSelf = isFromSelf, + timestamp = Instant.fromEpochSeconds(1_000), + capabilities = emptySet(), + ) + + @Test + fun rendersAttentionFlashAcrossItsFade() { + composeRule.mainClock.autoAdvance = false + composeRule.setContent { + FlipcashPreview(showBackground = true) { + Column( + modifier = Modifier.width(360.dp), + verticalArrangement = Arrangement.spacedBy(8.dp), + ) { + listOf(1f, 0.5f, 0f).forEach { strength -> + ContentBubble( + item = bubble(isFromSelf = false, text = "theirs at $strength"), + position = BubblePosition.Solo, + attention = { strength }, + ) + ContentBubble( + item = bubble(isFromSelf = true, text = "mine at $strength"), + position = BubblePosition.Solo, + attention = { strength }, + ) + } + } + } + } + repeat(10) { composeRule.mainClock.advanceTimeByFrame() } + + capture("chat_jump_attention.png") + } + + 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 outDir = File("build/screenshots").apply { mkdirs() } + val file = File(outDir, name) + file.outputStream().use { bitmap.compress(Bitmap.CompressFormat.PNG, 100, it) } + println("SCREENSHOT_WRITTEN: ${file.absolutePath} (${bitmap.width}x${bitmap.height})") + } +} diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt new file mode 100644 index 0000000000..01412e1433 --- /dev/null +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt @@ -0,0 +1,46 @@ +package com.flipcash.app.messenger.internal.screens.components + +import kotlin.test.Test +import kotlin.test.assertEquals + +/** + * The jump centres the message it lands on, and it does that in two scrolls: one that parks the + * target's leading edge mid-band and measures it, and one that corrects by where it actually came + * to rest. This is the correction — the only part of the landing that can be checked without a + * laid-out list. + * + * The band is what is left of the viewport once content padding is off it, in the coordinates + * `LazyListItemInfo.offset` is reported in. + */ +class JumpCenteringTest { + + private val band = 100..1100 // a 1000px band starting 100px in + + @Test + fun `an item already centred needs no correction`() { + // Spans 500..700, so its centre is at 600 — the middle of the band. + assertEquals(0f, centeringDelta(itemOffset = 500, itemHeight = 200, band = band)) + } + + @Test + fun `an item short of the middle is scrolled on`() { + // The first pass parks the leading edge at the middle, so this is the usual case: the item + // sits half its own height past centre and comes back by exactly that. + assertEquals(100f, centeringDelta(itemOffset = 600, itemHeight = 200, band = band)) + } + + @Test + fun `an item past the middle is scrolled back`() { + assertEquals(-150f, centeringDelta(itemOffset = 350, itemHeight = 200, band = band)) + } + + @Test + fun `an item too tall to centre is aligned to the band's start`() { + assertEquals(400f, centeringDelta(itemOffset = 500, itemHeight = 2000, band = band)) + } + + @Test + fun `an item exactly as tall as the band is aligned to it too`() { + assertEquals(400f, centeringDelta(itemOffset = 500, itemHeight = 1000, band = band)) + } +} 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 74716ee756..f205559c30 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 @@ -30,6 +30,7 @@ import androidx.compose.runtime.remember import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip +import androidx.compose.ui.draw.drawWithContent import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Shape import androidx.compose.ui.platform.LocalContext @@ -78,6 +79,9 @@ private const val CASH_BUBBLE_MAX_WIDTH_FRACTION = 0.64f * 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. + * @param attention how strongly this bubble is currently being pointed at, 0f to 1f — the flash a + * jump leaves on the message it landed on. A lambda because it is read while drawing: an animation + * running through it repaints the bubble without recomposing it or the list carrying it. */ @Composable fun ContentBubble( @@ -86,6 +90,7 @@ fun ContentBubble( modifier: Modifier = Modifier, interactive: Boolean = true, onLongClick: (() -> Unit)? = null, + attention: () -> Float = { 0f }, ) { val actionHandler = LocalChatActionHandler.current BoxWithConstraints(modifier = Modifier.fillMaxWidth()) { @@ -110,6 +115,7 @@ fun ContentBubble( position = position, maxWidth = bubbleMaxWidth, isEdited = item.isEdited, + attention = attention, ) // A tombstone is a text bubble with different words. Rendering it through the same @@ -127,6 +133,7 @@ fun ContentBubble( position = position, maxWidth = bubbleMaxWidth, isTombstone = true, + attention = attention, ) is MessageContent.Cash -> CashBubble( @@ -149,6 +156,7 @@ fun ContentBubble( // 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 }, + attention = attention, ) // A reply is a text bubble with a citation above the body. Routing it through @@ -169,6 +177,7 @@ fun ContentBubble( { actionHandler(ChatAction.JumpToMessage(quote.messageId)) } }, onQuoteLongClick = onLongClick?.takeIf { interactive }, + attention = attention, ) // TODO @@ -181,6 +190,9 @@ fun ContentBubble( private const val EDITED_MARKER_SLOT = "edited-marker" +/** How white a bubble goes at the peak of the flash a jump leaves on it. */ +private const val ATTENTION_SCRIM_ALPHA = 0.14f + /** Space between a reply's citation and its body. */ private val QUOTE_GAP = 6.dp @@ -196,8 +208,9 @@ private fun TextBubble( quote: ChatQuote? = null, onQuoteClick: (() -> Unit)? = null, onQuoteLongClick: (() -> Unit)? = null, + attention: () -> Float = { 0f }, ) { - Bubble(isFromSelf, position, maxWidth, modifier) { + Bubble(isFromSelf, position, maxWidth, modifier, attention = attention) { val linkStyle = SpanStyle( color = CodeTheme.colors.textMain, textDecoration = TextDecoration.Underline, @@ -307,6 +320,7 @@ private fun CashBubble( onClick: (() -> Unit)? = null, onLongClick: (() -> Unit)? = null, modifier: Modifier = Modifier, + attention: () -> Float = { 0f }, ) { Bubble( isFromSelf = isFromSelf, @@ -315,7 +329,8 @@ private fun CashBubble( maxWidth = maxWidth, onClick = onClick, onLongClick = onLongClick, - modifier = modifier + modifier = modifier, + attention = attention, ) { val exchange = LocalExchange.current @@ -410,6 +425,7 @@ private fun Bubble( minWidth: Dp = 0.dp, onClick: (() -> Unit)? = null, onLongClick: (() -> Unit)? = null, + attention: () -> Float = { 0f }, content: @Composable BoxScope.() -> Unit, ) { val bubble = if (isFromSelf) { @@ -426,6 +442,20 @@ private fun Bubble( Modifier.border(1.dp, bubble.border, shape) } .background(bubble.background) + // Over the content, not under it: a scrim behind the text would be hidden by the + // bubble's own fill. Drawn here rather than as a background layer so it also lightens + // the words, which is what makes a lit bubble read as one thing. + // + // Every bubble variant comes through here, so a cash card flashes like a text bubble + // does, and the shape is the one already clipped above — corners included, mid-animation + // included. + .drawWithContent { + drawContent() + val strength = attention() + if (strength > 0f) { + drawRect(Color.White, alpha = ATTENTION_SCRIM_ALPHA * strength) + } + } .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. From fa4d110db5754109ed6110bce17b45ac34d56eab Mon Sep 17 00:00:00 2001 From: Brandon McAnsh Date: Tue, 8 Sep 2026 14:45:09 -0400 Subject: [PATCH 2/2] fix(chat): centre a jumped-to message in one scroll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The jump ran two animations: one to put the target's leading edge at the middle of the viewport, then a corrective scroll for half the message's height once it had been laid out. The second one starts from a standstill, so the list moved, stopped, and moved again. Measure the target instead. `measuredHeight` scrolls it on screen and straight back off, both with the non-animated `scrollToItem`, which forces a remeasure and returns without suspending — the pair runs inside one dispatch, so nothing is drawn at the intermediate position. With the height known up front, the whole jump is one `animateScrollToItem` and the `scrollOffset` that centres it. Frame-differencing a screen recording of the tap shows the change: the old build's per-frame motion is 16.4, 11.5, 13.5, 8.8, 7.3, 7.8, 4.4 — two acceleration bumps — against 18.2, 12.9, 4.4, 1.1, 1.1 now, a single decay. --- .../screens/components/MessageList.kt | 58 +++++++++++-------- .../screens/components/JumpCenteringTest.kt | 38 ++++++------ 2 files changed, 53 insertions(+), 43 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 64dfd20b1f..9b46611a60 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 @@ -425,15 +425,13 @@ private fun indexOf(messages: LazyPagingItems, messageId: Long): I * Brings [index] to the middle of the transcript, which is where iOS lands a jump too * (`ChatViewController.scrollToRow`, `.centeredVertically`). * - * Two scrolls, because the height of the target is not known until something has laid it out. The - * first parks its leading edge — visually its bottom, this list is reverseLayout — at the middle, - * which is also what measures it; the second corrects by where it actually came to rest. Measuring - * rather than assuming is what makes the ends behave: a target close enough to the newest message - * that the list cannot scroll it to the middle stops short on the first pass, and the correction is - * then computed from where it stopped instead of pushing it further off. + * One animation, and it has to be one: a second scroll to correct the first starts from a + * standstill, so what it reads as is the list moving, stopping, and moving again rather than as one + * move to the message. * - * The correction is small whenever the first pass was free to land where it was asked, so what it - * reads as is the scroll settling rather than a second move. + * That leaves the target's height, which decides how far past its leading edge — visually its + * bottom, this list is reverseLayout — the message extends, and which nothing knows until the + * message has been laid out. [measuredHeight] lays it out without showing it. */ private suspend fun LazyListState.centerItem(index: Int) { // Content padding is not transcript a message can sit in: the top of it is under the bar's fade, @@ -444,29 +442,43 @@ private suspend fun LazyListState.centerItem(index: Int) { return } - animateScrollToItem(index, scrollOffset = -(band.last - band.first) / 2) - - val landed = layoutInfo.visibleItemsInfo.firstOrNull { it.index == index } ?: return - val delta = centeringDelta(landed.offset, landed.size, layoutInfo.centeringBand()) - if (delta != 0f) animateScrollBy(delta) + val height = measuredHeight(index) ?: 0 + animateScrollToItem(index, scrollOffset = centeringOffset(band.last - band.first, height)) } -/** The part of the viewport a message can actually be centred in, in item-offset coordinates. */ -private fun LazyListLayoutInfo.centeringBand(): IntRange = - (viewportStartOffset + beforeContentPadding)..(viewportEndOffset - afterContentPadding) +/** + * The laid-out height of [index], by putting it on screen and taking it straight back off. + * + * Both scrolls are the non-animated kind, which force a remeasure and return without suspending, so + * they run inside one dispatch and the list is back where it started before anything is drawn. What + * is left behind is the measurement. + */ +private suspend fun LazyListState.measuredHeight(index: Int): Int? { + layoutInfo.visibleItemsInfo.firstOrNull { it.index == index }?.let { return it.size } + + val anchorIndex = firstVisibleItemIndex + val anchorOffset = firstVisibleItemScrollOffset + scrollToItem(index) + val size = layoutInfo.visibleItemsInfo.firstOrNull { it.index == index }?.size + scrollToItem(anchorIndex, anchorOffset) + return size +} /** - * How far to scroll to bring an item of [itemHeight] currently at [itemOffset] to the middle of - * [band]. Positive is on toward the newest message. + * The `scrollOffset` that lands an item of [itemHeight] in the middle of a band [bandHeight] tall. + * + * `animateScrollToItem` positions the item's leading edge at `-scrollOffset` from the band's start, + * so centring is half the room the message leaves over, negated. * * An item taller than the band cannot be centred, so it is aligned to the band's start instead — * the edge the jump arrives from, since the reply that quoted it sits below it. */ -internal fun centeringDelta(itemOffset: Int, itemHeight: Int, band: IntRange): Float { - val height = band.last - band.first - if (itemHeight >= height) return (itemOffset - band.first).toFloat() - return (itemOffset + itemHeight / 2f) - (band.first + height / 2f) -} +internal fun centeringOffset(bandHeight: Int, itemHeight: Int): Int = + -((bandHeight - itemHeight) / 2).coerceAtLeast(0) + +/** The part of the viewport a message can actually be centred in, in item-offset coordinates. */ +private fun LazyListLayoutInfo.centeringBand(): IntRange = + (viewportStartOffset + beforeContentPadding)..(viewportEndOffset - afterContentPadding) /** Handed to every row that is not the one a jump just landed on. */ private val NoAttention: () -> Float = { 0f } diff --git a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt index 01412e1433..60ad54ffcd 100644 --- a/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt +++ b/apps/flipcash/features/messenger/src/test/kotlin/com/flipcash/app/messenger/internal/screens/components/JumpCenteringTest.kt @@ -4,43 +4,41 @@ import kotlin.test.Test import kotlin.test.assertEquals /** - * The jump centres the message it lands on, and it does that in two scrolls: one that parks the - * target's leading edge mid-band and measures it, and one that corrects by where it actually came - * to rest. This is the correction — the only part of the landing that can be checked without a - * laid-out list. + * The jump centres the message it lands on in one scroll, and the whole of that scroll is one + * number: the `scrollOffset` handed to `animateScrollToItem`, which positions the target's leading + * edge at `-scrollOffset` from the start of the band. * - * The band is what is left of the viewport once content padding is off it, in the coordinates - * `LazyListItemInfo.offset` is reported in. + * The band is what is left of the viewport once content padding is off it. */ class JumpCenteringTest { - private val band = 100..1100 // a 1000px band starting 100px in + private val band = 1000 // a 1000px band @Test - fun `an item already centred needs no correction`() { - // Spans 500..700, so its centre is at 600 — the middle of the band. - assertEquals(0f, centeringDelta(itemOffset = 500, itemHeight = 200, band = band)) + fun `a message is offset by half the room it leaves over`() { + // 200px in a 1000px band leaves 800, so its leading edge goes 400 in and its middle at 500. + assertEquals(-400, centeringOffset(bandHeight = band, itemHeight = 200)) } @Test - fun `an item short of the middle is scrolled on`() { - // The first pass parks the leading edge at the middle, so this is the usual case: the item - // sits half its own height past centre and comes back by exactly that. - assertEquals(100f, centeringDelta(itemOffset = 600, itemHeight = 200, band = band)) + fun `a message filling half the band still centres`() { + assertEquals(-250, centeringOffset(bandHeight = band, itemHeight = 500)) } @Test - fun `an item past the middle is scrolled back`() { - assertEquals(-150f, centeringDelta(itemOffset = 350, itemHeight = 200, band = band)) + fun `a message too tall to centre is aligned to the band's start`() { + assertEquals(0, centeringOffset(bandHeight = band, itemHeight = 2000)) } @Test - fun `an item too tall to centre is aligned to the band's start`() { - assertEquals(400f, centeringDelta(itemOffset = 500, itemHeight = 2000, band = band)) + fun `a message exactly as tall as the band is aligned to it too`() { + assertEquals(0, centeringOffset(bandHeight = band, itemHeight = band)) } @Test - fun `an item exactly as tall as the band is aligned to it too`() { - assertEquals(400f, centeringDelta(itemOffset = 500, itemHeight = 1000, band = band)) + fun `a height that could not be measured puts the leading edge on the middle`() { + // centerItem falls back to 0 when the target never reported a size. Half a message off is + // the worst it can then be, and it is off toward the top, where there is transcript to see. + assertEquals(-500, centeringOffset(bandHeight = band, itemHeight = 0)) } }