From 0607abf9e562691d675812a1992680bcc824e30d Mon Sep 17 00:00:00 2001 From: Ahmed Allam <49919286+0xallam@users.noreply.github.com> Date: Fri, 7 Aug 2026 16:14:22 +0300 Subject: [PATCH] fix(tui): scrollbar visibility, findings scrolling, and report navigation (#1006) --- .../tui/internal/app/findings_test.go | 299 ++++++++++++++++ strix/interface/tui/internal/app/model.go | 15 +- .../interface/tui/internal/app/model_test.go | 17 +- strix/interface/tui/internal/app/update.go | 109 ++++-- strix/interface/tui/internal/app/view.go | 24 +- .../tui/internal/app/vulnerabilities.go | 319 ++++++++++++------ 6 files changed, 634 insertions(+), 149 deletions(-) create mode 100644 strix/interface/tui/internal/app/findings_test.go diff --git a/strix/interface/tui/internal/app/findings_test.go b/strix/interface/tui/internal/app/findings_test.go new file mode 100644 index 00000000..0e865f57 --- /dev/null +++ b/strix/interface/tui/internal/app/findings_test.go @@ -0,0 +1,299 @@ +package app + +import ( + "encoding/json" + "fmt" + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/x/ansi" + "github.com/usestrix/strix/tui/internal/protocol" +) + +func findingsModel(t *testing.T, titles ...string) Model { + t.Helper() + m := New(nil) + m.width, m.height = 130, 30 + m.showSplash = false + m.handleEnvelope(stateEnvelope(t, 1, protocol.Snapshot{ScanState: "running"})) + items := make([]json.RawMessage, 0, len(titles)) + for i, title := range titles { + items = append(items, rawJSON(t, map[string]any{ + "id": string(rune('a' + i)), "title": title, "severity": "high", + })) + } + m.handleEnvelope(protocol.Envelope{Version: protocol.Version, Type: "collection_bootstrap", + Payload: rawJSON(t, protocol.CollectionBootstrap{ + Collection: "vulnerabilities", Revision: 1, Cursor: 0, + NextCursor: len(items), Done: true, Items: items, + })}) + m.resizeViewport() + return m +} + +// The list scrolls by row, not by finding. Stepping a whole entry at a time is +// what made a list of wrapped titles feel paginated. +func TestFindingsScrollByRow(t *testing.T) { + long := "A deliberately long finding title that wraps across several rows in the sidebar" + m := findingsModel(t, long, long, long) + + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + if len(rows) <= 3 { + t.Fatalf("titles did not wrap, so this proves nothing: %d rows", len(rows)) + } + total, offset := m.vulnerabilityScrollRows() + if total != len(rows) || offset != 0 { + t.Fatalf("scroll metrics are not in rows: total=%d offset=%d rows=%d", total, offset, len(rows)) + } + + // One step of the offset moves one row, and the first visible line follows it. + first := strings.Split(ansi.Strip(m.vulnerabilitiesView(40, 4)), "\n")[0] + m.vulnOffset = 1 + second := strings.Split(ansi.Strip(m.vulnerabilitiesView(40, 4)), "\n")[0] + if first == second { + t.Fatalf("advancing one row did not move the list: %q", first) + } + // That row still belongs to the first finding, which an item-stepping list + // would have skipped past entirely. + if got := m.vulnerabilityIndexAtRow(0); got != 0 { + t.Fatalf("one row in, the top line belongs to finding %d, want 0", got) + } +} + +// Selecting a finding scrolls the least it can, and never past its own start. +func TestSelectingAFindingBringsItIntoView(t *testing.T) { + long := "A deliberately long finding title that wraps across several rows in the sidebar" + m := findingsModel(t, long, long, long, long) + + m.selectedVuln = 3 + m.ensureVulnerabilityVisible() + + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + height := m.vulnerabilityPageSize() + end := min(len(rows), m.vulnOffset+height) + found := false + for _, row := range rows[m.vulnOffset:end] { + if row.index == 3 { + found = true + break + } + } + if !found { + t.Fatalf("the selected finding is not on screen: offset=%d height=%d", m.vulnOffset, height) + } + if m.vulnOffset > len(rows)-height && len(rows) > height { + t.Fatalf("scrolled past the end: offset=%d rows=%d height=%d", m.vulnOffset, len(rows), height) + } +} + +func reportModel(t *testing.T, count int) Model { + t.Helper() + titles := make([]string, 0, count) + for i := range count { + titles = append(titles, fmt.Sprintf("Finding number %d", i+1)) + } + m := findingsModel(t, titles...) + m.openModal(modalVulnerability) + return m +} + +// The open report can be stepped through the list without closing it. +func TestReportStepsBetweenFindings(t *testing.T) { + m := reportModel(t, 3) + + updated, _ := m.updateModal(tea.KeyMsg{Type: tea.KeyRight}) + m = updated.(Model) + if m.selectedVuln != 1 { + t.Fatalf("right moved to %d, want 1", m.selectedVuln) + } + if m.modal != modalVulnerability { + t.Fatal("stepping closed the report") + } + updated, _ = m.updateModal(tea.KeyMsg{Type: tea.KeyLeft}) + m = updated.(Model) + if m.selectedVuln != 0 { + t.Fatalf("left moved to %d, want 0", m.selectedVuln) + } +} + +// The ends do not wrap: rolling from the last report to the first would hide +// that you had reached the end. +func TestReportStepsStopAtTheEnds(t *testing.T) { + m := reportModel(t, 3) + + updated, _ := m.updateModal(tea.KeyMsg{Type: tea.KeyLeft}) + m = updated.(Model) + if m.selectedVuln != 0 { + t.Fatalf("left from the first report moved to %d, want 0", m.selectedVuln) + } + + m.selectedVuln = 2 + updated, _ = m.updateModal(tea.KeyMsg{Type: tea.KeyRight}) + m = updated.(Model) + if m.selectedVuln != 2 { + t.Fatalf("right from the last report moved to %d, want 2", m.selectedVuln) + } +} + +// Each direction is offered only when there is a report that way, and a lone +// finding is offered neither. +func TestReportNavigationHintsFollowAvailability(t *testing.T) { + m := reportModel(t, 3) + for _, testCase := range []struct { + index int + wantPrev, wantNext bool + position string + }{ + {index: 0, wantNext: true, position: "1/3"}, + {index: 1, wantPrev: true, wantNext: true, position: "2/3"}, + {index: 2, wantPrev: true, position: "3/3"}, + } { + m.selectedVuln = testCase.index + view := ansi.Strip(m.modalView()) + if !strings.Contains(view, testCase.position) { + t.Fatalf("report %d does not show %q", testCase.index, testCase.position) + } + if got := strings.Contains(view, reportPrev); got != testCase.wantPrev { + t.Fatalf("report %d prev hint = %v, want %v", testCase.index, got, testCase.wantPrev) + } + if got := strings.Contains(view, reportNext); got != testCase.wantNext { + t.Fatalf("report %d next hint = %v, want %v", testCase.index, got, testCase.wantNext) + } + } + + lone := reportModel(t, 1) + view := ansi.Strip(lone.modalView()) + if strings.Contains(view, reportPrev) || strings.Contains(view, reportNext) || strings.Contains(view, "1/1") { + t.Fatalf("a lone finding offered navigation:\n%s", view) + } +} + +// A new report opens at its top, and the copy state does not carry over. +func TestSteppingResetsTheReportView(t *testing.T) { + m := reportModel(t, 3) + m.vulnerabilityCopied = true + m.vulnViewport.SetYOffset(3) + + m.showVulnerability(1) + + if m.vulnViewport.YOffset != 0 { + t.Fatalf("the next report opened scrolled to %d", m.vulnViewport.YOffset) + } + if m.vulnerabilityCopied { + t.Fatal("the copy state carried over to another report") + } +} + +// Prev and Next are buttons, not just key hints: they can be clicked. +func TestReportStepButtonsAreClickable(t *testing.T) { + m := reportModel(t, 3) + m.selectedVuln = 1 + + click := func(label string) Model { + t.Helper() + view := m.modalView() + left, top, _, _ := m.centeredViewBounds(view) + for row, line := range strings.Split(view, "\n") { + plain := ansi.Strip(line) + index := strings.Index(plain, label) + if index < 0 { + continue + } + updated, _ := m.updateModalMouse(tea.MouseMsg{ + X: left + ansi.StringWidth(plain[:index]) + 1, Y: top + row, + Button: tea.MouseButtonLeft, Action: tea.MouseActionPress, + }) + return updated.(Model) + } + t.Fatalf("%q was not rendered", label) + return m + } + + if got := click(reportNext).selectedVuln; got != 2 { + t.Fatalf("clicking Next selected %d, want 2", got) + } + if got := click(reportPrev).selectedVuln; got != 0 { + t.Fatalf("clicking Prev selected %d, want 0", got) + } + if got := click(reportNext).modal; got != modalVulnerability { + t.Fatalf("clicking Next closed the report: modal=%v", got) + } +} + +// Tab walks the whole row, so the step buttons are reachable from the keyboard +// as well, and Enter presses whichever one is focused. +func TestTabReachesTheStepButtons(t *testing.T) { + m := reportModel(t, 3) + m.selectedVuln = 1 + + if got := m.focusedReportButton(); got != reportDone { + t.Fatalf("the report opened focused on %q, want %q", got, reportDone) + } + seen := map[string]bool{} + for range len(m.reportButtons()) { + updated, _ := m.updateModal(tea.KeyMsg{Type: tea.KeyTab}) + m = updated.(Model) + seen[m.focusedReportButton()] = true + } + for _, want := range []string{reportPrev, reportNext, reportCopy, reportDone} { + if !seen[want] { + t.Fatalf("tab never reached %q: %v", want, seen) + } + } + + // Enter on a focused step button steps. + m.reportFocus = reportNext + updated, _ := m.updateModal(tea.KeyMsg{Type: tea.KeyEnter}) + if got := updated.(Model).selectedVuln; got != 2 { + t.Fatalf("enter on Next selected %d, want 2", got) + } +} + +// Stepping to an end drops that button from the row; focus must not be stranded +// on it. +func TestFocusFallsBackWhenAStepButtonDisappears(t *testing.T) { + m := reportModel(t, 2) + m.selectedVuln = 0 + m.reportFocus = reportNext + + updated, _ := m.updateModal(tea.KeyMsg{Type: tea.KeyEnter}) + m = updated.(Model) + + if m.selectedVuln != 1 { + t.Fatalf("enter on Next selected %d, want 1", m.selectedVuln) + } + // Next is gone at the last report, so the focus cannot still be on it. + if got := m.focusedReportButton(); got == reportNext { + t.Fatalf("focus stayed on a button that is no longer shown: %q", got) + } + if got := m.focusedReportButton(); got != reportDone { + t.Fatalf("focus fell back to %q, want %q", got, reportDone) + } +} + +// The list must be laid out at one width. Rendering at one and hit-testing at +// another gives two different row counts for the same title, and then a click +// resolves to the wrong finding and the scrollbar reports the wrong length. +func TestFindingsUseOneWidthForRenderAndInteraction(t *testing.T) { + // This title wraps to one row at 21 columns and two at 20, which is exactly + // the pair of widths the two paths used to disagree on. + m := findingsModel(t, "ffffff dddd a a a a", "eeeee eeeee a a a a", "header dddd a a a a") + + width := m.vulnerabilityListWidth() + rows := m.vulnerabilityRows(width) + rendered := strings.Split(ansi.Strip(m.vulnerabilitiesView(width, len(rows))), "\n") + + if len(rendered) != len(rows) { + t.Fatalf("rendered %d rows, interaction counts %d", len(rendered), len(rows)) + } + for row := range rendered { + if got := m.vulnerabilityIndexAtRow(row); got != rows[row].index { + t.Fatalf("row %d shows finding %d but a click resolves to %d", + row, rows[row].index, got) + } + } + if total, _ := m.vulnerabilityScrollRows(); total != len(rendered) { + t.Fatalf("the scrollbar reports %d rows, %d are rendered", total, len(rendered)) + } +} diff --git a/strix/interface/tui/internal/app/model.go b/strix/interface/tui/internal/app/model.go index f2e876e4..2cacb8eb 100644 --- a/strix/interface/tui/internal/app/model.go +++ b/strix/interface/tui/internal/app/model.go @@ -110,6 +110,7 @@ type Model struct { agentOffset int vulnOffset int modalChoice int + reportFocus string ready bool quitting bool showSplash bool @@ -157,12 +158,16 @@ const ( treeCursorBg = lipgloss.Color("#0178d4") ) -// Scrollbar thumbs. Each panel keeps its own, and the track stays blank so a -// scrollable panel does not gain a visible rule down its edge. +// Scrollbar thumbs. The track stays blank so a scrollable panel does not gain a +// visible rule down its edge, and the thumb brightens while it is dragged, which +// is the feedback Textual gave through scrollbar-color-active. +// +// One resting color for every panel, rather than the three the stylesheet named. +// The chat pane's was #1a1a1a on black, which is invisible - the bar could not be +// found, let alone grabbed (#1005). const ( - thumbTrace = lipgloss.Color("#1a1a1a") - thumbAgents = lipgloss.Color("#404040") - thumbFindings = lipgloss.Color("#333333") + thumbResting = lipgloss.Color("#3f3f46") + thumbActive = lipgloss.Color("#9ca3af") ) // Composer placeholders. The launch screen falls back to the short prompt when diff --git a/strix/interface/tui/internal/app/model_test.go b/strix/interface/tui/internal/app/model_test.go index e971d392..2b9f0dfb 100644 --- a/strix/interface/tui/internal/app/model_test.go +++ b/strix/interface/tui/internal/app/model_test.go @@ -527,7 +527,8 @@ func TestVulnerabilityCopySupportsKeyboardAndMouse(t *testing.T) { } model := newModel() - updated, _ := model.updateModal(tea.KeyMsg{Type: tea.KeyLeft}) + // Tab moves between the buttons; the arrows step between reports. + updated, _ := model.updateModal(tea.KeyMsg{Type: tea.KeyTab}) model = updated.(Model) updated, cmd := model.updateModal(tea.KeyMsg{Type: tea.KeyEnter}) model = updated.(Model) @@ -560,8 +561,8 @@ func TestVulnerabilityCopySupportsKeyboardAndMouse(t *testing.T) { X: copyX, Y: copyY, Button: tea.MouseButtonLeft, Action: tea.MouseActionPress, }) model = updated.(Model) - if cmd == nil || model.modalChoice != 0 { - t.Fatalf("mouse Copy was not activated: choice=%d cmd=%v", model.modalChoice, cmd) + if cmd == nil || model.reportFocus != reportCopy { + t.Fatalf("mouse Copy was not activated: focus=%q cmd=%v", model.reportFocus, cmd) } cmd() if len(copied) != 2 { @@ -820,8 +821,8 @@ func TestRunningViewerShowsCompleteWrappedURL(t *testing.T) { } func TestVerticalScrollbarThumbTracksScrollOffset(t *testing.T) { - top := strings.Split(ansi.Strip(verticalScrollbar(6, 24, 6, 0, thumbAgents)), "\n") - bottom := strings.Split(ansi.Strip(verticalScrollbar(6, 24, 6, 18, thumbAgents)), "\n") + top := strings.Split(ansi.Strip(verticalScrollbar(6, 24, 6, 0, thumbResting)), "\n") + bottom := strings.Split(ansi.Strip(verticalScrollbar(6, 24, 6, 18, thumbResting)), "\n") // The track is blank, so only the thumb is drawn. if top[0] != "█" || top[5] != " " { @@ -830,10 +831,10 @@ func TestVerticalScrollbarThumbTracksScrollOffset(t *testing.T) { if bottom[0] != " " || bottom[5] != "█" { t.Fatalf("bottom scrollbar is incorrect: %#v", bottom) } - if full := verticalScrollbar(4, 4, 4, 0, thumbAgents); full != "" { + if full := verticalScrollbar(4, 4, 4, 0, thumbResting); full != "" { t.Fatalf("non-overflowing scrollbar should be hidden: %q", full) } - withoutBar := ansi.Strip(withVerticalScrollbar("content", 12, 2, 2, 2, 0, thumbAgents)) + withoutBar := ansi.Strip(withVerticalScrollbar("content", 12, 2, 2, 2, 0, thumbResting)) if strings.ContainsAny(withoutBar, "█") { t.Fatalf("non-overflowing panel rendered a scrollbar: %q", withoutBar) } @@ -841,7 +842,7 @@ func TestVerticalScrollbarThumbTracksScrollOffset(t *testing.T) { // The bar takes exactly one column, so a scrolling panel keeps the rest. func TestVerticalScrollbarOccupiesOneColumn(t *testing.T) { - rows := strings.Split(withVerticalScrollbar("content", 12, 2, 24, 2, 0, thumbTrace), "\n") + rows := strings.Split(withVerticalScrollbar("content", 12, 2, 24, 2, 0, thumbResting), "\n") for _, row := range rows { if width := ansi.StringWidth(row); width != 12 { t.Fatalf("scrolling panel row width = %d, want 12", width) diff --git a/strix/interface/tui/internal/app/update.go b/strix/interface/tui/internal/app/update.go index 0cccea5d..2a22c4a9 100644 --- a/strix/interface/tui/internal/app/update.go +++ b/strix/interface/tui/internal/app/update.go @@ -219,7 +219,8 @@ func (m Model) updateMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { case vulnHeight > 0 && y < viewerHeight+agentHeight+vulnHeight: m.focus = focusVulnerabilities m.input.Blur() - m.vulnOffset = min(max(0, len(m.snapshot.Vulnerabilities)-1), m.vulnOffset+3) + totalRows, _ := m.vulnerabilityScrollRows() + m.vulnOffset = min(max(0, totalRows-m.vulnerabilityPageSize()), m.vulnOffset+3) m.keepVulnerabilitySelectionInWindow() } return m, nil @@ -318,24 +319,7 @@ func (m *Model) updateMainScrollbarMouse( if msg.Action != tea.MouseActionPress || msg.Button != tea.MouseButtonLeft { return false } - - target := scrollbarNone - switch { - case msg.X == chatWidth-2 && msg.Y >= 1 && msg.Y < chatHeight-1 && - m.viewport.TotalLineCount() > m.viewport.VisibleLineCount(): - target = scrollbarTrace - case showSidebar && msg.X == m.width-3 && msg.Y >= viewerHeight+2 && - msg.Y < viewerHeight+agentHeight-2 && - len(agentTreeEntries(m.snapshot.Agents, m.collapsedAgents)) > m.agentPageSize(): - target = scrollbarAgents - case showSidebar && vulnHeight > 0 && msg.X == m.width-3 && - msg.Y >= viewerHeight+agentHeight+1 && - msg.Y < viewerHeight+agentHeight+vulnHeight-1: - totalRows, _ := m.vulnerabilityScrollRows() - if totalRows > m.vulnerabilityPageSize() { - target = scrollbarFindings - } - } + target := m.scrollbarAt(msg, showSidebar, chatWidth, chatHeight, viewerHeight, agentHeight, vulnHeight) if target == scrollbarNone { return false } @@ -344,6 +328,40 @@ func (m *Model) updateMainScrollbarMouse( return true } +// scrollbarGrab is how far either side of the bar still counts as grabbing it. A +// one column target is unreasonable to hit with a mouse, and nothing else lives +// in the column beside it. +const scrollbarGrab = 1 + +func nearColumn(x, column int) bool { + return x >= column-scrollbarGrab && x <= column+scrollbarGrab +} + +// scrollbarAt reports which scrollbar, if any, the pointer is over. +func (m Model) scrollbarAt( + msg tea.MouseMsg, + showSidebar bool, + chatWidth, chatHeight, viewerHeight, agentHeight, vulnHeight int, +) scrollbarTarget { + switch { + case nearColumn(msg.X, chatWidth-2) && msg.Y >= 1 && msg.Y < chatHeight-1 && + m.viewport.TotalLineCount() > m.viewport.VisibleLineCount(): + return scrollbarTrace + case showSidebar && nearColumn(msg.X, m.width-3) && msg.Y >= viewerHeight+2 && + msg.Y < viewerHeight+agentHeight-2 && + len(agentTreeEntries(m.snapshot.Agents, m.collapsedAgents)) > m.agentPageSize(): + return scrollbarAgents + case showSidebar && vulnHeight > 0 && nearColumn(msg.X, m.width-3) && + msg.Y >= viewerHeight+agentHeight+1 && + msg.Y < viewerHeight+agentHeight+vulnHeight-1: + totalRows, _ := m.vulnerabilityScrollRows() + if totalRows > m.vulnerabilityPageSize() { + return scrollbarFindings + } + } + return scrollbarNone +} + func (m *Model) scrollFromMouse( target scrollbarTarget, y, chatHeight, viewerHeight, agentHeight int, @@ -367,10 +385,10 @@ func (m *Model) scrollFromMouse( case scrollbarFindings: height := m.vulnerabilityPageSize() totalRows, _ := m.vulnerabilityScrollRows() - rowOffset := scrollbarOffset(y-viewerHeight-agentHeight-1, height, totalRows, height) m.focus = focusVulnerabilities m.input.Blur() - m.vulnOffset = m.vulnerabilityOffsetAtRow(rowOffset) + // The offset is a row, so dragging moves the list continuously. + m.vulnOffset = scrollbarOffset(y-viewerHeight-agentHeight-1, height, totalRows, height) m.keepVulnerabilitySelectionInWindow() } } @@ -405,6 +423,22 @@ func (m Model) updateSetupMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { return m, nil } +// pressReportButton performs a button of the report row, however it was reached. +func (m Model) pressReportButton(button string) (tea.Model, tea.Cmd) { + switch button { + case reportPrev: + m.showVulnerability(m.selectedVuln - 1) + case reportNext: + m.showVulnerability(m.selectedVuln + 1) + case reportCopy: + m.reportFocus = reportCopy + return m, m.startVulnerabilityCopy() + default: + m.closeModal() + } + return m, nil +} + func (m Model) updateModalMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { if m.modal == modalVulnerability { view := m.modalView() @@ -441,13 +475,22 @@ func (m Model) updateModalMouse(msg tea.MouseMsg) (tea.Model, tea.Cmd) { return m.updateModal(tea.KeyMsg{Type: tea.KeyEnter}) } case modalVulnerability: + for _, button := range m.reportButtons() { + if button == reportCopy || button == reportDone { + continue + } + if m.centeredLabelHit(view, button, msg.X, msg.Y) { + m.reportFocus = button + return m.pressReportButton(button) + } + } if m.centeredLabelHit(view, "Copy", msg.X, msg.Y) { - m.modalChoice = 0 + m.reportFocus = reportCopy cmd := m.startVulnerabilityCopy() return m, cmd } if m.centeredLabelHit(view, "Done", msg.X, msg.Y) { - m.modalChoice = 1 + m.reportFocus = reportDone m.closeModal() } } @@ -516,16 +559,19 @@ func (m Model) updateModal(key tea.KeyMsg) (tea.Model, tea.Cmd) { switch key.String() { case "esc": m.closeModal() - case "left", "right", "tab", "shift+tab": - m.modalChoice = 1 - m.modalChoice + // The arrows step between reports directly; tab walks the button row. + case "left": + m.showVulnerability(m.selectedVuln - 1) + case "right": + m.showVulnerability(m.selectedVuln + 1) + case "tab": + m.stepReportFocus(1) + case "shift+tab": + m.stepReportFocus(-1) case "enter": - if m.modalChoice == 0 { - cmd := m.startVulnerabilityCopy() - return m, cmd - } - m.closeModal() + return m.pressReportButton(m.focusedReportButton()) case "c": - m.modalChoice = 0 + m.reportFocus = reportCopy cmd := m.startVulnerabilityCopy() return m, cmd case "up": @@ -584,6 +630,7 @@ func (m *Model) openModal(mode modalMode) { m.modalChoice = 1 } if mode == modalVulnerability { + m.reportFocus = reportDone m.modalChoice = 1 m.vulnerabilityCopied = false m.vulnerabilityCopyError = "" diff --git a/strix/interface/tui/internal/app/view.go b/strix/interface/tui/internal/app/view.go index dec2b09b..9ac0a55e 100644 --- a/strix/interface/tui/internal/app/view.go +++ b/strix/interface/tui/internal/app/view.go @@ -164,6 +164,15 @@ func wrapBlock(value string, width int) string { return strings.Join(out, "\n") } +// scrollbarThumb brightens the bar being dragged so the grab reads as taking +// hold of it. +func (m Model) scrollbarThumb(target scrollbarTarget) lipgloss.Color { + if m.draggingScrollbar == target { + return thumbActive + } + return thumbResting +} + func verticalScrollbar(height, total, visible, offset int, thumb lipgloss.Color) string { if height <= 0 || total <= visible { return "" @@ -424,7 +433,7 @@ func (m Model) renderChatPane(width, height int, border lipgloss.Color) string { m.viewport.TotalLineCount(), m.viewport.VisibleLineCount(), m.viewport.YOffset, - thumbTrace, + m.scrollbarThumb(scrollbarTrace), ) out := lipgloss.NewStyle().Width(width).Height(height). Border(lipgloss.RoundedBorder()).BorderForeground(border).Render(trace) @@ -489,7 +498,7 @@ func (m Model) sidebarView(width, height int) string { len(agentEntries), agentRows, m.agentOffset, - thumbAgents, + m.scrollbarThumb(scrollbarAgents), ) parts := []string{ lipgloss.NewStyle().Width(width-2).Height(m.viewerHeight()-2).Border(lipgloss.RoundedBorder()).BorderForeground(dark).Padding(0, 1).Render(m.viewerView(width - 4)), @@ -503,13 +512,13 @@ func (m Model) sidebarView(width, height int) string { vulnRows := max(1, vulnHeight-2) totalRows, offsetRows := m.vulnerabilityScrollRows() findings := withVerticalScrollbar( - m.vulnerabilitiesView(max(1, width-5), vulnRows), + m.vulnerabilitiesView(m.vulnerabilityListWidth(), vulnRows), width-4, vulnRows, totalRows, vulnRows, offsetRows, - thumbFindings, + m.scrollbarThumb(scrollbarFindings), ) parts = append(parts, lipgloss.NewStyle().Width(width-2).Height(vulnRows).Border(lipgloss.RoundedBorder()).BorderForeground(vulnBorder).Padding(0, 1).Render(findings)) } @@ -524,12 +533,7 @@ func (m Model) sidebarHeights() (statsHeight, vulnHeight, agentHeight int) { statsRows := lipgloss.Height(lipgloss.NewStyle().Width(m.viewerContentWidth()).Render(m.statsView())) statsHeight = min(15, statsRows+2) if len(m.snapshot.Vulnerabilities) > 0 { - rows := 0 - width := m.vulnerabilityListWidth() - for i := range m.snapshot.Vulnerabilities { - rows += len(m.vulnerabilityTitleLines(i, width)) - } - vulnHeight = min(12, rows+2) + vulnHeight = min(12, len(m.vulnerabilityRows(m.vulnerabilityListWidth()))+2) } agentHeight = max(3, m.height-m.viewerHeight()-statsHeight-vulnHeight) return diff --git a/strix/interface/tui/internal/app/vulnerabilities.go b/strix/interface/tui/internal/app/vulnerabilities.go index 0d4f23b4..b8d988bf 100644 --- a/strix/interface/tui/internal/app/vulnerabilities.go +++ b/strix/interface/tui/internal/app/vulnerabilities.go @@ -1,6 +1,7 @@ package app import ( + "fmt" "strings" tea "github.com/charmbracelet/bubbletea" @@ -13,115 +14,123 @@ var panelSeverityColors = map[string]lipgloss.Color{ "critical": render.SevCrit, "high": render.SevHigh, "medium": render.SevMed, "low": green, "info": blue, } -func (m Model) vulnerabilitiesView(width, height int) string { - var lines []string - start := min(max(0, m.vulnOffset), max(0, len(m.snapshot.Vulnerabilities)-1)) - for i := start; i < len(m.snapshot.Vulnerabilities) && len(lines) < height; i++ { - vuln := m.snapshot.Vulnerabilities[i] - severity := strings.ToLower(render.StringValue(vuln["severity"])) - color, ok := panelSeverityColors[severity] - if !ok { - color = blue // matches SEVERITY_COLORS.get(severity, "#3b82f6") +// vulnerabilityRow is one rendered line of the findings list. The list scrolls by +// row rather than by finding, so a long title does not make the panel jump a +// whole entry at a time. +type vulnerabilityRow struct { + index int // the finding this line belongs to + text string // one wrapped line of its title + first bool // the line that carries the number and the severity dot +} + +// vulnerabilityRows lays every finding out as the lines it will occupy. +func (m Model) vulnerabilityRows(width int) []vulnerabilityRow { + // Wrapped lines sit under the title rather than under the severity dot. + body := max(1, width-2) + rows := make([]vulnerabilityRow, 0, len(m.snapshot.Vulnerabilities)) + for i := range m.snapshot.Vulnerabilities { + for line, text := range strings.Split(wrapBlock(m.vulnerabilityTitle(i), body), "\n") { + rows = append(rows, vulnerabilityRow{index: i, text: text, first: line == 0}) } - marker := lipgloss.NewStyle().Foreground(color).Render("● ") + } + return rows +} + +func (m Model) vulnerabilitiesView(width, height int) string { + rows := m.vulnerabilityRows(width) + start := min(max(0, m.vulnOffset), max(0, len(rows)-1)) + end := min(len(rows), start+height) + lines := make([]string, 0, max(0, end-start)) + for _, row := range rows[start:end] { style := lipgloss.NewStyle().Foreground(textColor) - if i == m.selectedVuln { + if row.index == m.selectedVuln { style = style.Bold(true).Foreground(white) } - for row, titleLine := range m.vulnerabilityTitleLines(i, width) { - if len(lines) >= height { - break + prefix := " " + if row.first { + severity := strings.ToLower(render.StringValue(m.snapshot.Vulnerabilities[row.index]["severity"])) + color, ok := panelSeverityColors[severity] + if !ok { + color = blue // matches SEVERITY_COLORS.get(severity, "#3b82f6") } - prefix := " " - if row == 0 { - prefix = marker - } - lines = append(lines, prefix+style.Render(titleLine)) + prefix = lipgloss.NewStyle().Foreground(color).Render("● ") } + lines = append(lines, prefix+style.Render(row.text)) } return strings.Join(lines, "\n") } +// vulnerabilityListWidth is the one width the findings list is laid out at, for +// rendering and for every interaction alike. Wrapping a title at two widths a +// column apart gives two different row counts, and then a click resolves to the +// wrong finding and the scrollbar reports the wrong length. +// +// The panel is sidebarWidth-2 wide with a column of padding either side, and the +// scrollbar takes one more. That last column is reserved whether or not the bar +// is showing, so the layout does not shift as the list grows past the panel. func (m Model) vulnerabilityListWidth() int { _, sidebarWidth, _, _ := m.layout() - return max(1, sidebarWidth-6) + return max(1, sidebarWidth-5) } -func (m Model) vulnerabilityTitleLines(index, width int) []string { +func (m Model) vulnerabilityTitle(index int) string { title := render.StringValue(m.snapshot.Vulnerabilities[index]["title"]) if title == "" { title = "Unknown Vulnerability" } - return strings.Split(wrapBlock(title, max(1, width-2)), "\n") + return title } +// vulnerabilityScrollRows reports the list length and position in rows, which is +// what the scrollbar needs to move continuously. func (m Model) vulnerabilityScrollRows() (total, offset int) { - width := m.vulnerabilityListWidth() - for i := range m.snapshot.Vulnerabilities { - rows := len(m.vulnerabilityTitleLines(i, width)) - total += rows - if i < m.vulnOffset { - offset += rows - } - } - return total, offset -} - -func (m Model) vulnerabilityOffsetAtRow(targetRow int) int { - width := m.vulnerabilityListWidth() - row := 0 - for i := range m.snapshot.Vulnerabilities { - row += len(m.vulnerabilityTitleLines(i, width)) - if targetRow < row { - return i - } - } - return max(0, len(m.snapshot.Vulnerabilities)-1) -} - -func (m Model) vulnerabilityVisibleEnd(start int) int { - height := m.vulnerabilityPageSize() - width := m.vulnerabilityListWidth() - rows := 0 - end := min(max(0, start), len(m.snapshot.Vulnerabilities)) - for end < len(m.snapshot.Vulnerabilities) { - itemRows := len(m.vulnerabilityTitleLines(end, width)) - if rows > 0 && rows+itemRows > height { - break - } - rows += itemRows - end++ - if rows >= height { - break - } - } - return end + return len(m.vulnerabilityRows(m.vulnerabilityListWidth())), m.vulnOffset } +// vulnerabilityIndexAtRow maps a click on a visible row back to its finding. func (m Model) vulnerabilityIndexAtRow(row int) int { - width := m.vulnerabilityListWidth() - currentRow := 0 - for i := m.vulnOffset; i < m.vulnerabilityVisibleEnd(m.vulnOffset); i++ { - currentRow += len(m.vulnerabilityTitleLines(i, width)) - if row < currentRow { - return i - } + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + target := m.vulnOffset + row + if target < 0 || target >= len(rows) { + return -1 } - return -1 + return rows[target].index } +// ensureVulnerabilityVisible scrolls the least it can to bring the selected +// finding into view, keeping the whole entry visible where it fits. func (m *Model) ensureVulnerabilityVisible() { - if len(m.snapshot.Vulnerabilities) == 0 { + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + if len(rows) == 0 { m.vulnOffset = 0 return } - if m.selectedVuln < m.vulnOffset { - m.vulnOffset = m.selectedVuln + height := m.vulnerabilityPageSize() + firstRow, lastRow := -1, -1 + for row, entry := range rows { + if entry.index != m.selectedVuln { + continue + } + if firstRow < 0 { + firstRow = row + } + lastRow = row } - for m.selectedVuln >= m.vulnerabilityVisibleEnd(m.vulnOffset) && m.vulnOffset < m.selectedVuln { - m.vulnOffset++ + if firstRow < 0 { + m.vulnOffset = clampVulnerabilityOffset(m.vulnOffset, len(rows), height) + return } - m.vulnOffset = min(m.vulnOffset, len(m.snapshot.Vulnerabilities)-1) + if firstRow < m.vulnOffset { + m.vulnOffset = firstRow + } else if lastRow >= m.vulnOffset+height { + // Prefer showing the whole entry, but never scroll its start out of view. + m.vulnOffset = min(firstRow, lastRow-height+1) + } + m.vulnOffset = clampVulnerabilityOffset(m.vulnOffset, len(rows), height) +} + +func clampVulnerabilityOffset(offset, total, height int) int { + return min(max(0, offset), max(0, total-height)) } func (m Model) vulnerabilityPageSize() int { @@ -129,23 +138,52 @@ func (m Model) vulnerabilityPageSize() int { return max(1, vulnHeight-2) } +// vulnerabilityPageItems is how many findings a page step should move by: the +// number of distinct entries currently on screen. func (m Model) vulnerabilityPageItems() int { - return max(1, m.vulnerabilityVisibleEnd(m.vulnOffset)-m.vulnOffset) + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + height := m.vulnerabilityPageSize() + start := min(max(0, m.vulnOffset), max(0, len(rows))) + end := min(len(rows), start+height) + seen := 0 + previous := -1 + for _, row := range rows[start:end] { + if row.index != previous { + seen++ + previous = row.index + } + } + return max(1, seen) } func (m *Model) moveVulnerabilitySelection(delta int) { m.selectedVuln = max(0, min(len(m.snapshot.Vulnerabilities)-1, m.selectedVuln+delta)) } +// keepVulnerabilitySelectionInWindow pulls the selection to the nearest finding +// still on screen after the list has been scrolled directly. func (m *Model) keepVulnerabilitySelectionInWindow() { - if len(m.snapshot.Vulnerabilities) == 0 { + rows := m.vulnerabilityRows(m.vulnerabilityListWidth()) + if len(rows) == 0 { return } - if m.selectedVuln < m.vulnOffset { - m.selectedVuln = m.vulnOffset - } else if end := m.vulnerabilityVisibleEnd(m.vulnOffset); m.selectedVuln >= end { - m.selectedVuln = max(m.vulnOffset, end-1) + height := m.vulnerabilityPageSize() + start := min(max(0, m.vulnOffset), max(0, len(rows)-1)) + end := min(len(rows), start+height) + visible := rows[start:end] + if len(visible) == 0 { + return } + for _, row := range visible { + if row.index == m.selectedVuln { + return + } + } + if m.selectedVuln < visible[0].index { + m.selectedVuln = visible[0].index + return + } + m.selectedVuln = visible[len(visible)-1].index } // statsView ports build_tui_stats_text + the version line appended in @@ -373,25 +411,116 @@ func (m Model) vulnerabilityDetail() string { inner := max(1, width-8) // Button row: right-aligned Copy / Done above a top rule (#vuln_detail_buttons). rule := lipgloss.NewStyle().Foreground(lipgloss.Color("#1a1a1a")).Render(strings.Repeat("─", max(1, inner))) - copyLabel := "Copy" - if m.vulnerabilityCopied { - copyLabel = "Copied!" - } else if m.vulnerabilityCopyError != "" { - copyLabel = "Copy failed" + focused := m.focusedReportButton() + var stepping, acting []string + for _, button := range m.reportButtons() { + rendered := m.reportButton(button, button == focused) + if button == reportPrev || button == reportNext { + stepping = append(stepping, rendered) + continue + } + acting = append(acting, rendered) } - copyButton := lipgloss.NewStyle().Foreground(lipgloss.Color("#525252")) - doneButton := lipgloss.NewStyle().Foreground(mid) - if m.modalChoice == 0 { - copyButton = copyButton.Background(lipgloss.Color("#363636")).Foreground(brightWhite).Bold(true).Padding(0, 1) - } else { - doneButton = doneButton.Background(lipgloss.Color("#363636")).Foreground(brightWhite).Bold(true).Padding(0, 1) + // Stepping sits on the left behind the position, acting on the right. + right := strings.Join(acting, " ") + left := strings.Join(stepping, " ") + if total := len(m.snapshot.Vulnerabilities); total > 1 { + left = render.Dim().Render(fmt.Sprintf("%d/%d", m.selectedVuln+1, total)) + " " + left } - buttons := copyButton.Render(copyLabel) + " " + doneButton.Render("Done") - buttonRow := rule + "\n" + lipgloss.NewStyle().Width(inner).Align(lipgloss.Right).Render(buttons) + room := max(0, inner-lipgloss.Width(right)) + buttonRow := rule + "\n" + + lipgloss.NewStyle().Width(room).Render(truncate(left, room)) + right content := m.vulnerabilityScrollView() + "\n" + buttonRow return lipgloss.NewStyle().Width(width-2).Height(height-2).Border(lipgloss.NormalBorder()).BorderForeground(lipgloss.Color("#262626")).Background(lipgloss.Color("#0a0a0a")).Padding(2, 3).Render(content) } +// showVulnerability moves the open report to another finding, keeping the list +// behind it in step and starting the new report at its top. +func (m *Model) showVulnerability(index int) { + if index < 0 || index >= len(m.snapshot.Vulnerabilities) || index == m.selectedVuln { + return + } + m.selectedVuln = index + m.ensureVulnerabilityVisible() + // The copy state belongs to the report that was on screen, not this one. + m.vulnerabilityCopied = false + m.vulnerabilityCopyError = "" + m.resizeVulnerabilityViewport() + m.vulnViewport.GotoTop() +} + +// The report's buttons. Prev and Next carry their arrows so a click test cannot +// be fooled by the same word appearing in the body of a finding. +const ( + reportPrev = "‹ Prev" + reportNext = "Next ›" + reportCopy = "Copy" + reportDone = "Done" +) + +// reportButtons is the row as it stands, left to right. Stepping is offered only +// in the directions that have a report. +func (m Model) reportButtons() []string { + previous, next := m.vulnerabilityNeighbors() + buttons := make([]string, 0, 4) + if previous { + buttons = append(buttons, reportPrev) + } + if next { + buttons = append(buttons, reportNext) + } + return append(buttons, reportCopy, reportDone) +} + +// focusedReportButton is the button Enter would press. It falls back to Done when +// the focused one has gone, which happens when stepping to either end drops a +// direction from the row. +func (m Model) focusedReportButton() string { + for _, button := range m.reportButtons() { + if button == m.reportFocus { + return button + } + } + return reportDone +} + +// stepReportFocus moves along the row, wrapping at its ends. +func (m *Model) stepReportFocus(delta int) { + buttons := m.reportButtons() + current := 0 + for i, button := range buttons { + if button == m.focusedReportButton() { + current = i + } + } + m.reportFocus = buttons[clampCycle(current+delta, len(buttons))] +} + +// vulnerabilityNeighbors reports which way the open report can be stepped. The +// ends are not wrapped: a report is one of an ordered list, and rolling from the +// last to the first hides that you reached the end. +func (m Model) vulnerabilityNeighbors() (previous, next bool) { + return m.selectedVuln > 0, m.selectedVuln < len(m.snapshot.Vulnerabilities)-1 +} + +// reportButton renders one button of the report row. Copy reports the outcome of +// the last attempt in its own label. +func (m Model) reportButton(label string, focused bool) string { + if label == reportCopy { + switch { + case m.vulnerabilityCopied: + label = "Copied!" + case m.vulnerabilityCopyError != "": + label = "Copy failed" + } + } + if focused { + return lipgloss.NewStyle().Background(lipgloss.Color("#363636")). + Foreground(brightWhite).Bold(true).Padding(0, 1).Render(label) + } + return lipgloss.NewStyle().Foreground(lipgloss.Color("#525252")).Render(label) +} + func (m *Model) startVulnerabilityCopy() tea.Cmd { m.vulnerabilityCopied = false m.vulnerabilityCopyError = ""