diff --git a/internal/ui/tui/wizard/boot.go b/internal/ui/tui/wizard/boot.go index 8dae8c1..8563a8b 100644 --- a/internal/ui/tui/wizard/boot.go +++ b/internal/ui/tui/wizard/boot.go @@ -226,6 +226,9 @@ func (m Model) pickLoadout(i int) (tea.Model, tea.Cmd) { func (m Model) enterSelect(sel map[string]bool) (tea.Model, tea.Cmd) { m.selected = sel m.screen = scrSelect + // pinReviewCat prepends "selected", so catCur 0 opens on the loadout's own + // picks rather than on whichever category happens to be first. + m.pinReviewCat() m.catCur, m.rowCur, m.scroll = 0, 0, 0 m.query, m.typing = "", false m.hoverRow = -1 diff --git a/internal/ui/tui/wizard/select.go b/internal/ui/tui/wizard/select.go index d4e6004..b18d488 100644 --- a/internal/ui/tui/wizard/select.go +++ b/internal/ui/tui/wizard/select.go @@ -15,6 +15,15 @@ import ( var searchOnline = search.SearchOnline const ( + // reviewCatName is the synthetic sidebar category the select screen opens + // on: the packages the loadout picked, gathered out of every category. The + // screen's question is "install these?" (↵ is bound to install), so the + // cursor has to land on the answer. Opening on a real category instead + // answered with whichever one sits at index 0 — packages the user never + // chose — while the ones they did chose stayed scattered behind nine other + // categories, reachable only by clicking through each in turn. + reviewCatName = "selected" + // onlineCatName is the synthetic sidebar category that holds packages // picked from openboot.dev search results (they aren't in the local // catalog, so without a home they'd vanish when the filter clears). @@ -57,6 +66,59 @@ func categoriesFromConfig(rc *config.RemoteConfig) []config.Category { return cats } +// ── the "selected" review category ── + +// reviewPackages gathers every selected package out of cats, in catalog order. +func reviewPackages(cats []config.Category, selected map[string]bool) []config.Package { + var out []config.Package + for _, c := range cats { + if c.Name == reviewCatName { + continue // it is built from the real categories; don't fold it in + } + for _, p := range c.Packages { + if selected[p.Name] { + out = append(out, p) + } + } + } + return out +} + +// pinReviewCat puts the synthetic "selected" category first in the sidebar, so +// entering the screen with catCur 0 lands on the loadout's picks. +// +// Two cases skip it. Config mode: the sidebar already *is* the config's own +// package lists with everything preselected, so a review category would just +// concatenate all of them. Hand-picking from scratch ("c" on the boot screen): +// there is nothing to review, and an empty pane says less than the catalog. +func (m *Model) pinReviewCat() { + if m.rc != nil || m.selCount() == 0 { + return + } + review := config.Category{Name: reviewCatName, Packages: reviewPackages(m.cats, m.selected)} + m.cats = append([]config.Category{review}, m.cats...) +} + +func (m Model) isReviewCat(i int) bool { + return i >= 0 && i < len(m.cats) && m.cats[i].Name == reviewCatName +} + +// setCat moves the sidebar cursor to category i and resets the list position. +// +// The review category is a snapshot taken on entry, not a live view of the +// selection. Standing in it and unchecking a row leaves the row in place with +// an empty box, so a mis-toggle costs one keypress to undo — and so "a" and +// "x" mean re-check and clear-all rather than acting on a list that empties +// itself out from under them. +func (m Model) setCat(i int) Model { + m.catCur = i + m.rowCur, m.scroll = 0, 0 + if m.isReviewCat(i) { + m.cats[i].Packages = reviewPackages(m.cats, m.selected) + } + return m +} + // ── selection helpers ── func (m Model) isInstalled(name string) bool { return m.installed[name] } @@ -98,6 +160,9 @@ func (m Model) pool() []config.Package { if q != "" { var out []config.Package for _, c := range m.cats { + if c.Name == reviewCatName { + continue // a copy of rows that live in real categories — would double every hit + } for _, p := range c.Packages { if strings.Contains(strings.ToLower(p.Name+" "+p.Description), q) { out = append(out, p) @@ -359,7 +424,11 @@ func (m Model) updateSelect(msg tea.KeyMsg) (tea.Model, tea.Cmd) { //nolint:gocy case "down", "j": m = m.selMoveDown() case " ": - if len(pool) > 0 { + // Space acts on the list cursor, and renderRow draws that cursor only + // while the list holds focus — exactly one pane shows a pointer at a + // time. With the sidebar focused there is therefore no visible target, + // and toggling rowCur anyway flipped a row the user couldn't see. + if m.selFocus == focusList && len(pool) > 0 { m.togglePkg(pool[clamp(m.rowCur, 0, last)]) } case "a": @@ -382,8 +451,7 @@ func (m Model) updateSelect(msg tea.KeyMsg) (tea.Model, tea.Cmd) { //nolint:gocy func (m Model) selMoveUp() Model { if m.selFocus == focusCats && m.query == "" { if m.catCur > 0 { - m.catCur-- - m.rowCur, m.scroll = 0, 0 + m = m.setCat(m.catCur - 1) } return m } @@ -396,8 +464,7 @@ func (m Model) selMoveUp() Model { 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 + m = m.setCat(m.catCur + 1) } return m } @@ -450,9 +517,9 @@ func (m Model) updateSelectMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { } switch kind, idx := m.selectHitTest(msg.X, msg.Y); kind { case hitCat: - m.catCur, m.query, m.typing = idx, "", false + m.query, m.typing = "", false + m = m.setCat(idx) m.selFocus = focusCats - m.rowCur, m.scroll = 0, 0 m.clearOnline() case hitPkg: m.rowCur = idx @@ -547,10 +614,19 @@ func (m Model) selectSidebar(h int) []string { q := strings.TrimSpace(m.query) for i, c := range m.cats { - selN, total := 0, len(c.Packages) + // Two facts, two channels: the count says where your selection is, the + // colour says whether any of it will actually run. Folding them into + // one number (counting only not-yet-installed packages) left a machine + // that already has everything with a flat grey total on every category + // and no way to find its own selection. + selN, todo, total := 0, 0, len(c.Packages) for _, p := range c.Packages { - if m.selected[p.Name] && !m.installed[p.Name] { - selN++ + if !m.selected[p.Name] { + continue + } + selN++ + if !m.installed[p.Name] { + todo++ } } active := i == m.catCur && q == "" @@ -569,7 +645,7 @@ func (m Model) selectSidebar(h int) []string { nameStyle = fg(cText) } } - if selN > 0 { + if todo > 0 { countStyle = fg(cAccentHi) } count := fmt.Sprintf("%d", total) diff --git a/internal/ui/tui/wizard/wizard_test.go b/internal/ui/tui/wizard/wizard_test.go index e92510f..093bb25 100644 --- a/internal/ui/tui/wizard/wizard_test.go +++ b/internal/ui/tui/wizard/wizard_test.go @@ -1,6 +1,7 @@ package wizard import ( + "fmt" "strings" "testing" @@ -572,6 +573,138 @@ func TestHoverUsesReverseVideo(t *testing.T) { assert.False(t, strings.HasSuffix(styled, rev), "the closing reset is not re-opened") } +// Picking a loadout must open the select screen on a "selected" review +// category so the cursor lands on what the user actually chose, not on +// whichever catalog category happens to sit at index 0. +func TestSelectOpensOnReviewCategory(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("2")) // developer loadout + + require.NotEmpty(t, m.cats) + assert.Equal(t, reviewCatName, m.cats[0].Name, "review category is pinned first") + assert.Equal(t, 0, m.catCur, "cursor opens on the review category") + + want := config.GetPackagesForPreset("developer") + pool := m.pool() + assert.Len(t, pool, len(want), "review category holds exactly the loadout's picks") + for _, p := range pool { + assert.True(t, want[p.Name], "%s wasn't part of the loadout", p.Name) + } +} + +// Hand-picking from scratch has nothing to review yet, so it must fall back to +// browsing the catalog rather than opening on an empty pane. +func TestHandPickSkipsReviewCategory(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) + for _, c := range m.cats { + assert.NotEqual(t, reviewCatName, c.Name, "nothing selected yet — no review category") + } +} + +// Config-mode installs already show the config's own package lists with +// everything preselected; a review category on top would just repeat them. +func TestConfigModeSkipsReviewCategory(t *testing.T) { + rc := &config.RemoteConfig{ + Packages: config.PackageEntryList{{Name: "curl"}}, + } + m := sizedConfig(rc) + m = finishProbes(m) + for _, c := range m.cats { + assert.NotEqual(t, reviewCatName, c.Name, "config mode browses the config's own lists") + } +} + +// Space acts on the list cursor, and the pointer glyph is only drawn while the +// list holds focus (TestSelectFocusIsVisuallyIndicated). Toggling on sidebar +// focus flipped a row the user had no way to see was the target. +func TestSpaceIgnoredWhileSidebarFocused(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("c")) // hand-pick, empty selection + m.installed = map[string]bool{} + require.Zero(t, m.selCount()) + + m = send(m, key("left")) // focus the sidebar + require.Equal(t, focusCats, m.selFocus) + m = send(m, key("space")) + assert.Zero(t, m.selCount(), "space with sidebar focused must not toggle an invisible row") + + m = send(m, key("right")) // back to the list + m = send(m, key("space")) + assert.Equal(t, 1, m.selCount(), "space with list focused toggles the cursor row as before") +} + +// The sidebar count must distinguish "selected" from "will actually run": +// folding installed-but-selected packages out of the count left every +// category on a fully up-to-date machine showing a flat, colourless total, +// with no way to tell where the loadout's picks landed. +func TestSidebarCountShowsSelectedEvenWhenAllInstalled(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("2")) // developer loadout + + // Mark every selected package installed, so nothing is left "to do". + installed := map[string]bool{} + for name, on := range m.selected { + if on { + installed[name] = true + } + } + m.installed = installed + + rows := m.selectSidebar(28) + joined := strings.Join(rows, "\n") + assert.Contains(t, joined, fmt.Sprintf("%d/%d", len(installed), len(installed)), + "review category must still report its full selected count, got:\n%s", joined) +} + +// The review category is a snapshot taken on entry; re-entering it after a +// selection change must refresh, not keep showing stale rows. +func TestReviewCategoryRefreshesOnReentry(t *testing.T) { + m := finishProbes(sized(96, 30)) + m = send(m, key("2")) // developer loadout, opens on "selected" + before := len(m.pool()) + require.Positive(t, before) + + // Move off the review category to one with an unselected package — not + // necessarily the very next one, since the developer preset already fills + // some catalog categories completely. + m = send(m, key("left")) // sidebar focus + var extra config.Package + for { + for _, p := range m.pool() { + if !m.selected[p.Name] { + extra = p + break + } + } + if extra.Name != "" { + break + } + before := m.catCur + m = send(m, key("down")) + require.NotEqual(t, before, m.catCur, "ran out of categories without finding an unselected package") + } + require.NotEqual(t, 0, m.catCur, "must have moved off the review category") + m = send(m, key("right")) // back to the list to toggle + pool := m.pool() + m.rowCur = 0 + for i, p := range pool { + if p.Name == extra.Name { + m.rowCur = i + break + } + } + m = send(m, key("space")) + require.True(t, m.selected[extra.Name]) + + // Back to the review category: it must include the new pick. + m = send(m, key("left")) + for m.catCur != 0 { + m = send(m, key("up")) + } + assert.Equal(t, before+1, len(m.pool()), "review category refreshed to include the new pick") +} + // The text ramp must stay terminal-relative: a hex grey is a guess about the // user's background, and the guess is what made pending rows and key hints // invisible on a translucent terminal. Brand hues are exempt — they're bright