From a74b1df2a593b82bbaad499e5725bb03a1578ba4 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 22 Aug 2026 10:52:42 +0200 Subject: [PATCH] fix(tui): keep palette highlight on the selected row when scrolled The ^K popup renders a scrolling window of maxPalRows entries but compared the window-relative index against the absolute selection, so the highlight vanished past row 8 and drifted onto the wrong row while scrolled. windowEntries now returns the window offset and the highlight compares absolute indices. --- internal/tui/palette.go | 13 +++++---- internal/tui/palette_test.go | 52 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/internal/tui/palette.go b/internal/tui/palette.go index 5a27f45..ab4b301 100644 --- a/internal/tui/palette.go +++ b/internal/tui/palette.go @@ -291,13 +291,14 @@ func (m *Model) palPopup() string { case len(m.pal.items) == 0 && m.pal.query != "": rows = append(rows, th.acDim.Render("no matches for “"+m.pal.query+"”")) default: - for i, e := range windowEntries(m.pal.items, m.pal.sel, maxPalRows) { + window, start := windowEntries(m.pal.items, m.pal.sel, maxPalRows) + for i, e := range window { prefix, label := " ", th.acItem.Render(e.title) detail := th.acDetail.Render(e.kind) if e.hint != "" { detail += th.acDetail.Render(" · ") + th.footerKey.Render(e.hint) } - if i == m.pal.sel { + if start+i == m.pal.sel { prefix, label = th.acSel.Render("› "), th.acSel.Render(e.title) } rows = append(rows, prefix+label+" "+detail) @@ -310,9 +311,11 @@ func (m *Model) palPopup() string { } // windowEntries windows entries around sel without changing indices. -func windowEntries(entries []palEntry, sel, n int) []palEntry { +// It returns the window and the absolute index of its first row, so +// callers can map window-relative positions back onto sel. +func windowEntries(entries []palEntry, sel, n int) ([]palEntry, int) { if len(entries) <= n { - return entries + return entries, 0 } start := sel - n/2 if start < 0 { @@ -321,7 +324,7 @@ func windowEntries(entries []palEntry, sel, n int) []palEntry { if start+n > len(entries) { start = len(entries) - n } - return entries[start : start+n] + return entries[start : start+n], start } // palHeight is the palette's rendered height (border + title + rows). diff --git a/internal/tui/palette_test.go b/internal/tui/palette_test.go index 89e9072..c2d043c 100644 --- a/internal/tui/palette_test.go +++ b/internal/tui/palette_test.go @@ -87,6 +87,58 @@ func TestPaletteOpenFilterRun(t *testing.T) { } } +// TestPaletteSelectionTracksScroll is a regression test for the ^K popup: +// the highlight must follow sel even after the list scrolls past the +// visible window (the highlight used to compare a window-relative index +// against the absolute sel, so it vanished or landed on the wrong row). +func TestPaletteSelectionTracksScroll(t *testing.T) { + m := wired(t) + _, cmd := m.Update(key("ctrl+k")) + m.Update(exec(cmd)) // sessions fetch + + if len(m.pal.items) <= maxPalRows { + t.Fatalf("fixture too small to scroll: %d entries", len(m.pal.items)) + } + + highlighted := func(m *Model) string { + var row string + n := 0 + for _, ln := range strings.Split(plain(m.palPopup()), "\n") { + if strings.Contains(ln, "›") { + row, n = ln, n+1 + } + } + if n != 1 { + t.Fatalf("expected exactly one highlighted row, got %d", n) + } + return row + } + + // Walk the selection past the visible window; the highlight must stay + // on the row the selection actually points at. + for i := 0; i < maxPalRows+2; i++ { + m.Update(key("down")) + } + sel := m.pal.sel + if sel != maxPalRows+2 { + t.Fatalf("down did not advance sel: got %d", sel) + } + if row := highlighted(m); !strings.Contains(row, m.pal.items[sel].title) { + t.Fatalf("scrolled highlight is on the wrong row:\n%q\nwant title %q", row, m.pal.items[sel].title) + } + + // Walk back to the top; the highlight must still track. + for i := 0; i < sel; i++ { + m.Update(key("up")) + } + if m.pal.sel != 0 { + t.Fatalf("up did not return to top: got %d", m.pal.sel) + } + if row := highlighted(m); !strings.Contains(row, m.pal.items[0].title) { + t.Fatalf("top highlight is on the wrong row:\n%q\nwant title %q", row, m.pal.items[0].title) + } +} + // TestPaletteFromApproval verifies the palette works from the approval rung. func TestPaletteFromApproval(t *testing.T) { m := wired(t)