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

Commit 977abc0d authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Fix regression in SwipeToScene 

This CL fixes a regression caused by ag/25992835: now that
MultiPointerDraggableNode.enabled is a lambda (that might never change),
we need to explicitly observe the result of this lambda to reset the
associated pointer input whenever enabled is changed.

Bug: 291071158
Test: atest SwipeToSceneTest
Flag: N/A
Change-Id: I476228f9f5e66d1b69de33b0879f62507ce31980
parent ac9016aa

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

+23 −1
Original line number Diff line number Diff line
@@ -40,8 +40,10 @@ 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
@@ -118,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) {
@@ -141,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(

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

+57 −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,29 +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())

            }
@@ -431,4 +443,36 @@ class SwipeToSceneTest { 
        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()

    }

}