From 4adf08fe074463f744f51900f20cbf0beec93a17 Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Thu, 20 Aug 2026 23:25:34 +0900 Subject: [PATCH 1/2] fix(ui): scroll tree selector viewport instead of dumping every row The tree selector rendered every scan result unconditionally, so a scan with more rows than fit the terminal height broke bubbletea's redraw math - cursor position and checkbox state visually desynced once the terminal started scrolling on its own. Render rows through a bubbles/viewport that scrolls to keep the cursor row visible, and run the selector in the alt screen so bubbletea owns the full frame. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012jb9mxkG51dbLmRxPsYfLJ --- go.mod | 2 +- internal/ui/treeselector.go | 153 +++++++++++++++++++++++-------- internal/ui/treeselector_test.go | 74 +++++++++++++++ 3 files changed, 188 insertions(+), 41 deletions(-) diff --git a/go.mod b/go.mod index eca941e..c20d7fc 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/ohing504/devclean go 1.26.1 require ( + github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 github.com/charmbracelet/bubbletea v1.3.6 github.com/charmbracelet/huh v1.0.0 github.com/charmbracelet/lipgloss v1.1.0 @@ -14,7 +15,6 @@ require ( github.com/atotto/clipboard v0.1.4 // indirect github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect github.com/catppuccin/go v0.3.0 // indirect - github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc // indirect github.com/charmbracelet/x/ansi v0.9.3 // indirect github.com/charmbracelet/x/cellbuf v0.0.13 // indirect diff --git a/internal/ui/treeselector.go b/internal/ui/treeselector.go index a32925a..a1a7ba8 100644 --- a/internal/ui/treeselector.go +++ b/internal/ui/treeselector.go @@ -5,11 +5,16 @@ import ( "strings" "time" + "github.com/charmbracelet/bubbles/viewport" tea "github.com/charmbracelet/bubbletea" "github.com/ohing504/devclean/internal/model" "github.com/ohing504/devclean/internal/pathutil" ) +// chromeLines is the number of lines occupied by the static header and +// footer around the scrollable item viewport (see renderHeader/renderFooter). +const chromeLines = 7 + // ItemType distinguishes rows in the tree selector. type ItemType int @@ -42,9 +47,11 @@ type TreeSelectorResult struct { // treeModel is the bubbletea model for tree selection. type treeModel struct { - items []TreeItem - cursor int - aborted bool + items []TreeItem + cursor int + aborted bool + viewport viewport.Model + ready bool } // BuildTreeItems constructs the flat item list from grouped scan results. @@ -152,7 +159,7 @@ func RunTreeSelector(results []model.ScanResult) TreeSelectorResult { cursor: cursor, } - p := tea.NewProgram(m) + p := tea.NewProgram(m, tea.WithAltScreen()) finalModel, err := p.Run() if err != nil { return TreeSelectorResult{Aborted: true} @@ -178,38 +185,91 @@ func (m treeModel) Init() tea.Cmd { } func (m treeModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { - key, ok := msg.(tea.KeyMsg) - if !ok { + switch msg := msg.(type) { + case tea.WindowSizeMsg: + height := msg.Height - chromeLines + if height < 1 { + height = 1 + } + if !m.ready { + m.viewport = viewport.New(msg.Width, height) + m.ready = true + } else { + m.viewport.Width = msg.Width + m.viewport.Height = height + } + m.syncViewport() + return m, nil + + case tea.KeyMsg: + switch msg.String() { + case "up", "k": + m.moveCursor(-1) + case "down", "j": + m.moveCursor(1) + case "left", "h": + m.jumpProject(-1) + case "right", "l": + m.jumpProject(1) + case " ": + m.toggleCurrent() + case "enter": + return m, tea.Quit + case "q", "esc", "ctrl+c": + m.aborted = true + return m, tea.Quit + case "a": + m.selectAll() + case "n": + m.selectNone() + case "s": + m.selectBySafety(model.SafetySafe) + case "d": + m.selectByActivity(model.StatusDormant) + } + m.syncViewport() return m, nil - } - switch key.String() { - case "up", "k": - m.moveCursor(-1) - case "down", "j": - m.moveCursor(1) - case "left", "h": - m.jumpProject(-1) - case "right", "l": - m.jumpProject(1) - case " ": - m.toggleCurrent() - case "enter": - return m, tea.Quit - case "q", "esc", "ctrl+c": - m.aborted = true - return m, tea.Quit - case "a": - m.selectAll() - case "n": - m.selectNone() - case "s": - m.selectBySafety(model.SafetySafe) - case "d": - m.selectByActivity(model.StatusDormant) } return m, nil } +// layoutItems renders every item to its display lines and records, per item, +// the line index each one starts at and how many lines it occupies (eco +// headers and projects span 2 lines, artifacts span 1). +func (m treeModel) layoutItems() (lines []string, starts, counts []int) { + starts = make([]int, len(m.items)) + counts = make([]int, len(m.items)) + for i, item := range m.items { + starts[i] = len(lines) + parts := strings.Split(m.renderItem(i, item, i == m.cursor), "\n") + counts[i] = len(parts) + lines = append(lines, parts...) + } + return lines, starts, counts +} + +// syncViewport rebuilds the viewport content from the current item list and +// scrolls just enough to keep the cursor row visible. +func (m *treeModel) syncViewport() { + if !m.ready { + return + } + + lines, starts, counts := m.layoutItems() + m.viewport.SetContent(strings.Join(lines, "\n")) + + if m.cursor < 0 || m.cursor >= len(starts) { + return + } + top := starts[m.cursor] + bottom := top + counts[m.cursor] - 1 + if top < m.viewport.YOffset { + m.viewport.SetYOffset(top) + } else if bottom > m.viewport.YOffset+m.viewport.Height-1 { + m.viewport.SetYOffset(bottom - m.viewport.Height + 1) + } +} + func (m *treeModel) moveCursor(dir int) { for { m.cursor += dir @@ -348,38 +408,51 @@ func (m *treeModel) selectByActivity(activity model.ActivityStatus) { } func (m treeModel) View() string { + if !m.ready { + return "Initializing...\n" + } + var b strings.Builder + b.WriteString(m.renderHeader()) + b.WriteString(m.viewport.View()) + b.WriteString("\n\n") + b.WriteString(m.renderFooter()) + return b.String() +} + +// renderHeader renders the static key-binding help block above the +// scrollable item viewport. Must stay at 3 lines to match chromeLines. +func (m treeModel) renderHeader() string { + var b strings.Builder fmt.Fprintf(&b, "%s move %s jump project %s toggle %s confirm %s cancel", DimStyle.Render("[↑↓]"), DimStyle.Render("[←→]"), DimStyle.Render("[space]"), DimStyle.Render("[enter]"), DimStyle.Render("[esc]")) b.WriteString("\n") fmt.Fprintf(&b, "%s all %s none %s safe only %s dormant only", DimStyle.Render("[a]"), DimStyle.Render("[n]"), DimStyle.Render("[s]"), DimStyle.Render("[d]")) b.WriteString("\n\n") + return b.String() +} +// renderFooter renders the selection summary and legend below the +// scrollable item viewport. Must stay at 4 lines to match chromeLines. +func (m treeModel) renderFooter() string { var selectedCount int var selectedSize int64 - - for i, item := range m.items { + for _, item := range m.items { if item.Type == ItemArtifact && item.Selected { selectedCount++ selectedSize += item.Size } - - isCursor := i == m.cursor - line := m.renderItem(i, item, isCursor) - b.WriteString(line) - b.WriteString("\n") } - b.WriteString("\n") + var b strings.Builder b.WriteString(InfoStyle.Render(fmt.Sprintf("Selected: %d items (%s)", selectedCount, model.HumanSize(selectedSize)))) b.WriteString("\n\n") fmt.Fprintf(&b, "%s %s safe %s caution %s protected %s Active %s Recent %s Stale %s Dormant\n", DimStyle.Render("Legend:"), SafeStyle.Render("✔"), CautionStyle.Render("⚠"), ProtectedStyle.Render("✖"), ActiveStyle.Render("●"), RecentStyle.Render("●"), StaleStyle.Render("●"), DormantStyle.Render("●")) - return b.String() } diff --git a/internal/ui/treeselector_test.go b/internal/ui/treeselector_test.go index 8b624fe..a26855c 100644 --- a/internal/ui/treeselector_test.go +++ b/internal/ui/treeselector_test.go @@ -341,3 +341,77 @@ func TestSelectAllConsistentWithSelectBySafety(t *testing.T) { } } } + +// newManyArtifactsFixture builds a single unprotected project with n +// artifacts — enough rows to overflow a small terminal window. +func newManyArtifactsFixture(t *testing.T, n int) treeModel { + t.Helper() + results := make([]model.ScanResult, n) + for i := range results { + results[i] = model.ScanResult{ + Path: "/proj/many/artifact" + string(rune('a'+i%26)) + string(rune('0'+i/26)), + Ecosystem: model.EcoNode, + Size: int64(i + 1), + Safety: model.SafetySafe, + ProjectRoot: "/proj/many", + } + } + items := BuildTreeItems(results) + if len(items) == 0 { + t.Fatal("BuildTreeItems returned no items") + } + return treeModel{items: items} +} + +// TestCursorStaysWithinViewport reproduces the reported bug: with more rows +// than fit on screen, moving the cursor down must scroll the viewport so the +// highlighted row is always inside [YOffset, YOffset+Height). Before the +// viewport-backed rendering was added, View() dumped every row unconditionally +// and the cursor could scroll off the visible terminal region entirely. +func TestCursorStaysWithinViewport(t *testing.T) { + m := newManyArtifactsFixture(t, 40) + + // Small window: forces scrolling almost immediately. + updated, _ := m.Update(tea.WindowSizeMsg{Width: 80, Height: 17}) + m = updated.(treeModel) + if !m.ready { + t.Fatal("model did not become ready after WindowSizeMsg") + } + if m.viewport.Height != 10 { // 17 - chromeLines(7) + t.Fatalf("viewport height = %d, want 10", m.viewport.Height) + } + + assertCursorVisible := func(t *testing.T, m treeModel) { + t.Helper() + _, starts, counts := m.layoutItems() + top := starts[m.cursor] + bottom := top + counts[m.cursor] - 1 + if top < m.viewport.YOffset || bottom > m.viewport.YOffset+m.viewport.Height-1 { + t.Fatalf("cursor row [%d,%d] outside visible window [%d,%d]", + top, bottom, m.viewport.YOffset, m.viewport.YOffset+m.viewport.Height-1) + } + } + + sawScroll := false + for i := 0; i < 39; i++ { + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyDown}) + m = updated.(treeModel) + assertCursorVisible(t, m) + if m.viewport.YOffset > 0 { + sawScroll = true + } + } + if !sawScroll { + t.Fatal("viewport never scrolled despite cursor moving past the visible window") + } + + // Scrolling back up must also keep the cursor visible, down to the top. + for i := 0; i < 39; i++ { + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyUp}) + m = updated.(treeModel) + assertCursorVisible(t, m) + } + if m.viewport.YOffset != 0 { + t.Fatalf("YOffset = %d after scrolling back to the first row, want 0", m.viewport.YOffset) + } +} From f0e11854778b670a50289e94031ac78a6b3957ce Mon Sep 17 00:00:00 2001 From: Youngsup Oh Date: Thu, 20 Aug 2026 23:34:57 +0900 Subject: [PATCH 2/2] fix(ui): match viewport-to-footer spacing to the chromeLines budget The blank line between the viewport and the footer used two newlines, one line more than chromeLines(7) accounts for, so the frame overflowed the terminal height by one row on every render. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_012jb9mxkG51dbLmRxPsYfLJ --- internal/ui/treeselector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/ui/treeselector.go b/internal/ui/treeselector.go index a1a7ba8..6ea094b 100644 --- a/internal/ui/treeselector.go +++ b/internal/ui/treeselector.go @@ -415,7 +415,7 @@ func (m treeModel) View() string { var b strings.Builder b.WriteString(m.renderHeader()) b.WriteString(m.viewport.View()) - b.WriteString("\n\n") + b.WriteString("\n") b.WriteString(m.renderFooter()) return b.String()