Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d23717de authored by omarmt's avatar omarmt
Browse files

LargeTopAppBarNestedScrollConnection use PriorityNestedScrollConnection

The LargeTopAppBarNestedScrollConnection used to consume all onPreScroll
 events when a scrollable item within it was scrolled up. While this was
  correct for the simplest cases, it could cause unexpected behavior
  when other components wanted to consume the onPreScroll event.
To resolve this ambiguity, this CL uses a priority concept that allows
the connection to start consuming scroll events only if it has priority
at that moment. The priority for this connection is obtained if the
gesture is starting at that moment and the connection is scrolling up,
or if the connection is scrolling down and there is an offset available.

Test: atest LargeTopAppBarNestedScrollConnectionTest
Bug: 291053278
Flag: NA
Change-Id: I5e642ee21a18d12ea304aa07c4f9a785b65ed200
parent 9eb42136
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -604,6 +604,7 @@ internal class SceneNestedScrollHandler(
                behavior.canStartOnPostFling && hasNextScene(velocityAvailable)
            },
            canContinueScroll = { true },
            canScrollOnFling = false,
            onStart = { offsetAvailable ->
                gestureHandler.gestureWithPriority = this
                gestureHandler.onDragStarted(
+40 −56
Original line number Diff line number Diff line
@@ -16,9 +16,8 @@

package com.android.compose.nestedscroll

import androidx.compose.ui.geometry.Offset
import androidx.compose.foundation.gestures.Orientation
import androidx.compose.ui.input.nestedscroll.NestedScrollConnection
import androidx.compose.ui.input.nestedscroll.NestedScrollSource

/**
 * A [NestedScrollConnection] that listens for all vertical scroll events and responds in the
@@ -34,59 +33,44 @@ import androidx.compose.ui.input.nestedscroll.NestedScrollSource
 *
 * @sample com.android.compose.animation.scene.demo.Shade
 */
class LargeTopAppBarNestedScrollConnection(
    private val height: () -> Float,
    private val onChangeHeight: (Float) -> Unit,
    private val minHeight: Float,
    private val maxHeight: Float,
) : NestedScrollConnection {

    constructor(
fun LargeTopAppBarNestedScrollConnection(
    height: () -> Float,
    onHeightChanged: (Float) -> Unit,
    heightRange: ClosedFloatingPointRange<Float>,
    ) : this(
        height = height,
        onChangeHeight = onHeightChanged,
        minHeight = heightRange.start,
        maxHeight = heightRange.endInclusive,
    )

    /**
     * When swiping up, the LargeTopAppBar will shrink (to [minHeight]) and the content will expand.
     * Then, you can then scroll down the content.
     */
    override fun onPreScroll(available: Offset, source: NestedScrollSource): Offset {
        val y = available.y
): PriorityNestedScrollConnection {
    val minHeight = heightRange.start
    val maxHeight = heightRange.endInclusive
    return PriorityNestedScrollConnection(
        orientation = Orientation.Vertical,
        // When swiping up, the LargeTopAppBar will shrink (to [minHeight]) and the content will
        // expand. Then, you can then scroll down the content.
        canStartPreScroll = { offsetAvailable, offsetBeforeStart ->
            offsetAvailable < 0 && offsetBeforeStart == 0f && height() > minHeight
        },
        // When swiping down, the content will scroll up until it reaches the top. Then, the
        // LargeTopAppBar will expand until it reaches its [maxHeight].
        canStartPostScroll = { offsetAvailable, _ -> offsetAvailable > 0 && height() < maxHeight },
        canStartPostFling = { false },
        canContinueScroll = {
            val currentHeight = height()
        if (y >= 0 || currentHeight <= minHeight) {
            return Offset.Zero
        }

        val amountLeft = minHeight - currentHeight
        val amountConsumed = y.coerceAtLeast(amountLeft)
        onChangeHeight(currentHeight + amountConsumed)
        return Offset(0f, amountConsumed)
    }

    /**
     * When swiping down, the content will scroll up until it reaches the top. Then, the
     * LargeTopAppBar will expand until it reaches its [maxHeight].
     */
    override fun onPostScroll(
        consumed: Offset,
        available: Offset,
        source: NestedScrollSource
    ): Offset {
        val y = available.y
            minHeight < currentHeight && currentHeight < maxHeight
        },
        canScrollOnFling = true,
        onStart = { /* do nothing */},
        onScroll = { offsetAvailable ->
            val currentHeight = height()
        if (y <= 0 || currentHeight >= maxHeight) {
            return Offset.Zero
        }

            val amountConsumed =
                if (offsetAvailable > 0) {
                    val amountLeft = maxHeight - currentHeight
        val amountConsumed = y.coerceAtMost(amountLeft)
        onChangeHeight(currentHeight + amountConsumed)
        return Offset(0f, amountConsumed)
                    offsetAvailable.coerceAtMost(amountLeft)
                } else {
                    val amountLeft = minHeight - currentHeight
                    offsetAvailable.coerceAtLeast(amountLeft)
                }
            onHeightChanged(currentHeight + amountConsumed)
            amountConsumed
        },
        // Don't consume the velocity on pre/post fling
        onStop = { 0f },
    )
}
+13 −2
Original line number Diff line number Diff line
@@ -38,6 +38,7 @@ class PriorityNestedScrollConnection(
    private val canStartPostScroll: (offsetAvailable: Offset, offsetBeforeStart: Offset) -> Boolean,
    private val canStartPostFling: (velocityAvailable: Velocity) -> Boolean,
    private val canContinueScroll: () -> Boolean,
    private val canScrollOnFling: Boolean,
    private val onStart: (offsetAvailable: Offset) -> Unit,
    private val onScroll: (offsetAvailable: Offset) -> Offset,
    private val onStop: (velocityAvailable: Velocity) -> Velocity,
@@ -59,7 +60,7 @@ class PriorityNestedScrollConnection(

        if (
            isPriorityMode ||
                source == NestedScrollSource.Fling ||
                (source == NestedScrollSource.Fling && !canScrollOnFling) ||
                !canStartPostScroll(available, offsetBeforeStart)
        ) {
            // The priority mode cannot start so we won't consume the available offset.
@@ -71,7 +72,7 @@ class PriorityNestedScrollConnection(

    override fun onPreScroll(available: Offset, source: NestedScrollSource): Offset {
        if (!isPriorityMode) {
            if (source != NestedScrollSource.Fling) {
            if (source != NestedScrollSource.Fling || canScrollOnFling) {
                if (canStartPreScroll(available, offsetScrolledBeforePriorityMode)) {
                    return onPriorityStart(available)
                }
@@ -98,12 +99,20 @@ class PriorityNestedScrollConnection(
    }

    override suspend fun onPreFling(available: Velocity): Velocity {
        if (isPriorityMode && canScrollOnFling) {
            // We don't want to consume the velocity, we prefer to continue receiving scroll events.
            return Velocity.Zero
        }
        // Step 3b: The finger is lifted, we can stop intercepting scroll events and use the speed
        // of the fling gesture.
        return onPriorityStop(velocity = available)
    }

    override suspend fun onPostFling(consumed: Velocity, available: Velocity): Velocity {
        if (isPriorityMode) {
            return onPriorityStop(velocity = available)
        }

        if (!canStartPostFling(available)) {
            return Velocity.Zero
        }
@@ -156,6 +165,7 @@ fun PriorityNestedScrollConnection(
    canStartPostScroll: (offsetAvailable: Float, offsetBeforeStart: Float) -> Boolean,
    canStartPostFling: (velocityAvailable: Float) -> Boolean,
    canContinueScroll: () -> Boolean,
    canScrollOnFling: Boolean,
    onStart: (offsetAvailable: Float) -> Unit,
    onScroll: (offsetAvailable: Float) -> Float,
    onStop: (velocityAvailable: Float) -> Float,
@@ -172,6 +182,7 @@ fun PriorityNestedScrollConnection(
                canStartPostFling(velocityAvailable.toFloat())
            },
            canContinueScroll = canContinueScroll,
            canScrollOnFling = canScrollOnFling,
            onStart = { offsetAvailable -> onStart(offsetAvailable.toFloat()) },
            onScroll = { offsetAvailable: Offset ->
                onScroll(offsetAvailable.toFloat()).toOffset()
+66 −0
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@
package com.android.compose.nestedscroll

import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.nestedscroll.NestedScrollConnection
import androidx.compose.ui.input.nestedscroll.NestedScrollSource
import com.google.common.truth.Truth.assertThat
import org.junit.Test
@@ -36,6 +37,15 @@ class LargeTopAppBarNestedScrollConnectionTest(testCase: TestCase) {
            heightRange = heightRange,
        )

    private fun NestedScrollConnection.scroll(
        available: Offset,
        consumedByScroll: Offset = Offset.Zero,
    ) {
        val consumedByPreScroll = onPreScroll(available = available, source = scrollSource)
        val consumed = consumedByPreScroll + consumedByScroll
        onPostScroll(consumed = consumed, available = available - consumed, source = scrollSource)
    }

    @Test
    fun onScrollUp_consumeHeightFirst() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
@@ -49,6 +59,41 @@ class LargeTopAppBarNestedScrollConnectionTest(testCase: TestCase) {
        assertThat(height).isEqualTo(0f)
    }

    @Test
    fun onScrollUpAfterContentScrolled_ignoreUpEvent() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
        height = 1f

        // scroll down consumed by a child
        scrollConnection.scroll(available = Offset(0f, 1f), consumedByScroll = Offset(0f, 1f))

        val offsetConsumed =
            scrollConnection.onPreScroll(available = Offset(x = 0f, y = -1f), source = scrollSource)

        // It should ignore all onPreScroll events
        assertThat(offsetConsumed).isEqualTo(Offset.Zero)
        assertThat(height).isEqualTo(1f)
    }

    @Test
    fun onScrollUpAfterContentReturnedToZero_consumeHeight() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
        height = 1f

        // scroll down consumed by a child
        scrollConnection.scroll(available = Offset(0f, 1f), consumedByScroll = Offset(0f, 1f))

        // scroll up consumed by a child, the child is in its original position
        scrollConnection.scroll(available = Offset(0f, -1f), consumedByScroll = Offset(0f, -1f))

        val offsetConsumed =
            scrollConnection.onPreScroll(available = Offset(x = 0f, y = -1f), source = scrollSource)

        // It should ignore all onPreScroll events
        assertThat(offsetConsumed).isEqualTo(Offset(0f, -1f))
        assertThat(height).isEqualTo(0f)
    }

    @Test
    fun onScrollUp_consumeDownToMin() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
@@ -109,6 +154,27 @@ class LargeTopAppBarNestedScrollConnectionTest(testCase: TestCase) {
        assertThat(height).isEqualTo(2f)
    }

    @Test
    fun onScrollDownAfterPostScroll_consumeHeightPreScroll() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
        height = 1f
        scrollConnection.onPostScroll(
            consumed = Offset.Zero,
            available = Offset(x = 0f, y = 0.5f),
            source = scrollSource
        )

        val offsetConsumed =
            scrollConnection.onPreScroll(
                available = Offset(x = 0f, y = 0.5f),
                source = scrollSource
            )
        assertThat(offsetConsumed).isEqualTo(Offset(0f, 0.5f))

        // It can increase by 1 (0.5f + 0.5f) the height
        assertThat(height).isEqualTo(2f)
    }

    @Test
    fun onScrollDown_consumeUpToMax() {
        val scrollConnection = buildScrollConnection(heightRange = 0f..2f)
+1 −0
Original line number Diff line number Diff line
@@ -46,6 +46,7 @@ class PriorityNestedScrollConnectionTest {
            canStartPostScroll = { _, _ -> canStartPostScroll },
            canStartPostFling = { canStartPostFling },
            canContinueScroll = { canContinueScroll },
            canScrollOnFling = false,
            onStart = { isStarted = true },
            onScroll = {
                lastScroll = it