From 851967b3daaac5a87debe59085042f3e22e81daf Mon Sep 17 00:00:00 2001 From: kirillk Date: Mon, 20 Jul 2026 17:42:03 -0400 Subject: [PATCH] fix(jetbrains): keep settings row active during popups --- .../autoapprove/SettingsInlineList.kt | 13 +++--- .../settings/base/SettingsInlineListPanel.kt | 7 ++++ .../settings/base/SettingsListRenderer.kt | 6 ++- .../client/settings/base/SettingsListView.kt | 41 +++++++++++++------ .../autoapprove/AutoApproveSettingsUiTest.kt | 5 ++- .../autoapprove/SettingsInlineListTest.kt | 9 ++-- .../settings/base/SettingsListViewTest.kt | 38 ++++++++--------- 7 files changed, 72 insertions(+), 47 deletions(-) diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineList.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineList.kt index 4423d534f4..c0c2b599ee 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineList.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineList.kt @@ -10,6 +10,7 @@ import ai.kilocode.client.settings.base.settingsListCellBounds import com.intellij.icons.AllIcons import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.ui.Messages +import com.intellij.openapi.ui.popup.JBPopup import com.intellij.openapi.ui.popup.JBPopupFactory import com.intellij.ui.SimpleListCellRenderer import com.intellij.ui.awt.RelativePoint @@ -38,18 +39,16 @@ internal sealed interface LevelChoice { * cell; tests substitute a picker that resolves a choice directly. */ internal fun interface LevelPicker { - fun show(anchor: JComponent, at: Point, choices: List, choose: (LevelChoice) -> Unit) + fun popup(choices: List, choose: (LevelChoice) -> Unit): JBPopup? } internal object PopupLevelPicker : LevelPicker { - override fun show(anchor: JComponent, at: Point, choices: List, choose: (LevelChoice) -> Unit) { + override fun popup(choices: List, choose: (LevelChoice) -> Unit): JBPopup = JBPopupFactory.getInstance() .createPopupChooserBuilder(choices) .setRenderer(SimpleListCellRenderer.create("") { levelChoiceLabel(it) }) .setItemChosenCallback(choose) .createPopup() - .show(RelativePoint(anchor, at)) - } } internal fun levelChoiceLabel(choice: LevelChoice): String = when (choice) { @@ -142,9 +141,9 @@ internal class SettingsInlineList( val item = item(key) ?: return val idx = index(key) ?: return val bounds = settingsListCellBounds(view.list, idx, idx == view.list.selectedIndex)[LEVEL_CELL] ?: return - picker.show(view.list, Point(bounds.x, bounds.y + bounds.height), choices(item.row)) { choice -> - choose(key, choice) - } + val popup = picker.popup(choices(item.row)) { choice -> choose(key, choice) } ?: return + trackPopup(popup) + popup.show(RelativePoint(view.list, Point(bounds.x, bounds.y + bounds.height))) } private fun item(key: String): PermissionItem? { diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsInlineListPanel.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsInlineListPanel.kt index db3c9e26e7..9236ebd8ac 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsInlineListPanel.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsInlineListPanel.kt @@ -8,6 +8,7 @@ import com.intellij.openapi.actionSystem.ActionToolbar import com.intellij.openapi.actionSystem.AnAction import com.intellij.openapi.actionSystem.DefaultActionGroup import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.ui.popup.JBPopup import com.intellij.ui.DocumentAdapter import com.intellij.ui.SearchTextField import com.intellij.util.concurrency.annotations.RequiresEdt @@ -62,6 +63,12 @@ internal abstract class SettingsInlineListPanel( view.filter(query) } + @RequiresEdt + protected fun trackPopup(popup: JBPopup) { + checkEdt() + view.trackPopup(popup) + } + @RequiresEdt fun setItems(items: List, enabled: Boolean) { checkEdt() diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListRenderer.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListRenderer.kt index c9f0f794c8..349f6f0d3c 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListRenderer.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListRenderer.kt @@ -68,7 +68,7 @@ internal class SettingsListRenderer( selected: Boolean, focused: Boolean, ): JPanel { - val active = selected && list.hasFocus() + val active = selected && (list.hasFocus() || (list as? SettingsListActive)?.active() == true) val fg = UIUtil.getListForeground(active, active || focused) val weak = if (active) fg else UiStyle.Colors.weak() val current = model.items.getOrNull(index) @@ -132,6 +132,10 @@ internal class SettingsListRenderer( } } +internal interface SettingsListActive { + fun active(): Boolean +} + internal class SettingsListActionCell : JBLabel() { var cellId: String = "" private set diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListView.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListView.kt index 499146cfee..13fce43bb4 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListView.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListView.kt @@ -2,11 +2,13 @@ package ai.kilocode.client.settings.base import ai.kilocode.client.session.ui.model.ModelSearch import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.ui.popup.JBPopup +import com.intellij.openapi.ui.popup.JBPopupListener +import com.intellij.openapi.ui.popup.LightweightWindowEvent import com.intellij.ui.CollectionListModel import com.intellij.ui.ScrollingUtil import com.intellij.ui.components.JBList import com.intellij.util.concurrency.annotations.RequiresEdt -import com.intellij.xml.util.XmlStringUtil import com.intellij.util.ui.UIUtil import java.awt.event.KeyEvent import java.awt.event.FocusAdapter @@ -24,17 +26,8 @@ internal class SettingsListView( private val onCell: (String, String) -> Unit, ) : BaseContentPanel() { private val model = CollectionListModel() - internal val list = object : JBList(model) { - override fun getToolTipText(event: MouseEvent): String? { - if (!cfg.description) return null - val idx = locationToIndex(event.point) - if (idx < 0) return null - val bounds = getCellBounds(idx, idx) ?: return null - if (!bounds.contains(event.point)) return null - val note = model.getElementAt(idx).description?.takeIf { it.isNotBlank() } ?: return null - val text = note.lines().joinToString("
") { XmlStringUtil.escapeString(it) } - return XmlStringUtil.wrapInHtml(text) - } + internal val list: JBList = object : JBList(model), SettingsListActive { + override fun active(): Boolean = popups > 0 }.apply { selectionMode = ListSelectionModel.SINGLE_SELECTION setExpandableItemsEnabled(false) @@ -43,6 +36,7 @@ internal class SettingsListView( private var items = emptyList() private var filter = "" private var press: Press? = null + private var popups = 0 internal var onSelect: (() -> Unit)? = null fun setEmptyText(text: String) { @@ -136,6 +130,29 @@ internal class SettingsListView( list.repaint() } + @RequiresEdt + fun trackPopup(popup: JBPopup) { + checkEdt() + var tracked = false + fun activate() { + if (tracked) return + tracked = true + popups++ + list.repaint() + } + popup.addListener(object : JBPopupListener { + override fun beforeShown(event: LightweightWindowEvent) = activate() + + override fun onClosed(event: LightweightWindowEvent) { + if (!tracked) return + tracked = false + popups = maxOf(0, popups - 1) + list.repaint() + } + }) + if (popup.isVisible) activate() + } + @RequiresEdt fun filter(query: String) { checkEdt() diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/AutoApproveSettingsUiTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/AutoApproveSettingsUiTest.kt index c0db7fd63a..0c3e81ce01 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/AutoApproveSettingsUiTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/AutoApproveSettingsUiTest.kt @@ -40,7 +40,10 @@ class AutoApproveSettingsUiTest : BasePlatformTestCase() { private lateinit var workspaces: KiloWorkspaceService private var ui: AutoApproveSettingsUi? = null private var pick: (List) -> LevelChoice = { it.first() } - private val picker = LevelPicker { _, _, choices, choose -> choose(pick(choices)) } + private val picker = LevelPicker { choices, choose -> + choose(pick(choices)) + null + } override fun setUp() { super.setUp() diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineListTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineListTest.kt index 6432ed16e9..78aa8cbf22 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineListTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/autoapprove/SettingsInlineListTest.kt @@ -4,6 +4,7 @@ import ai.kilocode.client.ui.UiStyle import ai.kilocode.client.settings.base.SettingsListItem import ai.kilocode.client.settings.base.settingsListCellBounds import com.intellij.openapi.application.ApplicationManager +import com.intellij.openapi.ui.popup.JBPopup import com.intellij.testFramework.fixtures.BasePlatformTestCase import com.intellij.ui.components.JBList import com.intellij.util.ui.UIUtil @@ -148,14 +149,10 @@ class SettingsInlineListTest : BasePlatformTestCase() { var offered: List = emptyList() private set - override fun show( - anchor: JComponent, - at: Point, - choices: List, - choose: (LevelChoice) -> Unit, - ) { + override fun popup(choices: List, choose: (LevelChoice) -> Unit): JBPopup? { offered = choices choose(select(choices)) + return null } } diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/base/SettingsListViewTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/base/SettingsListViewTest.kt index 2d46cbd5cb..f5c5700bfc 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/base/SettingsListViewTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/base/SettingsListViewTest.kt @@ -19,7 +19,7 @@ import java.awt.event.MouseEvent import javax.swing.SwingUtilities class SettingsListViewTest : BasePlatformTestCase() { - fun `test list owns formatted description tooltip`() { + fun `test list does not duplicate description in tooltip`() { edt { val view = SettingsListView("Empty") { _, _ -> } val row = item("with", "Alpha", "Use text\nAcross lines") @@ -31,25 +31,7 @@ class SettingsListViewTest : BasePlatformTestCase() { val bounds = view.list.getCellBounds(0, 0) val tip = view.list.getToolTipText(event(view.list, Point(bounds.x + 4, bounds.y + 4))) - assertNotNull(tip) - assertTrue(tip, tip!!.startsWith("")) - assertTrue(tip, tip.contains("Use <safe> text")) - assertTrue(tip, tip.contains("
Across lines")) - } - } - - fun `test list description tooltip ignores blank rows and outside points`() { - edt { - val view = SettingsListView("Empty") { _, _ -> } - view.update(listOf(item("without", "Beta", null))) - view.list.size = Dimension(320, 80) - view.list.doLayout() - UIUtil.dispatchAllInvocationEvents() - - val bounds = view.list.getCellBounds(0, 0) - - assertNull(view.list.getToolTipText(event(view.list, Point(bounds.x + 4, bounds.y + 4)))) - assertNull(view.list.getToolTipText(event(view.list, Point(4, bounds.y + bounds.height + 20)))) + assertNull(tip) } } @@ -252,6 +234,22 @@ class SettingsListViewTest : BasePlatformTestCase() { } } + fun `test active popup paints selected row as active without focus`() { + edt { + val row = item("with", "Alpha", "Description") + val model = CollectionListModel(listOf(row)) + val list = object : JBList(model), SettingsListActive { + override fun active(): Boolean = true + } + val renderer = SettingsListRenderer(model, SettingsListConfig.Equal) + + renderer.getListCellRendererComponent(list, row, 0, true, false) + + val desc = components(renderer).filterIsInstance().single { it.text == "Description" } + assertEquals(UIUtil.getListForeground(true, true), desc.foreground) + } + } + fun `test preserve no scroll keeps scroll position after row change`() { edt { val view = SettingsListView("Empty") { _, _ -> }