Close Popup/Dialog by clicking any mouse button outside (#1280)

## Proposed Changes - Remove condition for primary mouse button to send onClickOutside event ## Testing Test: `PopupTest`/`DialogTest` ## Issues Fixed Fixes JetBrains/compose-multiplatform#4549
JetBrains · Apr 19, 2024 · b6061d3 · b6061d3
1 parent 9eb3960
commit b6061d3
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 14 deletions.
diff --git a/...i/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/DesktopComposeSceneLayer.desktop.kt b/...i/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/DesktopComposeSceneLayer.desktop.kt
@@ -188,7 +188,7 @@ internal abstract class DesktopComposeSceneLayer(
      * @param event the mouse event
      */
     fun onMouseEventOutside(event: MouseEvent) {
-        if (isClosed || !event.isMainAction() || inBounds(event)) {
+        if (isClosed) {
             return
         }
         val eventType = when (event.id) {
@@ -215,7 +215,9 @@ internal abstract class DesktopComposeSceneLayer(
     private inner class DetectEventOutsideLayer : AwtEventListener {
         override fun onMouseEvent(event: MouseEvent): Boolean {
             layersAbove.toList().fastForEachReversed {
-                it.onMouseEventOutside(event)
+                if (!inBounds(event)) {
+                    it.onMouseEventOutside(event)
+                }
                 if (it.focusable) {
                     return false
                 }
@@ -263,9 +265,6 @@ internal abstract class DesktopComposeSceneLayer(
     }
 }
 
-private fun MouseEvent.isMainAction() =
-    button == MouseEvent.BUTTON1
-
 private fun maxInflate(baseBounds: IntRect, currentBounds: IntRect, maxInflate: IntRect) = IntRect(
     left = max(baseBounds.left - currentBounds.left, maxInflate.left),
     top = max(baseBounds.top - currentBounds.top, maxInflate.top),

diff --git a/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/MultiLayerComposeScene.skiko.kt b/compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/MultiLayerComposeScene.skiko.kt
@@ -618,8 +618,7 @@ private class MultiLayerComposeSceneImpl(
 private val PointerInputEvent.isGestureInProgress get() = pointers.fastAny { it.down }
 
 private fun PointerInputEvent.isMainAction() =
-    button == PointerButton.Primary ||
-        button == null && pointers.size == 1
+    button != null || pointers.size == 1
 
 private class CopiedList<T>(
     private val populate: (MutableList<T>) -> Unit

diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/DialogTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/DialogTest.kt
@@ -152,7 +152,7 @@ class DialogTest {
     }
 
     @Test
-    fun secondClickDoesNotDismissPopup() = runSkikoComposeUiTest(
+    fun secondTouchDoesNotDismissPopup() = runSkikoComposeUiTest(
         size = Size(100f, 100f)
     ) {
         val background = FillBox()
@@ -197,25 +197,41 @@ class DialogTest {
     }
 
     @Test
-    fun nonPrimaryButtonClickDoesNotDismissDialog() = runSkikoComposeUiTest(
+    fun secondaryButtonClickDismissDialog() = runSkikoComposeUiTest(
         size = Size(100f, 100f)
     ) {
+        val openDialog = mutableStateOf(true)
         val background = FillBox()
         val dialog = DialogState(
             IntSize(40, 40),
-            onDismissRequest = { fail() }
+            onDismissRequest = {
+                openDialog.value = false
+            }
         )
 
         setContent {
             background.Content()
-            dialog.Content()
+            if (openDialog.value) {
+                dialog.Content()
+            }
         }
 
         val buttons = PointerButtons(
             isSecondaryPressed = true
         )
-        scene.sendPointerEvent(PointerEventType.Press, Offset(10f, 10f), buttons = buttons, button = PointerButton.Secondary)
-        scene.sendPointerEvent(PointerEventType.Release, Offset(10f, 10f), button = PointerButton.Secondary)
+        scene.sendPointerEvent(
+            PointerEventType.Press,
+            position = Offset(10f, 10f),
+            buttons = buttons,
+            button = PointerButton.Secondary
+        )
+        scene.sendPointerEvent(
+            PointerEventType.Release,
+            position = Offset(10f, 10f),
+            button = PointerButton.Secondary
+        )
+
+        onNodeWithTag(dialog.tag).assertDoesNotExist()
     }
 
     @OptIn(InternalTestApi::class)

diff --git a/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/PopupTest.kt b/compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/PopupTest.kt
@@ -556,7 +556,7 @@ class PopupTest {
     }
 
     @Test
-    fun secondClickDoesNotDismissPopup() = runSkikoComposeUiTest(
+    fun secondTouchDoesNotDismissPopup() = runSkikoComposeUiTest(
         size = Size(100f, 100f)
     ) {
         val background = FillBox()
@@ -599,6 +599,40 @@ class PopupTest {
         )
     }
 
+    @Test
+    fun secondaryButtonClickDismissPopup() = runSkikoComposeUiTest(
+        size = Size(100f, 100f)
+    ) {
+        val openPopup = mutableStateOf(true)
+        val background = FillBox()
+        val popup = PopupState(
+            IntRect(20, 20, 60, 60),
+            focusable = true,
+            onDismissRequest = {
+                openPopup.value = false
+            }
+        )
+
+        setContent {
+            background.Content()
+            if (openPopup.value) {
+                popup.Content()
+            }
+        }
+
+        val buttons = PointerButtons(
+            isSecondaryPressed = true
+        )
+        scene.sendPointerEvent(
+            PointerEventType.Press,
+            position = Offset(10f, 10f),
+            buttons = buttons,
+            button = PointerButton.Secondary
+        )
+
+        onNodeWithTag(popup.tag).assertDoesNotExist()
+    }
+
     @Test
     fun clippingEnabledPopup() = runSkikoComposeUiTest(
         size = Size(100f, 100f)