From 6469e9c19694d63bfedcba7d69df244ab9bf7d14 Mon Sep 17 00:00:00 2001 From: kirillk Date: Sat, 4 Jul 2026 11:57:35 -0400 Subject: [PATCH] fix(jetbrains): make settings list action buttons fully clickable Hit-testing for inline action cells (Connect/OAuth/Disconnect/Enable) re-derived cell rectangles by hand, ignoring the horizontal insets the platform SelectablePanel adds in the New UI. The click target was offset from the drawn button, so only a small strip responded to clicks. Read the rectangles back from the actual rendered component tree instead, giving one source of geometry shared by the provider, agent, and MCP settings lists. --- .../fix-jetbrains-provider-action-clicks.md | 5 ++ .../client/settings/base/SettingsListModel.kt | 90 ++++++++++--------- .../settings/base/SettingsListRenderer.kt | 4 + .../client/settings/base/SettingsListView.kt | 4 +- .../settings/agents/AgentsSettingsUiTest.kt | 8 +- .../settings/agents/McpSettingsUiTest.kt | 4 +- .../settings/base/SettingsListViewTest.kt | 6 +- .../providers/ProvidersSettingsUiTest.kt | 71 +++++++++------ 8 files changed, 109 insertions(+), 83 deletions(-) create mode 100644 .changeset/fix-jetbrains-provider-action-clicks.md diff --git a/.changeset/fix-jetbrains-provider-action-clicks.md b/.changeset/fix-jetbrains-provider-action-clicks.md new file mode 100644 index 0000000000..c026cc8bb1 --- /dev/null +++ b/.changeset/fix-jetbrains-provider-action-clicks.md @@ -0,0 +1,5 @@ +--- +"@kilocode/kilo-jetbrains": patch +--- + +Fix unreliable clicks on inline action buttons (Connect, OAuth, Disconnect, Enable) in the JetBrains provider, agent, and MCP settings lists so the whole button is clickable. diff --git a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListModel.kt b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListModel.kt index 01e106ec8a..7e6a57389f 100644 --- a/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListModel.kt +++ b/packages/kilo-jetbrains/frontend/src/main/kotlin/ai/kilocode/client/settings/base/SettingsListModel.kt @@ -2,11 +2,14 @@ package ai.kilocode.client.settings.base import ai.kilocode.client.ui.UiStyle import com.intellij.util.ui.JBUI -import java.awt.Dimension +import java.awt.Component +import java.awt.Container import java.awt.Point import java.awt.Rectangle import javax.swing.Icon import javax.swing.JList +import javax.swing.ListCellRenderer +import javax.swing.SwingUtilities private const val CELL_GAP = 8 @@ -57,55 +60,62 @@ internal fun settingsListVisibleCells(item: SettingsListItem, selected: Boolean) return item.cells.filter { selected || it.alwaysVisible } } +internal fun settingsListCellGap() = JBUI.scale(CELL_GAP) + +/** + * Clickable action-cell rectangles for a row, in list coordinates. + * + * The rectangles are read back from the actual rendered component tree instead of being + * re-derived by hand. This keeps the click targets identical to what the [SettingsListRenderer] + * draws — including the horizontal insets the platform's [com.intellij.ui.popup.list.SelectablePanel] + * adds in the New UI, which a hand-computed layout would miss. + */ +internal fun settingsListCellBounds( + list: JList<*>, + index: Int, + selected: Boolean, +): Map { + val model = list.model + if (index < 0 || index >= model.size) return emptyMap() + @Suppress("UNCHECKED_CAST") + val renderer = list.cellRenderer as? ListCellRenderer ?: return emptyMap() + val cell = list.getCellBounds(index, index) ?: return emptyMap() + val comp = renderer.getListCellRendererComponent(list, model.getElementAt(index), index, selected, list.hasFocus()) + comp.setBounds(0, 0, cell.width, cell.height) + settingsListLayout(comp) + val out = linkedMapOf() + for (action in settingsListActionCells(comp)) { + val origin = SwingUtilities.convertPoint(action, 0, 0, comp) + out[action.cellId] = Rectangle(cell.x + origin.x, cell.y + origin.y, action.width, action.height) + } + return out +} + internal fun settingsListCellAt( list: JList<*>, - bounds: Rectangle, + index: Int, point: Point, - item: SettingsListItem, selected: Boolean, ): String? { - val cells = settingsListCellBounds(list, bounds, item, selected) + val item = list.model.getElementAt(index) as? SettingsListItem ?: return null + val cells = settingsListCellBounds(list, index, selected) return settingsListVisibleCells(item, selected) .firstOrNull { cell -> cell.enabled && cells[cell.id]?.contains(point) == true } ?.id } -internal fun settingsListCellBounds( - list: JList<*>, - bounds: Rectangle, - item: SettingsListItem, - selected: Boolean, -): Map { - val height = settingsListCellHeight(list) - var edge = bounds.x + bounds.width - UiStyle.Gap.pad() - val out = linkedMapOf() - for (cell in settingsListVisibleCells(item, selected).asReversed()) { - val size = settingsListCellSize(list, cell) - val width = size.width - val h = height.coerceAtLeast(size.height) - val top = bounds.y + (bounds.height - h) / 2 - val left = edge - width - out[cell.id] = Rectangle(left, top, width, h) - edge = left - JBUI.scale(CELL_GAP) +private fun settingsListLayout(component: Component) { + if (component !is Container) return + component.doLayout() + for (child in component.components) settingsListLayout(child) +} + +private fun settingsListActionCells(component: Component): List { + val out = mutableListOf() + fun visit(c: Component) { + if (c is SettingsListActionCell && c.isVisible) out += c + if (c is Container) c.components.forEach(::visit) } + visit(component) return out } - -internal fun settingsListCellSize(list: JList<*>, cell: SettingsListCell): Dimension { - val label = SettingsListActionCell().apply { - update(cell) - font = list.font - isEnabled = cell.enabled - } - val size = label.preferredSize - if (!cell.iconOnly) return size - val min = settingsListCellHeight(list) - return Dimension(size.width.coerceAtLeast(min), size.height.coerceAtLeast(min)) -} - -private fun settingsListCellHeight(list: JList<*>): Int { - val metrics = list.getFontMetrics(list.font) - return metrics.height + UiStyle.Gap.sm() * 2 -} - -internal fun settingsListCellGap() = JBUI.scale(CELL_GAP) 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 04da2eb039..314e2c64b7 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 @@ -133,7 +133,11 @@ internal class SettingsListRenderer( } internal class SettingsListActionCell : JBLabel() { + var cellId: String = "" + private set + fun update(cell: SettingsListCell) { + cellId = cell.id text = if (cell.iconOnly) "" else cell.label icon = cell.icon toolTipText = cell.label.takeIf { it.isNotBlank() } 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 b42042473f..661ac90103 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 @@ -203,9 +203,9 @@ internal class SettingsListView( val item = model.getElementAt(idx) val selected = idx == list.selectedIndex val id = if (enabled) { - settingsListCellAt(list, bounds, e.point, item, selected) + settingsListCellAt(list, idx, e.point, selected) } else { - settingsListCellBounds(list, bounds, item, selected) + settingsListCellBounds(list, idx, selected) .entries .firstOrNull { it.value.contains(e.point) } ?.key diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/AgentsSettingsUiTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/AgentsSettingsUiTest.kt index 71912dbbfa..55ee443275 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/AgentsSettingsUiTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/AgentsSettingsUiTest.kt @@ -286,9 +286,7 @@ class AgentsSettingsUiTest : BasePlatformTestCase() { list.doLayout() val idx = rows(panel).indexOfFirst { it.key == "hidden" } list.selectedIndex = idx - val row = rows(panel)[idx] - val bounds = list.getCellBounds(idx, idx) - val area = settingsListCellBounds(list, bounds, row, selected = true).getValue(DELETE_CELL) + val area = settingsListCellBounds(list, idx, selected = true).getValue(DELETE_CELL) click(list, center(area)) true } @@ -483,9 +481,7 @@ class AgentsSettingsUiTest : BasePlatformTestCase() { list.doLayout() val idx = rows(panel).indexOfFirst { it.key == key } list.selectedIndex = idx - val row = rows(panel)[idx] - val bounds = list.getCellBounds(idx, idx) - val area = settingsListCellBounds(list, bounds, row, selected = true).getValue(cell) + val area = settingsListCellBounds(list, idx, selected = true).getValue(cell) click(list, center(area)) } diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/McpSettingsUiTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/McpSettingsUiTest.kt index ddaf9ef93f..aff659e9fb 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/McpSettingsUiTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/agents/McpSettingsUiTest.kt @@ -350,9 +350,7 @@ class McpSettingsUiTest : BasePlatformTestCase() { list.doLayout() val idx = rows(panel).indexOfFirst { it.key == key } list.selectedIndex = idx - val row = rows(panel)[idx] - val bounds = list.getCellBounds(idx, idx) - val area = settingsListCellBounds(list, bounds, row, selected = true).getValue(id) + val area = settingsListCellBounds(list, idx, selected = true).getValue(id) click(list, center(area)) true } 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 6bc0e25ae5..1dffba4081 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 @@ -176,8 +176,7 @@ class SettingsListViewTest : BasePlatformTestCase() { view.list.doLayout() UIUtil.dispatchAllInvocationEvents() - val bounds = view.list.getCellBounds(0, 0) - val area = settingsListCellBounds(view.list, bounds, row, selected = true).getValue("edit") + val area = settingsListCellBounds(view.list, 0, selected = true).getValue("edit") val point = Point(area.x + area.width - 1, area.y + area.height - 1) click(view, point) @@ -218,8 +217,7 @@ class SettingsListViewTest : BasePlatformTestCase() { view.list.doLayout() UIUtil.dispatchAllInvocationEvents() - val bounds = view.list.getCellBounds(0, 0) - val area = settingsListCellBounds(view.list, bounds, row, selected = true).getValue("edit") + val area = settingsListCellBounds(view.list, 0, selected = true).getValue("edit") click(view, center(area)) diff --git a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUiTest.kt b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUiTest.kt index 5db81bbf41..6c6da0923c 100644 --- a/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUiTest.kt +++ b/packages/kilo-jetbrains/frontend/src/test/kotlin/ai/kilocode/client/settings/providers/ProvidersSettingsUiTest.kt @@ -1,6 +1,7 @@ package ai.kilocode.client.settings.providers import ai.kilocode.client.app.KiloProviderService +import ai.kilocode.client.settings.base.SettingsListConfig import ai.kilocode.client.settings.base.SettingsListItem import ai.kilocode.client.settings.base.SettingsListRenderer import ai.kilocode.client.settings.base.SettingsListActionCell @@ -366,36 +367,33 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() { fun `test renderer hit test maps actions`() { edt { val row = ProviderListRow(provider("cloudflare", "Cloudflare"), "All providers", listOf(ProviderListAction.OAUTH, ProviderListAction.CONNECT)) - val list = JBList(listOf(row)) - val bounds = Rectangle(0, 0, 320, 48) - val areas = actionBounds(list, bounds, row, selected = true) + val list = hitList(row) + val areas = actionBounds(list, selected = true) - assertEquals(ProviderListAction.CONNECT, actionAt(list, bounds, center(areas.getValue(ProviderListAction.CONNECT)), row, selected = true)) - assertEquals(ProviderListAction.OAUTH, actionAt(list, bounds, center(areas.getValue(ProviderListAction.OAUTH)), row, selected = true)) - assertNull(actionAt(list, bounds, Point(4, 4), row, selected = true)) - assertTrue(actionBounds(list, bounds, row, selected = false).isEmpty()) + assertEquals(ProviderListAction.CONNECT, actionAt(list, center(areas.getValue(ProviderListAction.CONNECT)), selected = true)) + assertEquals(ProviderListAction.OAUTH, actionAt(list, center(areas.getValue(ProviderListAction.OAUTH)), selected = true)) + assertNull(actionAt(list, Point(4, 4), selected = true)) + assertTrue(actionBounds(list, selected = false).isEmpty()) } } fun `test renderer keeps connected disconnect action visible when unselected`() { edt { val row = ProviderListRow(provider("openai", "OpenAI"), "Connected providers", listOf(ProviderListAction.DISCONNECT), connected = true) - val list = JBList(listOf(row)) - val bounds = Rectangle(0, 0, 320, 48) - val area = actionBounds(list, bounds, row, selected = false).getValue(ProviderListAction.DISCONNECT) + val list = hitList(row) + val area = actionBounds(list, selected = false).getValue(ProviderListAction.DISCONNECT) - assertEquals(ProviderListAction.DISCONNECT, actionAt(list, bounds, center(area), row, selected = false)) + assertEquals(ProviderListAction.DISCONNECT, actionAt(list, center(area), selected = false)) } } fun `test renderer ignores disabled env disconnect action`() { edt { val row = ProviderListRow(provider("env", "Env", source = "env"), "All providers", listOf(ProviderListAction.DISCONNECT)) - val list = JBList(listOf(row)) - val bounds = Rectangle(0, 0, 320, 48) - val area = actionBounds(list, bounds, row, selected = true).getValue(ProviderListAction.DISCONNECT) + val list = hitList(row) + val area = actionBounds(list, selected = true).getValue(ProviderListAction.DISCONNECT) - assertNull(actionAt(list, bounds, center(area), row, selected = true)) + assertNull(actionAt(list, center(area), selected = true)) } } @@ -444,15 +442,14 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() { fun `test disabled provider rows hide action labels and hit targets`() { edt { val row = ProviderListRow(provider("cloudflare", "Cloudflare"), "All providers", listOf(ProviderListAction.OAUTH, ProviderListAction.CONNECT), disabled = true) - val list = JBList(listOf(row)) - val bounds = Rectangle(0, 0, 320, 48) + val list = hitList(row) val renderer = renderer(row) render(renderer, list, row, selected = true) assertTrue(visibleActions(row, selected = true).isEmpty()) - assertTrue(actionBounds(list, bounds, row, selected = true).isEmpty()) - assertNull(actionAt(list, bounds, Point(300, 24), row, selected = true)) + assertTrue(actionBounds(list, selected = true).isEmpty()) + assertNull(actionAt(list, Point(300, 24), selected = true)) assertTrue(actionTexts(renderer).isEmpty()) } } @@ -529,15 +526,22 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() { } } - fun `test action bounds are vertically centered`() { + fun `test action hit target spans the full rendered button`() { edt { val row = ProviderListRow(provider("openai", "OpenAI"), "Popular providers", listOf(ProviderListAction.CONNECT)) - val list = JBList(listOf(row)) - val bounds = Rectangle(0, 10, 320, 80) - val area = actionBounds(list, bounds, row, selected = true).getValue(ProviderListAction.CONNECT) + val list = hitList(row) + val bounds = list.getCellBounds(0, 0) + val area = actionBounds(list, selected = true).getValue(ProviderListAction.CONNECT) - assertTrue(kotlin.math.abs((bounds.y + bounds.height / 2) - (area.y + area.height / 2)) <= 1) assertTrue(bounds.contains(area)) + // The button is right-aligned within the row. + assertTrue(area.x >= bounds.x + bounds.width / 2) + // Every horizontal slice of the drawn button resolves to the action, including the left + // edge that regressed when hit-testing ignored the New UI selection insets. + val y = area.y + area.height / 2 + assertEquals(ProviderListAction.CONNECT, actionAt(list, Point(area.x + 1, y), selected = true)) + assertEquals(ProviderListAction.CONNECT, actionAt(list, Point(area.x + area.width - 1, y), selected = true)) + assertNull(actionAt(list, Point(area.x - 2, y), selected = true)) } } @@ -900,13 +904,24 @@ class ProvidersSettingsUiTest : BasePlatformTestCase() { .filter { it.iconWidth == JBUI.scale(20) && it.iconHeight == JBUI.scale(20) } .map { Dimension(it.iconWidth, it.iconHeight) } - private fun actionAt(list: JBList, bounds: Rectangle, point: Point, row: ProviderListRow, selected: Boolean): ProviderListAction? { - val id = settingsListCellAt(list, bounds, point, row, selected) ?: return null + /** Builds a list wired with the real [SettingsListRenderer] and laid out, so hit-testing matches what is drawn. */ + private fun hitList(row: ProviderListRow): JBList { + val model = CollectionListModel(listOf(row)) + val list = JBList(model) + list.cellRenderer = SettingsListRenderer(model as CollectionListModel, SettingsListConfig.Preferred) + list.size = Dimension(320, 200) + list.doLayout() + UIUtil.dispatchAllInvocationEvents() + return list + } + + private fun actionAt(list: JBList, point: Point, selected: Boolean): ProviderListAction? { + val id = settingsListCellAt(list, 0, point, selected) ?: return null return ProviderListAction.entries.firstOrNull { it.name == id } } - private fun actionBounds(list: JBList, bounds: Rectangle, row: ProviderListRow, selected: Boolean): Map { - val cells = settingsListCellBounds(list, bounds, row, selected) + private fun actionBounds(list: JBList, selected: Boolean): Map { + val cells = settingsListCellBounds(list, 0, selected) return cells.mapNotNull { (id, rect) -> ProviderListAction.entries.firstOrNull { it.name == id }?.let { it to rect } }.toMap() }