diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarState.kt index d426b09e7d..8d743246db 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarState.kt @@ -86,8 +86,8 @@ class DisappearingBarState( */ suspend fun resetToVisible() { coroutineScope { - launch { animateOne({ topHeightOffset }, 0f, 0f) { topHeightOffset = it } } - launch { animateOne({ bottomHeightOffset }, 0f, 0f) { bottomHeightOffset = it } } + launch { animateOne({ topHeightOffset }, 0f, 0f, topHeightLimit) { topHeightOffset = it } } + launch { animateOne({ bottomHeightOffset }, 0f, 0f, bottomHeightLimit) { bottomHeightOffset = it } } } } @@ -110,19 +110,27 @@ class DisappearingBarState( positionBiasToHide -> -limit else -> 0f } - animateOne(get, target, initialVelocityY, set) + animateOne(get, target, initialVelocityY, limit, set) } private suspend fun animateOne( get: () -> Float, target: Float, initialVelocity: Float, + limit: Float, set: (Float) -> Unit, ) { val start = get() if (start == target && initialVelocity == 0f) return Animatable(start) - .animateTo( + .apply { + // Clamp to the visible travel range. A critically-damped spring still crosses its + // target once when given an initial velocity in the target's direction, so without + // these bounds a fast reveal fling would push the offset past 0 (or past -limit on + // a hide) and render the bar overshooting its resting edge before springing back. + // Hitting a bound ends the animation at the edge — a crisp settle with no rebound. + if (limit > 0f) updateBounds(lowerBound = -limit, upperBound = 0f) + }.animateTo( targetValue = target, animationSpec = SETTLE_SPRING, initialVelocity = initialVelocity, diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarStateTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarStateTest.kt new file mode 100644 index 0000000000..bae001b849 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/layouts/DisappearingBarStateTest.kt @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.amethyst.ui.layouts + +import androidx.compose.runtime.BroadcastFrameClock +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class DisappearingBarStateTest { + private fun state( + topLimit: Float = 100f, + bottomLimit: Float = 50f, + ) = DisappearingBarState().apply { + topHeightLimit = topLimit + bottomHeightLimit = bottomLimit + } + + /** + * Runs [block] (a settle/reset animation) under a manually-stepped frame clock, sampling both + * offsets after every frame. Returns the highest (closest-to-or-past zero) value each offset + * reached — a bar overshooting its fully-visible resting edge briefly pushes its offset above 0. + */ + @OptIn(ExperimentalCoroutinesApi::class) + private fun TestScope.peakOffsetsDuring( + state: DisappearingBarState, + block: suspend () -> Unit, + ): Pair { + val clock = BroadcastFrameClock() + var maxTop = state.topHeightOffset + var maxBottom = state.bottomHeightOffset + + val job = launch(clock) { block() } + runCurrent() + + var frameNanos = 0L + val frameStep = 16_000_000L + // Hard cap so a regression that never settles fails the test instead of hanging it. + repeat(2_000) { + if (!job.isActive) return@repeat + frameNanos += frameStep + clock.sendFrame(frameNanos) + runCurrent() + maxTop = maxOf(maxTop, state.topHeightOffset) + maxBottom = maxOf(maxBottom, state.bottomHeightOffset) + } + assertTrue("animation did not settle within the frame budget", !job.isActive) + return maxTop to maxBottom + } + + @Test + fun `a fast reveal fling settles at the visible edge without overshooting`() = + runTest { + val state = state(topLimit = 100f, bottomLimit = 50f) + // Mid-collapse, as a reveal fling leaves the bars when the list hits the top edge. + // Settling toward the visible edge (0) with a strong reveal velocity is what made the + // critically-damped spring shoot past 0 before the fix clamped it. + state.topHeightOffset = -40f + state.bottomHeightOffset = -20f + + val (peakTop, peakBottom) = + peakOffsetsDuring(state) { + state.settleToNearestEdge(initialVelocityY = 12000f) + } + + // Lands exactly on the visible edge... + assertEquals(0f, state.topHeightOffset, 0.01f) + assertEquals(0f, state.bottomHeightOffset, 0.01f) + // ...and never travels past it on the way there (no slide-down-then-back wobble). + assertTrue("top bar overshot to $peakTop", peakTop <= 0.5f) + assertTrue("bottom bar overshot to $peakBottom", peakBottom <= 0.5f) + } +}