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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Apr 19, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1dafc3e
Close Popup/Dialog by clicking any mouse button outside
MatkovIvan Apr 18, 2024
be5e0b1
Update tests
MatkovIvan Apr 18, 2024
a4158d7
Move inBounds check to DetectEventOutsideLayer
MatkovIvan Apr 18, 2024
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
9 changes: 4 additions & 5 deletions ...i/ui/src/desktopMain/kotlin/androidx/compose/ui/scene/DesktopComposeSceneLayer.desktop.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
}
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 1 addition & 2 deletions compose/ui/ui/src/skikoMain/kotlin/androidx/compose/ui/scene/MultiLayerComposeScene.skiko.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 22 additions & 6 deletions compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/DialogTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ class DialogTest {
}

@Test
fun secondClickDoesNotDismissPopup() = runSkikoComposeUiTest(
fun secondTouchDoesNotDismissPopup() = runSkikoComposeUiTest(
size = Size(100f, 100f)
) {
val background = FillBox()
Expand Down Expand Up @@ -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)
Expand Down
36 changes: 35 additions & 1 deletion compose/ui/ui/src/skikoTest/kotlin/androidx/compose/ui/window/PopupTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ class PopupTest {
}

@Test
fun secondClickDoesNotDismissPopup() = runSkikoComposeUiTest(
fun secondTouchDoesNotDismissPopup() = runSkikoComposeUiTest(
size = Size(100f, 100f)
) {
val background = FillBox()
Expand Down Expand Up @@ -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
)

Copy link

Choose a reason for hiding this comment

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

Should send a release here too

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's on purpose - Popup should be closed on down event

Copy link

@m-sasha m-sasha Apr 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So dialogs close on release, but popups close on press? I see that in the code, but is there a reason for 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.

Yes. TLDR - Android

onNodeWithTag(popup.tag).assertDoesNotExist()
}

@Test
fun clippingEnabledPopup() = runSkikoComposeUiTest(
size = Size(100f, 100f)
Expand Down
Loading