onDragStart called after onDrag [187320697]

Fixed

Bug

Status Update

No update yet.

Description

ha...@google.com

created issue #1

May 5, 2021 07:01PM

Steps to reproduce: Run the following code:

Box(
    modifier = Modifier.background(Color.Cyan).pointerInput(Unit) {
        detectDragGestures(
            onDragStart = {
                dragStarted = true
                Log.e("debugTag", "onDragStart")
            },
            onDragEnd = {
                dragStarted = false
                Log.e("debugTag", "onDragEnd")
            },
            onDragCancel = {
                dragStarted = false
                Log.e("debugTag", "onDragCancel")
            },
        ) { change, dragAmount ->
            change.consumeAllChanges()
            if (!dragStarted) {
                Log.e("debugTag", "onDrag called before onStart")
            } else {
                Log.e("debugTag", "onDrag")
            }
        }
    }
)

Drag anywhere in the Box.
Check the log, we can find that onDrag is always called once before onDragStart get called.

Example:

2021-05-05 11:59:43.653 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag called before onStart
2021-05-05 11:59:43.653 19756-19756/androidx.compose.integration.demos E/debugTag: onDragStart
2021-05-05 11:59:43.669 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.686 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.703 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.719 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.736 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.748 19756-19756/androidx.compose.integration.demos E/debugTag: onDrag
2021-05-05 11:59:43.749 19756-19756/androidx.compose.integration.demos E/debugTag: onDragEnd

Comments

an...@google.com <an...@google.com> May 20, 2021 11:26AM

Assigned to ma...@google.com.

ap...@google.com <ap...@google.com> #2May 20, 2021 12:18PM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ch...@gmail.com <ch...@gmail.com> #3Jun 11, 2021 02:37PM

I have been trying to implement zoomable composable in a scrollable layout like LazyColumn. When the zoomable composable is zoomed, user can drag to pan. But once the panning reaches the edge of the content, the subsequent pan will scroll the LazyColumn. Here's my code that was working prior to beta08:

@ExperimentalFoundationApi
@Composable
fun ZoomableComposable(modifier: Modifier = Modifier, content: @Composable () -> Unit) {
    val width = remember { mutableStateOf(0f) }
    val height = remember { mutableStateOf(0f) }
    val offsetX = remember { mutableStateOf(0f) }
    val offsetY = remember { mutableStateOf(0f) }
    val scale = remember { mutableStateOf(1f) }
    val state = rememberTransformableState { zoomChange, offsetChange, rotationChange ->
        scale.value *= zoomChange
        scale.value = maxOf(1f, minOf(3f, scale.value))
        if (scale.value == 1f) {
            offsetX.value = 0f
            offsetY.value = 0f
        }
    }
    Box(
        modifier = modifier
            .clip(RectangleShape)
            .onGloballyPositioned { coordinates ->
                // This will be the size of the Box.
                width.value = coordinates.size.width.toFloat()
                height.value = coordinates.size.height.toFloat()
            }
            .pointerInput(Unit) {
                detectDragGestures { change, dragAmount ->
                    Log.d("detectDragGestures", "scale: ${scale.value}")
                    offsetX.value += dragAmount.x
                    offsetY.value += dragAmount.y

                    var consumeChange = true
                    if (offsetX.value <= width.value * (1f - scale.value) / 2f) {
                        offsetX.value = width.value * (1f - scale.value) / 2f
                        consumeChange = false
                    } else if (offsetX.value >= width.value * (scale.value - 1f) / 2f) {
                        offsetX.value = width.value * (scale.value - 1f) / 2f
                        consumeChange = false
                    }

                    if (offsetY.value <= height.value * (1f - scale.value) / 2f) {
                        offsetY.value = height.value * (1f - scale.value) / 2f
                        consumeChange = false
                    } else if (offsetY.value >= height.value * (scale.value - 1f) / 2f) {
                        offsetY.value = height.value * (scale.value - 1f) / 2f
                        consumeChange = false
                    }

                    if (consumeChange) {
                        change.consumeAllChanges()
                    }
                }
            }
            .graphicsLayer(
                scaleX = scale.value,
                scaleY = scale.value,
                translationX = offsetX.value,
                translationY = offsetY.value
            )
            .transformable(state = state)
    ) {
        content()
    }
}

I would like to know how I can disable auto consume of the changes. Thank you.

Message last modified on Jun 12, 2021 03:06AM

ma...@google.com <ma...@google.com> #4Jun 11, 2021 02:46PM

There's no way to not to consume pointer input events in the detectDragGestures since beta08. If you need to not to consume changes, you can use lower level blocks such as awaitTouchSlop or drag().

ch...@gmail.com <ch...@gmail.com> #5Jun 12, 2021 02:40AM

Thank you for your reply. Is there any documentation or sample that I can refer to?

ma...@google.com <ma...@google.com> #6Jun 14, 2021 10:49AM

Sure, I think the implementation of detectDragGestures, which is ~25 LOC might be a good start: https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/DragGestureDetector.kt;l=195?q=DragGestureDetector

Issue 187320697