From 1b38579b97ed9741deab3e63d25c827232b572cd Mon Sep 17 00:00:00 2001 From: iris-ai <4+iris-ai@noreply.localhost> Date: Wed, 16 Sep 2026 02:38:48 -0400 Subject: [PATCH] Land a closed card centred on the tap that closed it Two things were wrong with the hold, and each hid the other. It held a *share* of the row's height: the point the finger was on stayed, in proportion, which is the same miss in miniature as holding an edge. Tap away from the middle of a long card and the heading landed most of a card's height from the finger, and off it. A shut card is a heading, and the only place it belongs is centred under the hand that shut it, wherever down the card the tap was. And the correction was worked out from the change in height, which needs the list to behave the way the arithmetic assumed. It does not: which item it holds still across a resize depends on what it has composed -- a row taller than the screen is anchored on itself -- and a scroll it cannot honour in full is honoured in part with nothing said. Measured rather than predicted now: each measure pass asks for the error it can see, and the pass that answers is where the rest becomes askable. Three passes is the worst seen, including the one where a card that reached past the bottom of the screen has shut, left the viewport entirely, and has to be asked back to the bottom edge before there is anything to measure at all. The pass has to be the *measure* one. A scroll asked for during placement is never picked up by another measure and does nothing whatever -- which is what the first version of this did, and why a close moved nothing -- so the list took an `afterMeasure` hook and the correction lives there. Checked with ktfmtFormat, compileDebugKotlin, lintDebug and testDebugUnitTest, and on the emulator against the sandbox, against a 2,785px card in a conversation with room on both sides: closed at 1450 it lands 1387..1513, at 1700 it lands 1637..1763, at 1950 it lands 1887..2013 -- centred on the tap to the pixel each time. Opening by the heading still holds the heading still, and a group closed from its footer bar lands on the bar. Where the conversation runs out -- a card at the very start with nothing above it to scroll -- it lands as close as the list can put it, which is what it could always do. --- PLAN.md | 50 ++-- .../kotlin/com/example/aiapp/SessionScreen.kt | 231 +++++++++++------- .../com/example/aiapp/TranscriptList.kt | 11 + 3 files changed, 179 insertions(+), 113 deletions(-) diff --git a/PLAN.md b/PLAN.md index c18c209..a6c4d6e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -1189,28 +1189,34 @@ dev-updater (Kotlin 2.4.x, CMP 1.11.x, JDK 21). question *was*, and "Deny" alone does not say Allow was the alternative. One rule in two places (`AskedQuestion` and `PermissionAsk`). An answer typed into **Other** matches no option, so that one is still written out. - - **Opening a row keeps still the end nearest the tap, and closing one - keeps still the point touched** (2026-09-16). A row about to open is - small, so both its edges are within a heading's height of the finger and - the wanted one is the edge pressed: touch the upper half and the top - edge holds, so it opens downwards; touch the lower half and the bottom - edge holds, as the list does by default. A row about to close leaves - almost nothing of itself, and the only place the closed card can - sensibly appear is under the finger that closed it — the case that - settles it is a card taller than the screen, where holding either edge - throws the card a screen's length from the hand. One number carries - both: the share of the row's height above the touch, spent as it is on a - close and rounded to the nearer edge on an open. Which half, rather than - which control, so everything behaves alike whether or not it has a - control at each end. The transcript is laid out from the bottom, so a - bottom edge is anchored for free and anything else has to be arranged: - `Modifier.holdTouchedPoint` asks the list to shift during the *layout* - phase, before anything is drawn. From an effect instead, the wrong - position is drawn once first, which reads as a flick. The scroll offset - that asks for it **goes negative on a close** — that is the list being - asked for the rows below what it has composed — and clamping it at zero, - which is what the growing-only version did, leaves every close - uncorrected. + - **Opening a row keeps still the end nearest the tap; closing one lands + the closed row centred on the tap itself** (2026-09-16). A row about to + open is small, so both its edges are within a heading's height of the + finger and the wanted one is the edge pressed: touch the upper half and + the top edge holds, so it opens downwards; touch the lower half and the + bottom edge holds, as the list does by default. A row about to close + leaves a heading where a screenful of card was, and the place that + heading belongs is under the finger that shut it — a card taller than the + screen settles it, since both of its edges can be a screen's length from + the hand, and holding a *share* of the card's height was the same miss in + miniature: tap away from the middle of a long card and the heading landed + a card's height off. Where there is not enough conversation on the far + side to scroll, the row lands as close as the list can put it. + **The correction is measured, never predicted, and it is asked for from + inside the list's own measure** (`settleHold`, called from + `TranscriptList`'s `afterMeasure`). Both halves of that were paid for: + working the scroll out from the change in height needs the list to behave + the way the arithmetic assumed, and it does not — which item it holds + still across a resize, and how much of a scroll it can honour, depend on + what it has composed, and a row taller than the screen is anchored on + *itself*. And a scroll asked for during the *placement* phase is never + picked up by another measure, so it does nothing at all; from a coroutine + it arrives a frame late, which is the flick this mechanism exists to + avoid. So each pass asks for the error it can see, and the pass that + answers is where the rest becomes askable — three passes is the worst + case seen, including the one where the shut card has left the viewport + entirely and has to be asked back to the bottom edge before it can be + measured at all. - **The full-screen image lives on the screen, not in the row that drew the thumbnail** (`SessionImageViewer`). A `Read` whose result is an image is a row of one call until the next call arrives and makes it a group — a diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt index 9db63b1..d4c2620 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/SessionScreen.kt @@ -82,7 +82,7 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.repeatOnLifecycle import java.util.concurrent.atomic.AtomicLong -import kotlin.math.roundToInt +import kotlin.math.abs import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.awaitCancellation import kotlinx.coroutines.cancelAndJoin @@ -169,34 +169,61 @@ private data class HistoryLoadSignal( ) /** - * Which row was asked to hold a point of itself still, and what share of its height change that - * point is worth; see [holdTouchedPoint]. + * Where a row has been asked to put itself, and how close it has got; see [holdTouchedPoint] and + * `SessionScreen`'s `settleHold`. * - * Deliberately *not* snapshot state, which is the point of the class. Both fields are written from + * Positions here are the list's own: the distance of an edge *above* the bottom of the viewport, + * which is what [LazyListItemInfo.offset] measures and the coordinate the whole correction is done + * in. [target] is the point the row has to arrive at -- the finger for a row shutting, its own top + * edge as it was for one opening -- and [onCentre] says which part of the row arrives there: the + * middle of it for a close, since what is left of a shut card is a heading and the finger should be + * on it wherever down the card it pressed, and the top edge for an open, where the row is a heading + * already and the edge pressed is what the reader wants held. + * + * Deliberately *not* snapshot state, which is the point of the class. The fields are written from * the layout phase; a snapshot write there that composition reads would schedule another * recomposition, and the correction has to land inside the frame already being laid out. [key] is - * cleared by the resize it was set for, so it cannot be spent on an unrelated one. + * cleared once the row has arrived, so a hold cannot be spent on an unrelated resize. */ private class TouchHold { var key: Any? = null + var target = 0 + var onCentre = false - /** - * 1 holds the row's top edge, 0 its bottom, and anything between the point that share names. - */ - var share = 0f -} + /** Where the row was in the list, for the pass where its own resize has hidden it. */ + var index = 0 -/** One row's height between layouts, so a change in it can be noticed. See [holdTouchedPoint]. */ -private class LastHeight { - var value: Int? = null + /** The row's height as this pass measured it, which its own modifier reports. */ + var height = 0 + + /** How far out the last pass was, so a pass that cannot improve on it is the last one. */ + var error = Int.MAX_VALUE + var passes = 0 } /** - * Which row the last touch landed in, and where down that row it landed -- as the share of the - * row's height that sits above the touch; see [toggleAnchored]. + * How many measure passes one hold may ask for. * - * A share rather than a distance because that is what both readings of the touch want: a close - * spends it as it is, and an open rounds it to the nearer edge. + * More than one because a single request is not always granted in full: a card giving back more + * than a screenful asks the list for rows below what it had composed, and it can only scroll as far + * as it has. The pass that answers is also the pass that composes them, so the next can ask for the + * rest. The cap is what stops a row the list will not move from asking forever. + */ +private const val HOLD_PASSES = 4 + +/** + * Reports this row's height to a hold on it, which is the one thing the hold cannot measure itself. + * + * In the measure phase, so the height is this pass's before the list's own measure hook reads it -- + * see `settleHold`, which is where the correction is made and why it is not made here. + */ +@Composable +private fun Modifier.holdTouchedPoint(key: Any, held: TouchHold): Modifier = onSizeChanged { size -> + if (held.key == key) held.height = size.height +} + +/** + * Which row the last touch landed in, and where in that row it landed. * * One slot rather than a map, because only the touch about to toggle something matters: * [toggleAnchored] reads it in the same gesture that wrote it. Written from a detector on each @@ -204,34 +231,10 @@ private class LastHeight { */ private class LastTouch { var key: Any? = null - var share = 0f -} -/** - * Keeps a point of this row still when the row changes height, if it was asked to. - * - * This runs in the *layout* phase, from the measurement that discovers the new height, and that is - * why it is a modifier rather than an effect. A correction posted to a coroutine arrives a frame or - * more after the layout it is correcting, so the wrong position is drawn once first -- visible as a - * flick, and worse the faster the screen refreshes. - * - * [hold] is given how far the list has to scroll. The row's bottom edge is held by the list and all - * growth goes upward, so scrolling by the whole change keeps the top edge where it was, by none of - * it the bottom edge, and by [TouchHold.share] of it the point that share names. - */ -@Composable -private fun Modifier.holdTouchedPoint(key: Any, held: TouchHold, hold: (Int) -> Unit): Modifier { - val last = remember { LastHeight() } - return onSizeChanged { size -> - val previous = last.value - last.value = size.height - // A first measurement has no previous height to have moved from, and a row that came back - // after being scrolled away is a first measurement again. - if (previous == null || previous == size.height || held.key != key) return@onSizeChanged - val share = held.share - held.key = null - hold(((size.height - previous) * share).roundToInt()) - } + /** How far down the row the finger landed, and how tall the row was when it did. */ + var y = 0 + var height = 1 } // The transcript's data model -- TranscriptItem, foldEvent, joinPages, warm -- lives in @@ -600,31 +603,104 @@ fun SessionScreen( * shrink therefore travels, and how far it travels depends on where down the row it landed. * * [closing] is the difference between the two ends of that, and they want different answers. - * Shutting a row leaves almost nothing of it, so the point under the finger is the only place - * the closed row can sensibly appear -- a row whose top is far off the screen is the case that - * settles it, since holding either edge throws the card the length of the screen away from the - * hand that shut it. Opening one is the opposite: the row is small, every point in it is within - * a heading's height of both edges, and what the reader wants held is the edge they pressed -- - * so the touch is rounded to the nearer of the two rather than splitting the difference and - * carrying the heading part of the way up an unbounded expansion. + * Shutting a row leaves a heading where a screenful of card was, and the place that heading + * belongs is under the finger that shut it -- centred on it, wherever down the card the tap + * landed. A card taller than the screen is the case that settles it: both of its edges can be a + * screen's length from the hand. Opening one is the opposite: the row is small, every point in + * it is within a heading's height of both edges, and what the reader wants held is the edge + * they pressed -- so the touch picks the nearer of the two rather than carrying the heading + * part of the way up an unbounded expansion. * * Where the touch landed decides it, rather than which control was pressed, so everything * behaves the same way whether or not it has a control at each end. * - * The correction itself belongs to the measurement -- see [holdTouchedPoint]. + * Asking for it is `settleHold`'s, which walks the row there once its new height is known. */ fun toggleAnchored(row: TranscriptRow, closing: Boolean, toggle: () -> Unit) = expanding { - if (lastTouch.key == row.key) { - val share = if (closing) lastTouch.share else if (lastTouch.share > 0.5f) 1f else 0f - // Nothing to correct at zero: the bottom edge is what the list holds on its own. - if (share > 0f) { - touchHeld.key = row.key - touchHeld.share = share - } + val item = listState.layoutInfo.visibleItemsInfo.firstOrNull { it.key == row.key } + // An open in the row's lower half wants its bottom edge held, which is what the list does + // on its own -- there is nothing to ask for and nothing to correct. + val wanted = closing || lastTouch.y < lastTouch.height / 2 + if (lastTouch.key == row.key && item != null && wanted) { + // An item's offset is its bottom edge, counted up from the bottom of the viewport, and + // the row sits on that edge with its gap above it -- so the touch is that far up, less + // how far down the row the finger landed. The finger for a close; for an open the edge + // nearest it, which is the row's top because the other half of `wanted` is the other + // edge. + val top = item.offset + lastTouch.height + touchHeld.key = row.key + touchHeld.index = item.index + touchHeld.onCentre = closing + touchHeld.target = if (closing) top - lastTouch.y else top + touchHeld.height = 0 + touchHeld.error = Int.MAX_VALUE + touchHeld.passes = 0 } toggle() } + /** + * Walks the held row to where the touch on it asked for, one measure pass at a time. + * + * Called from inside the list's own measure ([TranscriptList]'s `afterMeasure`), which is the + * only place a correction lands: a scroll asked for from the placement phase is not picked up + * by another measure and so does nothing at all, and one posted to a coroutine arrives a frame + * late, which is the flick this whole mechanism exists to avoid. + * + * It measures rather than predicts, and that is the point. Working out the scroll from the + * change in height needs the list to behave the way the arithmetic assumed, and it does not: + * which item it holds still across a resize, and how much of a scroll it can honour, both + * depend on what it has composed -- a row taller than the screen is anchored on *itself*, and a + * close asks for the rows below one that has been giving the screen its whole height. Asking + * for the error this pass can see, and asking again on the pass that answers, is what does not + * have to know any of that. A row the list will not move gets one pass that changes nothing and + * is then let go. + */ + fun settleHold() { + val key = touchHeld.key ?: return + val height = touchHeld.height + val info = listState.layoutInfo + val item = info.visibleItemsInfo.firstOrNull { it.key == key } + touchHeld.passes++ + // Measured before its own row was, or out of passes: nothing to do, and a correction aimed + // at a guess is worse than none. + if (height == 0 || touchHeld.passes > HOLD_PASSES) { + touchHeld.key = null + return + } + // A row shut while it reached past the bottom of the screen has just left it: 2,600px of + // card became a heading, and the heading is below the viewport with the rest of the + // conversation above it. It is not in this pass's layout to be measured against, so this + // pass spends itself asking for it back -- against the bottom edge, which is the one + // position the list can always be asked for -- and the next one, which has it, corrects + // the rest. + if (item == null) { + Snapshot.withoutReadObservation { listState.requestScrollToItem(touchHeld.index, 0) } + return + } + // Where the row's bottom edge has to be for the part that matters to land on the target. + val wanted = touchHeld.target - if (touchHeld.onCentre) height / 2 else height + val error = wanted - item.offset + // Arrived, or as far as the list will go -- a request it cannot honour leaves the error + // where it was, and asking again would only spend passes. + if (error == 0 || abs(error) >= abs(touchHeld.error)) { + touchHeld.key = null + return + } + touchHeld.error = error + // Reads unobserved: this runs inside the list's measure, and observing the scroll position + // here would make every frame's measure depend on it. + Snapshot.withoutReadObservation { + // A scroll offset counts *down* from the viewport's bottom edge, so moving the row up + // by the error means taking the error off it. Negative is allowed and is what a close + // needs: it is the list being asked for the rows below what it has composed. + listState.requestScrollToItem( + listState.firstVisibleItemIndex, + listState.firstVisibleItemScrollOffset - error, + ) + } + } + /** * Whether the row holding transcript position [seq] is loaded, with older history behind it. * @@ -1562,6 +1638,7 @@ fun SessionScreen( selection = selection, modifier = Modifier.fillMaxSize().drawWithContent { if (settled) drawContent() }, + afterMeasure = ::settleHold, below = { // The last thing in the transcript, because that is where they are in // the session's reading of events: after everything it has taken in, @@ -1628,32 +1705,7 @@ fun SessionScreen( is TranscriptUnit.Whole -> { val row = unit.row Box( - Modifier.holdTouchedPoint(row.key, touchHeld) { by -> - // A *request*, not a raw scroll delta: this runs - // inside the measure pass that discovered the new - // height, and a raw delta forces a synchronous - // remeasure from within measure, which is fatal. The - // request is applied by the same frame's next - // remeasure, so the correction still lands before - // anything is drawn. Reads unobserved, or this row's - // measure would inherit the scroll position as a - // dependency and remeasure on every frame. - // - // The offset goes negative on a close, and has to: - // that is the list being asked for room below what it - // has composed, which is where the newer rows come - // from when a card gives a screenful back. Clamping it - // at zero -- which is what this did while every - // correction was a row *growing* -- leaves a close - // uncorrected, and the card lands wherever the list - // felt like putting it. - Snapshot.withoutReadObservation { - listState.requestScrollToItem( - listState.firstVisibleItemIndex, - listState.firstVisibleItemScrollOffset + by, - ) - } - } + Modifier.holdTouchedPoint(row.key, touchHeld) // Where down this row the touch landed, for // [toggleAnchored]. On the initial pass and consuming // nothing, so every control inside still gets the gesture; @@ -1667,12 +1719,9 @@ fun SessionScreen( pass = PointerEventPass.Initial, ) lastTouch.key = row.key - lastTouch.share = - 1f - - (down.position.y / size.height).coerceIn( - 0f, - 1f, - ) + lastTouch.height = size.height + lastTouch.y = + down.position.y.toInt().coerceIn(0, size.height) } } ) { diff --git a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt index 5da4a9c..ec3cd97 100644 --- a/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt +++ b/app/androidApp/src/main/kotlin/com/example/aiapp/TranscriptList.kt @@ -57,6 +57,16 @@ fun TranscriptList( onRetryHistory: () -> Unit, selection: SelectionState, modifier: Modifier = Modifier, + /** + * Called inside the list's own measure, once its items have been measured and its + * [LazyListState.layoutInfo] describes this pass rather than the last one. + * + * It is where a correction to the scroll position belongs, and the only place one lands: a + * scroll asked for during the *placement* phase is not picked up by another measure, so it does + * nothing at all until something else moves the list. Asked for here it is served by this + * frame's next pass, before anything is drawn. See `SessionScreen`'s hold. + */ + afterMeasure: () -> Unit = {}, below: @Composable () -> Unit, unit: @Composable (TranscriptUnit) -> Unit, ) { @@ -73,6 +83,7 @@ fun TranscriptList( .layout { measurable, constraints -> val started = System.nanoTime() val placeable = measurable.measure(constraints) + afterMeasure() DebugStats.record( "measure: the whole transcript", System.nanoTime() - started,