From 6e788f989a9c26f1d4e80daf645ef589ea4187f6 Mon Sep 17 00:00:00 2001 From: Sebastiano Poggi Date: Wed, 12 Aug 2026 19:21:53 +0200 Subject: [PATCH] [JEWEL-1396] Keep Escape in Jewel ComboBox popups Handle Escape in a speed-search ComboBox before it reaches the popup, so the first press clears the query and the second closes the popup. Route native key events by popup ownership, while still forwarding owner-window events for focusable popups. Avoid renderer dismissal callbacks when speed-search focus loss dismissal is disabled, and make non-native popup renderers explicitly ignore window shapes. Add renderer contract and headful Spectre coverage.  Conflicts:  platform/jewel/int-ui/int-ui-standalone-tests/build.gradle.kts closes https://github.com/JetBrains/intellij-community/pull/3610 (cherry picked from commit 75292aa91e3903a659f914f03c7d20f92c1da906) IJ-MR-220560 GitOrigin-RevId: ee54893658771cfec864854f2c0cc00171f007b8 --- platform/jewel/gradle/libs.versions.toml | 3 + .../jewel/bridge/component/JBPopupRenderer.kt | 24 ++ .../int-ui-standalone-tests/build.gradle.kts | 35 ++- .../popup/CustomPopupRendererSpectreTest.kt | 212 ++++++++++++++++++ .../intui/standalone/popup/JDialogRenderer.kt | 15 +- .../showcase/components/SpeedSearches.kt | 29 ++- .../jewel/ui/component/ComboBoxTest.kt | 40 ++++ .../SpeedSearchAreaDismissRequestTest.kt | 106 +++++++++ .../search/SpeedSearchableComboBoxTest.kt | 6 +- .../org/jetbrains/jewel/ui/component/Popup.kt | 77 ++++++- .../jewel/ui/component/SpeedSearchArea.kt | 15 +- .../search/SpeedSearchableComboBox.kt | 4 +- 12 files changed, 544 insertions(+), 22 deletions(-) create mode 100644 platform/jewel/int-ui/int-ui-standalone-tests/src/spectreTest/kotlin/org/jetbrains/jewel/intui/standalone/popup/CustomPopupRendererSpectreTest.kt create mode 100644 platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchAreaDismissRequestTest.kt diff --git a/platform/jewel/gradle/libs.versions.toml b/platform/jewel/gradle/libs.versions.toml index a14716848530..0087ebc4b47a 100644 --- a/platform/jewel/gradle/libs.versions.toml +++ b/platform/jewel/gradle/libs.versions.toml @@ -22,6 +22,7 @@ ktfmtGradlePlugin = "0.23.0" metalava = "1.0.0-alpha13" jsoup = "1.21.2" mockk = "1.14.5" +spectre = "0.5.0" [libraries] assertj-core = { module = "org.assertj:assertj-core", version.ref = "assertJ"} @@ -52,6 +53,8 @@ ktor-client-java = { module = "io.ktor:ktor-client-java", version = "3.0.3" } metalava = { module = "com.android.tools.metalava:metalava", version.ref = "metalava" } jsoup = { module = "org.jsoup:jsoup", version.ref = "jsoup" } mockk = { module = "io.mockk:mockk", version.ref = "mockk" } +spectre-core = { module = "dev.sebastiano.spectre:spectre-core", version.ref = "spectre" } +spectre-testing = { module = "dev.sebastiano.spectre:spectre-testing", version.ref = "spectre" } # Plugin libraries for build-logic's convention plugins to use to resolve the types/tasks coming from these plugins detekt-gradlePlugin = { module = "dev.detekt:detekt-gradle-plugin", version.ref = "detekt" } diff --git a/platform/jewel/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/component/JBPopupRenderer.kt b/platform/jewel/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/component/JBPopupRenderer.kt index 271d3bc24968..19801f1618bb 100644 --- a/platform/jewel/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/component/JBPopupRenderer.kt +++ b/platform/jewel/ide-laf-bridge/src/main/kotlin/org/jetbrains/jewel/bridge/component/JBPopupRenderer.kt @@ -71,6 +71,30 @@ internal object JBPopupRenderer : PopupRenderer { 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, + ) { + // JBPopup does not expose native window shaping, so windowShape is ignored. JBPopup( popupPositionProvider = popupPositionProvider, onDismissRequest = onDismissRequest, diff --git a/platform/jewel/int-ui/int-ui-standalone-tests/build.gradle.kts b/platform/jewel/int-ui/int-ui-standalone-tests/build.gradle.kts index 73e4962d9fbd..4fffa7d3b3cb 100644 --- a/platform/jewel/int-ui/int-ui-standalone-tests/build.gradle.kts +++ b/platform/jewel/int-ui/int-ui-standalone-tests/build.gradle.kts @@ -4,9 +4,13 @@ plugins { alias(libs.plugins.compose.compiler) } +val spectreTest = sourceSets.create("spectreTest") { java.srcDir("src/spectreTest/kotlin") } + dependencies { api(projects.intUi.intUiStandalone) + testImplementation(projects.foundation) + testImplementation(projects.ui) testImplementation(kotlin("test")) testImplementation(libs.junit.jupiter) testRuntimeOnly(libs.junit.platform.engine) @@ -15,6 +19,35 @@ dependencies { testImplementation(libs.kotlinx.coroutines.test) testImplementation(compose.desktop.currentOs) { exclude(group = "org.jetbrains.compose.material") } testImplementation(libs.jna.core) + + "spectreTestImplementation"(projects.foundation) + "spectreTestImplementation"(projects.ui) + "spectreTestImplementation"(projects.intUi.intUiStandalone) + "spectreTestImplementation"(compose.desktop.currentOs) { exclude(group = "org.jetbrains.compose.material") } + "spectreTestImplementation"(libs.spectre.core) + "spectreTestImplementation"(libs.spectre.testing) + "spectreTestImplementation"(kotlin("test")) + "spectreTestImplementation"(libs.junit.jupiter) + "spectreTestImplementation"(libs.kotlinx.coroutines.core) + "spectreTestRuntimeOnly"(libs.junit.platform.engine) + "spectreTestRuntimeOnly"(libs.junit.platform.launcher) } -tasks.test { useJUnitPlatform() } +val jdkLevel = project.property("jdk.level") as String + +tasks { + named("test") { useJUnitPlatform() } + + register("spectreTest") { + group = "verification" + description = "Runs opt-in headful Compose Desktop UI tests with Spectre." + testClassesDirs = spectreTest.output.classesDirs + classpath = spectreTest.runtimeClasspath + useJUnitPlatform() + maxParallelForks = 1 + javaLauncher = project.javaToolchains.launcherFor { languageVersion = JavaLanguageVersion.of(jdkLevel) } + systemProperty("apple.awt.UIElement", "true") + systemProperty("java.awt.headless", "false") + systemProperty("jewel.customPopupRender", "true") + } +} diff --git a/platform/jewel/int-ui/int-ui-standalone-tests/src/spectreTest/kotlin/org/jetbrains/jewel/intui/standalone/popup/CustomPopupRendererSpectreTest.kt b/platform/jewel/int-ui/int-ui-standalone-tests/src/spectreTest/kotlin/org/jetbrains/jewel/intui/standalone/popup/CustomPopupRendererSpectreTest.kt new file mode 100644 index 000000000000..4a9e34e185c0 --- /dev/null +++ b/platform/jewel/int-ui/int-ui-standalone-tests/src/spectreTest/kotlin/org/jetbrains/jewel/intui/standalone/popup/CustomPopupRendererSpectreTest.kt @@ -0,0 +1,212 @@ +@file:OptIn(ExperimentalJewelApi::class) + +package org.jetbrains.jewel.intui.standalone.popup + +import androidx.compose.foundation.layout.Arrangement +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.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.platform.testTag +import androidx.compose.ui.unit.dp +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.AtomicReference +import kotlin.concurrent.thread +import kotlin.test.assertTrue +import kotlinx.coroutines.CoroutineScope +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.PopupManager +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.rememberSpeedSearchState +import org.jetbrains.jewel.ui.component.search.SpeedSearchableComboBox +import org.junit.jupiter.api.Test + +// Note: these tests run intentionally Gradle-only until JEWEL-1390 +class CustomPopupRendererSpectreTest { + @Test + fun `escape closes a hovered combo box`(): Unit = runSpectreTestWithCustomPopupRenderer { + val app = SpectreTestApplication() + app.start() + try { + 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") + + // 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") + } finally { + app.stop() + } + } + + @Test + fun `escape closes a focusable menu when the owner window receives the key`(): Unit = + runSpectreTestWithCustomPopupRenderer { + val app = SpectreTestApplication() + app.start() + try { + val automator = ComposeAutomator.inProcess(RobotDriver.synthetic(app.awaitWindow())) + + val menuButton = automator.waitForNode(tag = MENU_BUTTON_TAG) + automator.click(menuButton) + automator.waitForNode(tag = MENU_VISIBLE_TAG, text = "true") + + // 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. + automator.moveTo(menuButton) + automator.waitForIdle() + + automator.pressKey(KeyEvent.VK_ESCAPE) + automator.waitForNode(tag = MENU_VISIBLE_TAG, text = "false") + } finally { + app.stop() + } + } + + @Test + fun `escape prioritizes speed search when dismiss on lose focus is disabled`(): Unit = + runSpectreTestWithCustomPopupRenderer { + val app = SpectreTestApplication() + app.start() + try { + 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.typeText("Alpha") + automator.waitForNode(tag = SPEED_SEARCH_QUERY_TAG, text = "Alpha") + automator.waitForNode(tag = SPEED_SEARCH_VISIBLE_TAG, text = "true") + + 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.pressKey(KeyEvent.VK_ESCAPE) + automator.waitForNode(tag = SPEED_SEARCH_POPUP_VISIBLE_TAG, text = "false") + } finally { + app.stop() + } + } +} + +private fun runSpectreTestWithCustomPopupRenderer(block: suspend CoroutineScope.() -> Unit): Unit = runSpectreTest { + assertTrue(JewelFlags.useCustomPopupRenderer, "spectreTest must enable JDialogRenderer") + block() +} + +@Composable +private fun PopupEscapeScreen() { + val regularPopupManager = remember { PopupManager() } + 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, + ) { + Text("Regular popup content") + } + + Text(text = menuVisible.toString(), modifier = Modifier.testTag(MENU_VISIBLE_TAG)) + Box { + DefaultButton(onClick = { menuVisible = true }, modifier = Modifier.testTag(MENU_BUTTON_TAG)) { + Text("Show menu") + } + + if (menuVisible) { + PopupMenu( + onDismissRequest = { + menuVisible = false + true + }, + horizontalAlignment = Alignment.Start, + ) { + 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 }, + ) + } + } +} + +private class SpectreTestApplication(private val content: @Composable () -> Unit = { PopupEscapeScreen() }) { + private val exitApplication = AtomicReference<(() -> Unit)?>(null) + private val window = AtomicReference(null) + + fun start() { + thread(name = "spectre-popup-test-window", isDaemon = true) { + application(exitProcessOnExit = false) { + exitApplication.set(::exitApplication) + JewelWindow(onCloseRequest = ::exitApplication, title = "Jewel Spectre popup test") { + this@SpectreTestApplication.window.compareAndSet(null, window) + IntUiTheme { content() } + } + } + } + } + + fun stop() { + exitApplication.get()?.invoke() + } + + suspend fun awaitWindow(): ComposeWindow { + repeat(100) { + window.get()?.let { + return it + } + delay(100) + } + error("The Compose test window was not created") + } +} + +private const val REGULAR_COMBO_TAG = "spectre.regularCombo" +private const val REGULAR_POPUP_VISIBLE_TAG = "spectre.regularPopupVisible" +private const val MENU_BUTTON_TAG = "spectre.menuButton" +private const val MENU_VISIBLE_TAG = "spectre.menuVisible" +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" diff --git a/platform/jewel/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/popup/JDialogRenderer.kt b/platform/jewel/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/popup/JDialogRenderer.kt index 63bb75f2421c..73c8a50892e7 100644 --- a/platform/jewel/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/popup/JDialogRenderer.kt +++ b/platform/jewel/int-ui/int-ui-standalone/src/main/kotlin/org/jetbrains/jewel/intui/standalone/popup/JDialogRenderer.kt @@ -418,16 +418,25 @@ private fun JPopupImpl( } } is AWTKeyEvent -> { - if (!dialog.isVisible) return@AWTEventListener + val eventWindow = SwingUtilities.getWindowAncestor(event.component) ?: event.component as? Window + // Focusable popups do not necessarily own native focus immediately (see the JEWEL-1276 + // focus-stealing workaround below), so their key events can arrive from the owner window. + val ownsEvent = eventWindow == dialog || (currentProperties.focusable && eventWindow == window) + if (event.isConsumed || !dialog.isVisible || !ownsEvent) return@AWTEventListener val composeEvent = event.toComposeKeyEvent() val consumed = currentOnPreviewKeyEvent?.invoke(composeEvent) == true || currentOnKeyEvent?.invoke(composeEvent) == true + val dismissRequest = currentOnDismissRequest + val dismissed = !consumed && composeEvent.isDismissRequest() && dismissRequest != null - if (!consumed && composeEvent.isDismissRequest()) { - currentOnDismissRequest?.invoke() + if (dismissed) { + dismissRequest.invoke() + } + if (consumed || dismissed) { + event.consume() } } } diff --git a/platform/jewel/samples/showcase/src/main/kotlin/org/jetbrains/jewel/samples/showcase/components/SpeedSearches.kt b/platform/jewel/samples/showcase/src/main/kotlin/org/jetbrains/jewel/samples/showcase/components/SpeedSearches.kt index 1d379fe171f2..62a9cdb73219 100644 --- a/platform/jewel/samples/showcase/src/main/kotlin/org/jetbrains/jewel/samples/showcase/components/SpeedSearches.kt +++ b/platform/jewel/samples/showcase/src/main/kotlin/org/jetbrains/jewel/samples/showcase/components/SpeedSearches.kt @@ -9,7 +9,9 @@ import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth +import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.widthIn @@ -18,6 +20,7 @@ import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableIntStateOf import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember import androidx.compose.runtime.setValue @@ -34,6 +37,8 @@ import org.jetbrains.jewel.foundation.lazy.tree.rememberTreeState import org.jetbrains.jewel.foundation.search.filter import org.jetbrains.jewel.foundation.theme.JewelTheme import org.jetbrains.jewel.foundation.util.JewelLogger +import org.jetbrains.jewel.ui.Orientation +import org.jetbrains.jewel.ui.component.Divider import org.jetbrains.jewel.ui.component.GroupHeader import org.jetbrains.jewel.ui.component.InlineWarningBanner import org.jetbrains.jewel.ui.component.SimpleListItem @@ -41,6 +46,7 @@ import org.jetbrains.jewel.ui.component.SpeedSearchArea import org.jetbrains.jewel.ui.component.Text import org.jetbrains.jewel.ui.component.VerticalScrollbar import org.jetbrains.jewel.ui.component.rememberSpeedSearchState +import org.jetbrains.jewel.ui.component.search.SpeedSearchableComboBox import org.jetbrains.jewel.ui.component.search.SpeedSearchableLazyColumn import org.jetbrains.jewel.ui.component.search.SpeedSearchableTree import org.jetbrains.jewel.ui.component.search.highlightSpeedSearchMatches @@ -62,8 +68,15 @@ internal fun SpeedSearches(modifier: Modifier = Modifier) { Modifier.widthIn(max = 200.dp).weight(1f, fill = false).semantics { isTraversalGroup = true }, verticalArrangement = Arrangement.spacedBy(8.dp), ) { + GroupHeader(text = "Combo box", modifier = Modifier.fillMaxWidth()) + SpeedSearchComboBoxExample() + + Spacer(Modifier.height(8.dp)) + Divider(Orientation.Horizontal, Modifier.fillMaxWidth()) + Spacer(Modifier.height(8.dp)) + GroupHeader(text = "Tree", modifier = Modifier.fillMaxWidth()) - SpeedSearchTreeExample() + SpeedSearchTreeExample(Modifier.weight(1f)) } Column( @@ -85,6 +98,20 @@ internal fun SpeedSearches(modifier: Modifier = Modifier) { } } +@Composable +private fun SpeedSearchComboBoxExample(modifier: Modifier = Modifier) { + var selectedIndex by remember { mutableIntStateOf(0) } + + SpeedSearchArea(modifier) { + SpeedSearchableComboBox( + items = TEST_LIST, + selectedIndex = selectedIndex, + onSelectedItemChange = { selectedIndex = it }, + modifier = Modifier.fillMaxWidth(), + ) + } +} + /** * Demonstrates speed search functionality in a tree structure. * diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ComboBoxTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ComboBoxTest.kt index 3ef70808664c..81f32235efd9 100644 --- a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ComboBoxTest.kt +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/ComboBoxTest.kt @@ -2,12 +2,18 @@ package org.jetbrains.jewel.ui.component import androidx.compose.foundation.layout.Arrangement +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.layout.widthIn import androidx.compose.runtime.mutableStateOf import androidx.compose.ui.Modifier +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.test.SemanticsNodeInteraction import androidx.compose.ui.test.assertHeightIsEqualTo @@ -21,8 +27,10 @@ import androidx.compose.ui.test.performClick import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.size +import kotlin.test.assertFalse import kotlin.test.assertTrue import org.jetbrains.jewel.intui.standalone.theme.IntUiTheme +import org.jetbrains.jewel.ui.component.interactions.performKeyPress import org.junit.Rule import org.junit.Test @@ -35,6 +43,38 @@ class ComboBoxTest { private val comboBox: SemanticsNodeInteraction get() = composeRule.onNodeWithTag("ComboBox") + @Test + fun `escape closes popup without propagating to parent`() { + var receivedEscape = false + composeRule.setContent { + IntUiTheme { + Box( + modifier = + Modifier.onKeyEvent { + if (it.type == KeyEventType.KeyDown && it.key == Key.Escape) { + receivedEscape = true + } + false + } + ) { + ComboBox( + labelText = "Just a label", + modifier = Modifier.testTag("ComboBox"), + popupContent = { Text("Popup content") }, + ) + } + } + } + + comboBox.performClick() + popupMenu.assertIsDisplayed() + + comboBox.performKeyPress(Key.Escape, rule = composeRule) + + popupMenu.assertDoesNotExist() + assertFalse(receivedEscape) + } + @Test fun `popup width can be bigger than combo box width when nothing is set`() { composeRule.setContent { diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchAreaDismissRequestTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchAreaDismissRequestTest.kt new file mode 100644 index 000000000000..b83d0150d571 --- /dev/null +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchAreaDismissRequestTest.kt @@ -0,0 +1,106 @@ +// 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.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 + +@OptIn(ExperimentalJewelApi::class) +class SpeedSearchAreaDismissRequestTest { + @get:Rule val composeRule = createComposeRule() + + @Test + fun `passes null dismiss request when dismiss on lose focus is disabled`() { + val renderer = recordSpeedSearchPopup(dismissOnLoseFocus = false) + + assertTrue(renderer.dismissRequests.isNotEmpty()) + assertNull(renderer.dismissRequests.last()) + } + + @Test + fun `passes dismiss request when dismiss on lose focus is enabled`() { + val renderer = recordSpeedSearchPopup(dismissOnLoseFocus = true) + + assertTrue(renderer.dismissRequests.any { it != null }) + } + + private fun recordSpeedSearchPopup(dismissOnLoseFocus: Boolean): RecordingPopupRenderer { + val renderer = RecordingPopupRenderer() + var speedSearchState: SpeedSearchState? = null + val oldUseCustomPopupRenderer = JewelFlags.useCustomPopupRenderer + JewelFlags.useCustomPopupRenderer = true + try { + composeRule.setContent { + IntUiTheme { + CompositionLocalProvider(LocalPopupRenderer provides renderer) { + val state = rememberSpeedSearchState() + speedSearchState = state + + SpeedSearchArea(state = state, dismissOnLoseFocus = dismissOnLoseFocus) { + Text("Searchable content") + } + } + } + } + composeRule.runOnIdle { speedSearchState?.isVisible = true } + composeRule.waitForIdle() + } finally { + JewelFlags.useCustomPopupRenderer = oldUseCustomPopupRenderer + } + 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() + } +} diff --git a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBoxTest.kt b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBoxTest.kt index f555654328a1..e6f76720f26f 100644 --- a/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBoxTest.kt +++ b/platform/jewel/ui-tests/src/test/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBoxTest.kt @@ -77,7 +77,7 @@ class SpeedSearchableComboBoxTest { } @Test - fun `should hide on esc press`() = runComposeTest { + fun `should hide speed search before closing the popup on esc press`() = runComposeTest { comboBox.performClick() comboBox.performKeyPress("Item", rule = this) @@ -85,6 +85,10 @@ class SpeedSearchableComboBoxTest { comboBox.performKeyPress(Key.Escape, rule = this) onSpeedSearchAreaInput.assertDoesNotExist() + onNodeWithTag("Jewel.ComboBox.Popup").assertIsDisplayed() + + comboBox.performKeyPress(Key.Escape, rule = this) + onNodeWithTag("Jewel.ComboBox.Popup").assertDoesNotExist() } @Test diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Popup.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Popup.kt index 13caa5bc4285..18f4dcf1d3ae 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Popup.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/Popup.kt @@ -27,6 +27,7 @@ import androidx.compose.ui.unit.LayoutDirection import androidx.compose.ui.window.Popup as ComposePopup import androidx.compose.ui.window.PopupPositionProvider import androidx.compose.ui.window.PopupProperties +import java.awt.Shape import org.jetbrains.jewel.foundation.JewelFlags /** @@ -89,10 +90,10 @@ public fun Popup( * consume the event. * @param onKeyEvent Callback invoked for key events after they are dispatched to children. Return `true` to consume the * event. - * @param windowShape An optional factory that produces the [java.awt.Shape] used to clip the native popup window. The - * lambda receives the window's measured size in AWT logical units and **must** return a shape in the same coordinate - * system. Only applied by JDialogRenderer when `useCustomPopupRenderer = true` and the panel is not transparent (be - * it by enabling `compose.interop.blending` or setting the alpha channel of the panel's background to 0); all other + * @param windowShape An optional factory that produces the [Shape] used to clip the native popup window. The lambda + * receives the window's measured size in AWT logical units and **must** return a shape in the same coordinate system. + * Only applied by JDialogRenderer when `useCustomPopupRenderer = true` and the panel is not transparent (be it by + * enabling `compose.interop.blending` or setting the alpha channel of the panel's background to 0); all other * renderers ignore it. When null, window clipping falls back to the `cornerSize`-based rounded corners (via JBR) if * the platform supports it. * @param content The composable content to be displayed inside the popup. @@ -104,7 +105,7 @@ public fun Popup( properties: PopupProperties = PopupProperties(), onPreviewKeyEvent: ((KeyEvent) -> Boolean)? = null, onKeyEvent: ((KeyEvent) -> Boolean)? = null, - windowShape: ((IntSize) -> java.awt.Shape)? = null, + windowShape: ((IntSize) -> Shape)? = null, content: @Composable () -> Unit, ) { Popup( @@ -184,10 +185,10 @@ public fun Popup( * consume the event. * @param onKeyEvent Callback invoked for key events after they are dispatched to children. Return `true` to consume the * event. - * @param windowShape An optional factory that produces the [java.awt.Shape] used to clip the native popup window. The - * lambda receives the window's measured size in AWT logical units and must return a shape in the same coordinate - * system. Only applied by JDialogRenderer when `useCustomPopupRenderer = true` and the panel is not transparent (be - * it by enabling `compose.interop.blending` or setting the alpha channel of the panel's background to 0); all other + * @param windowShape An optional factory that produces the [Shape] used to clip the native popup window. The lambda + * receives the window's measured size in AWT logical units and must return a shape in the same coordinate system. + * Only applied by `JDialogRenderer` when `useCustomPopupRenderer = true` and the panel is not transparent (be it by + * enabling `compose.interop.blending` or setting the alpha channel of the panel's background to 0); all other * renderers ignore it. When null, window clipping falls back to the `cornerSize`-based rounded corners (via JBR) if * the platform supports it. * @param content The composable content to be displayed inside the popup. @@ -200,7 +201,7 @@ public fun Popup( properties: PopupProperties = PopupProperties(), onPreviewKeyEvent: ((KeyEvent) -> Boolean)? = null, onKeyEvent: ((KeyEvent) -> Boolean)? = null, - windowShape: ((IntSize) -> java.awt.Shape)? = null, + windowShape: ((IntSize) -> Shape)? = null, content: @Composable () -> Unit, ) { DisableSelection { @@ -236,6 +237,11 @@ public fun Popup( * [JewelFlags.useCustomPopupRenderer] flag to use it. */ public interface PopupRenderer { + /** + * Compatibility overload without native window-shape support. Implement the shaped overload instead. + * + * Parameters match the shaped overload. + */ @Deprecated(message = "Please use the overload with windowShape.") @Composable public fun Popup( @@ -248,6 +254,19 @@ public interface PopupRenderer { content: @Composable () -> Unit, ) + /** + * Renders a popup, optionally clipping its native window to [windowShape]. Only native-window renderers apply the + * shape; the default fallback delegates to the deprecated overload and drops it. + * + * @param popupPositionProvider Determines the popup position. + * @param properties Popup focus and dismissal behavior. + * @param onDismissRequest Callback invoked when dismissal is requested. + * @param onPreviewKeyEvent Preview key handler; return `true` to consume an event. + * @param onKeyEvent Key handler; return `true` to consume an event. + * @param cornerSize Popup corner size. + * @param windowShape Native shape factory in AWT logical units, or `null` for the default shape. + * @param content Popup content. + */ @Composable public fun Popup( popupPositionProvider: PopupPositionProvider, @@ -256,9 +275,10 @@ public interface PopupRenderer { onPreviewKeyEvent: ((KeyEvent) -> Boolean)?, onKeyEvent: ((KeyEvent) -> Boolean)?, cornerSize: CornerSize, - windowShape: ((IntSize) -> java.awt.Shape)? = null, + windowShape: ((IntSize) -> Shape)? = null, content: @Composable () -> Unit, ) { + @Suppress("DEPRECATION") Popup( popupPositionProvider = popupPositionProvider, properties = properties, @@ -270,6 +290,7 @@ public interface PopupRenderer { ) } + /** Companion object for [PopupRenderer]. Currently empty; kept as an extension point for extension functions. */ public companion object } @@ -283,7 +304,14 @@ public val LocalPopupRenderer: ProvidableCompositionLocal = stati } private object DefaultPopupRenderer : PopupRenderer { - @Suppress("OVERRIDE_DEPRECATION") + @Deprecated( + "Please use the overload with windowShape.", + ReplaceWith( + "Popup(popupPositionProvider, properties, onDismissRequest, onPreviewKeyEvent, onKeyEvent, cornerSize, " + + "windowShape = null, content)", + "org.jetbrains.jewel.ui.component.DefaultPopupRenderer.Popup", + ), + ) @Composable override fun Popup( popupPositionProvider: PopupPositionProvider, @@ -294,6 +322,31 @@ private object DefaultPopupRenderer : PopupRenderer { 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) -> Shape)?, + content: @Composable () -> Unit, + ) { + // Compose popups are rendered in the owner window and do not support native window shapes, so + // windowShape is ignored. ComposePopup( popupPositionProvider = popupPositionProvider, onDismissRequest = onDismissRequest, diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchArea.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchArea.kt index 59a0fc822f3c..33bf9d55fa83 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchArea.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/SpeedSearchArea.kt @@ -112,7 +112,8 @@ public fun SpeedSearchArea( * @param textStyle The text style for the search input text. * @param searchMatchStyle The styling for highlighting matched text in search results. * @param interactionSource The interaction source for tracking focus state. If null, a new one will be created. - * @param dismissOnLoseFocus Whether to automatically hide the search input when it loses focus. Defaults to true. + * @param dismissOnLoseFocus Whether focus loss or renderer-level outside dismissal hides the search input. Escape + * behavior is defined by the content's [SpeedSearchScope.processKeyEvent] implementation. Defaults to true. * @param content The content to be displayed within the speed search area. Use [SpeedSearchScope] to access search * state and process key events. */ @@ -157,7 +158,8 @@ public fun SpeedSearchArea( * @param textStyle The text style for the search input text. * @param searchMatchStyle The styling for highlighting matched text in search results. * @param interactionSource The interaction source for tracking focus state. If null, a new one will be created. - * @param dismissOnLoseFocus Whether to automatically hide the search input when it loses focus. Defaults to true. + * @param dismissOnLoseFocus Whether focus loss or renderer-level outside dismissal hides the search input. Escape + * behavior is defined by the content's [SpeedSearchScope.processKeyEvent] implementation. Defaults to true. * @param content The content to be displayed within the speed search area. Use [SpeedSearchScope] to access search * state and process key events. */ @@ -190,9 +192,11 @@ public fun SpeedSearchArea( if (state.isVisible) { SpeedSearchInput( + speedSearchState = state, state = state.textFieldState, hasMatch = state.hasMatches, position = state.position, + dismissOnLoseFocus = dismissOnLoseFocus, styling = styling, textStyle = textStyle, textFieldStyle = textFieldStyle, @@ -390,9 +394,11 @@ public fun ProvideSearchMatchState( @Composable private fun SpeedSearchInput( + speedSearchState: SpeedSearchState, state: TextFieldState, hasMatch: Boolean, position: Alignment.Vertical, + dismissOnLoseFocus: Boolean, styling: SpeedSearchStyle, textStyle: TextStyle, textFieldStyle: TextFieldStyle, @@ -407,7 +413,10 @@ private fun SpeedSearchInput( } } - Popup(popupPositionProvider = rememberComponentRectPositionProvider(anchor, alignment)) { + Popup( + popupPositionProvider = rememberComponentRectPositionProvider(anchor, alignment), + onDismissRequest = if (dismissOnLoseFocus) ({ speedSearchState.hideSearch() }) else null, + ) { val focusRequester = remember { FocusRequester() } BasicTextField( diff --git a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBox.kt b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBox.kt index 1ccfac0c1ffe..a3234d62eb70 100644 --- a/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBox.kt +++ b/platform/jewel/ui/src/main/kotlin/org/jetbrains/jewel/ui/component/search/SpeedSearchableComboBox.kt @@ -299,7 +299,9 @@ private fun SpeedSearchScope.SpeedSearchableComboBoxImpl( modifier.onPreviewKeyEvent { event -> if (!popupVisible) return@onPreviewKeyEvent false - if (!processKeyEvent(event) && speedSearchState.isVisibleAndNotEmpty) { + if (processKeyEvent(event)) return@onPreviewKeyEvent true + + if (speedSearchState.isVisibleAndNotEmpty) { val actionHandled = speedSearchKeyActions .handleOnKeyEvent(