From 981e4794e07166da0094d2bc89c8c4f4329b7675 Mon Sep 17 00:00:00 2001 From: fullstackjam Date: Thu, 9 Jul 2026 22:49:19 +0800 Subject: [PATCH] feat(tui): two-pane focus navigation and mouse on the select screen MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The select screen paired a category sidebar with a package list, but ← → just cycled categories while ↑ ↓ always drove the list — two columns, one confusing model. Rework it into an explicit two-pane focus model: - ← → (and tab) move focus between the sidebar and the list; the active pane's cursor is bright, the other dimmed, so "which column am I in" reads at a glance. - ↑ ↓ act on the focused pane: change category (list live-previews) when the sidebar has focus, move the package cursor otherwise. - Mouse: left-click a category to switch to it, left-click a package to toggle it, wheel to scroll the list (enabled via WithMouseCellMotion). Click→row mapping is a pure, unit-tested function (selectHitTest) whose geometry mirrors the renderer, so it's verified rather than eyeballed. Visual style is unchanged — interaction only. Claude-Session: https://claude.ai/code/session_01HLoiSd2k9EJJ1HPjjDgUgo --- go.mod | 2 +- internal/ui/tui/wizard/frames_test.go | 4 + internal/ui/tui/wizard/select.go | 164 ++++++++++++++++++++++---- internal/ui/tui/wizard/status.go | 2 +- internal/ui/tui/wizard/wizard.go | 18 ++- internal/ui/tui/wizard/wizard_test.go | 127 +++++++++++++++++++- test/e2e/install_wizard_e2e_test.go | 38 ++++++ 7 files changed, 328 insertions(+), 27 deletions(-) diff --git a/go.mod b/go.mod index cb94540..c4e18e2 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/charmbracelet/bubbletea v1.3.0 github.com/charmbracelet/huh v0.6.0 github.com/charmbracelet/lipgloss v1.0.0 + github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a github.com/sahilm/fuzzy v0.1.1 github.com/spf13/cobra v1.8.1 github.com/stretchr/testify v1.11.1 @@ -32,7 +33,6 @@ require ( github.com/mitchellh/hashstructure/v2 v2.0.2 // indirect github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect - github.com/muesli/termenv v0.15.3-0.20240618155329-98d742f6907a // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/rivo/uniseg v0.4.7 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/internal/ui/tui/wizard/frames_test.go b/internal/ui/tui/wizard/frames_test.go index 58db161..0ab5923 100644 --- a/internal/ui/tui/wizard/frames_test.go +++ b/internal/ui/tui/wizard/frames_test.go @@ -34,6 +34,10 @@ func key(s string) tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyDown} case "up": return tea.KeyMsg{Type: tea.KeyUp} + case "left": + return tea.KeyMsg{Type: tea.KeyLeft} + case "right": + return tea.KeyMsg{Type: tea.KeyRight} case "backspace": return tea.KeyMsg{Type: tea.KeyBackspace} } diff --git a/internal/ui/tui/wizard/select.go b/internal/ui/tui/wizard/select.go index eced975..b199576 100644 --- a/internal/ui/tui/wizard/select.go +++ b/internal/ui/tui/wizard/select.go @@ -136,27 +136,25 @@ func (m Model) updateSelect(msg tea.KeyMsg) (tea.Model, tea.Cmd) { //nolint:gocy m.quit = true return m, tea.Quit case "/": - m.typing, m.query, m.rowCur, m.scroll = true, "", 0, 0 + // Filtering searches across categories, so it's a list operation — + // pull focus to the list so ↑↓ behaves predictably after it clears. + m.typing, m.query, m.rowCur, m.scroll, m.selFocus = true, "", 0, 0, focusList case "esc": m.query, m.rowCur, m.scroll = "", 0, 0 - case "up", "k": - if m.rowCur > 0 { - m.rowCur-- + case "left", "h": + m.selFocus = focusCats + case "right", "l": + m.selFocus = focusList + case "tab", "shift+tab": + if m.selFocus == focusCats { + m.selFocus = focusList + } else { + m.selFocus = focusCats } + case "up", "k": + m = m.selMoveUp() case "down", "j": - if m.rowCur < last { - m.rowCur++ - } - case "tab", "right": - if m.query == "" && len(m.cats) > 0 { - m.catCur = (m.catCur + 1) % len(m.cats) - m.rowCur, m.scroll = 0, 0 - } - case "shift+tab", "left": - if m.query == "" && len(m.cats) > 0 { - m.catCur = (m.catCur - 1 + len(m.cats)) % len(m.cats) - m.rowCur, m.scroll = 0, 0 - } + m = m.selMoveDown() case " ": if len(pool) > 0 { p := pool[clamp(m.rowCur, 0, last)] @@ -178,6 +176,114 @@ func (m Model) updateSelect(msg tea.KeyMsg) (tea.Model, tea.Cmd) { //nolint:gocy return m.clampSelScroll(), nil } +// selMoveUp/selMoveDown move the cursor within the focused pane: the category +// sidebar when it has focus (right pane live-previews the new category), the +// package list otherwise. +func (m Model) selMoveUp() Model { + if m.selFocus == focusCats && m.query == "" { + if m.catCur > 0 { + m.catCur-- + m.rowCur, m.scroll = 0, 0 + } + return m + } + if m.rowCur > 0 { + m.rowCur-- + } + return m +} + +func (m Model) selMoveDown() Model { + if m.selFocus == focusCats && m.query == "" { + if m.catCur < len(m.cats)-1 { + m.catCur++ + m.rowCur, m.scroll = 0, 0 + } + return m + } + if m.rowCur < len(m.pool())-1 { + m.rowCur++ + } + return m +} + +// ── mouse ── + +type selHit int + +const ( + hitNone selHit = iota + hitCat + hitPkg +) + +// updateSelectMouse handles clicks and wheel scrolls on the select screen: +// left-click a category to switch to it, left-click a package to toggle it, +// wheel to scroll the package list. +func (m Model) updateSelectMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { + switch msg.Button { + case tea.MouseButtonWheelUp: + m.selFocus = focusList + if m.rowCur > 0 { + m.rowCur-- + } + case tea.MouseButtonWheelDown: + m.selFocus = focusList + if m.rowCur < len(m.pool())-1 { + m.rowCur++ + } + case tea.MouseButtonLeft: + if msg.Action != tea.MouseActionPress { + return m, nil + } + switch kind, idx := m.selectHitTest(msg.X, msg.Y); kind { + case hitCat: + m.catCur, m.query, m.typing = idx, "", false + m.selFocus = focusCats + m.rowCur, m.scroll = 0, 0 + case hitPkg: + m.rowCur = idx + m.selFocus = focusList + pool := m.pool() + if idx < len(pool) && !m.isInstalled(pool[idx].Name) { + m.toggle(pool[idx].Name) + } + case hitNone: + // click landed on chrome or blank space — nothing to do + } + default: + // other buttons (middle, right, drag motion) aren't actionable here + return m, nil + } + return m.clampSelScroll(), nil +} + +// selectHitTest maps a screen coordinate to a category or package row. The +// geometry mirrors View + selectBody exactly: screen row 0 is the title bar so +// body row = y-1; the sidebar lists categories from body row 3, and the package +// list starts at body row 2 offset by the scroll. Kept a pure, testable +// function so the mapping is verified rather than eyeballed. +func (m Model) selectHitTest(x, y int) (selHit, int) { + bodyRow := y - 1 + if bodyRow < 0 { + return hitNone, -1 + } + if x < sidebarW { + if ci := bodyRow - 3; ci >= 0 && ci < len(m.cats) { + return hitCat, ci + } + return hitNone, -1 + } + if bodyRow < 2 { + return hitNone, -1 + } + pool := m.pool() + if pj := m.scroll + bodyRow - 2; pj >= 0 && pj < len(pool) { + return hitPkg, pj + } + return hitNone, -1 +} + func (m Model) tryInstall() (tea.Model, tea.Cmd) { if m.toInstallCount() == 0 { return m, nil // nothing to install — stay put @@ -233,8 +339,15 @@ func (m Model) selectSidebar(h int) []string { nameStyle := fg(cDim) countStyle := fg(cDim4) if active { - edge = fg(cAccent).Render("▎") - nameStyle = fg(cTextHi) + // Bright marker when the sidebar holds focus; muted when the + // package list does, so the active pane reads at a glance. + if m.selFocus == focusCats { + edge = fg(cAccent).Render("▎") + nameStyle = fg(cTextHi) + } else { + edge = fg(cDim2).Render("▎") + nameStyle = fg(cText) + } } if selN > 0 { countStyle = fg(cAccentHi) @@ -317,13 +430,14 @@ func (m Model) renderRow(p config.Package, cursor bool, w int) string { box = fg(cAccent).Render("◉") } + listFocused := m.selFocus == focusList nameStyle := fg(cMuted) switch { case installed: nameStyle = fg(cDim2) - case cursor: + case cursor && listFocused: nameStyle = fg(cWhite).Bold(true) - case on: + case cursor, on: nameStyle = fg(cText) } @@ -335,7 +449,13 @@ func (m Model) renderRow(p config.Package, cursor bool, w int) string { name := padTo(nameStyle.Render(p.Name), 20) rowPrefix := " " if cursor { - rowPrefix = fg(cAccent).Render("› ") + // Dim the cursor when focus is on the sidebar, so the two panes' + // cursors don't compete for attention. + if listFocused { + rowPrefix = fg(cAccent).Render("› ") + } else { + rowPrefix = fg(cDim3).Render("› ") + } } left := rowPrefix + box + " " + name + " " + fg(cDim).Render(p.Description) return bar(truncCell(left, w-12), tail+" ", w) diff --git a/internal/ui/tui/wizard/status.go b/internal/ui/tui/wizard/status.go index d91827e..2333017 100644 --- a/internal/ui/tui/wizard/status.go +++ b/internal/ui/tui/wizard/status.go @@ -18,7 +18,7 @@ func (m Model) statusContent() (mode string, color lipgloss.Color, keys, right s case scrSelect: return "SELECT", cAccent, - "↑↓/jk move · space toggle · ⇥ category · / filter · a all · x clear · ↵ install", + "←→ pane · ↑↓ move · space toggle · / filter · a all · x clear · ↵ install", fmt.Sprintf("%d pkgs · ~%d min", m.selCount(), m.estMin()) case scrGit: diff --git a/internal/ui/tui/wizard/wizard.go b/internal/ui/tui/wizard/wizard.go index 3409c37..1f6ec09 100644 --- a/internal/ui/tui/wizard/wizard.go +++ b/internal/ui/tui/wizard/wizard.go @@ -32,6 +32,15 @@ const ( scrInstall ) +// focusPane is which of the two select-screen columns holds keyboard focus. +// ← → (and tab) move focus between them; ↑ ↓ act on the focused one. +type focusPane int + +const ( + focusList focusPane = iota // package list (right column) — the default + focusCats // category sidebar (left column) +) + // ErrAborted is returned by Run when the user cancels a running install with // ctrl+c. It distinguishes a deliberate abort from install failures. var ErrAborted = errors.New("installation aborted") @@ -66,6 +75,7 @@ type Model struct { scroll int query string typing bool + selFocus focusPane selected map[string]bool // ── git identity (captured only when none is configured) ── @@ -169,6 +179,12 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m.updateInstall(msg) } + case tea.MouseMsg: + if m.screen == scrSelect { + return m.updateSelectMouse(msg) + } + return m, nil + case probeDoneMsg: return m.onProbeDone(msg) @@ -326,7 +342,7 @@ func Run(version string, opts *config.InstallOptions) (plan installer.InstallPla }() } - p := tea.NewProgram(m, tea.WithAltScreen(), tea.WithOutput(realOut), tea.WithInput(os.Stdin)) + p := tea.NewProgram(m, tea.WithAltScreen(), tea.WithMouseCellMotion(), tea.WithOutput(realOut), tea.WithInput(os.Stdin)) final, runErr := p.Run() if runErr != nil { return installer.InstallPlan{}, false, fmt.Errorf("run wizard: %w", runErr) diff --git a/internal/ui/tui/wizard/wizard_test.go b/internal/ui/tui/wizard/wizard_test.go index aaf5cb0..eac4a36 100644 --- a/internal/ui/tui/wizard/wizard_test.go +++ b/internal/ui/tui/wizard/wizard_test.go @@ -8,6 +8,7 @@ import ( tea "github.com/charmbracelet/bubbletea" "github.com/charmbracelet/lipgloss" + "github.com/muesli/termenv" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -116,12 +117,134 @@ func TestGitFieldsAcceptPastedText(t *testing.T) { assert.Equal(t, "ci@example.com", m.gitEmail) } -func TestSelectCategoryCycle(t *testing.T) { +// The select screen is a two-pane focus model: ← → (and tab) move focus +// between the category sidebar and the package list; ↑ ↓ act on the focused +// pane. These tests pin that contract plus the mouse mapping. + +func TestSelectFocusAndCategoryNav(t *testing.T) { m := finishProbes(sized(96, 30)) m = send(m, key("c")) + require.Equal(t, focusList, m.selFocus, "the package list has focus by default") + require.GreaterOrEqual(t, len(m.cats), 2) + + // ← focuses the sidebar; ↓ then advances the category and resets the row. + m = send(m, key("left")) + assert.Equal(t, focusCats, m.selFocus) start := m.catCur + m = send(m, key("down")) + assert.Equal(t, start+1, m.catCur, "↓ under sidebar focus moves to the next category") + assert.Equal(t, 0, m.rowCur, "switching category resets the package cursor") + require.GreaterOrEqual(t, len(m.pool()), 2, "chosen category needs rows to move through") + + // → returns focus to the list; ↓ now moves the package cursor, not category. + m = send(m, key("right")) + assert.Equal(t, focusList, m.selFocus) + cat := m.catCur + m = send(m, key("down")) + assert.Equal(t, cat, m.catCur, "↓ under list focus leaves the category unchanged") + assert.Equal(t, 1, m.rowCur, "↓ under list focus advances the package cursor") +} + +// TestSelectFocusIsVisuallyIndicated proves the focus highlight is automatable +// (contra "you have to eyeball it"): with identical state but different focus, +// the rendered sidebar and cursor row must differ, because the active pane's +// marker is styled brighter than the other's. What a test can't judge is +// whether that difference is *clear enough* to a human — that stays taste. +func TestSelectFocusIsVisuallyIndicated(t *testing.T) { + // lipgloss strips color when stdout isn't a TTY (as under `go test`), which + // would make both focus states render identically — the trap that makes a + // naive colour assertion silently test nothing. Force a profile so the test + // actually sees the styling; SetColorProfile exists for exactly this. + orig := lipgloss.ColorProfile() + lipgloss.SetColorProfile(termenv.TrueColor) + defer lipgloss.SetColorProfile(orig) + + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + require.GreaterOrEqual(t, len(m.pool()), 1) + + catsFocus, listFocus := m, m + catsFocus.selFocus = focusCats + listFocus.selFocus = focusList + + assert.NotEqual(t, + strings.Join(catsFocus.selectSidebar(28), "\n"), + strings.Join(listFocus.selectSidebar(28), "\n"), + "active category must render differently when the sidebar has focus") + assert.NotEqual(t, + catsFocus.renderRow(catsFocus.pool()[0], true, 90), + listFocus.renderRow(listFocus.pool()[0], true, 90), + "cursor row must render differently when the list has focus") +} + +func TestSelectTabTogglesFocus(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + require.Equal(t, focusList, m.selFocus) + m = send(m, key("tab")) + assert.Equal(t, focusCats, m.selFocus, "tab toggles focus to the sidebar") m = send(m, key("tab")) - assert.Equal(t, (start+1)%len(m.cats), m.catCur) + assert.Equal(t, focusList, m.selFocus, "tab toggles focus back to the list") +} + +func TestSelectHitTest(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + require.GreaterOrEqual(t, len(m.cats), 2) + require.GreaterOrEqual(t, len(m.pool()), 1) + + // Sidebar: category i renders at screen row 4+i (title bar + 3 header rows). + kind, idx := m.selectHitTest(2, 4) + assert.Equal(t, hitCat, kind) + assert.Equal(t, 0, idx) + kind, idx = m.selectHitTest(2, 5) + assert.Equal(t, hitCat, kind) + assert.Equal(t, 1, idx) + + // List: first package at screen row 3 (title + search + blank), x past sidebar. + kind, idx = m.selectHitTest(sidebarW+5, 3) + assert.Equal(t, hitPkg, kind) + assert.Equal(t, 0, idx) + + // Out of range → none. + kind, _ = m.selectHitTest(2, 0) // title bar + assert.Equal(t, hitNone, kind) + kind, _ = m.selectHitTest(sidebarW+5, 999) // below the list + assert.Equal(t, hitNone, kind) +} + +func TestSelectMouseClickCategory(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + require.GreaterOrEqual(t, len(m.cats), 2) + m = send(m, tea.MouseMsg{X: 2, Y: 5, Action: tea.MouseActionPress, Button: tea.MouseButtonLeft}) + assert.Equal(t, 1, m.catCur, "clicking a category switches to it") + assert.Equal(t, focusCats, m.selFocus, "clicking a category focuses the sidebar") +} + +func TestSelectMouseClickTogglesPackage(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + m.installed = map[string]bool{} + pool := m.pool() + require.NotEmpty(t, pool) + click := tea.MouseMsg{X: sidebarW + 5, Y: 3, Action: tea.MouseActionPress, Button: tea.MouseButtonLeft} + m = send(m, click) + assert.True(t, m.selected[pool[0].Name], "clicking a package toggles it on") + assert.Equal(t, focusList, m.selFocus) + m = send(m, click) + assert.False(t, m.selected[pool[0].Name], "clicking again toggles it off") +} + +func TestSelectMouseWheelScrolls(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + require.GreaterOrEqual(t, len(m.pool()), 2) + require.Equal(t, 0, m.rowCur) + m = send(m, tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonWheelDown}) + assert.Equal(t, 1, m.rowCur, "wheel down advances the list cursor") + m = send(m, tea.MouseMsg{Action: tea.MouseActionPress, Button: tea.MouseButtonWheelUp}) + assert.Equal(t, 0, m.rowCur, "wheel up moves it back") } func TestTryInstallNoopWhenNothingToInstall(t *testing.T) { diff --git a/test/e2e/install_wizard_e2e_test.go b/test/e2e/install_wizard_e2e_test.go index 2beb2a3..c82f9c1 100644 --- a/test/e2e/install_wizard_e2e_test.go +++ b/test/e2e/install_wizard_e2e_test.go @@ -209,3 +209,41 @@ func TestE2E_InstallWizard_FullChoreography(t *testing.T) { assert.Contains(t, got, "REVIEW", "confirm screen status badge rendered") assert.Contains(t, got, "\x1b[?1049l", "terminal restored") } + +// TestE2E_InstallWizard_MouseTogglesPackage proves the select screen's mouse +// support works through a real pty — the one interaction the keyboard +// choreography can't reach. It filters to a single uninstalled formula, then +// sends an SGR left-click escape sequence at that row's cell and asserts the +// status bar reflects the toggle. Nothing else can flip "0 pkgs" → "1 pkgs" +// here, so the assertion genuinely proves the click landed and was handled. +func TestE2E_InstallWizard_MouseTogglesPackage(t *testing.T) { + formula := wizardTestFormula(t) + t.Logf("clicking to select formula %q", formula) + + binary := testutil.BuildTestBinary(t) + s := startWizardPty(t, binary, wizardEnv(t.TempDir())) + + require.True(t, s.waitFor("Choose a starting point", 90*time.Second), + "boot: loadout list; output:\n%s", s.out.String()) + s.send(t, "c") // hand-pick: empty selection → status shows "0 pkgs" + + require.True(t, s.waitFor("type to filter", 10*time.Second), + "select: filter placeholder; output:\n%s", s.out.String()) + // Filter (no enter) so the single uninstalled formula is package row 0. + s.sendPaced(t, "/", formula) + require.True(t, s.waitFor(formula, 10*time.Second), + "filtered formula should render as the top row; output:\n%s", s.out.String()) + + // SGR left-click on the first package row. Geometry mirrors selectHitTest: + // screen row 3 (0-based) = title(0) + search(1) + blank(2) + pkg0(3), at a + // column past the 22-wide sidebar. SGR coords are 1-based (bubbletea + // subtracts 1), so send col 28, row 4. "M" = press, "m" = release. + s.send(t, "\x1b[<0;28;4M") + s.send(t, "\x1b[<0;28;4m") + + require.True(t, s.waitFor("1 pkgs", 10*time.Second), + "the mouse click should toggle the package on (status → 1 pkgs); output:\n%s", s.out.String()) + + s.send(t, "\x03") // ctrl+c — installs nothing + s.expectExit(t, 15*time.Second) +}