Skip to content

Commit

Permalink
isPressed
Browse files Browse the repository at this point in the history
Fix crash when we press right mouse button during dragging with left button

Fixes JetBrains/compose-multiplatform#1426
Fixes JetBrains/compose-multiplatform#1176

Fixes JetBrains/compose-multiplatform#1430

Get rid of mousePressed from ComposeScene

mousePressed is unreliable on Windows (we can miss the release event), and doesn't work well with multiple buttons.

After this fix, mouseClickable only reacts to the first pressed button. Right button click doesn't trigger callback, if there is already left mouse button is pressed.

`Clickable` shouldn't be able to handle these cases. If users would want simultaneously handle multiple buttons, they have to use low-level api:
```
Modifier.pointerInput(Unit) {
    while (true) {
        val event = awaitPointerEventScope { awaitPointerEvent() }

        if (event.type == PointerEventType.Press && event.buttons.isPrimaryPressed) {
            // do something
        } else if (event.type == PointerEventType.Press && event.buttons.isSecondaryPressed) {
            // do something
        }
    }
}
```
(it is verbose, there is a field for improvement)

Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
Test: the snippet from JetBrains/compose-multiplatform#1149, because changed the code for that fix

Remove pointerId from ComposeScene

pointerId was indroduced in https://android-review.googlesource.com/c/platform/frameworks/support/+/1402607, because double click didn't work (see https://jetbrains.slack.com/archives/GT449QBCK/p1597328095373000)

But it messes with hover and clicking multiple mouse buttons at the same time.

Double clicking still works after removing it:
```
import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.foundation.background
import androidx.compose.foundation.combinedClickable
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.size
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.unit.dp
import androidx.compose.ui.window.singleWindowApplication

@OptIn(ExperimentalFoundationApi::class)
fun main() = singleWindowApplication {
    Box(
        Modifier
        .size(300.dp)
        .background(Color.Red)
            .combinedClickable(onDoubleClick = {
                println("onDoubleClick")
            }, onClick = {
                println("onClick")
            })
    ) {
    }
}
```

Fixes JetBrains/compose-multiplatform#1176

Test: ./gradlew jvmTest desktopTest -Pandroidx.compose.multiplatformEnabled=true
Test: manual (see the snippet)
  • Loading branch information
igordmn committed Jan 18, 2022
1 parent b7b32ae commit 0feb3da
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import androidx.compose.ui.input.pointer.PointerInputEvent
import androidx.compose.ui.input.pointer.PointerInputEventData
import androidx.compose.ui.input.pointer.PointerKeyboardModifiers
import androidx.compose.ui.input.pointer.PointerType
import androidx.compose.ui.input.pointer.areAnyPressed
import androidx.compose.ui.platform.AccessibilityController
import androidx.compose.ui.platform.AccessibilityControllerImpl
import androidx.compose.ui.platform.PlatformComponent
Expand Down Expand Up @@ -52,7 +53,6 @@ internal actual fun pointerInputEvent(
timeMillis: Long,
nativeEvent: Any?,
type: PointerType,
isMousePressed: Boolean,
pointerId: Long,
scrollDelta: Offset,
buttons: PointerButtons,
Expand All @@ -67,7 +67,7 @@ internal actual fun pointerInputEvent(
timeMillis,
position,
position,
isMousePressed,
buttons.areAnyPressed,
type,
scrollDelta = scrollDelta
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,6 @@ class ComposeScene internal constructor(

private val defaultPointerStateTracker = DefaultPointerStateTracker()

private var pointerId = 0L
private var isMousePressed = false

private val job = Job()
private val coroutineScope = CoroutineScope(coroutineContext + job)
// We use FlushCoroutineDispatcher for effectDispatcher not because we need `flush` for
Expand Down Expand Up @@ -390,22 +387,18 @@ class ComposeScene internal constructor(
val actualButtons = buttons ?: defaultPointerStateTracker.buttons
val actualKeyboardModifiers = keyboardModifiers ?: defaultPointerStateTracker.keyboardModifiers

when (eventType) {
PointerEventType.Press -> isMousePressed = true
PointerEventType.Release -> isMousePressed = false
}
val event = pointerInputEvent(
eventType,
position,
timeMillis,
nativeEvent,
type,
isMousePressed,
pointerId,
pointerId = 0,
scrollDelta,
actualButtons,
actualKeyboardModifiers
)

when (eventType) {
PointerEventType.Press -> onMousePressed(event)
PointerEventType.Release -> onMouseReleased(event)
Expand Down Expand Up @@ -435,7 +428,6 @@ class ComposeScene internal constructor(
private fun onMouseReleased(event: PointerInputEvent) {
val owner = hoveredOwner ?: focusedOwner
owner?.processPointerInput(event)
pointerId += 1
}

private var pointLocation = Offset.Zero
Expand Down Expand Up @@ -474,7 +466,6 @@ internal expect fun pointerInputEvent(
timeMillis: Long,
nativeEvent: Any?,
type: PointerType,
isMousePressed: Boolean,
pointerId: Long,
scrollDelta: Offset,
buttons: PointerButtons,
Expand Down

0 comments on commit 0feb3da

Please sign in to comment.