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

Commit 880ef4fa authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Disable nested drag only in disableSwipesWhenScrolling() 

This CL is changing the implementation of disableSwipesWhenScrolling()
so that only the nested scroll behavior of Modifier.nestedDraggable() is
disabled when scrolls have been consumed by a nested scrollable, instead
of disabling the entire PointerInputModifierNode and removing it from
composition.

This change is necessary because Compose does not really support the
removal of a PointerInputModifierNode during a gesture (see b/389906055
for details).

Bug: 389906055
Test: atest NestedDraggableTest
Test: atest ContentTest
Flag: com.android.systemui.scene_container
Change-Id: I6b0bca8f6915c8f7acf67ff81241ae36daf30dd2
parent 6459fc72

packages/SystemUI/compose/core/src/com/android/compose/gesture/NestedDraggable.kt

+33 −16
Original line number Diff line number Diff line
@@ -137,9 +137,18 @@ fun Modifier.nestedDraggable( 
    orientation: Orientation,

    overscrollEffect: OverscrollEffect? = null,

    enabled: Boolean = true,

    nestedDragsEnabled: Boolean = true,

): Modifier {

    return this.thenIf(overscrollEffect != null) { Modifier.overscroll(overscrollEffect) }

        .then(NestedDraggableElement(draggable, orientation, overscrollEffect, enabled))

        .then(

            NestedDraggableElement(

                draggable,

                orientation,

                overscrollEffect,

                enabled,

                nestedDragsEnabled,

            )

        )

}



