Stop a fully zoomed-out timeline from scrolling (#315)
The pinch clamped its lower bound to `ceil(fillPx)`. Rounding a fill height up by the fraction of a pixel that 24 hours don't divide the viewport into makes the timeline up to one pixel per hour taller than the viewport those hours are supposed to fill — roughly 24px of leftover scroll, enough to bounce off Android's overscroll stretch. `FitDay` resolves to the unrounded height and sits still, so the two ways of reaching "the whole day on one screen" disagreed. That inconsistency is what #290 is about. Clamps to `fillPx` itself. The whole-pixel rounding exists so the hour gutter's 24 stacked boxes share a grid with the lines and blocks drawn at the fractional height, and it still applies everywhere the pinch is free to move. The floor is the one height where matching `FitDay` matters more — and because it is the clamp *result* rather than a bound the gesture is merely held against, the pinch lands on it exactly, so the next frame reads it back unchanged and the focal anchor gets no correction to apply. Week and Day both measure their viewport inside a `BoxWithConstraints` below the all-day strip, so the strip appearing only changes the height both paths agree on — that half of the report needed no change. Tests: the two cases that encoded the old rounding are updated (the dead-space invariant still holds, now exactly rather than by a pixel), plus one for the reported symptom and one asserting the pinch floor equals what `FitDay` resolves to at the same viewport. Closes #290 Co-authored-by: Jean-Luc Makiola <business@jeanlucmakiola.de> Reviewed-on: https://codeberg.org/jlmakiola/calendula/pulls/315
This commit is contained in:
@@ -19,7 +19,6 @@ import androidx.compose.ui.input.pointer.PointerEventPass
|
||||
import androidx.compose.ui.input.pointer.pointerInput
|
||||
import androidx.compose.ui.unit.Dp
|
||||
import kotlin.math.abs
|
||||
import kotlin.math.ceil
|
||||
import kotlin.math.floor
|
||||
import kotlin.math.roundToInt
|
||||
|
||||
@@ -197,18 +196,26 @@ fun rememberTimelinePinchZoom(
|
||||
* names, jumping the whole column as a pinch drifts across each half pixel.
|
||||
* Pinning the hour to whole pixels keeps every part of the timeline on one grid.
|
||||
*
|
||||
* The bounds themselves are pulled onto that grid too, each in the direction
|
||||
* that keeps its own promise — up for the fill floor, so no dead space opens
|
||||
* under midnight, down for the ceiling. A fractional bound would be a height the
|
||||
* pinch can be held against but never actually land on, and the difference feeds
|
||||
* the focal anchor a scroll correction on every frame the fingers sit still.
|
||||
* The ceiling is pulled onto that grid too, downwards, so it stays a height the
|
||||
* pinch can actually land on.
|
||||
*
|
||||
* [fillPx] is deliberately *not* rounded (#290). It is the one height the whole
|
||||
* day exactly fills the viewport at, and it is the same value
|
||||
* [TimelineScale.FitDay] resolves to — rounding it up by the fraction of a pixel
|
||||
* that 24 hours don't divide the viewport into leaves the timeline a pixel per
|
||||
* hour taller than its own viewport, so a pinched-all-the-way-out day still
|
||||
* scrolls a hair and bounces off Android's overscroll stretch, while the
|
||||
* identical FitDay preset sits still. Being the clamp result rather than a bound
|
||||
* the gesture is merely held against, it is a height the pinch does land on: the
|
||||
* next frame reads it back unchanged and the focal anchor is handed nothing to
|
||||
* correct.
|
||||
*/
|
||||
internal fun pinchedHourHeightPx(target: Float, fillPx: Float, maxPx: Float): Float =
|
||||
// Filling the viewport wins over the ceiling: on a screen tall enough for
|
||||
// the two to disagree, dead space is the worse of the two failures.
|
||||
target.roundToInt().toFloat()
|
||||
.coerceAtMost(floor(maxPx))
|
||||
.coerceAtLeast(ceil(fillPx))
|
||||
.coerceAtLeast(fillPx)
|
||||
|
||||
/**
|
||||
* The scroll offset that keeps the moment under [centroidY] under it after the
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
package de.jeanlucmakiola.calendula.ui.common
|
||||
|
||||
import androidx.compose.ui.unit.Density
|
||||
import androidx.compose.ui.unit.dp
|
||||
import com.google.common.truth.Truth.assertThat
|
||||
import org.junit.jupiter.api.Test
|
||||
@@ -75,17 +76,16 @@ class TimelineZoomTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a pinch held against a fractional bound stays put`() {
|
||||
// A bound that is not a whole pixel is a height the pinch can be pushed
|
||||
// against but never land on, so every frame of a held gesture would look
|
||||
// like a scale change and hand the focal anchor a scroll correction.
|
||||
fun `a pinch held against either bound stays put`() {
|
||||
val fillPx = 62.083f
|
||||
val maxPx = 616.5f
|
||||
|
||||
val floor = pinchedHourHeightPx(target = 1f, fillPx, maxPx)
|
||||
val ceiling = pinchedHourHeightPx(target = 9_000f, fillPx, maxPx)
|
||||
|
||||
assertThat(floor).isEqualTo(63f)
|
||||
// The ceiling is pulled onto the pixel grid so it stays landable; the
|
||||
// fill floor is landable as it is, being the clamp result itself.
|
||||
assertThat(floor).isEqualTo(fillPx)
|
||||
assertThat(ceiling).isEqualTo(616f)
|
||||
// Landing there and being pushed further must not move them again.
|
||||
assertThat(pinchedHourHeightPx(floor * 0.9f, fillPx, maxPx)).isEqualTo(floor)
|
||||
@@ -100,6 +100,35 @@ class TimelineZoomTest {
|
||||
assertThat(floor * 24).isAtLeast(viewport)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `pinching all the way out leaves nothing to scroll`() {
|
||||
// #290: rounding the fill floor up to a whole pixel made the day one
|
||||
// pixel per hour taller than the viewport it was supposed to fill, so a
|
||||
// fully zoomed-out timeline still scrolled a hair and bounced off
|
||||
// Android's overscroll stretch -- while FitDay, at the same zoom, sat
|
||||
// still. 1490 is deliberately not divisible by 24.
|
||||
val viewport = 1490f
|
||||
val floor = pinchedHourHeightPx(target = 1f, fillPx = viewport / 24f, maxPx = 616f)
|
||||
assertThat(floor * 24).isWithin(0.01f).of(viewport)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the pinch floor is the height FitDay resolves to`() {
|
||||
// The inconsistency the issue is about: the two ways to reach "the whole
|
||||
// day on one screen" have to arrive at the same height.
|
||||
val density = Density(2.5f)
|
||||
val viewport = 596.dp
|
||||
with(density) {
|
||||
val fitDay = TimelineScale.FitDay.hourHeight(viewport).toPx()
|
||||
val pinched = pinchedHourHeightPx(
|
||||
target = 1f,
|
||||
fillPx = fillHourHeight(viewport).toPx(),
|
||||
maxPx = MAX_PINCH_HOUR_HEIGHT.toPx(),
|
||||
)
|
||||
assertThat(pinched).isWithin(0.01f).of(fitDay)
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a settled pinch is what gets persisted`() {
|
||||
var persisted: TimelineScale? = null
|
||||
|
||||
Reference in New Issue
Block a user