Reapply "Don't install a pointer input when there are no user actions"
This is a reland of ag/29669127, which was broken (and reverted in
ag/29690066) because SwipeToSceneNode was not properly handling the
update of its draggableHandler.
This reland fixes the issue by introducing a SwipeToSceneRootNode, which
recreates a new delegate node whenever draggableHandler has changed.
This approach was suggested by the Compose team in
http://shortn/_MUgPp4TRmY. A test covering this case was added in
SwipeToSceneTest.swipeToSceneSupportsUpdates().
Bug: 370913106
Test: atest SwipeToSceneTest
Flag: com.android.systemui.scene_container
Change-Id: Ifc1b88a813b6de153028db64aac6cd1103c94a66
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
index dc3135d..aa70a0c 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/MultiPointerDraggable.kt
@@ -42,10 +42,8 @@
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
@@ -79,7 +77,6 @@
@Stable
internal fun Modifier.multiPointerDraggable(
orientation: Orientation,
- enabled: () -> Boolean,
startDragImmediately: (startedPosition: Offset) -> Boolean,
onDragStarted: (startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,
onFirstPointerDown: () -> Unit = {},
@@ -89,7 +86,6 @@
this.then(
MultiPointerDraggableElement(
orientation,
- enabled,
startDragImmediately,
onDragStarted,
onFirstPointerDown,
@@ -100,7 +96,6 @@
private data class MultiPointerDraggableElement(
private val orientation: Orientation,
- private val enabled: () -> Boolean,
private val startDragImmediately: (startedPosition: Offset) -> Boolean,
private val onDragStarted:
(startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,
@@ -111,7 +106,6 @@
override fun create(): MultiPointerDraggableNode =
MultiPointerDraggableNode(
orientation = orientation,
- enabled = enabled,
startDragImmediately = startDragImmediately,
onDragStarted = onDragStarted,
onFirstPointerDown = onFirstPointerDown,
@@ -121,7 +115,6 @@
override fun update(node: MultiPointerDraggableNode) {
node.orientation = orientation
- node.enabled = enabled
node.startDragImmediately = startDragImmediately
node.onDragStarted = onDragStarted
node.onFirstPointerDown = onFirstPointerDown
@@ -131,27 +124,23 @@
internal class MultiPointerDraggableNode(
orientation: Orientation,
- enabled: () -> Boolean,
var startDragImmediately: (startedPosition: Offset) -> Boolean,
var onDragStarted:
(startedPosition: Offset, overSlop: Float, pointersDown: Int) -> DragController,
var onFirstPointerDown: () -> Unit,
- var swipeDetector: SwipeDetector = DefaultSwipeDetector,
+ swipeDetector: SwipeDetector = DefaultSwipeDetector,
private val dispatcher: NestedScrollDispatcher,
) :
DelegatingNode(),
PointerInputModifierNode,
CompositionLocalConsumerModifierNode,
- ObserverModifierNode,
SpaceVectorConverter {
private val pointerTracker = delegate(SuspendingPointerInputModifierNode { pointerTracker() })
private val pointerInput = delegate(SuspendingPointerInputModifierNode { pointerInput() })
private val velocityTracker = VelocityTracker()
- private var previousEnabled: Boolean = false
- var enabled: () -> Boolean = enabled
+ var swipeDetector: SwipeDetector = swipeDetector
set(value) {
- // Reset the pointer input whenever enabled changed.
if (value != field) {
field = value
pointerInput.resetPointerInputHandler()
@@ -178,21 +167,6 @@
}
}
- override fun onAttach() {
- previousEnabled = enabled()
- onObservedReadsChanged()
- }
-
- override fun onObservedReadsChanged() {
- observeReads {
- val newEnabled = enabled()
- if (newEnabled != previousEnabled) {
- pointerInput.resetPointerInputHandler()
- }
- previousEnabled = newEnabled
- }
- }
-
override fun onCancelPointerInput() {
pointerTracker.onCancelPointerInput()
pointerInput.onCancelPointerInput()
@@ -254,9 +228,7 @@
velocityTracker.resetTracking()
velocityTracker.addPointerInputChange(firstPointerDown)
startedPosition = firstPointerDown.position
- if (enabled()) {
- onFirstPointerDown()
- }
+ onFirstPointerDown()
}
// Changes with at least one pointer
@@ -295,10 +267,6 @@
}
private suspend fun PointerInputScope.pointerInput() {
- if (!enabled()) {
- return
- }
-
val currentContext = currentCoroutineContext()
awaitPointerEventScope {
while (currentContext.isActive) {
diff --git a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
index 98d4aaa..061583a 100644
--- a/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
+++ b/packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SwipeToScene.kt
@@ -41,22 +41,62 @@
draggableHandler: DraggableHandlerImpl,
swipeDetector: SwipeDetector,
): Modifier {
- return this.then(SwipeToSceneElement(draggableHandler, swipeDetector))
+ return if (draggableHandler.enabled()) {
+ this.then(SwipeToSceneElement(draggableHandler, swipeDetector))
+ } else {
+ this
+ }
+}
+
+private fun DraggableHandlerImpl.enabled(): Boolean {
+ return isDrivingTransition || contentForSwipes().shouldEnableSwipes(orientation)
+}
+
+private fun DraggableHandlerImpl.contentForSwipes(): Content {
+ return layoutImpl.contentForUserActions()
+}
+
+/** Whether swipe should be enabled in the given [orientation]. */
+private fun Content.shouldEnableSwipes(orientation: Orientation): Boolean {
+ if (userActions.isEmpty()) {
+ return false
+ }
+
+ return userActions.keys.any { it is Swipe.Resolved && it.direction.orientation == orientation }
}
private data class SwipeToSceneElement(
val draggableHandler: DraggableHandlerImpl,
val swipeDetector: SwipeDetector,
-) : ModifierNodeElement<SwipeToSceneNode>() {
- override fun create(): SwipeToSceneNode = SwipeToSceneNode(draggableHandler, swipeDetector)
+) : ModifierNodeElement<SwipeToSceneRootNode>() {
+ override fun create(): SwipeToSceneRootNode =
+ SwipeToSceneRootNode(draggableHandler, swipeDetector)
- override fun update(node: SwipeToSceneNode) {
- node.draggableHandler = draggableHandler
+ override fun update(node: SwipeToSceneRootNode) {
+ node.update(draggableHandler, swipeDetector)
+ }
+}
+
+private class SwipeToSceneRootNode(
+ draggableHandler: DraggableHandlerImpl,
+ swipeDetector: SwipeDetector,
+) : DelegatingNode() {
+ private var delegate = delegate(SwipeToSceneNode(draggableHandler, swipeDetector))
+
+ fun update(draggableHandler: DraggableHandlerImpl, swipeDetector: SwipeDetector) {
+ if (draggableHandler == delegate.draggableHandler) {
+ // Simple update, just update the swipe detector directly and keep the node.
+ delegate.swipeDetector = swipeDetector
+ } else {
+ // The draggableHandler changed, force recreate the underlying SwipeToSceneNode.
+ undelegate(delegate)
+ delegate = delegate(SwipeToSceneNode(draggableHandler, swipeDetector))
+ }
}
}
private class SwipeToSceneNode(
- draggableHandler: DraggableHandlerImpl,
+ val draggableHandler: DraggableHandlerImpl,
swipeDetector: SwipeDetector,
) : DelegatingNode(), PointerInputModifierNode {
private val dispatcher = NestedScrollDispatcher()
@@ -64,7 +104,6 @@
delegate(
MultiPointerDraggableNode(
orientation = draggableHandler.orientation,
- enabled = ::enabled,
startDragImmediately = ::startDragImmediately,
onDragStarted = draggableHandler::onDragStarted,
onFirstPointerDown = ::onFirstPointerDown,
@@ -73,18 +112,10 @@
)
)
- private var _draggableHandler = draggableHandler
- var draggableHandler: DraggableHandlerImpl
- get() = _draggableHandler
+ var swipeDetector: SwipeDetector
+ get() = multiPointerDraggableNode.swipeDetector
set(value) {
- if (_draggableHandler != value) {
- _draggableHandler = value
-
- // Make sure to update the delegate orientation. Note that this will automatically
- // reset the underlying pointer input handler, so previous gestures will be
- // cancelled.
- multiPointerDraggableNode.orientation = value.orientation
- }
+ multiPointerDraggableNode.swipeDetector = value
}
private val nestedScrollHandlerImpl =
@@ -124,22 +155,6 @@
override fun onCancelPointerInput() = multiPointerDraggableNode.onCancelPointerInput()
- private fun enabled(): Boolean {
- return draggableHandler.isDrivingTransition ||
- contentForSwipes().shouldEnableSwipes(multiPointerDraggableNode.orientation)
- }
-
- private fun contentForSwipes(): Content {
- return draggableHandler.layoutImpl.contentForUserActions()
- }
-
- /** Whether swipe should be enabled in the given [orientation]. */
- private fun Content.shouldEnableSwipes(orientation: Orientation): Boolean {
- return userActions.keys.any {
- it is Swipe.Resolved && it.direction.orientation == orientation
- }
- }
-
private fun startDragImmediately(startedPosition: Offset): Boolean {
// Immediately start the drag if the user can't swipe in the other direction and the gesture
// handler can intercept it.
@@ -152,7 +167,7 @@
Orientation.Vertical -> Orientation.Horizontal
Orientation.Horizontal -> Orientation.Vertical
}
- return contentForSwipes().shouldEnableSwipes(oppositeOrientation)
+ return draggableHandler.contentForSwipes().shouldEnableSwipes(oppositeOrientation)
}
}
diff --git a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt
index 493f3a1..c8f6e6d 100644
--- a/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt
+++ b/packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/MultiPointerDraggableTest.kt
@@ -45,6 +45,7 @@
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Velocity
import androidx.test.ext.junit.runners.AndroidJUnit4
+import com.android.compose.modifiers.thenIf
import com.android.compose.nestedscroll.SuspendedValue
import com.google.common.truth.Truth.assertThat
import kotlin.properties.Delegates
@@ -94,19 +95,20 @@
Box(
Modifier.size(with(LocalDensity.current) { Size(size, size).toDpSize() })
.nestedScrollDispatcher()
- .multiPointerDraggable(
- orientation = Orientation.Vertical,
- enabled = { enabled },
- startDragImmediately = { false },
- onDragStarted = { _, _, _ ->
- started = true
- SimpleDragController(
- onDrag = { dragged = true },
- onStop = { stopped = true },
- )
- },
- dispatcher = defaultDispatcher,
- )
+ .thenIf(enabled) {
+ Modifier.multiPointerDraggable(
+ orientation = Orientation.Vertical,
+ startDragImmediately = { false },
+ onDragStarted = { _, _, _ ->
+ started = true
+ SimpleDragController(
+ onDrag = { dragged = true },
+ onStop = { stopped = true },
+ )
+ },
+ dispatcher = defaultDispatcher,
+ )
+ }
)
}
@@ -164,7 +166,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
// We want to start a drag gesture immediately
startDragImmediately = { true },
onDragStarted = { _, _, _ ->
@@ -238,7 +239,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
started = true
@@ -358,7 +358,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
started = true
@@ -464,7 +463,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
verticalStarted = true
@@ -477,7 +475,6 @@
)
.multiPointerDraggable(
orientation = Orientation.Horizontal,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
horizontalStarted = true
@@ -570,7 +567,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
swipeDetector =
object : SwipeDetector {
@@ -672,7 +668,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
SimpleDragController(
@@ -744,7 +739,6 @@
.nestedScrollDispatcher()
.multiPointerDraggable(
orientation = Orientation.Vertical,
- enabled = { true },
startDragImmediately = { false },
onDragStarted = { _, _, _ ->
SimpleDragController(
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 25e8713..28d0a47 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
@@ -22,11 +22,15 @@
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.offset
import androidx.compose.foundation.layout.size
+import androidx.compose.material3.Button
+import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.CompositionLocalProvider
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
+import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
+import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.input.nestedscroll.NestedScrollConnection
@@ -36,9 +40,14 @@
import androidx.compose.ui.platform.LocalViewConfiguration
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertPositionInRootIsEqualTo
+import androidx.compose.ui.test.assertTextEquals
import androidx.compose.ui.test.junit4.createComposeRule
+import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onRoot
+import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTouchInput
+import androidx.compose.ui.test.swipeRight
+import androidx.compose.ui.test.swipeUp
import androidx.compose.ui.test.swipeWithVelocity
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.IntSize
@@ -844,4 +853,62 @@
assertThat(transition.progress).isEqualTo(1f)
assertThat(availableOnPostScroll).isEqualTo(ovescrollPx)
}
+
+ @Test
+ fun sceneWithoutSwipesDoesNotConsumeGestures() {
+ val buttonTag = "button"
+
+ rule.setContent {
+ Box {
+ var count by remember { mutableStateOf(0) }
+ Button(onClick = { count++ }, Modifier.testTag(buttonTag).align(Alignment.Center)) {
+ Text("Count: $count")
+ }
+
+ SceneTransitionLayout(remember { MutableSceneTransitionLayoutState(SceneA) }) {
+ scene(SceneA) { Box(Modifier.fillMaxSize()) }
+ }
+ }
+ }
+
+ rule.onNodeWithTag(buttonTag).assertTextEquals("Count: 0")
+
+ // Click on the root at its center, where the button is located. Clicks should go through
+ // the STL and reach the button given that there is no swipes for the current scene.
+ repeat(3) { rule.onRoot().performClick() }
+ rule.onNodeWithTag(buttonTag).assertTextEquals("Count: 3")
+ }
+
+ @Test
+ fun swipeToSceneSupportsUpdates() {
+ val state = rule.runOnUiThread { MutableSceneTransitionLayoutState(SceneA) }
+
+ rule.setContent {
+ SceneTransitionLayout(state) {
+ // SceneA only has vertical actions, so only one vertical Modifier.swipeToScene()
+ // is composed.
+ scene(SceneA, mapOf(Swipe.Up to SceneB)) { Box(Modifier.fillMaxSize()) }
+
+ // SceneB only has horizontal actions, so only one vertical Modifier.swipeToScene()
+ // is composed, which will be force update it with a new draggableHandler.
+ scene(SceneB, mapOf(Swipe.Right to SceneC)) { Box(Modifier.fillMaxSize()) }
+ scene(SceneC) { Box(Modifier.fillMaxSize()) }
+ }
+ }
+
+ assertThat(state.transitionState).isIdle()
+ assertThat(state.transitionState).hasCurrentScene(SceneA)
+
+ // Swipe up to scene B.
+ rule.onRoot().performTouchInput { swipeUp() }
+ rule.waitForIdle()
+ assertThat(state.transitionState).isIdle()
+ assertThat(state.transitionState).hasCurrentScene(SceneB)
+
+ // Swipe right to scene C.
+ rule.onRoot().performTouchInput { swipeRight() }
+ rule.waitForIdle()
+ assertThat(state.transitionState).isIdle()
+ assertThat(state.transitionState).hasCurrentScene(SceneC)
+ }
}