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

4 files changed, 94 insertions, 5 deletions

diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
index 03fa971f332d..638a71965063 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/DraggableHandler.kt
@@ -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()
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt b/packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt
index e3cddc1a8f1d..f5f566f703dc 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/nestedscroll/PriorityNestedScrollConnection.kt
@@ -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()
 
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
index fd0214823eaf..bfb7736397ca 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/DraggableHandlerTest.kt
@@ -1296,6 +1296,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.
         layoutState.transitions = transitions {
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
index 3001505d0e02..2bc9b3826548 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SwipeToSceneTest.kt
@@ -938,6 +938,71 @@ class SwipeToSceneTest {
     }
 
     @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"