State where a resized row goes instead of walking it there
Correcting by the error each pass could see, and asking again on the pass that answered, was a frame per pass with the ones in between drawn: opening a card visibly stepped. It also still missed, because two of the passes were spent finding out what the list would do rather than telling it. `requestScrollToItem` against the row itself says it outright, in one pass: the row is placed wherever it has ended up and whether or not it is still on screen, and a negative offset -- which is the list being asked for the rows below a card that has just given the screen its whole height back -- is exactly what a close needs and works. Everything else follows from where that puts it. A call opened or shut *inside* a group is a third case, and it was being treated as the second: the group is not the thing opening, it is the container, and centring it on the tap threw a group of six the length of the screen. What keeps the call under the finger is holding the group's top edge, so that everything above the change -- that call's own heading included -- stays where it is. Checked with ktfmtFormat, compileDebugKotlin, lintDebug and testDebugUnitTest, and on the emulator against the sandbox with sampling as fast as the device will report it, so an intermediate frame would show: a 2,785px card closed at 600, 1200 and 1800 lands centred on 600, 1200 and 1800 to the pixel, each in one step; opening by the heading holds the heading still; and opening or closing a call inside a group of five leaves the group's heading exactly where it was.
This commit is contained in:
1 parent
1b38579b97
commit
581e07624f
2 files changed
+73
-86
No files matched your search
@@ -82,7 +82,6 @@ import androidx.lifecycle.Lifecycle
|
||||
import androidx.lifecycle.compose.LocalLifecycleOwner
|
||||
import androidx.lifecycle.repeatOnLifecycle
|
||||
import java.util.concurrent.atomic.AtomicLong
|
||||
import kotlin.math.abs
|
||||
import kotlinx.coroutines.Dispatchers
|
||||
import kotlinx.coroutines.awaitCancellation
|
||||
import kotlinx.coroutines.cancelAndJoin
|
||||
@@ -190,25 +189,17 @@ private class TouchHold {
|
||||
var target = 0
|
||||
var onCentre = false
|
||||
|
||||
/** Where the row was in the list, for the pass where its own resize has hidden it. */
|
||||
/** Where the row is in the list, which is what the correction is asked for against. */
|
||||
var index = 0
|
||||
|
||||
/** 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
|
||||
/** Passes spent waiting for the row's own height; see `settleHold`. */
|
||||
var passes = 0
|
||||
}
|
||||
|
||||
/**
|
||||
* How many measure passes one hold may ask for.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
/** How many passes a hold waits for its row to be measured before giving up on it. */
|
||||
private const val HOLD_PASSES = 4
|
||||
|
||||
/**
|
||||
@@ -611,94 +602,82 @@ fun SessionScreen(
|
||||
* 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
|
||||
* [inner] is neither, and is what a call inside an open group is: the row is not the thing
|
||||
* being opened or shut, it is the container of it, and it stays a container either way. What
|
||||
* the reader is looking at is the call under their finger, and what keeps it there is holding
|
||||
* the row's *top* edge -- everything above the change keeps its place, so the call's own
|
||||
* heading does too, and the rest of the group moves below it. Centring the group on the tap
|
||||
* instead, which is what it did for a day, threw a group of six the length of the screen for a
|
||||
* call closed inside it.
|
||||
*
|
||||
* Where the touch landed decides the rest, rather than which control was pressed, so everything
|
||||
* behaves the same way whether or not it has a control at each end.
|
||||
*
|
||||
* Asking for it is `settleHold`'s, which walks the row there once its new height is known.
|
||||
* Asking for it is `settleHold`'s, which puts the row there once its new height is known.
|
||||
*/
|
||||
fun toggleAnchored(row: TranscriptRow, closing: Boolean, toggle: () -> Unit) = expanding {
|
||||
fun toggleAnchored(
|
||||
row: TranscriptRow,
|
||||
closing: Boolean,
|
||||
inner: Boolean = false,
|
||||
toggle: () -> Unit,
|
||||
) = expanding {
|
||||
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
|
||||
val onCentre = closing && !inner
|
||||
// What is left is an open in the row's lower half, which wants its bottom edge held -- and
|
||||
// that is what the list does on its own, so there is nothing to ask for.
|
||||
val wanted = onCentre || inner || 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.
|
||||
// the row sits on that edge with its gap above it -- so its top is that far up plus its
|
||||
// height, and the finger is that less how far down the row it landed.
|
||||
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.onCentre = onCentre
|
||||
touchHeld.target = if (onCentre) 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.
|
||||
* Puts the held row where the touch on it asked for, in the pass that measured its new height.
|
||||
*
|
||||
* 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.
|
||||
* One request, never a walk towards the answer. A correction asking for the error it can see
|
||||
* and then asking again is the obvious way to stop guessing at what the list will do with a
|
||||
* request -- and it is wrong here, because the pass that answers is a *frame* later and the one
|
||||
* in between is drawn: opening a card visibly stepped. So the position is stated outright
|
||||
* rather than converged on, which the list can do exactly: [LazyListState.requestScrollToItem]
|
||||
* places the row itself, wherever it has ended up and whether or not it is still on screen, and
|
||||
* everything else follows from where that puts it.
|
||||
*/
|
||||
fun settleHold() {
|
||||
val key = touchHeld.key ?: return
|
||||
if (touchHeld.key == null) return
|
||||
// The row has not measured yet -- the pass that measures it is the one to correct in, and
|
||||
// this one has nothing to work from. Bounded, so a row that never measures lets the hold go
|
||||
// rather than keeping it for an unrelated resize.
|
||||
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) {
|
||||
if (height == 0) {
|
||||
if (touchHeld.passes++ < HOLD_PASSES) return
|
||||
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.
|
||||
touchHeld.key = null
|
||||
// Where the row's bottom edge has to be for the part that matters to land on the target,
|
||||
// and then the same in the list's own terms: a scroll offset is that distance counted the
|
||||
// other way, negative when the edge ends up above the bottom of the viewport. Negative is
|
||||
// both allowed and what a close needs -- it is the list being asked for the rows below the
|
||||
// card that has been giving the screen its whole height.
|
||||
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,
|
||||
)
|
||||
}
|
||||
Snapshot.withoutReadObservation { listState.requestScrollToItem(touchHeld.index, -wanted) }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1749,6 +1728,7 @@ fun SessionScreen(
|
||||
toggleAnchored(
|
||||
row,
|
||||
closing = id in expandedTools,
|
||||
inner = true,
|
||||
) {
|
||||
expandedTools =
|
||||
if (id in expandedTools)
|
||||
|
||||
Reference in new issue
Block a user