mirror of
https://github.com/mark3labs/kit.git
synced 2026-06-13 19:20:06 +00:00
fix(ui): correct mouse selection drift with extension widgets
- Match View() and getItemAndLineAtY() row counts for empty items so streaming-reasoning placeholders no longer offset hit-testing by one row each (exposed when extension widgets like subagent-monitor shrink the scrollback). - Honor IsLineInRange's endCol=-1 'to end of line' sentinel in HighlightLine and ExtractText so the start row of a multi-line drag actually renders highlighted and is included in clipboard copies. - Add regression tests for both invariants in scrolllist and selection.
This commit is contained in:
@@ -538,6 +538,21 @@ func (s *ScrollList) View() string {
|
||||
for idx := s.offsetIdx; idx < len(s.items) && remainingHeight > 0; idx++ {
|
||||
item := s.items[idx]
|
||||
content := item.Render(s.width)
|
||||
|
||||
// Items that render to an empty string contribute zero height to
|
||||
// the viewport. This MUST match renderedHeight()'s semantics —
|
||||
// otherwise getItemAndLineAtY (which uses renderedHeight) treats
|
||||
// the item as 0 lines while View() emits one blank line via
|
||||
// strings.Split("", "\n") = [""], producing a 1-row downward
|
||||
// drift in mouse hit-testing per empty item between offsetIdx
|
||||
// and the cursor (most visibly streaming-reasoning items before
|
||||
// any reasoning has streamed, which extension widgets surface by
|
||||
// shrinking the scrollback).
|
||||
if content == "" {
|
||||
s.heightCache[item.ID()] = 0
|
||||
continue
|
||||
}
|
||||
|
||||
contentLines := strings.Split(content, "\n")
|
||||
|
||||
// Refresh height cache from the actual render (authoritative).
|
||||
|
||||
@@ -130,3 +130,52 @@ func TestScrollList_SetItemsRespectsMouseDown(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestScrollList_EmptyItemsDoNotShiftMouseMapping is the regression test
|
||||
// for the second drift bug: items that render to "" must contribute the
|
||||
// same number of rows in View() (zero) as in renderedHeight(), or mouse
|
||||
// hit-testing drifts by one row per empty item between offsetIdx and the
|
||||
// cursor. This was surfaced by extension widgets (e.g. subagent-monitor)
|
||||
// that shrink the scrollback so empty streaming-reasoning items end up
|
||||
// in the visible window.
|
||||
//
|
||||
// Setup: 1 normal item + 1 empty item + 1 normal item. Click on the line
|
||||
// where the third item begins. With the bug, getItemAndLineAtY skips the
|
||||
// empty item (renderedHeight=0) and reports lineIdx pointing one row
|
||||
// past where View() actually painted that line.
|
||||
func TestScrollList_EmptyItemsDoNotShiftMouseMapping(t *testing.T) {
|
||||
sl := NewScrollList(80, 10)
|
||||
sl.SetItems([]MessageItem{
|
||||
&fakeItem{id: "a", lines: 2}, // viewY 0–1
|
||||
&fakeItem{id: "empty", lines: 0}, // renders "" — contributes 0 rows
|
||||
&fakeItem{id: "b", lines: 2}, // viewY 2–3
|
||||
})
|
||||
|
||||
// Render the viewport once so the cache reflects what View() actually
|
||||
// emits (this is the path that previously diverged from renderedHeight
|
||||
// for empty items).
|
||||
rendered := sl.View()
|
||||
lines := strings.Split(rendered, "\n")
|
||||
|
||||
// Sanity: View() must emit exactly height lines.
|
||||
if len(lines) != 10 {
|
||||
t.Fatalf("View() returned %d lines, want 10", len(lines))
|
||||
}
|
||||
// Item b's first line should appear at viewY=2, NOT viewY=3.
|
||||
if !strings.Contains(lines[2], "b-line-0") {
|
||||
t.Errorf("viewY=2 should render b-line-0 (empty item contributes 0 rows), got %q", lines[2])
|
||||
}
|
||||
|
||||
// Now the actual hit-test contract: clicking on viewY=2 must map to
|
||||
// item b line 0 — the same coordinate View() rendered there.
|
||||
idx, line := sl.getItemAndLineAtY(2)
|
||||
if idx != 2 || line != 0 {
|
||||
t.Errorf("getItemAndLineAtY(2) = (%d,%d), want (2,0)", idx, line)
|
||||
}
|
||||
|
||||
// And clicking on the second line of b (viewY=3) must map to b line 1.
|
||||
idx, line = sl.getItemAndLineAtY(3)
|
||||
if idx != 2 || line != 1 {
|
||||
t.Errorf("getItemAndLineAtY(3) = (%d,%d), want (2,1)", idx, line)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -230,8 +230,10 @@ func FindWordBoundaries(line string, col int) (startCol, endCol int) {
|
||||
|
||||
// HighlightLine applies reverse-video highlighting to a portion of a rendered
|
||||
// line (which may contain ANSI escape codes). startCol/endCol are in display
|
||||
// columns. If startCol == -1, the entire line is highlighted. If startCol ==
|
||||
// endCol, returns the line unchanged.
|
||||
// columns. If startCol == -1, the entire line is highlighted. If endCol ==
|
||||
// -1, the highlight runs from startCol to the end of the line (the sentinel
|
||||
// returned by IsLineInRange for the first line of a multi-line selection).
|
||||
// If startCol == endCol, returns the line unchanged.
|
||||
//
|
||||
// Uses ultraviolet ScreenBuffer for cell-level ANSI manipulation.
|
||||
func HighlightLine(line string, startCol, endCol int) string {
|
||||
@@ -250,6 +252,16 @@ func HighlightLine(line string, startCol, endCol int) string {
|
||||
endCol = lineWidth
|
||||
}
|
||||
|
||||
// "From startCol to end of line" sentinel (returned by IsLineInRange
|
||||
// for the first line of a multi-line selection). Without this branch,
|
||||
// the start line of a multi-line drag would never be highlighted —
|
||||
// the user perceives this as the selection being shifted one row down
|
||||
// from the cursor, especially when extension widgets shrink the
|
||||
// scrollback and make the start line land on a tall styled block.
|
||||
if endCol < 0 {
|
||||
endCol = lineWidth
|
||||
}
|
||||
|
||||
if startCol >= endCol || startCol >= lineWidth {
|
||||
return line
|
||||
}
|
||||
@@ -296,6 +308,11 @@ func ExtractText(line string, startCol, endCol int) string {
|
||||
endCol = lineWidth
|
||||
}
|
||||
|
||||
// "From startCol to end of line" sentinel (see HighlightLine).
|
||||
if endCol < 0 {
|
||||
endCol = lineWidth
|
||||
}
|
||||
|
||||
if startCol >= endCol || startCol >= lineWidth {
|
||||
return ""
|
||||
}
|
||||
|
||||
@@ -357,6 +357,54 @@ func TestHighlightLine_NoSelection(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestHighlightLine_EndOfLineSentinel verifies that endCol=-1 is interpreted
|
||||
// as "highlight from startCol to end of line", matching the sentinel
|
||||
// returned by IsLineInRange for the first line of a multi-line selection.
|
||||
//
|
||||
// Regression: without this contract, the start line of any multi-line drag
|
||||
// would silently fall through HighlightLine's startCol >= endCol guard and
|
||||
// render unstyled, making the selection appear to begin one row below the
|
||||
// cursor — the exact "tracking gets shifted" symptom users reported when
|
||||
// extension widgets shrank the scrollback enough that the click landed on a
|
||||
// styled tool-result block.
|
||||
func TestHighlightLine_EndOfLineSentinel(t *testing.T) {
|
||||
line := "Hello, World!"
|
||||
result := HighlightLine(line, 0, -1)
|
||||
if result == line {
|
||||
t.Errorf("endCol=-1 should highlight from startCol to end of line; got unchanged input")
|
||||
}
|
||||
if len(result) <= len(line) {
|
||||
t.Errorf("highlighted result should be longer than plain input (ANSI codes added); got len=%d want > %d", len(result), len(line))
|
||||
}
|
||||
}
|
||||
|
||||
// TestExtractText_EndOfLineSentinel mirrors TestHighlightLine_EndOfLineSentinel
|
||||
// for the extraction path used by the clipboard copy.
|
||||
func TestExtractText_EndOfLineSentinel(t *testing.T) {
|
||||
line := "Hello, World!"
|
||||
got := ExtractText(line, 7, -1)
|
||||
want := "World!"
|
||||
if got != want {
|
||||
t.Errorf("ExtractText(line, 7, -1) = %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsLineInRange_StartLineSentinelHighlights composes IsLineInRange with
|
||||
// HighlightLine end-to-end: the start line of a multi-line, single-item
|
||||
// selection must actually emit highlight ANSI codes. This is the contract
|
||||
// the rendering path in scrolllist.View() relies on.
|
||||
func TestIsLineInRange_StartLineSentinelHighlights(t *testing.T) {
|
||||
r := Range{StartItemIdx: 5, EndItemIdx: 5, StartLine: 0, EndLine: 2, StartCol: 0, EndCol: 10}
|
||||
inRange, sc, ec := IsLineInRange(r, 5, 0)
|
||||
if !inRange {
|
||||
t.Fatalf("item 5 line 0 should be in range")
|
||||
}
|
||||
highlighted := HighlightLine("first line of selection", sc, ec)
|
||||
if highlighted == "first line of selection" {
|
||||
t.Errorf("first line of multi-line selection was not highlighted (sc=%d ec=%d)", sc, ec)
|
||||
}
|
||||
}
|
||||
|
||||
// TestMultiClickDetection verifies the click counting logic.
|
||||
func TestMultiClickDetection(t *testing.T) {
|
||||
s := NewState()
|
||||
|
||||
Reference in New Issue
Block a user