mirror of
https://gitflic.ru/project/openide/openide.git
synced 2026-09-27 10:03:11 +07:00
[JEWEL-1396] Take popup dismissal policy from PopupProperties
The custom popup renderers decided both whether to dismiss a popup and whether to consume the Escape key from the shape of `onDismissRequest`: non-null meant "dismiss and consume", null meant "do neither". A caller wanting to suppress dismissal had to withhold the callback, which suppressed every dismissal path at once. `ComboBox` did exactly that while the pointer was over it, which is a pointer concern, and so Escape was consumed while nothing closed and the key never reached the surrounding dialog. Compose's own popup keeps these three concerns apart, and Jewel now does the same: `dismissOnClickOutside` governs the pointer path, `dismissOnBackPress` governs Escape, and `onDismissRequest` says only whether there is any way to act at all. Changes: * `ComboBox` and `EditableComboBox` suppress only the pointer path, via `dismissOnClickOutside`, and always pass a dismissal callback. Hovering no longer affects Escape. * Both renderers gate Escape dismissal on `dismissOnBackPress`, which neither honoured before. * Both renderers consume Escape whenever the popup is focusable, whether or not it dismissed, matching Compose's rule that any focusable popup consumes back events. A non-focusable popup no longer swallows a key it did not act on. * `JBPopupRenderer` pushes property changes onto the live `AbstractPopup`. It captured them once at creation, so a popup whose properties change could not be honoured at all. * `GotItTooltip` sets `dismissOnBackPress = false`, since it handles Escape on its anchor and the renderer must not claim the key. Without this it was measurably broken against the bridge renderer: Escape was swallowed and the tooltip stayed on screen. * `SpeedSearchArea` likewise expresses `dismissOnLoseFocus` as `dismissOnClickOutside`, so Escape always hides the search input whatever the outside-dismissal policy is. * Adds a Spectre parity test asserting the custom renderer is indistinguishable from Compose's own popup across nine scenarios, including popups with no component logic behind them, where the renderer's own decision is what is being observed. closes https://github.com/JetBrains/intellij-community/pull/3622 (cherry picked from commit 71c304a9fe976a8ac48f3ba6e7d1d6e343fb92d5) IJ-MR-220560 GitOrigin-RevId: 0a3b2988e04eac2ac1e3f50cc4c2b67364eb1772
This commit is contained in:
committed by
intellij-monorepo-bot
parent
2eba0e030f
commit
1b0dd5a4d2
@@ -17,6 +17,7 @@ kt_jvm_import(
|
||||
name = "spectre-core",
|
||||
testonly = True,
|
||||
jar = "@jewel_deps_spectre_core//file",
|
||||
srcjar = "@jewel_deps_spectre_core_sources//file",
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
@@ -24,6 +25,7 @@ kt_jvm_import(
|
||||
name = "spectre-testing",
|
||||
testonly = True,
|
||||
jar = "@jewel_deps_spectre_testing//file",
|
||||
srcjar = "@jewel_deps_spectre_testing_sources//file",
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
@@ -31,6 +33,7 @@ kt_jvm_import(
|
||||
name = "spectre-agent",
|
||||
testonly = True,
|
||||
jar = "@jewel_deps_spectre_agent//file",
|
||||
srcjar = "@jewel_deps_spectre_agent_sources//file",
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
@@ -38,6 +41,7 @@ kt_jvm_import(
|
||||
name = "spectre-recording",
|
||||
testonly = True,
|
||||
jar = "@jewel_deps_spectre_recording//file",
|
||||
srcjar = "@jewel_deps_spectre_recording_sources//file",
|
||||
visibility = ["//visibility:private"],
|
||||
)
|
||||
|
||||
|
||||
+29
-5
@@ -45,6 +45,7 @@ import com.intellij.openapi.ui.popup.JBPopupListener
|
||||
import com.intellij.openapi.ui.popup.LightweightWindowEvent
|
||||
import com.intellij.ui.ScreenUtil
|
||||
import com.intellij.ui.awt.RelativePoint
|
||||
import com.intellij.ui.popup.AbstractPopup
|
||||
import com.intellij.ui.scale.JBUIScale
|
||||
import java.awt.Component
|
||||
import java.awt.Dimension
|
||||
@@ -118,6 +119,8 @@ private fun JBPopup(
|
||||
val currentContent = rememberUpdatedState(content)
|
||||
val currentPopupPositionProvider by rememberUpdatedState(popupPositionProvider)
|
||||
val currentOnDismissRequest by rememberUpdatedState(onDismissRequest)
|
||||
val currentOnPreviewKeyEvent by rememberUpdatedState(onPreviewKeyEvent)
|
||||
val currentOnKeyEvent by rememberUpdatedState(onKeyEvent)
|
||||
val currentProperties by rememberUpdatedState(properties)
|
||||
|
||||
val owner = LocalComponent.current
|
||||
@@ -182,17 +185,29 @@ private fun JBPopup(
|
||||
.setCancelOnClickOutside(currentProperties.dismissOnClickOutside)
|
||||
.setCancelOnWindowDeactivation(currentProperties.dismissOnClickOutside)
|
||||
.setLocateWithinScreenBounds(false)
|
||||
// AbstractPopup cancels itself on Escape whenever the handler below declines the event, which would
|
||||
// close the popup behind Compose's back and leave the two out of sync. Take sole ownership instead.
|
||||
.setCancelKeyEnabled(false)
|
||||
.setKeyEventHandler { event ->
|
||||
val composeEvent = event.toComposeKeyEvent()
|
||||
val consumed =
|
||||
onPreviewKeyEvent?.invoke(composeEvent) == true || onKeyEvent?.invoke(composeEvent) == true
|
||||
currentOnPreviewKeyEvent?.invoke(composeEvent) == true ||
|
||||
currentOnKeyEvent?.invoke(composeEvent) == true
|
||||
// Whether Escape dismisses is the popup's declared policy, not something inferred from the shape
|
||||
// of its callbacks. A null callback means we have no way to dismiss, so we must not claim the key.
|
||||
val dismissed =
|
||||
!consumed &&
|
||||
composeEvent.isDismissRequest() &&
|
||||
currentProperties.dismissOnBackPress &&
|
||||
currentOnDismissRequest != null
|
||||
|
||||
if (!consumed && composeEvent.isDismissRequest()) {
|
||||
if (dismissed) {
|
||||
isVisible = false
|
||||
true
|
||||
} else {
|
||||
consumed
|
||||
}
|
||||
// A focusable popup swallows Escape whether or not it dismissed, matching Compose. A
|
||||
// non-focusable one must let a key it did not act on reach whatever owns it.
|
||||
val claimsDismissRequest = composeEvent.isDismissRequest() && currentProperties.focusable
|
||||
consumed || dismissed || claimsDismissRequest
|
||||
}
|
||||
.addListener(
|
||||
object : JBPopupListener {
|
||||
@@ -208,6 +223,15 @@ private fun JBPopup(
|
||||
.createPopup()
|
||||
}
|
||||
|
||||
// The builder above only captures the properties as they were on first composition. AbstractPopup reads these
|
||||
// flags live on every event, so pushing changes keeps a popup whose properties vary (a ComboBox suppressing
|
||||
// click-outside dismissal while its chevron is hovered) in step without recreating the native window.
|
||||
LaunchedEffect(popup, currentProperties) {
|
||||
val abstractPopup = popup as? AbstractPopup ?: return@LaunchedEffect
|
||||
abstractPopup.setCancelOnClickOutside(currentProperties.dismissOnClickOutside)
|
||||
abstractPopup.setCancelOnWindowDeactivation(currentProperties.dismissOnClickOutside)
|
||||
}
|
||||
|
||||
val rectValue = popupRectangle.value
|
||||
LaunchedEffect(rectValue) {
|
||||
val rectangle = rectValue ?: return@LaunchedEffect
|
||||
|
||||
+126
-33
@@ -7,6 +7,7 @@ import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.layout.width
|
||||
import androidx.compose.foundation.text.input.TextFieldState
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
@@ -18,13 +19,17 @@ import androidx.compose.ui.awt.ComposeWindow
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.window.application
|
||||
import dev.sebastiano.spectre.core.AutomatorNode
|
||||
import dev.sebastiano.spectre.core.ComposeAutomator
|
||||
import dev.sebastiano.spectre.core.RobotDriver
|
||||
import dev.sebastiano.spectre.testing.runSpectreTest
|
||||
import java.awt.event.KeyEvent
|
||||
import java.util.concurrent.atomic.AtomicReference
|
||||
import kotlin.concurrent.thread
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
import kotlinx.coroutines.CoroutineScope
|
||||
import kotlinx.coroutines.delay
|
||||
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
|
||||
@@ -33,7 +38,7 @@ import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme
|
||||
import org.jetbrains.jewel.intui.standalone.window.Window as JewelWindow
|
||||
import org.jetbrains.jewel.ui.component.ComboBox
|
||||
import org.jetbrains.jewel.ui.component.DefaultButton
|
||||
import org.jetbrains.jewel.ui.component.PopupManager
|
||||
import org.jetbrains.jewel.ui.component.EditableComboBox
|
||||
import org.jetbrains.jewel.ui.component.PopupMenu
|
||||
import org.jetbrains.jewel.ui.component.SpeedSearchArea
|
||||
import org.jetbrains.jewel.ui.component.Text
|
||||
@@ -44,6 +49,11 @@ import org.junit.jupiter.api.Test
|
||||
// Headful, and deliberately not a jps_test: the app under test must keep a standalone-only runtime closure,
|
||||
// with no IntelliJ Platform classes on the classpath. Runs in CI on any agent with a display:
|
||||
// bazel test //platform/jewel/int-ui/int-ui-standalone-tests:jewel-intUi-standalone-spectre-tests
|
||||
//
|
||||
// Every assertion here is on what is actually on screen. The renderers under test put popups in their own native
|
||||
// windows, so a popup can be gone while Compose still believes it is showing, or the reverse: asserting on the
|
||||
// components' own visibility state alone would pass straight through the bug class these tests exist to catch.
|
||||
// The state is still checked, but only ever as a cross-check that it agrees with the screen.
|
||||
class CustomPopupRendererSpectreTest {
|
||||
@Test
|
||||
fun `escape closes a hovered combo box`(): Unit = runSpectreTestWithCustomPopupRenderer {
|
||||
@@ -53,12 +63,12 @@ class CustomPopupRendererSpectreTest {
|
||||
val automator = ComposeAutomator.inProcess(RobotDriver.synthetic(app.awaitWindow()))
|
||||
|
||||
automator.click(automator.waitForNode(tag = REGULAR_COMBO_TAG))
|
||||
automator.waitForNode(tag = REGULAR_POPUP_VISIBLE_TAG, text = "true")
|
||||
automator.waitForNode(tag = COMBO_BOX_POPUP_TAG)
|
||||
|
||||
// The pointer remains inside the ComboBox after click, reproducing the hover path that
|
||||
// previously let the popup's JDialogRenderer consume Escape as a no-op dismissal.
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
automator.waitForNode(tag = REGULAR_POPUP_VISIBLE_TAG, text = "false")
|
||||
automator.waitUntilGone("The ComboBox popup") { findByTestTag(COMBO_BOX_POPUP_TAG) }
|
||||
} finally {
|
||||
app.stop()
|
||||
}
|
||||
@@ -74,7 +84,7 @@ class CustomPopupRendererSpectreTest {
|
||||
|
||||
val menuButton = automator.waitForNode(tag = MENU_BUTTON_TAG)
|
||||
automator.click(menuButton)
|
||||
automator.waitForNode(tag = MENU_VISIBLE_TAG, text = "true")
|
||||
automator.waitForNode(tag = MENU_POPUP_TAG)
|
||||
|
||||
// Spectre routes synthetic key events to the window under its injected pointer. Move that pointer back
|
||||
// to the owner window to cover focusable popups that have not taken native focus yet.
|
||||
@@ -82,7 +92,7 @@ class CustomPopupRendererSpectreTest {
|
||||
automator.waitForIdle()
|
||||
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
automator.waitForNode(tag = MENU_VISIBLE_TAG, text = "false")
|
||||
automator.waitUntilGone("The menu popup") { findByTestTag(MENU_POPUP_TAG) }
|
||||
} finally {
|
||||
app.stop()
|
||||
}
|
||||
@@ -97,21 +107,101 @@ class CustomPopupRendererSpectreTest {
|
||||
val automator = ComposeAutomator.inProcess(RobotDriver.synthetic(app.awaitWindow()))
|
||||
|
||||
automator.click(automator.waitForNode(tag = SPEED_SEARCH_COMBO_TAG))
|
||||
automator.waitForNode(tag = SPEED_SEARCH_POPUP_VISIBLE_TAG, text = "true")
|
||||
automator.waitForNode(tag = COMBO_BOX_POPUP_TAG)
|
||||
|
||||
automator.typeText("Alpha")
|
||||
automator.waitForNode(tag = SPEED_SEARCH_QUERY_TAG, text = "Alpha")
|
||||
automator.waitForNode(tag = SPEED_SEARCH_VISIBLE_TAG, text = "true")
|
||||
val searchInput = automator.waitForNode(tag = SPEED_SEARCH_INPUT_TAG)
|
||||
assertEquals("Alpha", searchInput.editableText, "The speed search field should show what was typed")
|
||||
|
||||
// The first Escape belongs to speed search, and must not reach the popup behind it.
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
automator.waitForNode(tag = SPEED_SEARCH_VISIBLE_TAG, text = "false")
|
||||
automator.waitForNode(tag = SPEED_SEARCH_POPUP_VISIBLE_TAG, text = "true")
|
||||
automator.waitUntilGone("The speed search field") { findByTestTag(SPEED_SEARCH_INPUT_TAG) }
|
||||
assertTrue(
|
||||
automator.isPresent { findByTestTag(COMBO_BOX_POPUP_TAG) },
|
||||
"The first Escape dismissed speed search, so the popup behind it must still be open",
|
||||
)
|
||||
|
||||
// Only once speed search is gone does Escape reach the popup.
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
automator.waitForNode(tag = SPEED_SEARCH_POPUP_VISIBLE_TAG, text = "false")
|
||||
automator.waitUntilGone("The speed searchable ComboBox popup") { findByTestTag(COMBO_BOX_POPUP_TAG) }
|
||||
} finally {
|
||||
app.stop()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `clicking the chevron of an open combo box closes it without reopening it`(): Unit =
|
||||
runSpectreTestWithCustomPopupRenderer {
|
||||
val app = SpectreTestApplication()
|
||||
app.start()
|
||||
try {
|
||||
val driver = RobotDriver.synthetic(app.awaitWindow())
|
||||
val automator = ComposeAutomator.inProcess(driver)
|
||||
|
||||
val comboBox = automator.waitForNode(tag = REGULAR_COMBO_TAG)
|
||||
automator.click(comboBox)
|
||||
automator.waitForNode(tag = COMBO_BOX_POPUP_TAG)
|
||||
|
||||
// Suppressing dismissal while the ComboBox is hovered is what stops the click-outside dismissal
|
||||
// from closing the popup before the chevron's own handler runs and immediately reopens it.
|
||||
val bounds = comboBox.boundsOnScreen
|
||||
driver.click(bounds.x + bounds.width - CHEVRON_INSET, bounds.y + bounds.height / 2)
|
||||
|
||||
automator.waitUntilGone("The ComboBox popup") { findByTestTag(COMBO_BOX_POPUP_TAG) }
|
||||
automator.waitForIdle()
|
||||
assertFalse(
|
||||
automator.isPresent { findByTestTag(COMBO_BOX_POPUP_TAG) },
|
||||
"The chevron click must not reopen the popup it just closed",
|
||||
)
|
||||
} finally {
|
||||
app.stop()
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `escape closes a hovered editable combo box`(): Unit = runSpectreTestWithCustomPopupRenderer {
|
||||
val app = SpectreTestApplication()
|
||||
app.start()
|
||||
try {
|
||||
val driver = RobotDriver.synthetic(app.awaitWindow())
|
||||
val automator = ComposeAutomator.inProcess(driver)
|
||||
|
||||
// Only the chevron opens the popup here; clicking the text field just focuses it.
|
||||
val comboBox = automator.waitForNode(tag = EDITABLE_COMBO_TAG)
|
||||
val bounds = comboBox.boundsOnScreen
|
||||
driver.click(bounds.x + bounds.width - CHEVRON_INSET, bounds.y + bounds.height / 2)
|
||||
automator.waitForNode(tag = COMBO_BOX_POPUP_TAG)
|
||||
|
||||
// The pointer is left on the chevron, which is the hover path that previously had the renderer
|
||||
// consume Escape as a no-op dismissal.
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
automator.waitUntilGone("The EditableComboBox popup") { findByTestTag(COMBO_BOX_POPUP_TAG) }
|
||||
} finally {
|
||||
app.stop()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Waits until [find] matches nothing, across every window Spectre tracks.
|
||||
*
|
||||
* Spectre only ships a wait-for-presence helper, but a popup lives in its own native window, and these tests need to
|
||||
* watch that window go away.
|
||||
*/
|
||||
private suspend fun ComposeAutomator.waitUntilGone(
|
||||
description: String,
|
||||
find: ComposeAutomator.() -> List<AutomatorNode>,
|
||||
) {
|
||||
repeat(POLL_ATTEMPTS) {
|
||||
if (!isPresent(find)) return
|
||||
delay(POLL_INTERVAL_MS.milliseconds)
|
||||
}
|
||||
error("$description was still on screen after ${POLL_ATTEMPTS * POLL_INTERVAL_MS} ms")
|
||||
}
|
||||
|
||||
private fun ComposeAutomator.isPresent(find: ComposeAutomator.() -> List<AutomatorNode>): Boolean {
|
||||
refreshWindows()
|
||||
return find().isNotEmpty()
|
||||
}
|
||||
|
||||
private fun runSpectreTestWithCustomPopupRenderer(block: suspend CoroutineScope.() -> Unit): Unit = runSpectreTest {
|
||||
@@ -121,25 +211,22 @@ private fun runSpectreTestWithCustomPopupRenderer(block: suspend CoroutineScope.
|
||||
|
||||
@Composable
|
||||
private fun PopupEscapeScreen() {
|
||||
val regularPopupManager = remember { PopupManager() }
|
||||
val editableTextFieldState = remember { TextFieldState("Editable") }
|
||||
var menuVisible by remember { mutableStateOf(false) }
|
||||
var speedSearchPopupVisible by remember { mutableStateOf(false) }
|
||||
val speedSearchState = rememberSpeedSearchState()
|
||||
|
||||
Column(modifier = Modifier.padding(24.dp), verticalArrangement = Arrangement.spacedBy(12.dp)) {
|
||||
Text(
|
||||
text = regularPopupManager.isPopupVisible.value.toString(),
|
||||
modifier = Modifier.testTag(REGULAR_POPUP_VISIBLE_TAG),
|
||||
)
|
||||
ComboBox(
|
||||
labelText = "Regular ComboBox",
|
||||
modifier = Modifier.testTag(REGULAR_COMBO_TAG).width(240.dp),
|
||||
popupManager = regularPopupManager,
|
||||
) {
|
||||
ComboBox(labelText = "Regular ComboBox", modifier = Modifier.testTag(REGULAR_COMBO_TAG).width(240.dp)) {
|
||||
Text("Regular popup content")
|
||||
}
|
||||
|
||||
Text(text = menuVisible.toString(), modifier = Modifier.testTag(MENU_VISIBLE_TAG))
|
||||
EditableComboBox(
|
||||
textFieldState = editableTextFieldState,
|
||||
modifier = Modifier.testTag(EDITABLE_COMBO_TAG).width(240.dp),
|
||||
) {
|
||||
Text("Editable popup content")
|
||||
}
|
||||
|
||||
Box {
|
||||
DefaultButton(onClick = { menuVisible = true }, modifier = Modifier.testTag(MENU_BUTTON_TAG)) {
|
||||
Text("Show menu")
|
||||
@@ -152,22 +239,19 @@ private fun PopupEscapeScreen() {
|
||||
true
|
||||
},
|
||||
horizontalAlignment = Alignment.Start,
|
||||
modifier = Modifier.testTag(MENU_POPUP_TAG),
|
||||
) {
|
||||
selectableItem(selected = false, onClick = {}) { Text("Menu item") }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Text(text = speedSearchPopupVisible.toString(), modifier = Modifier.testTag(SPEED_SEARCH_POPUP_VISIBLE_TAG))
|
||||
Text(text = speedSearchState.isVisible.toString(), modifier = Modifier.testTag(SPEED_SEARCH_VISIBLE_TAG))
|
||||
Text(text = speedSearchState.searchText, modifier = Modifier.testTag(SPEED_SEARCH_QUERY_TAG))
|
||||
SpeedSearchArea(state = speedSearchState, dismissOnLoseFocus = false) {
|
||||
SpeedSearchableComboBox(
|
||||
items = listOf("Alpha", "Beta", "Gamma"),
|
||||
selectedIndex = 0,
|
||||
onSelectedItemChange = {},
|
||||
modifier = Modifier.testTag(SPEED_SEARCH_COMBO_TAG).width(240.dp),
|
||||
onPopupVisibleChange = { speedSearchPopupVisible = it },
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -198,17 +282,26 @@ private class SpectreTestApplication(private val content: @Composable () -> Unit
|
||||
window.get()?.let {
|
||||
return it
|
||||
}
|
||||
delay(100)
|
||||
delay(100.milliseconds)
|
||||
}
|
||||
error("The Compose test window was not created")
|
||||
}
|
||||
}
|
||||
|
||||
/** Set by both ComboBox and EditableComboBox on their popup content, including the speed searchable variant. */
|
||||
private const val COMBO_BOX_POPUP_TAG = "Jewel.ComboBox.Popup"
|
||||
|
||||
/** Set by SpeedSearchArea on its search field. */
|
||||
private const val SPEED_SEARCH_INPUT_TAG = "SpeedSearchArea.Input"
|
||||
|
||||
private const val REGULAR_COMBO_TAG = "spectre.regularCombo"
|
||||
private const val REGULAR_POPUP_VISIBLE_TAG = "spectre.regularPopupVisible"
|
||||
private const val EDITABLE_COMBO_TAG = "spectre.editableCombo"
|
||||
private const val MENU_BUTTON_TAG = "spectre.menuButton"
|
||||
private const val MENU_VISIBLE_TAG = "spectre.menuVisible"
|
||||
private const val MENU_POPUP_TAG = "spectre.menuPopup"
|
||||
private const val SPEED_SEARCH_COMBO_TAG = "spectre.speedSearchCombo"
|
||||
private const val SPEED_SEARCH_POPUP_VISIBLE_TAG = "spectre.speedSearchPopupVisible"
|
||||
private const val SPEED_SEARCH_VISIBLE_TAG = "spectre.speedSearchVisible"
|
||||
private const val SPEED_SEARCH_QUERY_TAG = "spectre.speedSearchQuery"
|
||||
|
||||
/** Distance from the ComboBox's trailing edge that reliably lands on the chevron rather than the label. */
|
||||
private const val CHEVRON_INSET = 8
|
||||
|
||||
private const val POLL_ATTEMPTS = 100
|
||||
private const val POLL_INTERVAL_MS = 100L
|
||||
|
||||
+394
@@ -0,0 +1,394 @@
|
||||
@file:OptIn(ExperimentalJewelApi::class)
|
||||
|
||||
package org.jetbrains.jewel.intui.standalone.popup
|
||||
|
||||
// Differential test: the custom renderer must behave exactly as Compose's own popup does, for every component
|
||||
// that opens a popup. Compose's renderer is the oracle here, so a divergence is a bug in JDialogRenderer even
|
||||
// when both outcomes look individually reasonable.
|
||||
|
||||
import androidx.compose.foundation.layout.Box
|
||||
import androidx.compose.foundation.layout.Column
|
||||
import androidx.compose.foundation.layout.padding
|
||||
import androidx.compose.foundation.layout.width
|
||||
import androidx.compose.foundation.text.input.TextFieldState
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.Alignment
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.awt.ComposeWindow
|
||||
import androidx.compose.ui.input.key.Key
|
||||
import androidx.compose.ui.input.key.KeyEventType
|
||||
import androidx.compose.ui.input.key.key
|
||||
import androidx.compose.ui.input.key.onKeyEvent
|
||||
import androidx.compose.ui.input.key.type
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.window.PopupProperties
|
||||
import androidx.compose.ui.window.application
|
||||
import dev.sebastiano.spectre.core.ComposeAutomator
|
||||
import dev.sebastiano.spectre.core.RobotDriver
|
||||
import dev.sebastiano.spectre.testing.runSpectreTest
|
||||
import java.awt.event.KeyEvent
|
||||
import java.util.concurrent.atomic.AtomicBoolean
|
||||
import java.util.concurrent.atomic.AtomicReference
|
||||
import kotlin.concurrent.thread
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
import kotlinx.coroutines.delay
|
||||
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
|
||||
import org.jetbrains.jewel.foundation.JewelFlags
|
||||
import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme
|
||||
import org.jetbrains.jewel.intui.standalone.window.Window as JewelWindow
|
||||
import org.jetbrains.jewel.ui.component.ComboBox
|
||||
import org.jetbrains.jewel.ui.component.DefaultButton
|
||||
import org.jetbrains.jewel.ui.component.EditableComboBox
|
||||
import org.jetbrains.jewel.ui.component.PopupContainer
|
||||
import org.jetbrains.jewel.ui.component.PopupMenu
|
||||
import org.jetbrains.jewel.ui.component.SpeedSearchArea
|
||||
import org.jetbrains.jewel.ui.component.Text
|
||||
import org.jetbrains.jewel.ui.component.gotit.GotItButtons
|
||||
import org.jetbrains.jewel.ui.component.gotit.GotItTooltip
|
||||
import org.jetbrains.jewel.ui.component.rememberSpeedSearchState
|
||||
import org.jetbrains.jewel.ui.component.search.SpeedSearchableComboBox
|
||||
import org.junit.jupiter.api.Test
|
||||
|
||||
private const val OPEN_TAG = "matrix.open"
|
||||
private const val POPUP_TAG = "Jewel.ComboBox.Popup"
|
||||
private const val MENU_TAG = "matrix.menuPopup"
|
||||
private const val SEARCH_INPUT_TAG = "SpeedSearchArea.Input"
|
||||
private const val RAW_POPUP_TAG = "parity.rawPopup"
|
||||
|
||||
/** Set when Escape reaches the owner window's Compose tree, i.e. nothing above it consumed the key. */
|
||||
private val hostSawEscape = AtomicBoolean(false)
|
||||
|
||||
private class Case(
|
||||
val name: String,
|
||||
val open: suspend (ComposeAutomator, RobotDriver) -> Unit,
|
||||
val measure: suspend (ComposeAutomator) -> String,
|
||||
/**
|
||||
* The absolute measurement both renderers must produce, for scenarios with no component logic masking the
|
||||
* renderer's own decision. Differential parity alone cannot catch both renderers being wrong the same way.
|
||||
*/
|
||||
val expect: String? = null,
|
||||
val content: @Composable () -> Unit,
|
||||
)
|
||||
|
||||
private fun ComposeAutomator.hasTag(tag: String) = findByTestTag(tag).isNotEmpty()
|
||||
|
||||
private fun ComposeAutomator.hasText(text: String) = findByText(text).isNotEmpty()
|
||||
|
||||
private fun host() = "hostSawEscape=" + hostSawEscape.get()
|
||||
|
||||
private val openByClick: suspend (ComposeAutomator, RobotDriver) -> Unit = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = POPUP_TAG)
|
||||
}
|
||||
|
||||
/** Clicks the chevron rather than the centre, which on an EditableComboBox only focuses the text field. */
|
||||
private val openByChevron: suspend (ComposeAutomator, RobotDriver) -> Unit = { a, driver ->
|
||||
val bounds = a.waitForNode(tag = OPEN_TAG).boundsOnScreen
|
||||
driver.click(bounds.x + bounds.width - 8, bounds.y + bounds.height / 2)
|
||||
a.waitForNode(tag = POPUP_TAG)
|
||||
}
|
||||
|
||||
private val CONTROL_CASE =
|
||||
// Control: no popup at all. If this does not report hostSawEscape=true, the probe itself is broken
|
||||
// and every other hostSawEscape reading in this run is meaningless.
|
||||
Case(
|
||||
name = "control(noPopup)",
|
||||
open = { a, _ -> a.click(a.waitForNode(tag = OPEN_TAG)) },
|
||||
measure = { "hostSawEscape=" + hostSawEscape.get() },
|
||||
) {
|
||||
DefaultButton(onClick = {}, modifier = Modifier.testTag(OPEN_TAG)) { Text("Plain button") }
|
||||
}
|
||||
|
||||
private val CASES =
|
||||
listOf(
|
||||
Case(name = "comboBox", open = openByClick, measure = { "popupOpen=" + it.hasTag(POPUP_TAG) + " " + host() }) {
|
||||
ComboBox(labelText = "Combo", modifier = Modifier.testTag(OPEN_TAG).width(240.dp)) { Text("Content") }
|
||||
},
|
||||
Case(
|
||||
name = "editableComboBox",
|
||||
open = openByChevron,
|
||||
measure = { "popupOpen=" + it.hasTag(POPUP_TAG) + " " + host() },
|
||||
) {
|
||||
EditableComboBox(
|
||||
textFieldState = remember { TextFieldState("Editable") },
|
||||
modifier = Modifier.testTag(OPEN_TAG).width(240.dp),
|
||||
) {
|
||||
Text("Content")
|
||||
}
|
||||
},
|
||||
Case(
|
||||
name = "popupMenu(focusable=true)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = MENU_TAG)
|
||||
},
|
||||
measure = { "menuOpen=" + it.hasTag(MENU_TAG) + " " + host() },
|
||||
) {
|
||||
MenuWithSubmenu()
|
||||
},
|
||||
Case(
|
||||
name = "submenu(nested)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = MENU_TAG)
|
||||
a.click(a.waitForNode(text = "Open submenu"))
|
||||
a.waitForNode(text = "Sub item")
|
||||
},
|
||||
measure = {
|
||||
"parentMenuOpen=" + it.hasTag(MENU_TAG) + " submenuOpen=" + it.hasText("Sub item") + " " + host()
|
||||
},
|
||||
) {
|
||||
MenuWithSubmenu()
|
||||
},
|
||||
Case(
|
||||
name = "gotIt(withButtons)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(text = "Tooltip body")
|
||||
},
|
||||
measure = { "tooltipOpen=" + it.hasText("Tooltip body") + " " + host() },
|
||||
) {
|
||||
GotIt(buttons = GotItButtons.default())
|
||||
},
|
||||
Case(
|
||||
name = "gotIt(noButtons)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(text = "Tooltip body")
|
||||
},
|
||||
measure = { "tooltipOpen=" + it.hasText("Tooltip body") + " " + host() },
|
||||
) {
|
||||
GotIt(buttons = GotItButtons.None)
|
||||
},
|
||||
Case(
|
||||
name = "rawPopup(dismissOnBackPress=true)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = RAW_POPUP_TAG)
|
||||
},
|
||||
measure = { "popupOpen=" + it.hasTag(RAW_POPUP_TAG) + " " + host() },
|
||||
// Escape dismisses, and the focusable popup consumes the key whether it dismissed or not.
|
||||
expect = "popupOpen=false hostSawEscape=false",
|
||||
) {
|
||||
RawPopup(dismissOnBackPress = true)
|
||||
},
|
||||
Case(
|
||||
name = "rawPopup(nonFocusable)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = RAW_POPUP_TAG)
|
||||
},
|
||||
measure = { "popupOpen=" + it.hasTag(RAW_POPUP_TAG) + " " + host() },
|
||||
// A non-focusable popup cannot act on Escape at all: it must neither dismiss nor consume, so the
|
||||
// key reaches the host. Proves unconsumed Escape is forwarded while a native popup window exists.
|
||||
expect = "popupOpen=true hostSawEscape=true",
|
||||
) {
|
||||
RawPopup(focusable = false)
|
||||
},
|
||||
Case(
|
||||
name = "rawPopup(dismissOnBackPress=false)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = RAW_POPUP_TAG)
|
||||
},
|
||||
measure = { "popupOpen=" + it.hasTag(RAW_POPUP_TAG) + " " + host() },
|
||||
// No dismissal, but the focusable popup still consumes the key.
|
||||
expect = "popupOpen=true hostSawEscape=false",
|
||||
) {
|
||||
RawPopup(dismissOnBackPress = false)
|
||||
},
|
||||
Case(
|
||||
name = "speedSearchCombo(escape#1)",
|
||||
open = { a, _ ->
|
||||
a.click(a.waitForNode(tag = OPEN_TAG))
|
||||
a.waitForNode(tag = POPUP_TAG)
|
||||
a.typeText("Alpha")
|
||||
a.waitForNode(tag = SEARCH_INPUT_TAG)
|
||||
},
|
||||
measure = {
|
||||
"popupOpen=" + it.hasTag(POPUP_TAG) + " searchOpen=" + it.hasTag(SEARCH_INPUT_TAG) + " " + host()
|
||||
},
|
||||
) {
|
||||
SpeedSearchCombo()
|
||||
},
|
||||
)
|
||||
|
||||
class PopupRendererParitySpectreTest {
|
||||
@Test
|
||||
fun `the custom renderer matches Compose's own popup for every component`(): Unit = runSpectreTest {
|
||||
// The control doubles as the probe's canary: if Escape does not reach the host window with no popup
|
||||
// involved, every other hostSawEscape reading is meaningless, and the parity comparison below would
|
||||
// pass vacuously on a dead probe. Gate on it explicitly.
|
||||
for (custom in listOf(false, true)) {
|
||||
assertEquals(
|
||||
expected = "hostSawEscape=true",
|
||||
actual = measure(custom = custom, case = CONTROL_CASE),
|
||||
message =
|
||||
"Escape probe is broken on ${label(custom)}: the host window did not see Escape with no popup showing",
|
||||
)
|
||||
}
|
||||
|
||||
val divergences = mutableListOf<String>()
|
||||
for (case in CASES) {
|
||||
val withCompose = measure(custom = false, case = case)
|
||||
val withJDialog = measure(custom = true, case = case)
|
||||
if (withCompose != withJDialog) {
|
||||
divergences += "${case.name}: defaultCompose[$withCompose] != JDialogRenderer[$withJDialog]"
|
||||
}
|
||||
val expected = case.expect
|
||||
if (expected != null) {
|
||||
if (withCompose != expected) {
|
||||
divergences += "${case.name}: defaultCompose[$withCompose] != expected[$expected]"
|
||||
}
|
||||
if (withJDialog != expected) {
|
||||
divergences += "${case.name}: JDialogRenderer[$withJDialog] != expected[$expected]"
|
||||
}
|
||||
}
|
||||
}
|
||||
assertEquals(
|
||||
emptyList(),
|
||||
divergences,
|
||||
"JDialogRenderer must be indistinguishable from Compose's own popup for these components",
|
||||
)
|
||||
}
|
||||
|
||||
private suspend fun measure(custom: Boolean, case: Case): String {
|
||||
JewelFlags.useCustomPopupRenderer = custom
|
||||
val app = MatrixApp(case.content)
|
||||
app.start()
|
||||
try {
|
||||
val driver = RobotDriver.synthetic(app.awaitWindow())
|
||||
val automator = ComposeAutomator.inProcess(driver)
|
||||
case.open(automator, driver)
|
||||
automator.waitForIdle()
|
||||
hostSawEscape.set(false)
|
||||
|
||||
automator.pressKey(KeyEvent.VK_ESCAPE)
|
||||
delay(1500.milliseconds)
|
||||
automator.refreshWindows()
|
||||
|
||||
val result = case.measure(automator)
|
||||
println("PARITY renderer=" + label(custom) + " case=" + case.name + " " + result)
|
||||
return result
|
||||
} finally {
|
||||
app.stop()
|
||||
delay(500.milliseconds)
|
||||
}
|
||||
}
|
||||
|
||||
private fun label(custom: Boolean) = if (custom) "JDialogRenderer" else "defaultCompose"
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun MenuWithSubmenu() {
|
||||
var visible by remember { mutableStateOf(false) }
|
||||
Box {
|
||||
DefaultButton(onClick = { visible = true }, modifier = Modifier.testTag(OPEN_TAG)) { Text("Menu") }
|
||||
if (visible) {
|
||||
PopupMenu(
|
||||
onDismissRequest = {
|
||||
visible = false
|
||||
true
|
||||
},
|
||||
horizontalAlignment = Alignment.Start,
|
||||
modifier = Modifier.testTag(MENU_TAG),
|
||||
) {
|
||||
submenu(submenu = { selectableItem(false, onClick = {}) { Text("Sub item") } }) { Text("Open submenu") }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun GotIt(buttons: GotItButtons) {
|
||||
var visible by remember { mutableStateOf(false) }
|
||||
GotItTooltip(text = "Tooltip body", visible = visible, onDismiss = { visible = false }, buttons = buttons) {
|
||||
DefaultButton(onClick = { visible = true }, modifier = Modifier.testTag(OPEN_TAG)) { Text("Show") }
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A popup with no component logic behind it: nothing else handles Escape, so what happens is entirely the renderer's
|
||||
* decision. Every other case in this matrix has a component that closes itself on Escape, which masks whatever the
|
||||
* renderer does.
|
||||
*/
|
||||
@Composable
|
||||
private fun RawPopup(focusable: Boolean = true, dismissOnBackPress: Boolean = true) {
|
||||
var visible by remember { mutableStateOf(false) }
|
||||
Box {
|
||||
DefaultButton(onClick = { visible = true }, modifier = Modifier.testTag(OPEN_TAG)) { Text("Open raw popup") }
|
||||
if (visible) {
|
||||
PopupContainer(
|
||||
onDismissRequest = { visible = false },
|
||||
horizontalAlignment = Alignment.Start,
|
||||
modifier = Modifier.testTag(RAW_POPUP_TAG),
|
||||
popupProperties = PopupProperties(focusable = focusable, dismissOnBackPress = dismissOnBackPress),
|
||||
) {
|
||||
Text("Raw popup content")
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun SpeedSearchCombo() {
|
||||
val state = rememberSpeedSearchState()
|
||||
SpeedSearchArea(state = state, dismissOnLoseFocus = false) {
|
||||
SpeedSearchableComboBox(
|
||||
items = listOf("Alpha", "Beta", "Gamma"),
|
||||
selectedIndex = 0,
|
||||
onSelectedItemChange = {},
|
||||
modifier = Modifier.testTag(OPEN_TAG).width(240.dp),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private class MatrixApp(private val content: @Composable () -> Unit) {
|
||||
private val exitApplication = AtomicReference<(() -> Unit)?>(null)
|
||||
private val window = AtomicReference<ComposeWindow?>(null)
|
||||
|
||||
fun start() {
|
||||
thread(name = "spectre-matrix-window", isDaemon = true) {
|
||||
application(exitProcessOnExit = false) {
|
||||
exitApplication.set(::exitApplication)
|
||||
JewelWindow(onCloseRequest = ::exitApplication, title = "Jewel popup matrix") {
|
||||
this@MatrixApp.window.compareAndSet(null, window)
|
||||
IntUiTheme {
|
||||
Column(
|
||||
modifier =
|
||||
Modifier.padding(24.dp).onKeyEvent { event ->
|
||||
if (event.type == KeyEventType.KeyDown && event.key == Key.Escape) {
|
||||
hostSawEscape.set(true)
|
||||
}
|
||||
false
|
||||
}
|
||||
) {
|
||||
content()
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fun stop() {
|
||||
exitApplication.get()?.invoke()
|
||||
window.set(null)
|
||||
}
|
||||
|
||||
suspend fun awaitWindow(): ComposeWindow {
|
||||
repeat(100) {
|
||||
window.get()?.let {
|
||||
return it
|
||||
}
|
||||
delay(100.milliseconds)
|
||||
}
|
||||
error("The Compose test window was not created")
|
||||
}
|
||||
}
|
||||
+11
-2
@@ -430,12 +430,21 @@ private fun JPopupImpl(
|
||||
currentOnPreviewKeyEvent?.invoke(composeEvent) == true ||
|
||||
currentOnKeyEvent?.invoke(composeEvent) == true
|
||||
val dismissRequest = currentOnDismissRequest
|
||||
val dismissed = !consumed && composeEvent.isDismissRequest() && dismissRequest != null
|
||||
// Whether Escape dismisses is the popup's declared policy; a null callback just means we have
|
||||
// no way to act on it, so we must not claim the key either.
|
||||
val dismissed =
|
||||
!consumed &&
|
||||
composeEvent.isDismissRequest() &&
|
||||
currentProperties.dismissOnBackPress &&
|
||||
dismissRequest != null
|
||||
|
||||
if (dismissed) {
|
||||
dismissRequest.invoke()
|
||||
}
|
||||
if (consumed || dismissed) {
|
||||
// A focusable popup swallows Escape whether or not it dismissed, matching Compose. A
|
||||
// non-focusable one must let a key it did not act on reach whatever owns it.
|
||||
val claimsDismissRequest = composeEvent.isDismissRequest() && currentProperties.focusable
|
||||
if (consumed || dismissed || claimsDismissRequest) {
|
||||
event.consume()
|
||||
}
|
||||
}
|
||||
|
||||
@@ -30,6 +30,13 @@ http_file(
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-core/0.5.0/spectre-core-0.5.0.jar",
|
||||
)
|
||||
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_core_sources",
|
||||
downloaded_file_path = "spectre-core-0.5.0-sources.jar",
|
||||
sha256 = "798968ef359092d51beca43dbd2707792a21405761bf4f817a98addac038e1f9",
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-core/0.5.0/spectre-core-0.5.0-sources.jar",
|
||||
)
|
||||
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_testing",
|
||||
downloaded_file_path = "spectre-testing-0.5.0.jar",
|
||||
@@ -37,6 +44,13 @@ http_file(
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-testing/0.5.0/spectre-testing-0.5.0.jar",
|
||||
)
|
||||
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_testing_sources",
|
||||
downloaded_file_path = "spectre-testing-0.5.0-sources.jar",
|
||||
sha256 = "2f95170eb876feeb284261077257a019eb0073e89f91e24322fef07730972917",
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-testing/0.5.0/spectre-testing-0.5.0-sources.jar",
|
||||
)
|
||||
|
||||
# Compile dependency of spectre-testing; unused by the in-process lane, but must resolve.
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_agent",
|
||||
@@ -45,6 +59,13 @@ http_file(
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-agent/0.5.0/spectre-agent-0.5.0.jar",
|
||||
)
|
||||
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_agent_sources",
|
||||
downloaded_file_path = "spectre-agent-0.5.0-sources.jar",
|
||||
sha256 = "8117ee97f119d1fae12ee44a467ceb6d91f6c552edaea1a3d32d033e43e7a6e4",
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-agent/0.5.0/spectre-agent-0.5.0-sources.jar",
|
||||
)
|
||||
|
||||
# Runtime dependency of spectre-testing.
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_recording",
|
||||
@@ -53,6 +74,13 @@ http_file(
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-recording/0.5.0/spectre-recording-0.5.0.jar",
|
||||
)
|
||||
|
||||
http_file(
|
||||
name = "jewel_deps_spectre_recording_sources",
|
||||
downloaded_file_path = "spectre-recording-0.5.0-sources.jar",
|
||||
sha256 = "7c84c5c3e9bd2774d06183d802fbad5a8fe4bb3bd2bb2ea271b7e5d5b1bb61b8",
|
||||
url = "https://cache-redirector.jetbrains.com/repo1.maven.org/maven2/dev/sebastiano/spectre/spectre-recording/0.5.0/spectre-recording-0.5.0-sources.jar",
|
||||
)
|
||||
|
||||
# Runtime dependency of spectre-core, not otherwise in the monorepo.
|
||||
http_file(
|
||||
name = "jewel_deps_androidx_tracing_wire_desktop",
|
||||
|
||||
+147
@@ -0,0 +1,147 @@
|
||||
// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||
package org.jetbrains.jewel.ui.component
|
||||
|
||||
import androidx.compose.foundation.layout.width
|
||||
import androidx.compose.foundation.text.input.TextFieldState
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.geometry.Offset
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.test.MouseInjectionScope
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.test.onNodeWithTag
|
||||
import androidx.compose.ui.test.performMouseInput
|
||||
import androidx.compose.ui.unit.dp
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
|
||||
import org.jetbrains.jewel.foundation.JewelFlags
|
||||
import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* Both combo boxes suppress the *pointer* dismissal path while the pointer is over them, so that clicking the chevron
|
||||
* to close an open popup does not have the click-outside dismissal close it first and the chevron's own handler
|
||||
* immediately reopen it.
|
||||
*
|
||||
* That suppression belongs in `PopupProperties.dismissOnClickOutside`, which is what renderers consult for the pointer
|
||||
* path, and it must leave Escape alone. Expressing it by withholding `onDismissRequest` instead would suppress every
|
||||
* dismissal path at once, so Escape would be swallowed with nothing closing.
|
||||
*/
|
||||
@OptIn(ExperimentalJewelApi::class)
|
||||
class ComboBoxDismissRequestTest {
|
||||
@get:Rule val composeRule = createComposeRule()
|
||||
|
||||
@Test
|
||||
fun `combo box allows pointer dismissal when the pointer is elsewhere`() {
|
||||
val renderer = recordPopup(hoverAt = null) { popupManager -> ComboBox(popupManager) }
|
||||
|
||||
assertTrue(renderer.properties.isNotEmpty(), "The popup was never rendered")
|
||||
assertTrue(renderer.properties.last().dismissOnClickOutside, "An unhovered combo box must be dismissable")
|
||||
assertTrue(renderer.dismissRequests.last() != null, "A combo box must always be able to dismiss")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `combo box suppresses only pointer dismissal while it is hovered`() {
|
||||
// The whole ComboBox drives the hover flag, so its centre is as good a hover target as the chevron.
|
||||
val renderer = recordPopup(hoverAt = { center }) { popupManager -> ComboBox(popupManager) }
|
||||
|
||||
assertFalse(
|
||||
renderer.properties.last().dismissOnClickOutside,
|
||||
"A hovered combo box must opt out of the pointer dismissal path",
|
||||
)
|
||||
assertTrue(
|
||||
renderer.properties.last().dismissOnBackPress,
|
||||
"Hovering must not disable Escape: it is a separate dismissal path",
|
||||
)
|
||||
assertTrue(renderer.dismissRequests.last() != null, "A combo box must always be able to dismiss")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `editable combo box allows pointer dismissal when the pointer is elsewhere`() {
|
||||
val renderer = recordPopup(hoverAt = null) { popupManager -> EditableComboBox(popupManager) }
|
||||
|
||||
assertTrue(renderer.properties.isNotEmpty(), "The popup was never rendered")
|
||||
assertTrue(renderer.properties.last().dismissOnClickOutside, "An unhovered editable combo box is dismissable")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `editable combo box suppresses only pointer dismissal while its chevron is hovered`() {
|
||||
// Unlike ComboBox, only the text field and the chevron drive the hover flags here, and the tagged node also
|
||||
// spans the popup, so aim at the chevron: the right-hand end of the first row.
|
||||
val renderer =
|
||||
recordPopup(hoverAt = { Offset(width - CHEVRON_INSET, CHEVRON_INSET) }) { popupManager ->
|
||||
EditableComboBox(popupManager)
|
||||
}
|
||||
|
||||
assertFalse(
|
||||
renderer.properties.last().dismissOnClickOutside,
|
||||
"An editable combo box with a hovered chevron must opt out of the pointer dismissal path",
|
||||
)
|
||||
assertTrue(
|
||||
renderer.properties.last().dismissOnBackPress,
|
||||
"Hovering must not disable Escape: it is a separate dismissal path",
|
||||
)
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun ComboBox(popupManager: PopupManager) {
|
||||
ComboBox(labelText = "Label", modifier = comboBoxModifier, popupManager = popupManager) {
|
||||
Text("Popup content")
|
||||
}
|
||||
}
|
||||
|
||||
@Composable
|
||||
private fun EditableComboBox(popupManager: PopupManager) {
|
||||
EditableComboBox(
|
||||
textFieldState = remember { TextFieldState("Value") },
|
||||
modifier = comboBoxModifier,
|
||||
popupManager = popupManager,
|
||||
) {
|
||||
Text("Popup content")
|
||||
}
|
||||
}
|
||||
|
||||
private fun recordPopup(
|
||||
hoverAt: (MouseInjectionScope.() -> Offset)?,
|
||||
content: @Composable (PopupManager) -> Unit,
|
||||
): RecordingPopupRenderer {
|
||||
val renderer = RecordingPopupRenderer()
|
||||
lateinit var popupManager: PopupManager
|
||||
val oldUseCustomPopupRenderer = JewelFlags.useCustomPopupRenderer
|
||||
JewelFlags.useCustomPopupRenderer = true
|
||||
try {
|
||||
composeRule.setContent {
|
||||
IntUiTheme {
|
||||
CompositionLocalProvider(LocalPopupRenderer provides renderer) {
|
||||
popupManager = remember { PopupManager() }
|
||||
content(popupManager)
|
||||
}
|
||||
}
|
||||
}
|
||||
composeRule.runOnIdle { popupManager.setPopupVisible(true) }
|
||||
composeRule.waitForIdle()
|
||||
|
||||
if (hoverAt != null) {
|
||||
composeRule.onNodeWithTag(COMBO_BOX_TAG).performMouseInput {
|
||||
// moveTo, not updatePointerTo: onHover keys off Enter, which only a dispatched move produces.
|
||||
moveTo(hoverAt())
|
||||
advanceEventTime()
|
||||
}
|
||||
composeRule.waitForIdle()
|
||||
}
|
||||
} finally {
|
||||
JewelFlags.useCustomPopupRenderer = oldUseCustomPopupRenderer
|
||||
}
|
||||
return renderer
|
||||
}
|
||||
|
||||
private val comboBoxModifier
|
||||
get() = Modifier.testTag(COMBO_BOX_TAG).width(240.dp)
|
||||
}
|
||||
|
||||
private const val COMBO_BOX_TAG = "Jewel.Test.ComboBox"
|
||||
private const val CHEVRON_INSET = 8f
|
||||
+64
@@ -0,0 +1,64 @@
|
||||
// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||
package org.jetbrains.jewel.ui.component
|
||||
|
||||
import androidx.compose.foundation.shape.CornerSize
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.ui.input.key.KeyEvent
|
||||
import androidx.compose.ui.unit.IntSize
|
||||
import androidx.compose.ui.window.PopupPositionProvider
|
||||
import androidx.compose.ui.window.PopupProperties
|
||||
|
||||
/**
|
||||
* A [PopupRenderer] that records the `onDismissRequest` it is handed, and renders nothing.
|
||||
*
|
||||
* Renderers treat a null `onDismissRequest` as "this popup does not want renderer-driven dismissal", and must then
|
||||
* neither dismiss nor consume the key. Components that instead pass a callback which decides whether to dismiss look
|
||||
* identical from here, so the renderer swallows Escape while nothing closes: this renderer makes that distinction
|
||||
* observable in a headless test.
|
||||
*
|
||||
* The content is deliberately dropped rather than composed inline. A real popup is its own window, so composing it into
|
||||
* the caller's layout instead would let it overlap the component that opened it and swallow the pointer events a hover
|
||||
* test depends on.
|
||||
*/
|
||||
internal class RecordingPopupRenderer : PopupRenderer {
|
||||
val dismissRequests: MutableList<(() -> Unit)?> = mutableListOf()
|
||||
val properties: MutableList<PopupProperties> = mutableListOf()
|
||||
|
||||
@Suppress("OVERRIDE_DEPRECATION")
|
||||
@Composable
|
||||
override fun Popup(
|
||||
popupPositionProvider: PopupPositionProvider,
|
||||
properties: PopupProperties,
|
||||
onDismissRequest: (() -> Unit)?,
|
||||
onPreviewKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
onKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
cornerSize: CornerSize,
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
Popup(
|
||||
popupPositionProvider = popupPositionProvider,
|
||||
properties = properties,
|
||||
onDismissRequest = onDismissRequest,
|
||||
onPreviewKeyEvent = onPreviewKeyEvent,
|
||||
onKeyEvent = onKeyEvent,
|
||||
cornerSize = cornerSize,
|
||||
windowShape = null,
|
||||
content = content,
|
||||
)
|
||||
}
|
||||
|
||||
@Composable
|
||||
override fun Popup(
|
||||
popupPositionProvider: PopupPositionProvider,
|
||||
properties: PopupProperties,
|
||||
onDismissRequest: (() -> Unit)?,
|
||||
onPreviewKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
onKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
cornerSize: CornerSize,
|
||||
windowShape: ((IntSize) -> java.awt.Shape)?,
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
dismissRequests += onDismissRequest
|
||||
this.properties += properties
|
||||
}
|
||||
}
|
||||
+19
-54
@@ -1,15 +1,9 @@
|
||||
// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||
package org.jetbrains.jewel.ui.component
|
||||
|
||||
import androidx.compose.foundation.shape.CornerSize
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.ui.input.key.KeyEvent
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import androidx.compose.ui.unit.IntSize
|
||||
import androidx.compose.ui.window.PopupPositionProvider
|
||||
import androidx.compose.ui.window.PopupProperties
|
||||
import kotlin.test.assertNull
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
|
||||
import org.jetbrains.jewel.foundation.JewelFlags
|
||||
@@ -22,18 +16,31 @@ class SpeedSearchAreaDismissRequestTest {
|
||||
@get:Rule val composeRule = createComposeRule()
|
||||
|
||||
@Test
|
||||
fun `passes null dismiss request when dismiss on lose focus is disabled`() {
|
||||
fun `suppresses outside dismissal when dismiss on lose focus is disabled`() {
|
||||
val renderer = recordSpeedSearchPopup(dismissOnLoseFocus = false)
|
||||
|
||||
assertTrue(renderer.dismissRequests.isNotEmpty())
|
||||
assertNull(renderer.dismissRequests.last())
|
||||
assertTrue(renderer.properties.isNotEmpty(), "The popup was never rendered")
|
||||
assertFalse(renderer.properties.last().dismissOnClickOutside, "Outside dismissal should be suppressed")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `passes dismiss request when dismiss on lose focus is enabled`() {
|
||||
fun `allows outside dismissal when dismiss on lose focus is enabled`() {
|
||||
val renderer = recordSpeedSearchPopup(dismissOnLoseFocus = true)
|
||||
|
||||
assertTrue(renderer.dismissRequests.any { it != null })
|
||||
assertTrue(renderer.properties.last().dismissOnClickOutside, "Outside dismissal should be allowed")
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `keeps escape enabled regardless of dismiss on lose focus`() {
|
||||
for (dismissOnLoseFocus in listOf(false, true)) {
|
||||
val renderer = recordSpeedSearchPopup(dismissOnLoseFocus = dismissOnLoseFocus)
|
||||
|
||||
assertTrue(
|
||||
renderer.properties.last().dismissOnBackPress,
|
||||
"Escape must always hide the search input, whatever the outside-dismissal policy is",
|
||||
)
|
||||
assertTrue(renderer.dismissRequests.last() != null, "Speed search must always be able to hide its input")
|
||||
}
|
||||
}
|
||||
|
||||
private fun recordSpeedSearchPopup(dismissOnLoseFocus: Boolean): RecordingPopupRenderer {
|
||||
@@ -62,45 +69,3 @@ class SpeedSearchAreaDismissRequestTest {
|
||||
return renderer
|
||||
}
|
||||
}
|
||||
|
||||
private class RecordingPopupRenderer : PopupRenderer {
|
||||
val dismissRequests = mutableListOf<(() -> Unit)?>()
|
||||
|
||||
@Suppress("OVERRIDE_DEPRECATION")
|
||||
@Composable
|
||||
override fun Popup(
|
||||
popupPositionProvider: PopupPositionProvider,
|
||||
properties: PopupProperties,
|
||||
onDismissRequest: (() -> Unit)?,
|
||||
onPreviewKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
onKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
cornerSize: CornerSize,
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
Popup(
|
||||
popupPositionProvider = popupPositionProvider,
|
||||
properties = properties,
|
||||
onDismissRequest = onDismissRequest,
|
||||
onPreviewKeyEvent = onPreviewKeyEvent,
|
||||
onKeyEvent = onKeyEvent,
|
||||
cornerSize = cornerSize,
|
||||
windowShape = null,
|
||||
content = content,
|
||||
)
|
||||
}
|
||||
|
||||
@Composable
|
||||
override fun Popup(
|
||||
popupPositionProvider: PopupPositionProvider,
|
||||
properties: PopupProperties,
|
||||
onDismissRequest: (() -> Unit)?,
|
||||
onPreviewKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
onKeyEvent: ((KeyEvent) -> Boolean)?,
|
||||
cornerSize: CornerSize,
|
||||
windowShape: ((IntSize) -> java.awt.Shape)?,
|
||||
content: @Composable () -> Unit,
|
||||
) {
|
||||
dismissRequests += onDismissRequest
|
||||
content()
|
||||
}
|
||||
}
|
||||
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
// Copyright 2000-2026 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
|
||||
package org.jetbrains.jewel.ui.component.gotit
|
||||
|
||||
import androidx.compose.runtime.CompositionLocalProvider
|
||||
import androidx.compose.ui.test.junit4.v2.createComposeRule
|
||||
import kotlin.test.assertFalse
|
||||
import kotlin.test.assertTrue
|
||||
import org.jetbrains.jewel.foundation.ExperimentalJewelApi
|
||||
import org.jetbrains.jewel.foundation.JewelFlags
|
||||
import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme
|
||||
import org.jetbrains.jewel.ui.component.LocalPopupRenderer
|
||||
import org.jetbrains.jewel.ui.component.RecordingPopupRenderer
|
||||
import org.junit.Rule
|
||||
import org.junit.Test
|
||||
|
||||
/**
|
||||
* `GotItTooltip` handles Escape itself, on its anchor rather than inside the popup, so it has to declare that the
|
||||
* renderer should keep its hands off the key.
|
||||
*
|
||||
* Without this, a renderer that honours the default `dismissOnBackPress = true` will dismiss the popup — invoking a
|
||||
* callback that does nothing, since the tooltip's visibility is owned by the caller — and consume Escape on the way, so
|
||||
* the anchor's own handler never runs and the tooltip stays on screen. That was measured against the IDE bridge
|
||||
* renderer, which is the one with no window-ownership check to fall back on.
|
||||
*/
|
||||
@OptIn(ExperimentalJewelApi::class)
|
||||
class GotItTooltipDismissPolicyTest {
|
||||
@get:Rule val composeRule = createComposeRule()
|
||||
|
||||
@Test
|
||||
fun `opts out of renderer-driven escape dismissal`() {
|
||||
val renderer = RecordingPopupRenderer()
|
||||
val oldUseCustomPopupRenderer = JewelFlags.useCustomPopupRenderer
|
||||
JewelFlags.useCustomPopupRenderer = true
|
||||
try {
|
||||
composeRule.setContent {
|
||||
IntUiTheme {
|
||||
CompositionLocalProvider(LocalPopupRenderer provides renderer) {
|
||||
GotItTooltip(text = "Body", visible = true, onDismiss = {}) {}
|
||||
}
|
||||
}
|
||||
}
|
||||
composeRule.waitForIdle()
|
||||
} finally {
|
||||
JewelFlags.useCustomPopupRenderer = oldUseCustomPopupRenderer
|
||||
}
|
||||
|
||||
assertTrue(renderer.properties.isNotEmpty(), "The tooltip popup was never rendered")
|
||||
assertFalse(
|
||||
renderer.properties.last().dismissOnBackPress,
|
||||
"GotItTooltip handles Escape on its anchor, so the renderer must neither dismiss nor consume it",
|
||||
)
|
||||
assertFalse(
|
||||
renderer.properties.last().dismissOnClickOutside,
|
||||
"GotItTooltip is dismissed by its own buttons or timeout, not by clicking away",
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -389,11 +389,7 @@ internal fun ComboBoxImpl(
|
||||
val maxHeight = maxPopupHeight.takeOrElse { style.metrics.maxPopupHeight }
|
||||
|
||||
PopupContainer(
|
||||
onDismissRequest = {
|
||||
if (!chevronHovered) {
|
||||
popupManager.setPopupVisible(false)
|
||||
}
|
||||
},
|
||||
onDismissRequest = { popupManager.setPopupVisible(false) },
|
||||
modifier =
|
||||
Modifier.testTag("Jewel.ComboBox.Popup")
|
||||
.heightIn(max = maxHeight)
|
||||
@@ -401,7 +397,10 @@ internal fun ComboBoxImpl(
|
||||
.then(popupModifier)
|
||||
.onClick { popupManager.setPopupVisible(false) },
|
||||
horizontalAlignment = horizontalPopupAlignment,
|
||||
popupProperties = PopupProperties(focusable = false),
|
||||
// Suppressing the pointer path while the ComboBox is hovered stops the click-outside
|
||||
// dismissal from firing on pointer down, before the chevron's own toggle handler runs and
|
||||
// reopens the popup it just closed. Escape is governed separately, and stays enabled.
|
||||
popupProperties = PopupProperties(focusable = false, dismissOnClickOutside = !chevronHovered),
|
||||
style = popupStyle,
|
||||
popupPositionProvider = popupPositionProvider,
|
||||
content = popupContent,
|
||||
|
||||
+4
-6
@@ -191,11 +191,7 @@ public fun EditableComboBox(
|
||||
val popupVisible by popupManager.isPopupVisible
|
||||
if (popupVisible) {
|
||||
PopupContainer(
|
||||
onDismissRequest = {
|
||||
if (!chevronHovered && !textFieldHovered) {
|
||||
popupManager.setPopupVisible(false)
|
||||
}
|
||||
},
|
||||
onDismissRequest = { popupManager.setPopupVisible(false) },
|
||||
modifier =
|
||||
Modifier.testTag("Jewel.ComboBox.Popup")
|
||||
.semantics { contentDescription = "Jewel.EditableComboBox.Popup" }
|
||||
@@ -204,7 +200,9 @@ public fun EditableComboBox(
|
||||
.then(popupModifier)
|
||||
.onClick { popupManager.setPopupVisible(false) },
|
||||
horizontalAlignment = Alignment.Start,
|
||||
popupProperties = PopupProperties(focusable = false),
|
||||
// See ComboBox: only the pointer path is suppressed while hovered; Escape stays enabled.
|
||||
popupProperties =
|
||||
PopupProperties(focusable = false, dismissOnClickOutside = !chevronHovered && !textFieldHovered),
|
||||
content = popupContent,
|
||||
)
|
||||
}
|
||||
|
||||
+5
-1
@@ -49,6 +49,7 @@ import androidx.compose.ui.layout.onFirstVisible
|
||||
import androidx.compose.ui.platform.testTag
|
||||
import androidx.compose.ui.text.TextStyle
|
||||
import androidx.compose.ui.unit.dp
|
||||
import androidx.compose.ui.window.PopupProperties
|
||||
import androidx.compose.ui.window.rememberComponentRectPositionProvider
|
||||
import java.awt.event.KeyEvent as AWTKeyEvent
|
||||
import kotlinx.coroutines.CoroutineDispatcher
|
||||
@@ -415,7 +416,10 @@ private fun SpeedSearchInput(
|
||||
|
||||
Popup(
|
||||
popupPositionProvider = rememberComponentRectPositionProvider(anchor, alignment),
|
||||
onDismissRequest = if (dismissOnLoseFocus) ({ speedSearchState.hideSearch() }) else null,
|
||||
onDismissRequest = { speedSearchState.hideSearch() },
|
||||
// Only the outside-click path is conditional. Escape always hides the search input, which is what makes
|
||||
// the first Escape belong to speed search and the second to whatever the search is layered over.
|
||||
properties = PopupProperties(dismissOnClickOutside = dismissOnLoseFocus),
|
||||
) {
|
||||
val focusRequester = remember { FocusRequester() }
|
||||
|
||||
|
||||
+3
-1
@@ -275,7 +275,9 @@ public fun GotItTooltip(
|
||||
popupPositionProvider =
|
||||
rememberGotItTooltipBalloonPopupPositionProvider(gotItBalloonPosition, anchor, padding = offset),
|
||||
cornerSize = CornerSize(style.metrics.cornerRadius),
|
||||
properties = PopupProperties(focusable = false, dismissOnClickOutside = false),
|
||||
// Escape is handled on the anchor above, so the renderer must neither dismiss nor consume it.
|
||||
properties =
|
||||
PopupProperties(focusable = false, dismissOnBackPress = false, dismissOnClickOutside = false),
|
||||
onDismissRequest = {},
|
||||
windowShape = { logicalSize ->
|
||||
createBalloonAwtShape(
|
||||
|
||||
Reference in New Issue
Block a user