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

Commit 4e07be21 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Android (Google) Code Review
Browse files

Merge changes I7311f773,I476228f9,Ia99a388a into main

* changes:
  Remove tolerance from SceneGestureHandlerTest
  Fix regression in SwipeToScene
  Fix regression in SceneGestureHandler
parents 3b164ce3 ab726ea1
Loading
Loading
Loading
Loading
+48 −6
Original line number Diff line number Diff line
@@ -40,12 +40,15 @@ import androidx.compose.ui.input.pointer.util.addPointerInputChange
import androidx.compose.ui.node.CompositionLocalConsumerModifierNode
import androidx.compose.ui.node.DelegatingNode
import androidx.compose.ui.node.ModifierNodeElement
import androidx.compose.ui.node.ObserverModifierNode
import androidx.compose.ui.node.PointerInputModifierNode
import androidx.compose.ui.node.currentValueOf
import androidx.compose.ui.node.observeReads
import androidx.compose.ui.platform.LocalViewConfiguration
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.Velocity
import androidx.compose.ui.util.fastForEach
import kotlin.math.sign

/**
 * Make an element draggable in the given [orientation].
@@ -117,10 +120,15 @@ internal class MultiPointerDraggableNode(
    var onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> Unit,
    var onDragDelta: (Float) -> Unit,
    var onDragStopped: (velocity: Float) -> Unit,
) : PointerInputModifierNode, DelegatingNode(), CompositionLocalConsumerModifierNode {
) :
    PointerInputModifierNode,
    DelegatingNode(),
    CompositionLocalConsumerModifierNode,
    ObserverModifierNode {
    private val pointerInputHandler: suspend PointerInputScope.() -> Unit = { pointerInput() }
    private val delegate = delegate(SuspendingPointerInputModifierNode(pointerInputHandler))
    private val velocityTracker = VelocityTracker()
    private var previousEnabled: Boolean = false

    var enabled: () -> Boolean = enabled
        set(value) {
@@ -140,6 +148,21 @@ internal class MultiPointerDraggableNode(
            }
        }

    override fun onAttach() {
        previousEnabled = enabled()
        onObservedReadsChanged()
    }

    override fun onObservedReadsChanged() {
        observeReads {
            val newEnabled = enabled()
            if (newEnabled != previousEnabled) {
                delegate.resetPointerInputHandler()
            }
            previousEnabled = newEnabled
        }
    }

    override fun onCancelPointerInput() = delegate.onCancelPointerInput()

    override fun onPointerEvent(
@@ -223,12 +246,31 @@ private suspend fun PointerInputScope.detectDragGestures(

                // TODO(b/291055080): Replace by await[Orientation]PointerSlopOrCancellation once
                // it is public.
                val drag =
                    when (orientation) {
                        Orientation.Horizontal ->
                            awaitHorizontalTouchSlopOrCancellation(down.id, onSlopReached)
                        Orientation.Vertical ->
                            awaitVerticalTouchSlopOrCancellation(down.id, onSlopReached)
                    }

                // Make sure that overSlop is not 0f. This can happen when the user drags by exactly
                // the touch slop. However, the overSlop we pass to onDragStarted() is used to
                // compute the direction we are dragging in, so overSlop should never be 0f unless
                // we intercept an ongoing swipe transition (i.e. startDragImmediately() returned
                // true).
                if (drag != null && overSlop == 0f) {
                    val deltaOffset = drag.position - initialDown.position
                    val delta =
                        when (orientation) {
                            Orientation.Horizontal -> deltaOffset.y
                            Orientation.Vertical -> deltaOffset.y
                        }
                    check(delta != 0f)
                    overSlop = delta.sign
                }

                drag
            }

        if (drag != null) {
+22 −27
Original line number Diff line number Diff line
@@ -21,7 +21,6 @@ import androidx.compose.material3.Text
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.nestedscroll.NestedScrollConnection
import androidx.compose.ui.input.nestedscroll.NestedScrollSource
import androidx.compose.ui.test.ExperimentalTestApi
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.IntSize
import androidx.compose.ui.unit.Velocity
@@ -118,9 +117,6 @@ class SceneGestureHandlerTest {
        fun up(fractionOfScreen: Float) =
            if (fractionOfScreen < 0f) error("use down()") else -down(fractionOfScreen)

        // Float tolerance for comparisons
        val tolerance = 0.00001f

        // Offset y: 10% of the screen
        val offsetY10 = Offset(x = 0f, y = down(0.1f))

@@ -169,12 +165,11 @@ class SceneGestureHandlerTest {
            if (progress != null)
                assertWithMessage("progress does not match")
                    .that((transitionState as? Transition)?.progress)
                    .isWithin(tolerance)
                    .isWithin(0f) // returns true when comparing 0.0f with -0.0f
                    .of(progress)
        }
    }

    @OptIn(ExperimentalTestApi::class)
    private fun runGestureTest(block: suspend TestGestureScope.() -> Unit) {
        runMonotonicClockTest { TestGestureScope(coroutineScope = this).block() }
    }
@@ -248,7 +243,7 @@ class SceneGestureHandlerTest {
    @Test
    fun onDragReversedDirection_changeToScene() = runGestureTest {
        // Drag A -> B with progress 0.6
        draggable.onDragStarted(overSlop = up(0.6f))
        draggable.onDragStarted(overSlop = -60f)
        assertTransition(
            currentScene = SceneA,
            fromScene = SceneA,
@@ -257,7 +252,7 @@ class SceneGestureHandlerTest {
        )

        // Reverse direction such that A -> C now with 0.4
        draggable.onDelta(down(1f))
        draggable.onDelta(100f)
        assertTransition(
            currentScene = SceneA,
            fromScene = SceneA,
@@ -287,7 +282,7 @@ class SceneGestureHandlerTest {
        navigateToSceneC()

        // We are on SceneC which has no action in Down direction
        draggable.onDragStarted(down(0.1f))
        draggable.onDragStarted(10f)
        assertTransition(
            currentScene = SceneC,
            fromScene = SceneC,
@@ -296,7 +291,7 @@ class SceneGestureHandlerTest {
        )

        // Reverse drag direction, it will consume the previous drag
        draggable.onDelta(up(0.1f))
        draggable.onDelta(-10f)
        assertTransition(
            currentScene = SceneC,
            fromScene = SceneC,
@@ -305,7 +300,7 @@ class SceneGestureHandlerTest {
        )

        // Continue reverse drag direction, it should record progress to Scene B
        draggable.onDelta(up(0.1f))
        draggable.onDelta(-10f)
        assertTransition(
            currentScene = SceneC,
            fromScene = SceneC,
@@ -557,51 +552,51 @@ class SceneGestureHandlerTest {
    ) {
        val nestedScroll = nestedScrollConnection(nestedScrollBehavior = EdgeWithPreview)
        // start scene transition
        nestedScroll.scroll(available = Offset(0f, SCREEN_SIZE * firstScroll))
        nestedScroll.scroll(available = Offset(0f, firstScroll))

        // stop scene transition (start the "stop animation")
        nestedScroll.onPreFling(available = Velocity.Zero)

        // a pre scroll event, that could be intercepted by SceneGestureHandler
        nestedScroll.onPreScroll(Offset(0f, SCREEN_SIZE * secondScroll), NestedScrollSource.Drag)
        nestedScroll.onPreScroll(Offset(0f, secondScroll), NestedScrollSource.Drag)
    }

    @Test
    fun scrollAndFling_scrollLessThanInterceptable_goToIdleOnCurrentScene() = runGestureTest {
        val first = transitionInterceptionThreshold - tolerance
        val second = 0.01f
        val firstScroll = (transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
        val secondScroll = 1f

        preScrollAfterSceneTransition(firstScroll = first, secondScroll = second)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)

        assertIdle(SceneA)
    }

    @Test
    fun scrollAndFling_scrollMinInterceptable_interceptPreScrollEvents() = runGestureTest {
        val first = transitionInterceptionThreshold + tolerance
        val second = 0.01f
        val firstScroll = (transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
        val secondScroll = 1f

        preScrollAfterSceneTransition(firstScroll = first, secondScroll = second)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)

        assertTransition(progress = first + second)
        assertTransition(progress = (firstScroll + secondScroll) / SCREEN_SIZE)
    }

    @Test
    fun scrollAndFling_scrollMaxInterceptable_interceptPreScrollEvents() = runGestureTest {
        val first = 1f - transitionInterceptionThreshold - tolerance
        val second = 0.01f
        val firstScroll = (1f - transitionInterceptionThreshold - 0.0001f) * SCREEN_SIZE
        val secondScroll = 1f

        preScrollAfterSceneTransition(firstScroll = first, secondScroll = second)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)

        assertTransition(progress = first + second)
        assertTransition(progress = (firstScroll + secondScroll) / SCREEN_SIZE)
    }

    @Test
    fun scrollAndFling_scrollMoreThanInterceptable_goToIdleOnNextScene() = runGestureTest {
        val first = 1f - transitionInterceptionThreshold + tolerance
        val second = 0.01f
        val firstScroll = (1f - transitionInterceptionThreshold + 0.0001f) * SCREEN_SIZE
        val secondScroll = 0.01f

        preScrollAfterSceneTransition(firstScroll = first, secondScroll = second)
        preScrollAfterSceneTransition(firstScroll = firstScroll, secondScroll = secondScroll)

        assertIdle(SceneC)
    }
+96 −13
Original line number Diff line number Diff line
@@ -21,6 +21,9 @@ import androidx.compose.foundation.layout.Spacer
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.size
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.platform.LocalViewConfiguration
@@ -63,7 +66,10 @@ class SwipeToSceneTest {

    /** The content under test. */
    @Composable
    private fun TestContent(layoutState: SceneTransitionLayoutState) {
    private fun TestContent(
        layoutState: SceneTransitionLayoutState,
        swipesEnabled: () -> Boolean = { true },
    ) {
        SceneTransitionLayout(
            state = layoutState,
            modifier = Modifier.size(LayoutWidth, LayoutHeight).testTag(TestElements.Foo.debugName),
@@ -71,28 +77,35 @@ class SwipeToSceneTest {
            scene(
                TestScenes.SceneA,
                userActions =
                    if (swipesEnabled())
                        mapOf(
                            Swipe.Left to TestScenes.SceneB,
                            Swipe.Down to TestScenes.SceneC,
                    ),
                            Swipe.Up to TestScenes.SceneB,
                        )
                    else emptyMap(),
            ) {
                Box(Modifier.fillMaxSize())
            }
            scene(
                TestScenes.SceneB,
                userActions = mapOf(Swipe.Right to TestScenes.SceneA),
                userActions =
                    if (swipesEnabled()) mapOf(Swipe.Right to TestScenes.SceneA) else emptyMap(),
            ) {
                Box(Modifier.fillMaxSize())
            }
            scene(
                TestScenes.SceneC,
                userActions =
                    if (swipesEnabled())
                        mapOf(
                            Swipe.Down to TestScenes.SceneA,
                            Swipe(SwipeDirection.Down, pointerCount = 2) to TestScenes.SceneB,
                        Swipe(SwipeDirection.Right, fromSource = Edge.Left) to TestScenes.SceneB,
                            Swipe(SwipeDirection.Right, fromSource = Edge.Left) to
                                TestScenes.SceneB,
                            Swipe(SwipeDirection.Down, fromSource = Edge.Top) to TestScenes.SceneB,
                    ),
                        )
                    else emptyMap(),
            ) {
                Box(Modifier.fillMaxSize())
            }
@@ -357,7 +370,7 @@ class SwipeToSceneTest {
        // detected as a drag event.
        var touchSlop = 0f

        val layoutState = MutableSceneTransitionLayoutState(TestScenes.SceneA)
        val layoutState = layoutState()
        val verticalSwipeDistance = 50.dp
        assertThat(verticalSwipeDistance).isNotEqualTo(LayoutHeight)

@@ -392,4 +405,74 @@ class SwipeToSceneTest {
        assertThat(transition).isNotNull()
        assertThat(transition!!.progress).isEqualTo(0.5f)
    }

    @Test
    fun swipeByTouchSlop() {
        val layoutState = layoutState()
        var touchSlop = 0f
        rule.setContent {
            touchSlop = LocalViewConfiguration.current.touchSlop
            TestContent(layoutState)
        }

        // Swipe down by exactly touchSlop, so that the drag overSlop is 0f.
        rule.onRoot().performTouchInput {
            down(middle)
            moveBy(Offset(0f, touchSlop), delayMillis = 1_000)
        }

        // We should still correctly compute that we are swiping down to scene C.
        var transition = layoutState.currentTransition
        assertThat(transition).isNotNull()
        assertThat(transition?.toScene).isEqualTo(TestScenes.SceneC)

        // Release the finger, animating back to scene A.
        rule.onRoot().performTouchInput { up() }
        rule.waitForIdle()
        assertThat(layoutState.currentTransition).isNull()
        assertThat(layoutState.transitionState.currentScene).isEqualTo(TestScenes.SceneA)

        // Swipe up by exactly touchSlop, so that the drag overSlop is 0f.
        rule.onRoot().performTouchInput {
            down(middle)
            moveBy(Offset(0f, -touchSlop), delayMillis = 1_000)
        }

        // We should still correctly compute that we are swiping up to scene B.
        transition = layoutState.currentTransition
        assertThat(transition).isNotNull()
        assertThat(transition?.toScene).isEqualTo(TestScenes.SceneB)
    }

    @Test
    fun swipeEnabledLater() {
        val layoutState = MutableSceneTransitionLayoutState(TestScenes.SceneA)
        var swipesEnabled by mutableStateOf(false)
        var touchSlop = 0f
        rule.setContent {
            touchSlop = LocalViewConfiguration.current.touchSlop
            TestContent(layoutState, swipesEnabled = { swipesEnabled })
        }

        // Drag down from the middle. This should not do anything, because swipes are disabled.
        rule.onRoot().performTouchInput {
            down(middle)
            moveBy(Offset(0f, touchSlop), delayMillis = 1_000)
        }
        assertThat(layoutState.currentTransition).isNull()

        // Release finger.
        rule.onRoot().performTouchInput { up() }

        // Enable swipes.
        swipesEnabled = true
        rule.waitForIdle()

        // Drag down from the middle. Now it should start a transition.
        rule.onRoot().performTouchInput {
            down(middle)
            moveBy(Offset(0f, touchSlop), delayMillis = 1_000)
        }
        assertThat(layoutState.currentTransition).isNotNull()
    }
}