From 5bdc4a20b0623c8a4bbcf0d1c57cc92a3a36aa3c Mon Sep 17 00:00:00 2001 From: Jean-Luc Makiola Date: Fri, 31 Jul 2026 20:14:36 +0200 Subject: [PATCH] Fix the pinch's jumpy hour gutter and its zoom-out floor (#56) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both found reviewing the gesture on device. The gutter is 24 stacked hour-tall boxes, each rounding its own height to whole pixels, while the hour lines and event blocks are drawn at the fractional height. A pinch lands on fractional heights constantly, so the two sat on different grids — at 56.4px per hour the 23:00 label is ~9px off the line it names — and the whole label column jumped as the pinch drifted across each half pixel. A pinch now lands on whole pixels, so every part of the timeline shares one grid. Zooming out stopped at a fixed 24dp per hour, which is below the height that fills the screen on a phone, so you could shrink the day until it left dead space under midnight. The floor is now the height at which all 24 hours exactly fill the viewport: past that there is no more day to uncover. It is applied when the height is resolved rather than only in the gesture, so a height pinched in landscape still fills portrait. --- .../calendula/ui/common/TimelineScale.kt | 32 ++++++++++++++++--- .../calendula/ui/common/TimelineZoom.kt | 32 ++++++++++++++++--- .../calendula/ui/day/DayScreen.kt | 2 +- .../calendula/ui/week/WeekScreen.kt | 2 +- .../calendula/ui/common/TimelineScaleTest.kt | 29 ++++++++++------- .../calendula/ui/common/TimelineZoomTest.kt | 29 +++++++++++++++++ 6 files changed, 104 insertions(+), 22 deletions(-) diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt index a8133b1..c5f941a 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineScale.kt @@ -42,9 +42,13 @@ sealed interface TimelineScale { /** The named steps the settings picker offers, coarse to roomy. */ val presets: List = listOf(FitDay, Compact, Regular, Comfortable) - /** A pinched hour height, clamped into the legible zoom range. */ + /** + * A pinched hour height. The floor a pinch actually stops at depends on + * the viewport and is applied when the height is resolved (see + * [hourHeight]); this only holds a stored value to something sane. + */ fun custom(hourHeight: Dp): Custom = - Custom(hourHeight.coerceIn(MIN_PINCH_HOUR_HEIGHT, MAX_PINCH_HOUR_HEIGHT)) + Custom(hourHeight.coerceIn(MIN_STORED_HOUR_HEIGHT, MAX_PINCH_HOUR_HEIGHT)) } } @@ -56,6 +60,11 @@ sealed interface TimelineScale { * being legible, and above [FIT_DAY_MAX] a short landscape day would stretch its * blocks absurdly. On a screen too short for the whole day the clamp wins and * the timeline still scrolls a little — honest, rather than unreadable. + * + * A pinched height is held to [fillHourHeight] here and not only in the gesture, + * because the viewport can change under a height that was already stored: pinch + * all the way out in landscape and the same value would leave portrait with dead + * space under midnight. */ fun TimelineScale.hourHeight(viewportHeight: Dp): Dp = when (this) { TimelineScale.FitDay -> (viewportHeight / 24f).coerceIn(FIT_DAY_MIN, FIT_DAY_MAX) @@ -63,6 +72,8 @@ fun TimelineScale.hourHeight(viewportHeight: Dp): Dp = when (this) { TimelineScale.Regular -> 56.dp TimelineScale.Comfortable -> 80.dp is TimelineScale.Custom -> this.hourHeight + .coerceAtMost(MAX_PINCH_HOUR_HEIGHT) + .coerceAtLeast(fillHourHeight(viewportHeight)) } /** @@ -91,10 +102,14 @@ val FIT_DAY_MIN = 24.dp val FIT_DAY_MAX = 96.dp /** - * Floor for a pinch — the same legibility limit [TimelineScale.FitDay] stops at, - * since the gutter has the same 24 labels to fit either way. + * The hour height at which all 24 hours exactly fill [viewportHeight] — how far + * out a pinch can zoom. + * + * Below it there is no more day left to uncover, so the only thing shrinking + * further buys is empty space under midnight. Zooming out means "show me more of + * the day", and once the whole day is on screen that request is answered. */ -val MIN_PINCH_HOUR_HEIGHT = FIT_DAY_MIN +fun fillHourHeight(viewportHeight: Dp): Dp = viewportHeight / 24f /** * Ceiling for a pinch, deliberately far above [TimelineScale.Comfortable]: a @@ -103,6 +118,13 @@ val MIN_PINCH_HOUR_HEIGHT = FIT_DAY_MIN */ val MAX_PINCH_HOUR_HEIGHT = 240.dp +/** + * Floor for a *stored* pinched height, which only has to stay sane enough to be + * re-clamped against whatever viewport it is later shown in. The floor a pinch + * stops at is [fillHourHeight]. + */ +private val MIN_STORED_HOUR_HEIGHT = 1.dp + @get:StringRes val TimelineScale.labelRes: Int get() = when (this) { diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt index 2882298..597ab63 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoom.kt @@ -19,6 +19,7 @@ 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.roundToInt /** * The scale the timelines draw at right now: the stored preference, or whatever @@ -104,16 +105,19 @@ fun rememberTimelineZoom( * * [hourHeight] is the *resolved* height, so a pinch that starts from * `FitDay` picks up where the viewport left it rather than jumping. + * [viewportHeight] sets how far out it can go — see [fillHourHeight]. */ @Composable fun rememberTimelinePinchZoom( scrollState: ScrollState, + viewportHeight: Dp, hourHeight: Dp, zoom: TimelineZoom, ): Modifier { - // The gesture loop outlives any single composition, so it reads the height - // through a state handle rather than capturing the value it started with. + // The gesture loop outlives any single composition, so it reads these + // through state handles rather than capturing what they were when it started. val currentHourHeight = rememberUpdatedState(hourHeight) + val currentViewport = rememberUpdatedState(viewportHeight) return Modifier.pointerInput(scrollState, zoom) { awaitEachGesture { awaitFirstDown(requireUnconsumed = false, pass = PointerEventPass.Initial) @@ -136,8 +140,11 @@ fun rememberTimelinePinchZoom( } if (claimed) { val old = currentHourHeight.value.toPx() - val new = (old * step) - .coerceIn(MIN_PINCH_HOUR_HEIGHT.toPx(), MAX_PINCH_HOUR_HEIGHT.toPx()) + val new = pinchedHourHeightPx( + target = old * step, + fillPx = fillHourHeight(currentViewport.value).toPx(), + maxPx = MAX_PINCH_HOUR_HEIGHT.toPx(), + ) if (new != old) { val centroidY = event.calculateCentroid(useCurrent = true).y pending += anchoredScroll(scrollState.value, centroidY, old, new) - @@ -157,6 +164,23 @@ fun rememberTimelinePinchZoom( } } +/** + * Where a pinch aiming at [target] px per hour actually lands: clamped between + * "the whole day fills the screen" ([fillPx]) and [maxPx], then rounded to a + * whole pixel. + * + * The rounding is not cosmetic. The hour gutter is 24 stacked boxes one hour + * tall, and each rounds its own height to whole pixels, while the hour lines and + * event blocks are drawn at the fractional height — so a height of 56.4px lays + * the labels out at 56px and leaves the 23:00 label ~9px above the line it + * 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. + */ +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.coerceAtMost(maxPx).coerceAtLeast(fillPx).roundToInt().toFloat() + /** * The scroll offset that keeps the moment under [centroidY] under it after the * hour height changes from [oldHourPx] to [newHourPx]. diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt index 07558b1..93cc0a0 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/day/DayScreen.kt @@ -499,7 +499,7 @@ private fun Timeline( // The pinch sits on the Row, above both scroll viewports: it has to // outrank the vertical scroll, and it does that by watching the initial // pass, which only reaches it if it is their ancestor. - val pinch = rememberTimelinePinchZoom(scrollState, hourHeight, zoom) + val pinch = rememberTimelinePinchZoom(scrollState, maxHeight, hourHeight, zoom) // Gutter and day column are two scroll viewports that SHARE one scroll // state, so they stay perfectly aligned. The day-column viewport is a // static, rounded-clipped window — the content scrolls inside it, so the diff --git a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt index c53fdee..34101e1 100644 --- a/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt +++ b/app/src/main/java/de/jeanlucmakiola/calendula/ui/week/WeekScreen.kt @@ -634,7 +634,7 @@ private fun Timeline( // The pinch sits on the Row, above both scroll viewports: it has to // outrank the vertical scroll, and it does that by watching the initial // pass, which only reaches it if it is their ancestor. - val pinch = rememberTimelinePinchZoom(scrollState, hourHeight, zoom) + val pinch = rememberTimelinePinchZoom(scrollState, maxHeight, hourHeight, zoom) // Gutter and day columns are two scroll viewports that SHARE one scroll // state, so they stay perfectly aligned. The day-column viewport is a // static, rounded-clipped window — the content scrolls inside it, so the diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineScaleTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineScaleTest.kt index d1bcd67..c78bcd4 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineScaleTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineScaleTest.kt @@ -73,27 +73,34 @@ class TimelineScaleTest { } @Test - fun `a pinched height is used as given, whatever the viewport`() { - val scale = TimelineScale.custom(63.dp) - assertThat(scale.hourHeight(phoneViewport)).isEqualTo(63.dp) - assertThat(scale.hourHeight(120.dp)).isEqualTo(63.dp) + fun `a pinched height is used as given`() { + assertThat(TimelineScale.custom(63.dp).hourHeight(phoneViewport)).isEqualTo(63.dp) } @Test - fun `a pinch cannot zoom past the legible range`() { - // The gesture clamps as it goes, but the floor and ceiling belong to the - // model — nothing that constructs a scale may land outside them (#56). - assertThat(TimelineScale.custom(1.dp).hourHeight).isEqualTo(MIN_PINCH_HOUR_HEIGHT) - assertThat(TimelineScale.custom(10_000.dp).hourHeight).isEqualTo(MAX_PINCH_HOUR_HEIGHT) + fun `a pinched height never leaves the timeline short of the screen`() { + // Zooming out means "show me more of the day"; once the whole day is on + // screen that is answered, and going further would only open dead space + // under midnight. + val tooSmall = TimelineScale.custom(4.dp) + assertThat(tooSmall.hourHeight(phoneViewport) * 24).isAtLeast(phoneViewport) } @Test - fun `a pinch can go beyond every preset in both directions`() { + fun `a height pinched on one viewport still fills a taller one`() { + // Pinching all the way out in landscape stores a small height; rotating + // back to portrait must not leave the day floating in the top half. + val pinchedInLandscape = TimelineScale.custom(fillHourHeight(320.dp)) + assertThat(pinchedInLandscape.hourHeight(phoneViewport) * 24).isAtLeast(phoneViewport) + } + + @Test + fun `a pinch can zoom in past every preset`() { // Otherwise the gesture would be strictly less capable than the picker // it is meant to refine. val presetHeights = TimelineScale.presets.map { it.hourHeight(phoneViewport) } - assertThat(MIN_PINCH_HOUR_HEIGHT).isAtMost(presetHeights.min()) assertThat(MAX_PINCH_HOUR_HEIGHT).isGreaterThan(presetHeights.max()) + assertThat(TimelineScale.custom(200.dp).hourHeight(phoneViewport)).isEqualTo(200.dp) } @Test diff --git a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt index 087d1a3..1bc1994 100644 --- a/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt +++ b/app/src/test/java/de/jeanlucmakiola/calendula/ui/common/TimelineZoomTest.kt @@ -45,6 +45,35 @@ class TimelineZoomTest { assertThat(anchoredScroll(0, 0f, 56f, 84f)).isWithin(0.001f).of(0f) } + @Test + fun `a pinch lands on whole pixels`() { + // The gutter is 24 stacked hour-tall boxes, each rounding its own height, + // while the lines and blocks are drawn at the fractional one — a + // fractional hour puts the two on different grids and jumps the labels + // about as the pinch drifts across each half pixel. + for (target in listOf(56.4f, 56.6f, 83.5f, 120.01f)) { + val landed = pinchedHourHeightPx(target, fillPx = 28f, maxPx = 240f) + assertThat(landed).isEqualTo(landed.toInt().toFloat()) + } + } + + @Test + fun `a pinch stops where the day fills the screen`() { + assertThat(pinchedHourHeightPx(target = 5f, fillPx = 28f, maxPx = 240f)).isEqualTo(28f) + } + + @Test + fun `filling the screen outranks the ceiling`() { + // On a viewport tall enough for the two to disagree, dead space under + // midnight is the worse of the two failures. + assertThat(pinchedHourHeightPx(target = 10f, fillPx = 300f, maxPx = 240f)).isEqualTo(300f) + } + + @Test + fun `a pinch stops at the ceiling`() { + assertThat(pinchedHourHeightPx(target = 9_000f, fillPx = 28f, maxPx = 240f)).isEqualTo(240f) + } + @Test fun `a settled pinch is what gets persisted`() { var persisted: TimelineScale? = null