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

Commit 13c0ec43 authored by omarmt's avatar omarmt
Browse files

STL nested scroll keeps priority even if it cannot scroll 

Fixes a bug introduced with ag/29700348.

In an attempt to simplify PriorityNestedScrollConnection declaration, a
new behavior has been introduced: the NestedScrollConnection could lose
priority when it could no longer scroll.

This caused unexpected behavior (see ag/29996647) and for this reason it
has been reverted.

It is now possible to specify when the PriorityNestedScrollConnection
can lose priority during the scroll gesture.

Test: atest DraggableHandlerTest
Test: atest SwipeToSceneTest
Bug: 370949877
Flag: com.android.systemui.scene_container
Change-Id: Id8c295458bb4a82a128fe2370bccd5883d0f4889
parent 90bef8a8

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt

+3 −0
Original line number Diff line number Diff line
@@ -709,6 +709,9 @@ internal class NestedScrollHandlerImpl( 


                canStart

            },

            // We need to maintain scroll priority even if the scene transition can no longer

            // consume the scroll gesture to allow us to return to the previous scene.

            canStopOnScroll = { _, _ -> false },

            canStopOnPreFling = { true },

            onStart = { offsetAvailable ->

                val pointersInfo = pointersInfo()

packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt

+6 −5
Original line number Diff line number Diff line
@@ -40,6 +40,8 @@ internal typealias SuspendedValue<T> = suspend () -> T 
 *   events in post-scroll mode.

 * @param canStartPostFling lambda that returns true if the connection can start consuming scroll

 *   events in post-fling mode.

 * @param canStopOnScroll lambda that returns true if the connection can stop consuming scroll

 *   events in scroll mode.

 * @param canStopOnPreFling lambda that returns true if the connection can stop consuming scroll

 *   events in pre-fling (i.e. as soon as the user lifts their fingers).

 * @param onStart lambda that is called when the connection starts consuming scroll events.
@@ -57,6 +59,9 @@ class PriorityNestedScrollConnection( 
    private val canStartPostScroll:

        (offsetAvailable: Float, offsetBeforeStart: Float, source: NestedScrollSource) -> Boolean,

    private val canStartPostFling: (velocityAvailable: Float) -> Boolean,

    private val canStopOnScroll: (available: Float, consumed: Float) -> Boolean = { _, consumed ->

        consumed == 0f

    },

    private val canStopOnPreFling: () -> Boolean,

    private val onStart: (offsetAvailable: Float) -> Unit,

    private val onScroll: (offsetAvailable: Float, source: NestedScrollSource) -> Float,
@@ -155,10 +160,6 @@ class PriorityNestedScrollConnection( 
        }

    }



    private fun shouldStop(consumed: Float): Boolean {

        return consumed == 0f

    }



    private fun start(

        availableOffset: Float,

        source: NestedScrollSource,
@@ -183,7 +184,7 @@ class PriorityNestedScrollConnection( 
        // Step 2: We have the priority and can consume the scroll events.

        val consumedByScroll = onScroll(offsetAvailable, source)



        if (shouldStop(consumedByScroll)) {

        if (canStopOnScroll(offsetAvailable, consumedByScroll)) {

            // Step 3a: We have lost priority and we no longer need to intercept scroll events.

            cancel()

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt

+20 −0
Original line number Diff line number Diff line
@@ -1295,6 +1295,26 @@ class DraggableHandlerTest { 
        )

    }



    @Test

    fun scrollKeepPriorityEvenIfWeCanNoLongerScrollOnThatDirection() = runGestureTest {

        // Overscrolling on scene B does nothing.

        layoutState.transitions = transitions { overscrollDisabled(SceneB, Orientation.Vertical) }

        val nestedScroll = nestedScrollConnection(nestedScrollBehavior = EdgeAlways)



        // Overscroll is disabled, it will scroll up to 100%

        nestedScroll.scroll(available = upOffset(fractionOfScreen = 2f))

        assertTransition(fromScene = SceneA, toScene = SceneB, progress = 1f)



        // We need to maintain scroll priority even if the scene transition can no longer consume

        // the scroll gesture.

        nestedScroll.scroll(available = upOffset(fractionOfScreen = 0.1f))

        assertTransition(fromScene = SceneA, toScene = SceneB, progress = 1f)



        // A scroll gesture in the opposite direction allows us to return to the previous scene.

        nestedScroll.scroll(available = downOffset(fractionOfScreen = 0.5f))

        assertTransition(fromScene = SceneA, toScene = SceneB, progress = 0.5f)

    }



    @Test

    fun overscroll_releaseBetween0And100Percent_up() = runGestureTest {

        // Make scene B overscrollable.

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt

+65 −0
Original line number Diff line number Diff line
@@ -937,6 +937,71 @@ class SwipeToSceneTest { 
        assertThat(availableOnPostScroll).isEqualTo(ovescrollPx)

    }



    @Test

    fun scrollKeepPriorityEvenIfWeCanNoLongerScrollOnThatDirection() {

        val swipeDistance = 100.dp

        val state =

            rule.runOnUiThread {

                MutableSceneTransitionLayoutState(

                    SceneA,

                    transitions {

                        from(SceneA, to = SceneB) { distance = FixedDistance(swipeDistance) }

                        from(SceneB, to = SceneC) { distance = FixedDistance(swipeDistance) }

                        overscrollDisabled(SceneB, Orientation.Vertical)

                    },

                )

            }

        val layoutSize = 200.dp

        var touchSlop = 0f

        rule.setContent {

            touchSlop = LocalViewConfiguration.current.touchSlop

            SceneTransitionLayout(state, Modifier.size(layoutSize)) {

                scene(SceneA, userActions = mapOf(Swipe.Down to SceneB, Swipe.Right to SceneC)) {

                    Box(

                        Modifier.fillMaxSize()

                            // A scrollable that does not consume the scroll gesture

                            .scrollable(rememberScrollableState { 0f }, Orientation.Vertical)

                    )

                }

                scene(SceneB, userActions = mapOf(Swipe.Right to SceneC)) {

                    Box(Modifier.element(TestElements.Foo).fillMaxSize())

                }

                scene(SceneC) { Box(Modifier.fillMaxSize()) }

            }

        }



        fun assertTransition(from: SceneKey, to: SceneKey, progress: Float) {

            val transition = assertThat(state.transitionState).isSceneTransition()

            assertThat(transition).hasFromScene(from)

            assertThat(transition).hasToScene(to)

            assertThat(transition.progress).isEqualTo(progress)

        }



        // Vertical scroll 100%

        rule.onRoot().performTouchInput {

            val middle = (layoutSize / 2).toPx()

            down(Offset(middle, middle))

            moveBy(Offset(0f, y = touchSlop + swipeDistance.toPx()), delayMillis = 1_000)

        }

        assertTransition(from = SceneA, to = SceneB, progress = 1f)



        // Continue vertical scroll, should be ignored (overscrollDisabled)

        rule.onRoot().performTouchInput { moveBy(Offset(0f, y = touchSlop), delayMillis = 1_000) }

        assertTransition(from = SceneA, to = SceneB, progress = 1f)



        // Horizontal scroll, should be ignored

        rule.onRoot().performTouchInput {

            moveBy(Offset(x = touchSlop + swipeDistance.toPx(), 0f), delayMillis = 1_000)

        }

        assertTransition(from = SceneA, to = SceneB, progress = 1f)



        // Vertical scroll, in the opposite direction

        rule.onRoot().performTouchInput {

            moveBy(Offset(0f, -swipeDistance.toPx()), delayMillis = 1_000)

        }

        assertTransition(from = SceneA, to = SceneB, progress = 0f)

    }



    @Test

    fun sceneWithoutSwipesDoesNotConsumeGestures() {

        val buttonTag = "button"