diff --git a/internal/tui/accordion_input_test.go b/internal/tui/accordion_input_test.go index 239bd60..6c8b551 100644 --- a/internal/tui/accordion_input_test.go +++ b/internal/tui/accordion_input_test.go @@ -171,24 +171,34 @@ func rawSequence(cmd tea.Cmd) string { return "" } -func TestMouseMotionSetsPointerShapeOverAnAccordionRow(t *testing.T) { +// TestMouseMotionSetsPointerShapeOverTheAccordionZone: the hand shape tracks +// the block's whole zone, whether that is a lone collapsed header or every +// disclosed row once expanded, and resets on the first row past it. OSC 22 is +// only emitted on a shape change, so each step alternates the expected shape. +func TestMouseMotionSetsPointerShapeOverTheAccordionZone(t *testing.T) { m := accordionTestModel(t) row := accordionRow(t, m) - - _, cmd := m.Update(tea.MouseMotionMsg{X: 0, Y: row}) - if got := rawSequence(cmd); got != ansi.SetPointerShape("pointer") { - t.Fatalf("hovering the row did not set the pointer shape: got %q, want %q", got, ansi.SetPointerShape("pointer")) + hand, arrow := ansi.SetPointerShape("pointer"), ansi.SetPointerShape("default") + expect := func(y int, want, what string) { + t.Helper() + if _, cmd := m.Update(tea.MouseMotionMsg{X: 0, Y: y}); rawSequence(cmd) != want { + t.Fatalf("%s: got %q, want %q", what, rawSequence(cmd), want) + } } - _, cmd = m.Update(tea.MouseMotionMsg{X: 0, Y: row + 1}) - if got := rawSequence(cmd); got != ansi.SetPointerShape("default") { - t.Fatalf("un-hovering the row did not reset the pointer shape: got %q, want %q", got, ansi.SetPointerShape("default")) - } + expect(row, hand, "hovering the collapsed header") + expect(row+1, arrow, "leaving the collapsed header-only block") + + m.list.ToggleAllDisclosure() + // accordionTestOutput is 5 lines, so the expanded block spans row..row+5. + expect(row+5, hand, "hovering the last disclosed row") + expect(row+6, arrow, "leaving the expanded block") } // scrollableAccordionTestModel builds several identical tool blocks in a -// viewport too short to show them all, so ScrollBy actually moves the -// offset instead of no-oping at AtBottom(). +// viewport too short to show them all, so ScrollBy actually moves the offset +// instead of no-oping at AtBottom(). Single-line output keeps an expanded +// block at two rows, so one wheel notch crosses a block boundary. func scrollableAccordionTestModel(t *testing.T) *Model { t.Helper() m := newShell() @@ -198,7 +208,7 @@ func scrollableAccordionTestModel(t *testing.T) *Model { blocks = append(blocks, agent.Block{ ID: agent.BlockID{Scope: agent.ScopeTool, Key: fmt.Sprintf("call-%d", i)}, Kind: assistant.KindToolResult, - Tool: &agent.ToolBlock{Name: "search_logs", Status: agent.ToolSuccess, Output: accordionTestOutput}, + Tool: &agent.ToolBlock{Name: "search_logs", Status: agent.ToolSuccess, Output: "row 1"}, }) } m.transcript.Blocks = blocks @@ -223,22 +233,35 @@ func visibleAccordionRow(t *testing.T, m *Model) int { return -1 } +// TestMouseWheelScrollRefreshesHoverUnderTheStationaryPointer: a wheel notch +// that slides a different block under a stationary pointer must retarget +// hover to it. Hover stays set across the notch; the targeted block changes. func TestMouseWheelScrollRefreshesHoverUnderTheStationaryPointer(t *testing.T) { m := scrollableAccordionTestModel(t) - // Expanded blocks put detail rows, not the next header, under the pointer - // once the wheel scrolls. m.list.ToggleAllDisclosure() m.list.ScrollToTop() - row := visibleAccordionRow(t, m) + // One row below the chevron is the first block's lone detail row. + row := visibleAccordionRow(t, m) + 1 + before, ok := m.list.AccordionAt(row) + if !ok { + t.Fatalf("row %d is not an accordion target before scrolling", row) + } m.Update(tea.MouseMotionMsg{X: 0, Y: row}) if !m.list.Hovered() { - t.Fatal("expected hovering the chevron to set hover") + t.Fatal("expected hovering a detail row to set hover") } m.Update(tea.MouseWheelMsg{Button: tea.MouseWheelDown, X: 0, Y: row}) - if m.list.Hovered() { - t.Fatal("scrolling moved the accordion row out from under the pointer, but hover was not recomputed") + after, ok := m.list.AccordionAt(row) + if !ok { + t.Fatalf("row %d is not an accordion target after scrolling", row) + } + if !m.list.Hovered() { + t.Fatal("scrolling a new block under the stationary pointer did not hover it") + } + if after == before { + t.Fatalf("fixture did not advance to a different block after one wheel notch: still %v", before) } } diff --git a/internal/tui/chat/list.go b/internal/tui/chat/list.go index 6212107..600954a 100644 --- a/internal/tui/chat/list.go +++ b/internal/tui/chat/list.go @@ -371,23 +371,53 @@ func (l *List) SetPointerRow(y int) { l.pointerY = y } // SetPointer records the viewport cell under the mouse. func (l *List) SetPointer(x, y int) { l.pointerX, l.pointerY = x, y } -// Hovered reports whether the pointer sits over a clickable accordion row. +// Hovered reports whether the pointer sits over a clickable accordion zone. func (l *List) Hovered() bool { - _, ok := l.HeaderAt(l.pointerY) + _, ok := l.AccordionAt(l.pointerY) return ok } // HoveredLink returns the rendered link beneath the stationary pointer. func (l *List) HoveredLink() (LinkHit, bool) { return l.LinkAt(l.pointerX, l.pointerY) } -// HeaderAt returns the block whose clickable accordion header sits on viewport -// row y. The whole row is the target, so only y matters. -func (l *List) HeaderAt(y int) (agent.BlockID, bool) { - idx, line, ok := l.itemAtRow(y) - if ok && line == 0 && l.gutter(l.view[idx]) > 0 { - return l.view[idx].id, true +// AccordionAt returns the block whose clickable accordion zone sits on +// viewport row y: every row an expandable block renders, whether collapsed +// (header plus any compact preview) or expanded. +func (l *List) AccordionAt(y int) (agent.BlockID, bool) { + idx, _, ok := l.accordionAtRow(y) + if !ok { + return agent.BlockID{}, false } - return agent.BlockID{}, false + return l.view[idx].id, true +} + +// accordionAtRow maps a viewport row to the guttered item it targets and the +// row's line within it; line 0 is the header. +func (l *List) accordionAtRow(y int) (idx, line int, ok bool) { + idx, line, ok = l.itemAtRow(y) + if !ok || l.gutter(l.view[idx]) == 0 { + return 0, 0, false + } + return idx, line, true +} + +// hoverFill is the row span an accordion hover paints. header is unclamped +// and may be negative once the header scrolls above the viewport; start and +// end are clamped to [0, height]. +type hoverFill struct { + header, start, end int +} + +// hoverFillRows returns the rows the pointer's accordion hover should paint, +// clamped to the viewport. +func (l *List) hoverFillRows() (hoverFill, bool) { + idx, line, ok := l.accordionAtRow(l.pointerY) + if !ok { + return hoverFill{}, false + } + header := l.pointerY - line + end := header + len(l.entry(idx).lines) + return hoverFill{header: header, start: max(header, 0), end: min(end, l.height)}, true } // itemAtRow maps a viewport row to a rendered presentation item. Gaps and @@ -728,8 +758,14 @@ func (l *List) renderSurface(withPosition bool) Surface { for len(lines) < budget { lines = append(lines, "") } - if l.Hovered() { - lines[l.pointerY] = components.PaintRowBackground(lines[l.pointerY], l.width, l.sty.Accordion.HoverBackground) + if fill, ok := l.hoverFillRows(); ok { + for row := fill.start; row < fill.end; row++ { + bg := l.sty.Accordion.DetailHoverBackground + if row == fill.header { + bg = l.sty.Accordion.HoverBackground + } + lines[row] = components.PaintRowBackground(lines[row], l.width, bg) + } } top := 0 if withPosition { diff --git a/internal/tui/chat/list_test.go b/internal/tui/chat/list_test.go index b8f14aa..41bb80b 100644 --- a/internal/tui/chat/list_test.go +++ b/internal/tui/chat/list_test.go @@ -9,6 +9,7 @@ import ( "github.com/datadog-labs/bits-cli/internal/agent" "github.com/datadog-labs/bits-cli/internal/assistant" + "github.com/datadog-labs/bits-cli/internal/filediff" "github.com/datadog-labs/bits-cli/internal/tools/spec" "github.com/datadog-labs/bits-cli/internal/tui/components" "github.com/datadog-labs/bits-cli/internal/tui/styles" @@ -345,7 +346,7 @@ func TestReasoningExpandsToItsThinkingText(t *testing.T) { if !strings.Contains(compact, "▶ reasoning") || strings.Contains(compact, "idea") { t.Fatalf("reasoning did not start collapsed on its header:\n%s", compact) } - if _, ok := list.HeaderAt(0); !ok { + if _, ok := list.AccordionAt(0); !ok { t.Fatal("reasoning header is not clickable") } @@ -763,7 +764,7 @@ func TestNothingToExpandDrawsNoGutter(t *testing.T) { list.SetHeight(8) list.SetItems([]agent.Block{tc.block}) - if _, ok := list.HeaderAt(0); ok { + if _, ok := list.AccordionAt(0); ok { t.Fatal("block with nothing to expand is clickable") } bare := list.renderPresentationItem(list.view[0], renderContext{width: list.width, sty: list.sty, frame: list.frame}) @@ -815,25 +816,115 @@ func TestDocumentAgreesWithVisibleSurfaceAcrossCollapseAndScroll(t *testing.T) { } } -func TestHeaderAtOnlyMatchesAVisibleHeaderRow(t *testing.T) { - list := listWithTool(agent.ToolSuccess) - list.ToggleDisclosure(list.view[0].id) // give the header detail rows - list.ScrollToTop() // drop follow so the header starts visible - list.SetHeight(2) - if id, ok := list.HeaderAt(0); !ok || id != list.view[0].id { - t.Fatalf("HeaderAt(0) = %v, %t, want the tool's header", id, ok) +// Dark-theme hover fills, as SGR background parameters. +const ( + headerHoverFill = "48;2;34;36;43" // accordionHover (#22242B) + detailHoverFill = "48;2;28;30;38" // accordionDetailHover (#1C1E26) +) + +func hasHoverFill(line string) bool { + return strings.Contains(line, headerHoverFill) || strings.Contains(line, detailHoverFill) +} + +// listWithEditDiff returns a sized list holding one edit_file block whose +// collapsed view renders a multi-line diff preview below its header. +func listWithEditDiff() *List { + var before, after strings.Builder + for i := 1; i <= 5; i++ { + fmt.Fprintf(&before, "line-%d\n", i) + fmt.Fprintf(&after, "LINE-%d\n", i) + } + diff, display := filediff.BuildWithDisplay("a/f.go", "b/f.go", before.String(), after.String()) + block := editorToolBlock("edit_file", display, &filediff.State{ + Phase: filediff.PhaseApplied, + Change: &filediff.Change{Path: "f.go", Op: filediff.OpEdit, State: filediff.ChangeApplied, Diff: &diff}, + }) + block.ID = agent.BlockID{Scope: agent.ScopeTool, Key: "call-1"} + + list := NewList() + list.SetStyles(DefaultStyles(true)) + list.SetWidth(80) + list.SetHeight(20) + list.SetItems([]agent.Block{block}) + return list +} + +// TestAccordionZoneIsTheWholeRenderedBlock: whatever a guttered block renders +// (a lone header, a collapsed preview such as exec_command or edit_file, or +// the full disclosure) is both its click target and its hover region. From +// any of its rows, hover paints the header fill on row 0 and the detail fill +// below, and neither targeting nor painting reaches past its last row. +func TestAccordionZoneIsTheWholeRenderedBlock(t *testing.T) { + for _, tc := range []struct { + name string + list func() *List + expanded bool + }{ + {"collapsed header only", func() *List { return listWithTool(agent.ToolSuccess) }, false}, + {"expanded detail", func() *List { return listWithTool(agent.ToolSuccess) }, true}, + {"collapsed preview", listWithEditDiff, false}, + {"expanded preview", listWithEditDiff, true}, + } { + t.Run(tc.name, func(t *testing.T) { + list := tc.list() + if tc.expanded { + list.ToggleDisclosure(list.view[0].id) + } + list.ScrollToTop() + height := list.itemHeight(0) + if height >= list.height { + t.Fatalf("fixture is %d rows in a %d-row viewport, want room below it", height, list.height) + } + + for row := range height { + if id, ok := list.AccordionAt(row); !ok || id != list.view[0].id { + t.Fatalf("AccordionAt(%d) = %v, %t, want the block", row, id, ok) + } + } + if _, ok := list.AccordionAt(height); ok { + t.Fatalf("AccordionAt(%d) matched past the block's last row", height) + } + + for _, pointer := range []int{0, height - 1} { + list.SetPointerRow(pointer) + lines := strings.Split(list.Render(), "\n") + for row := range height { + want := detailHoverFill + if row == 0 { + want = headerHoverFill + } + if !strings.Contains(lines[row], want) { + t.Fatalf("pointer on row %d: row %d missing its hover fill %q:\n%s", pointer, row, want, lines[row]) + } + } + if hasHoverFill(lines[height]) { + t.Fatalf("pointer on row %d: hover leaked past the block into row %d:\n%s", pointer, height, lines[height]) + } + } + }) } - if _, ok := list.HeaderAt(1); ok { - t.Fatal("HeaderAt matched a detail row") +} + +// TestAccordionZoneSurvivesTheHeaderScrollingOff: the header is not the +// hit-test anchor, so scrolling it off must keep the rest targeted, and the +// rows left on screen keep the detail fill rather than taking the header's. +func TestAccordionZoneSurvivesTheHeaderScrollingOff(t *testing.T) { + list := listWithTool(agent.ToolSuccess) + list.ToggleDisclosure(list.view[0].id) + list.SetHeight(1) + list.ScrollBy(1) // scroll past the header; no chevron remains visible + + if id, ok := list.AccordionAt(0); !ok || id != list.view[0].id { + t.Fatalf("AccordionAt(0) = %v, %t, want the block even with its header scrolled off", id, ok) } - if _, ok := list.HeaderAt(2); ok { - t.Fatal("HeaderAt matched a row outside the viewport") + if _, ok := list.AccordionAt(1); ok { + t.Fatal("AccordionAt matched a row outside the viewport") } - list.SetHeight(1) - list.ScrollBy(1) - if _, ok := list.HeaderAt(0); ok { - t.Fatal("HeaderAt matched after the header scrolled off the top") + list.SetPointerRow(0) + line := list.Render() + if !strings.Contains(line, detailHoverFill) || strings.Contains(line, headerHoverFill) { + t.Fatalf("visible detail row not painted with the detail fill alone:\n%s", line) } } diff --git a/internal/tui/components/accordion.go b/internal/tui/components/accordion.go index 10980ad..25f6399 100644 --- a/internal/tui/components/accordion.go +++ b/internal/tui/components/accordion.go @@ -7,8 +7,8 @@ import ( ) // Accordion renders the disclosure control for a block's detail rows. The -// caller owns the expanded state and the click target; hover is painted by the -// caller across the whole row (see PaintRowBackground). +// caller owns the expanded state and the click target, which is every row the +// block renders, and paints hover across it (see PaintRowBackground). func Accordion(sty styles.Accordion, expanded bool) string { glyph := sty.Collapsed if expanded { diff --git a/internal/tui/model.go b/internal/tui/model.go index 307183e..89da93b 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -263,8 +263,8 @@ type Model struct { follow followControl // pendingAccordionToggle is the block a mouse-down landed on inside an - // accordion header row. Every left-click also begins a potential - // drag-select (see beginSelection), so the toggle stays pending until + // accordion zone: any row an expandable block renders, collapsed preview + // rows included. Every left-click also begins a potential drag-select (see beginSelection), so the toggle stays pending until // finishSelection sees the gesture through: it fires only if the release // never turned it into a real selection range. pendingAccordionToggle agent.BlockID diff --git a/internal/tui/styles/builder.go b/internal/tui/styles/builder.go index 7c41149..8283f06 100644 --- a/internal/tui/styles/builder.go +++ b/internal/tui/styles/builder.go @@ -118,7 +118,8 @@ func build(isDark bool, p palette) Theme { Foreground(lipgloss.Color(p.textSecondary)). Background(lipgloss.Color(p.accordionBackground)). Padding(0, 1), - HoverBackground: lipgloss.Color(p.accordionHover), + HoverBackground: lipgloss.Color(p.accordionHover), + DetailHoverBackground: lipgloss.Color(p.accordionDetailHover), } selector := Selector{ Item: text.Primary, diff --git a/internal/tui/styles/palette.go b/internal/tui/styles/palette.go index 7aeaa7d..5c5807a 100644 --- a/internal/tui/styles/palette.go +++ b/internal/tui/styles/palette.go @@ -38,6 +38,11 @@ type palette struct { // just reuses surfaceRaised, which already sits one step above surface. accordionHover string + // accordionDetailHover is accordionHover blended at 45% over the + // background, used for a hovered block's rows below its header so the header + // keeps the stronger tone. + accordionDetailHover string + // accordionBackground is the disclosure control's resting fill. Fixed to // the same hex in both palettes by design request, rather than tuned per // theme like the rest of this file. @@ -78,40 +83,41 @@ type palette struct { func darkPalette() palette { return palette{ - primary: "#5e6dd6", - interactive: "#8B80F9", - secondary: "#3f4ca5", - onAccent: "#FFFFFF", - onWarning: "#1A1A1A", - textPrimary: "#FFFFFF", - textSecondary: "#A2A3A6", - textTertiary: "#45474D", - background: "#171921", - surface: "#22252F", - surfaceRaised: "236", - approvalSurface: "#2C3142", - inputRule: "#383A40", - borderSubtle: "#474A54", - codeSurface: "#343336", - codeText: "#CECECE", - accordionHover: "#22242B", - accordionBackground: "#22242B", - sweepHot: "#A2C6FF", - link: "#3d8bd0", - busy: "#F5C453", - busySurface: "#2E2409", - busyDim: "#8A6A1F", - busyHot: "#FFE9A8", - success: "#349C50", - successSurface: "#0A2F10", - error: "#D33043", - errorSurface: "#2F0A0F", - feedbackSuccess: "#65A875", - feedbackError: "#D77480", - critical: "#C4314B", - info: "#632CA6", - warning: "#F5A623", - fullAccessSurface: "#812836", + primary: "#5e6dd6", + interactive: "#8B80F9", + secondary: "#3f4ca5", + onAccent: "#FFFFFF", + onWarning: "#1A1A1A", + textPrimary: "#FFFFFF", + textSecondary: "#A2A3A6", + textTertiary: "#45474D", + background: "#171921", + surface: "#22252F", + surfaceRaised: "236", + approvalSurface: "#2C3142", + inputRule: "#383A40", + borderSubtle: "#474A54", + codeSurface: "#343336", + codeText: "#CECECE", + accordionHover: "#22242B", + accordionDetailHover: "#1C1E26", + accordionBackground: "#22242B", + sweepHot: "#A2C6FF", + link: "#3d8bd0", + busy: "#F5C453", + busySurface: "#2E2409", + busyDim: "#8A6A1F", + busyHot: "#FFE9A8", + success: "#349C50", + successSurface: "#0A2F10", + error: "#D33043", + errorSurface: "#2F0A0F", + feedbackSuccess: "#65A875", + feedbackError: "#D77480", + critical: "#C4314B", + info: "#632CA6", + warning: "#F5A623", + fullAccessSurface: "#812836", } } @@ -121,39 +127,40 @@ func darkPalette() palette { // The three text levels are picked by eye; see the palette struct. func lightPalette() palette { return palette{ - primary: "#5e6dd6", - interactive: "#3F4991", - secondary: "#1d2140", - onAccent: "#FFFFFF", - onWarning: "#1A1A1A", - textPrimary: "#000000", - textSecondary: "#5E5F62", - textTertiary: "#989BA0", - background: "#ECEFF4", - surface: "#CCD3DF", - surfaceRaised: "#BDC5D3", - approvalSurface: "#B7C1D5", - inputRule: "#383A40", - borderSubtle: "#9AA3B2", - codeSurface: "#CED3DD", - codeText: "#1C2E38", - accordionHover: "#BDC5D3", - accordionBackground: "#22242B", - sweepHot: "#A2C6FF", - link: "#006bc2", - busy: "#7A5200", - busySurface: "#E5D198", - busyDim: "#D2A340", - busyHot: "#4A3000", - success: "#41C464", - successSurface: "#EAFDED", - error: "#B61225", - errorSurface: "#F9C6CC", - feedbackSuccess: "#30663E", - feedbackError: "#A13443", - critical: "#C4314B", - info: "#632CA6", - warning: "#F5A623", - fullAccessSurface: "#EC93A0", + primary: "#5e6dd6", + interactive: "#3F4991", + secondary: "#1d2140", + onAccent: "#FFFFFF", + onWarning: "#1A1A1A", + textPrimary: "#000000", + textSecondary: "#5E5F62", + textTertiary: "#989BA0", + background: "#ECEFF4", + surface: "#CCD3DF", + surfaceRaised: "#BDC5D3", + approvalSurface: "#B7C1D5", + inputRule: "#383A40", + borderSubtle: "#9AA3B2", + codeSurface: "#CED3DD", + codeText: "#1C2E38", + accordionHover: "#BDC5D3", + accordionDetailHover: "#D7DCE5", + accordionBackground: "#22242B", + sweepHot: "#A2C6FF", + link: "#006bc2", + busy: "#7A5200", + busySurface: "#E5D198", + busyDim: "#D2A340", + busyHot: "#4A3000", + success: "#41C464", + successSurface: "#EAFDED", + error: "#B61225", + errorSurface: "#F9C6CC", + feedbackSuccess: "#30663E", + feedbackError: "#A13443", + critical: "#C4314B", + info: "#632CA6", + warning: "#F5A623", + fullAccessSurface: "#EC93A0", } } diff --git a/internal/tui/styles/styles.go b/internal/tui/styles/styles.go index c058058..e9e8b8d 100644 --- a/internal/tui/styles/styles.go +++ b/internal/tui/styles/styles.go @@ -185,6 +185,11 @@ type Accordion struct { // wrapping it in a lipgloss style, since the line's own foreground colors // must survive the repaint. HoverBackground color.Color + + // DetailHoverBackground paints a block's rows below its header (compact + // preview or full disclosure) while any row of it is hovered. Softer than HoverBackground so the header + // stays distinct from its content. + DetailHoverBackground color.Color } // Selector styles the reusable two-column keyboard selector. diff --git a/internal/tui/update.go b/internal/tui/update.go index a8c8a8d..ae18a1d 100644 --- a/internal/tui/update.go +++ b/internal/tui/update.go @@ -590,7 +590,7 @@ func (m *Model) handleMouse(msg tea.MouseMsg) tea.Cmd { if msg.Button == tea.MouseLeft { m.beginLinkClick(msg) // Defer the toggle until release so drag selection can take over. - m.pendingAccordionToggle, m.hasPendingAccordionToggle = m.list.HeaderAt(msg.Y) + m.pendingAccordionToggle, m.hasPendingAccordionToggle = m.list.AccordionAt(msg.Y) return m.beginSelection(msg) } }