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.
2026-08-28 19:11:03 +08:00 · 2026-07-04 11:57:35 -04:00
parent eb59ad6b13
commit 6469e9c196
8 changed files with 109 additions and 83 deletions
@@ -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.
@@ -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<String, Rectangle> {
+    val model = list.model
+    if (index < 0 || index >= model.size) return emptyMap()
+    @Suppress("UNCHECKED_CAST")
+    val renderer = list.cellRenderer as? ListCellRenderer<Any?> ?: 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<String, Rectangle>()
+    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<String, Rectangle> {
-    val height = settingsListCellHeight(list)
-    var edge = bounds.x + bounds.width - UiStyle.Gap.pad()
-    val out = linkedMapOf<String, Rectangle>()
-    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<SettingsListActionCell> {
+    val out = mutableListOf<SettingsListActionCell>()
+    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)
@@ -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() }
@@ -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
@@ -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))
    }

@@ -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
        }
@@ -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))

@@ -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<ProviderListRow>, 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<ProviderListRow> {
+        val model = CollectionListModel<ProviderListRow>(listOf(row))
+        val list = JBList(model)
+        list.cellRenderer = SettingsListRenderer(model as CollectionListModel<SettingsListItem>, SettingsListConfig.Preferred)
+        list.size = Dimension(320, 200)
+        list.doLayout()
+        UIUtil.dispatchAllInvocationEvents()
+        return list
+    }
+
+    private fun actionAt(list: JBList<ProviderListRow>, 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<ProviderListRow>, bounds: Rectangle, row: ProviderListRow, selected: Boolean): Map<ProviderListAction, Rectangle> {
-        val cells = settingsListCellBounds(list, bounds, row, selected)
+    private fun actionBounds(list: JBList<ProviderListRow>, selected: Boolean): Map<ProviderListAction, Rectangle> {
+        val cells = settingsListCellBounds(list, 0, selected)
        return cells.mapNotNull { (id, rect) -> ProviderListAction.entries.firstOrNull { it.name == id }?.let { it to rect } }.toMap()
    }