private data class NestedDraggableElement(
@@ -147,13 +156,20 @@ private data class NestedDraggableElement( 
    private val orientation: Orientation,

    private val overscrollEffect: OverscrollEffect?,

    private val enabled: Boolean,

    private val nestedDragsEnabled: Boolean,

) : ModifierNodeElement<NestedDraggableRootNode>() {

    override fun create(): NestedDraggableRootNode {

        return NestedDraggableRootNode(draggable, orientation, overscrollEffect, enabled)

        return NestedDraggableRootNode(

            draggable,

            orientation,

            overscrollEffect,

            enabled,

            nestedDragsEnabled,

        )

    }



    override fun update(node: NestedDraggableRootNode) {

        node.update(draggable, orientation, overscrollEffect, enabled)

        node.update(draggable, orientation, overscrollEffect, enabled, nestedDragsEnabled)

    }

}
@@ -166,15 +182,17 @@ private class NestedDraggableRootNode( 
    orientation: Orientation,

    overscrollEffect: OverscrollEffect?,

    enabled: Boolean,

    nestedDragsEnabled: Boolean,

) : DelegatingNode() {

    private var delegateNode =

        if (enabled) create(draggable, orientation, overscrollEffect) else null

        if (enabled) create(draggable, orientation, overscrollEffect, nestedDragsEnabled) else null



    fun update(

        draggable: NestedDraggable,

        orientation: Orientation,

        overscrollEffect: OverscrollEffect?,

        enabled: Boolean,

        nestedDragsEnabled: Boolean,

    ) {

        // Disabled.

        if (!enabled) {
@@ -186,20 +204,23 @@ private class NestedDraggableRootNode( 
        // Disabled => Enabled.

        val nullableDelegate = delegateNode

        if (nullableDelegate == null) {

            delegateNode = create(draggable, orientation, overscrollEffect)

            delegateNode = create(draggable, orientation, overscrollEffect, nestedDragsEnabled)

            return

        }



        // Enabled => Enabled (update).

        nullableDelegate.update(draggable, orientation, overscrollEffect)

        nullableDelegate.update(draggable, orientation, overscrollEffect, nestedDragsEnabled)

    }



    private fun create(

        draggable: NestedDraggable,

        orientation: Orientation,

        overscrollEffect: OverscrollEffect?,

        nestedDragsEnabled: Boolean,

    ): NestedDraggableNode {

        return delegate(NestedDraggableNode(draggable, orientation, overscrollEffect))

        return delegate(

            NestedDraggableNode(draggable, orientation, overscrollEffect, nestedDragsEnabled)

        )

    }

}
@@ -207,6 +228,7 @@ private class NestedDraggableNode( 
    private var draggable: NestedDraggable,

    override var orientation: Orientation,

    private var overscrollEffect: OverscrollEffect?,

    private var nestedDragsEnabled: Boolean,

) :

    DelegatingNode(),

    PointerInputModifierNode,
@@ -247,18 +269,12 @@ private class NestedDraggableNode( 
        draggable: NestedDraggable,

        orientation: Orientation,

        overscrollEffect: OverscrollEffect?,

        nestedDragsEnabled: Boolean,

    ) {

        if (

            draggable == this.draggable &&

                orientation == this.orientation &&

                overscrollEffect == this.overscrollEffect

        ) {

            return

        }



        this.draggable = draggable

        this.orientation = orientation

        this.overscrollEffect = overscrollEffect

        this.nestedDragsEnabled = nestedDragsEnabled



        trackWheelScroll.resetPointerInputHandler()

        trackDownPositionDelegate.resetPointerInputHandler()
@@ -545,6 +561,7 @@ private class NestedDraggableNode( 


        val sign = offset.sign

        if (

            nestedDragsEnabled &&

                nestedScrollController == null &&

                // TODO(b/388231324): Remove this.

                !lastEventWasScrollWheel &&

packages/SystemUI/compose/core/tests/src/com/android/compose/gesture/NestedDraggableTest.kt

+21 −0
Original line number Diff line number Diff line
@@ -172,6 +172,27 @@ class NestedDraggableTest(override val orientation: Orientation) : OrientationAw 
        assertThat(effect.applyToFlingDone).isTrue()

    }



    @Test

    fun nestedScrollable_disabled() {

        val draggable = TestDraggable()

        val effect = TestOverscrollEffect(orientation) { 0f }

        val touchSlop =

            rule.setContentWithTouchSlop {

                Box(

                    Modifier.fillMaxSize()

                        .nestedDraggable(draggable, orientation, effect, nestedDragsEnabled = false)

                        .nestedScrollable(rememberScrollState())

                )

            }



        rule.onRoot().performTouchInput {

            down(center)

            moveBy((-touchSlop - 10f).toOffset())

        }



        assertThat(draggable.onDragStartedCalled).isFalse()

    }



    @Test

    fun onDragStoppedIsCalledWhenDraggableIsUpdatedAndReset() {

        val draggable = TestDraggable()

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

+0 −4
Original line number Diff line number Diff line
@@ -458,10 +458,6 @@ internal class Swipes(val upOrLeft: Swipe.Resolved, val downOrRight: Swipe.Resol 
     * @return The best matching [UserActionResult], or `null` if no match is found.

     */

    private fun Content.findActionResultBestMatch(swipe: Swipe.Resolved): UserActionResult? {

        if (!areSwipesAllowed()) {

            return null

        }



        var bestPoints = Int.MIN_VALUE

        var bestMatch: UserActionResult? = null

        userActions.forEach { (actionSwipe, actionResult) ->

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

+7 −4
Original line number Diff line number Diff line
@@ -27,16 +27,19 @@ import com.android.compose.gesture.nestedDraggable 
 */

@Stable

internal fun Modifier.swipeToScene(draggableHandler: DraggableHandler): Modifier {

    val contentForSwipes = draggableHandler.contentForSwipes()

    val enabled = draggableHandler.enabled(contentForSwipes)

    return this.nestedDraggable(

        draggable = draggableHandler,

        orientation = draggableHandler.orientation,

        overscrollEffect = draggableHandler.overscrollEffect,

        enabled = draggableHandler.enabled(),

        enabled = enabled,

        nestedDragsEnabled = enabled && contentForSwipes.areNestedSwipesAllowed(),

    )

}



internal fun DraggableHandler.enabled(): Boolean {

    return isDrivingTransition || contentForSwipes().shouldEnableSwipes(orientation)

internal fun DraggableHandler.enabled(contentForSwipes: Content = contentForSwipes()): Boolean {

    return isDrivingTransition || contentForSwipes.shouldEnableSwipes(orientation)

}



private fun DraggableHandler.contentForSwipes(): Content {
@@ -45,7 +48,7 @@ private fun DraggableHandler.contentForSwipes(): Content { 


/** Whether swipe should be enabled in the given [orientation]. */

private fun Content.shouldEnableSwipes(orientation: Orientation): Boolean {

    if (userActions.isEmpty() || !areSwipesAllowed()) {

    if (userActions.isEmpty()) {

        return false

    }

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/content/Content.kt

+1 −1
Original line number Diff line number Diff line
@@ -179,7 +179,7 @@ internal sealed class Content( 
        }

    }



    fun areSwipesAllowed(): Boolean = nestedScrollControlState.isOuterScrollAllowed

    fun areNestedSwipesAllowed(): Boolean = nestedScrollControlState.isOuterScrollAllowed



    fun maybeUpdateEffects(effectFactory: OverscrollFactory) {

        if (effectFactory != lastFactory) {