diff --git a/apps/tui-go/internal/app/diff_scroll_test.go b/apps/tui-go/internal/app/diff_scroll_test.go new file mode 100644 index 00000000..7e1e726a --- /dev/null +++ b/apps/tui-go/internal/app/diff_scroll_test.go @@ -0,0 +1,46 @@ +package app + +import ( + "strings" + "testing" +) + +// scrollDiff must clamp to [0, diffMaxScroll] against the same geometry the modal renders to, +// so PgUp/PgDn (and g/G) past either end land exactly on the edge — no dead presses unwinding +// an overshoot. Backs the fullscreen viewer for every tool preview (diff / command / plain). +func TestDiffScrollClampsToBounds(t *testing.T) { + m := inSessionModel(120, 30) + var b strings.Builder + b.WriteString("--- a/f\n+++ b/f\n@@ -0,0 +1,40 @@\n") + for i := 0; i < 40; i++ { + b.WriteString("+a new line\n") + } + // currentDiff() falls back to the last "tool" transcript entry when no approval is pending. + m.routerMessages[m.selectedID] = []RouterEntry{{Role: "tool", Content: b.String()}} + + max := m.diffMaxScroll() + if max <= 0 { + t.Fatalf("fixture diff must exceed the modal body height to test scrolling; max=%d", max) + } + + m.scrollDiff(10_000) // way past the bottom + if m.diffScrollOffset != max { + t.Fatalf("scroll past end clamped to %d, want max %d", m.diffScrollOffset, max) + } + m.scrollDiff(-10_000) // way past the top + if m.diffScrollOffset != 0 { + t.Fatalf("scroll past top clamped to %d, want 0", m.diffScrollOffset) + } +} + +// A diff that fits the modal body never scrolls. +func TestDiffScrollNoOpWhenFits(t *testing.T) { + m := inSessionModel(120, 30) + m.routerMessages[m.selectedID] = []RouterEntry{ + {Role: "tool", Content: "--- a/f\n+++ b/f\n@@ -1,1 +1,1 @@\n-x\n+y\n"}, + } + m.scrollDiff(5) + if m.diffScrollOffset != 0 { + t.Fatalf("short diff should not scroll, got %d", m.diffScrollOffset) + } +} diff --git a/apps/tui-go/internal/app/overlays.go b/apps/tui-go/internal/app/overlays.go index 634d860d..41d3a214 100644 --- a/apps/tui-go/internal/app/overlays.go +++ b/apps/tui-go/internal/app/overlays.go @@ -353,6 +353,20 @@ func (m Model) diffMaxScroll() int { return max } +// scrollDiff moves the diff/preview/command modal offset by delta rows, clamped to +// [0, diffMaxScroll] so paging past either end lands exactly on the edge (no dead +// presses unwinding an overshoot — same contract as the OUTPUT free-scroll). +func (m *Model) scrollDiff(delta int) { + off := m.diffScrollOffset + delta + if mx := m.diffMaxScroll(); off > mx { + off = mx + } + if off < 0 { + off = 0 + } + m.diffScrollOffset = off +} + func (m Model) diffModal() string { if cmd := m.pendingCommand(); cmd != nil { return m.commandModal(cmd) @@ -390,7 +404,7 @@ func (m Model) diffModal() string { for _, ln := range lines { b.WriteString(ln + "\n") } - b.WriteString("\n" + modalHints(t, [][2]string{{"↑↓", "scroll"}, {"^x/esc", "close"}})) + b.WriteString("\n" + modalHints(t, [][2]string{{"↑↓", "line"}, {"PgUp/Dn", "page"}, {"g/G", "ends"}, {"^x/esc", "close"}})) modal := t.Overlay.Width(w).Render(b.String()) return m.center(modal) @@ -424,7 +438,7 @@ func (m Model) commandModal(a *Approval) string { for i := off; i < end; i++ { b.WriteString(lines[i] + "\n") } - b.WriteString("\n" + modalHints(t, [][2]string{{"↑↓", "scroll"}, {"^x/esc", "close"}})) + b.WriteString("\n" + modalHints(t, [][2]string{{"↑↓", "line"}, {"PgUp/Dn", "page"}, {"g/G", "ends"}, {"^x/esc", "close"}})) return m.center(t.Overlay.Width(w).Render(b.String())) } @@ -742,11 +756,18 @@ func (m Model) helpModal() string { {"G / I", "grants / idea board"}, }) section(&b, "approval band", []kb{ - {"y / a / enter", "approve once"}, + {"y / a / enter", "approve once (T3+ asks again to confirm)"}, {"n / r", "reject"}, {"e / s", "steer (add a note)"}, {"A", "approve-always — choose scope"}, {"↑ / ↓", "walk the pending queue"}, + {"^x", "fullscreen the diff / command preview"}, + }) + section(&b, "diff / preview viewer (^x)", []kb{ + {"↑ / ↓", "scroll a line"}, + {"PgUp / PgDn", "scroll a page (^u / ^d half)"}, + {"g / G", "jump to top / end"}, + {"^x / esc", "close"}, }) b.WriteString(modalHints(t, [][2]string{{"any key", "close"}})) modal := t.Overlay.Width(w).Render(b.String()) diff --git a/apps/tui-go/internal/app/update.go b/apps/tui-go/internal/app/update.go index 30cf2cea..14946733 100644 --- a/apps/tui-go/internal/app/update.go +++ b/apps/tui-go/internal/app/update.go @@ -630,15 +630,27 @@ func (m Model) handleOverlayKey(k tea.KeyMsg) (tea.Model, tea.Cmd) { case OverlayPalette: return m.handlePaletteKey(k) case OverlayDiff: + // A long diff/preview is unreadable one line at a time, so the fullscreen + // viewer pages like the OUTPUT transcript: ↑↓/jk by line, PgUp/PgDn by a + // full body, ^u/^d by half, g/G to the ends. All clamp to the diff bounds. + page := m.diffBodyHeight() switch { case k.Type == tea.KeyUp || runeIs(k, "k"): - if m.diffScrollOffset > 0 { - m.diffScrollOffset-- - } + m.scrollDiff(-1) case k.Type == tea.KeyDown || runeIs(k, "j"): - if m.diffScrollOffset < m.diffMaxScroll() { - m.diffScrollOffset++ - } + m.scrollDiff(1) + case k.Type == tea.KeyPgUp: + m.scrollDiff(-page) + case k.Type == tea.KeyPgDown: + m.scrollDiff(page) + case k.Type == tea.KeyCtrlU: + m.scrollDiff(-page / 2) + case k.Type == tea.KeyCtrlD: + m.scrollDiff(page / 2) + case runeIs(k, "g"): + m.diffScrollOffset = 0 + case runeIs(k, "G"): + m.diffScrollOffset = m.diffMaxScroll() case k.Type == tea.KeyCtrlX: m.overlay = OverlayNone }