Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore button filter for dialog closing #1336

Merged
merged 2 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -714,11 +714,11 @@ private fun ComposeScene.onMouseEvent(
buttons = event.buttons,
keyboardModifiers = event.keyboardModifiers,
nativeEvent = event,
button = event.getPointerButton()
button = event.composePointerButton
)
}

private fun MouseEvent.getPointerButton(): PointerButton? {
internal val MouseEvent.composePointerButton: PointerButton? get() {
if (button == MouseEvent.NOBUTTON) return null
return when (button) {
MouseEvent.BUTTON2 -> PointerButton.Tertiary
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import androidx.compose.ui.awt.AwtEventFilter
import androidx.compose.ui.awt.AwtEventListener
import androidx.compose.ui.awt.AwtEventListeners
import androidx.compose.ui.awt.toAwtRectangle
import androidx.compose.ui.input.pointer.PointerButton
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.skiko.RecordDrawRectRenderDecorator
import androidx.compose.ui.unit.Density
Expand Down Expand Up @@ -73,7 +74,10 @@ internal abstract class DesktopComposeSceneLayer(
*/
private var maxDrawInflate = IntRect.Zero

private var outsidePointerCallback: ((eventType: PointerEventType) -> Unit)? = null
private var outsidePointerCallback: ((
eventType: PointerEventType,
button: PointerButton?
) -> Unit)? = null
Comment on lines +77 to +80
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worthwhile to typealias it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or some event type with reduced scope as parameter. But the problem here - it should be public

private var isClosed = false

final override var density: Density = density
Expand Down Expand Up @@ -119,7 +123,7 @@ internal abstract class DesktopComposeSceneLayer(
}

final override fun setOutsidePointerEventListener(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)?
onOutsidePointerEvent: ((eventType: PointerEventType, button: PointerButton?) -> Unit)?
) {
outsidePointerCallback = onOutsidePointerEvent
}
Expand Down Expand Up @@ -196,7 +200,7 @@ internal abstract class DesktopComposeSceneLayer(
MouseEvent.MOUSE_RELEASED -> PointerEventType.Release
else -> return
}
outsidePointerCallback?.invoke(eventType)
outsidePointerCallback?.invoke(eventType, event.composePointerButton)
}

private fun inBounds(event: MouseEvent): Boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.compose.runtime.rememberUpdatedState
import androidx.compose.ui.InternalComposeUiApi
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.pointer.PointerButton
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.node.LayoutNode
import androidx.compose.ui.platform.LocalDensity
Expand Down Expand Up @@ -145,7 +146,10 @@ interface ComposeSceneLayer {
* gesture that started outside of [boundsInWindow].
*/
fun setOutsidePointerEventListener(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
onOutsidePointerEvent: ((
eventType: PointerEventType,
button: PointerButton?
) -> Unit)? = null,
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,10 @@ private class MultiLayerComposeSceneImpl(
inputHandler = inputHandler,
)
private var composition: Composition? = null
private var outsidePointerCallback: ((eventType: PointerEventType) -> Unit)? = null
private var outsidePointerCallback: ((
eventType: PointerEventType,
button: PointerButton?
) -> Unit)? = null
private var isClosed = false

override var density: Density by owner::density
Expand Down Expand Up @@ -577,7 +580,7 @@ private class MultiLayerComposeSceneImpl(
}

override fun setOutsidePointerEventListener(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)?,
onOutsidePointerEvent: ((eventType: PointerEventType, button: PointerButton?) -> Unit)?,
) {
outsidePointerCallback = onOutsidePointerEvent
}
Expand Down Expand Up @@ -607,17 +610,17 @@ private class MultiLayerComposeSceneImpl(
fun isInBounds(position: Offset) = boundsInWindow.contains(position.round())

fun onOutsidePointerEvent(event: PointerInputEvent) {
if (!event.isMainAction()) {
if (!event.isMouseOrSingleTouch()) {
return
}
outsidePointerCallback?.invoke(event.eventType)
outsidePointerCallback?.invoke(event.eventType, event.button)
}
}
}

private val PointerInputEvent.isGestureInProgress get() = pointers.fastAny { it.down }

private fun PointerInputEvent.isMainAction() =
private fun PointerInputEvent.isMouseOrSingleTouch() =
button != null || pointers.size == 1

private class CopiedList<T>(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.key.KeyEventType
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.type
import androidx.compose.ui.input.pointer.PointerButton
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.layout.Layout
import androidx.compose.ui.platform.LocalWindowInfo
Expand Down Expand Up @@ -159,8 +160,13 @@ actual fun Dialog(
null
}
val onOutsidePointerEvent = if (properties.dismissOnClickOutside) {
{ eventType: PointerEventType ->
if (eventType == PointerEventType.Release) {
{ eventType: PointerEventType, button: PointerButton? ->
// Clicking outside dialog is clicking on scrim.
// So this behavior should match regular clicks or [detectTapGestures] that accepts
// only primary mouse button clicks.
if (eventType == PointerEventType.Release &&
(button == null || button == PointerButton.Primary)
) {
currentOnDismissRequest()
}
}
Expand All @@ -182,7 +188,7 @@ private fun DialogLayout(
modifier: Modifier = Modifier,
onPreviewKeyEvent: ((KeyEvent) -> Boolean)? = null,
onKeyEvent: ((KeyEvent) -> Boolean)? = null,
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
onOutsidePointerEvent: ((eventType: PointerEventType, button: PointerButton?) -> Unit)? = null,
content: @Composable () -> Unit
) {
val currentContent by rememberUpdatedState(content)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.key.KeyEventType
import androidx.compose.ui.input.key.key
import androidx.compose.ui.input.key.type
import androidx.compose.ui.input.pointer.PointerButton
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.layout.EmptyLayout
import androidx.compose.ui.layout.Layout
Expand Down Expand Up @@ -410,7 +411,10 @@ fun Popup(
}
val onOutsidePointerEvent = if (properties.dismissOnClickOutside && onDismissRequest != null) {
// No need to remember this lambda, as it doesn't capture any values that can change.
{ eventType: PointerEventType ->
{ eventType: PointerEventType, _: PointerButton? ->
// Popup should react on first event - [PointerEventType.Press],
// but at the same time trigger [onDismissRequest] only once per click.
// Any mouse buttons should be accepted to match regular dropdown behavior.
if (eventType == PointerEventType.Press) {
currentOnDismissRequest?.invoke()
}
Expand All @@ -436,7 +440,7 @@ private fun PopupLayout(
modifier: Modifier,
onPreviewKeyEvent: ((KeyEvent) -> Boolean)? = null,
onKeyEvent: ((KeyEvent) -> Boolean)? = null,
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null,
onOutsidePointerEvent: ((eventType: PointerEventType, button: PointerButton?) -> Unit)? = null,
content: @Composable () -> Unit
) {
val currentContent by rememberUpdatedState(content)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,23 +197,18 @@ class DialogTest {
}

@Test
fun secondaryButtonClickDismissDialog() = runSkikoComposeUiTest(
fun secondaryButtonClickDoesNotDismissDialog() = runSkikoComposeUiTest(
size = Size(100f, 100f)
) {
val openDialog = mutableStateOf(true)
val background = FillBox()
val dialog = DialogState(
IntSize(40, 40),
onDismissRequest = {
openDialog.value = false
}
onDismissRequest = { fail() }
)

setContent {
background.Content()
if (openDialog.value) {
dialog.Content()
}
dialog.Content()
}

val buttons = PointerButtons(
Expand All @@ -230,8 +225,6 @@ class DialogTest {
position = Offset(10f, 10f),
button = PointerButton.Secondary
)

onNodeWithTag(dialog.tag).assertDoesNotExist()
}

@OptIn(InternalTestApi::class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import androidx.compose.runtime.CompositionContext
import androidx.compose.runtime.CompositionLocalContext
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.input.key.KeyEvent
import androidx.compose.ui.input.pointer.PointerButton
import androidx.compose.ui.input.pointer.PointerEventType
import androidx.compose.ui.interop.UIKitInteropContext
import androidx.compose.ui.platform.PlatformContext
Expand Down Expand Up @@ -66,7 +67,10 @@ internal class UIViewComposeSceneLayer(
) : ComposeSceneLayer {

override var focusable: Boolean = focusStack != null
private var onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)? = null
private var onOutsidePointerEvent: ((
eventType: PointerEventType,
button: PointerButton?
) -> Unit)? = null
private val rootView = composeContainer.view.window ?: composeContainer.view
private val backgroundView: UIView = object : UIView(
frame = CGRectZero.readValue()
Expand All @@ -77,7 +81,7 @@ internal class UIViewComposeSceneLayer(
if (previousSuccessHitTestTimestamp != withEvent?.timestamp) {
// This workaround needs to send PointerEventType.Press just once
previousSuccessHitTestTimestamp = withEvent?.timestamp
onOutsidePointerEvent?.invoke(PointerEventType.Press)
onOutsidePointerEvent?.invoke(PointerEventType.Press, null)
}
}

Expand All @@ -91,7 +95,8 @@ internal class UIViewComposeSceneLayer(
// This view's coordinate space is equal to [ComposeScene]'s
val contains = boundsInWindow.contains(locationInView.toOffset(density).round())
if (!contains) {
onOutsidePointerEvent?.invoke(PointerEventType.Release)
// TODO: Send only for last pointer in case of multi-touch
onOutsidePointerEvent?.invoke(PointerEventType.Release, null)
}
}
super.touchesEnded(touches, withEvent)
Expand Down Expand Up @@ -202,7 +207,7 @@ internal class UIViewComposeSceneLayer(
}

override fun setOutsidePointerEventListener(
onOutsidePointerEvent: ((eventType: PointerEventType) -> Unit)?
onOutsidePointerEvent: ((eventType: PointerEventType, button: PointerButton?) -> Unit)?
) {
this.onOutsidePointerEvent = onOutsidePointerEvent
}
Expand Down
Loading