From 379715b82a6fe8b691001e119e77c7690d0060e0 Mon Sep 17 00:00:00 2001 From: Loren Segal Date: Thu, 10 Sep 2026 15:17:21 -0700 Subject: [PATCH 1/2] Start work on issue #647 [skip ci] From bb6d55f0066e8389eb8271d0245fe53d1f4e725e Mon Sep 17 00:00:00 2001 From: Loren Segal Date: Thu, 10 Sep 2026 15:17:49 -0700 Subject: [PATCH 2/2] Fix tui status bar layout for page/web/id Closes #647 --- CHANGELOG.md | 1 + ui.go | 78 +++++++++++++++++++++++++++++++++++----------------- ui_test.go | 78 ++++++++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 123 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1b6d8f..2b9376c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - Add `glorp watch --no-changelog` and its matching dashboard setting, so dispatched `gh-fix` runs skip adding a changelog entry for the fix (issue #643). `gh-fix` accepts the same directive as trailing `and no changelog` dispatch advice, mirroring `--no-merge`'s `and do not merge`. +- Combine the terminal dashboard's pager hint and web link onto one status-bar-styled line below the job counts, instead of each sitting on its own bare, unpainted line with a gap between them (issue #647). The instance id now shares that second line too, leftmost, and the web link is shortened from "web dashboard" to "web" and underlined. ## v1.3.5 - 2026-09-09 diff --git a/ui.go b/ui.go index b8c5c1c..622592e 100644 --- a/ui.go +++ b/ui.go @@ -360,22 +360,48 @@ func (m dashboard) View() string { tokens := quotaText(m.snapshot) push := deliveryText(m.snapshot) targets := "targets: " + strings.Join(formatTargets(m.snapshot.Targets, m.snapshot.IssueCounts), ", ") - items := []string{counts, tokens, push, targets} - if m.snapshot.Identity != "" { - items = append([]string{"id: " + m.snapshot.Identity}, items...) + footer := renderStatusBar(m.width, []string{counts, tokens, push, targets}) + // The id, pager hint, and web link join the counts line directly (no gap + // line) so the whole status bar reads as one colored block instead of the + // counts sitting on a colored line above bare, unstyled text (issue #647). + if info := m.renderStatusInfoLine(page, pages); info != "" { + footer += "\n" + info } - footer := renderStatusBar(m.width, items) sections := []string{logs, footer} + if grid != "" { + sections = append([]string{grid}, sections...) + } + return joinVerticalWithGap(sections, dashboardGap) +} + +// renderStatusInfoLine renders the instance id, the pager hint, and the web +// dashboard link as a second status-bar-styled line, so they share the +// counts line's background and page/web sit together on one line (issue +// #647). It returns "" when none of the three apply. +func (m dashboard) renderStatusInfoLine(page, pages int) string { + var items []string + if m.snapshot.Identity != "" { + items = append(items, "id: "+m.snapshot.Identity) + } if pages > 1 { - sections = append(sections, muted.Render(fmt.Sprintf(pagerHint, page+1, pages))) + items = append(items, fmt.Sprintf(pagerHint, page+1, pages)) } if m.snapshot.WebUIURL != "" { - sections = append(sections, muted.Render("web dashboard: "+m.snapshot.WebUIURL)) + items = append(items, "web: "+underlineSpanStyle(len(items)).Render(m.snapshot.WebUIURL)) } - if grid != "" { - sections = append([]string{grid}, sections...) + if len(items) == 0 { + return "" } - return joinVerticalWithGap(sections, dashboardGap) + return renderStatusBar(m.width, items) +} + +// underlineSpanStyle matches the background and foreground of the status bar +// cell a span will render inside, since a nested Lipgloss span resets its +// parent style when it ends (see renderJobCounts) and would otherwise leave +// the web link's cell unpainted around the underline. +func underlineSpanStyle(cellIndex int) lipgloss.Style { + cell := statusBars[cellIndex%len(statusBars)] + return lipgloss.NewStyle().Background(cell.GetBackground()).Foreground(cell.GetForeground()).Underline(true) } // pagerHint tells the operator that agent cards continue on another page and @@ -388,15 +414,17 @@ const pagerHint = "page %d/%d ←/→ (or h/l) for more agents" // a shorter terminal the top row was pushed off screen entirely and those // viewports could neither be read nor scrolled (issue #617). // -// extraLines counts the persistent bottom lines rendered under the status bar -// (the web dashboard URL, the pager hint), each of which also costs the gap -// line joinVerticalWithGap puts above it. -func jobsPerPage(height, extraLines int) int { +// hasInfoLine reports whether the status bar's second line (instance id, +// pager hint, web link) is present. That line is fused directly under the +// counts line inside the same footer section rather than joined as its own +// section, so it costs exactly one content line and no extra gap line +// (issue #647). +func jobsPerPage(height, hasInfoLine int) int { logHeight := max(3, height/3) - // The grid, the log panel, the status bar, and each extra line are joined - // with one blank gap line between them. - chrome := max(1, logHeight-2) + 1 + extraLines - available := height - chrome - (2 + extraLines) + // The grid, the log panel, and the status bar (whose optional second line + // is fused into it) are joined with one blank gap line between each. + chrome := max(1, logHeight-2) + 1 + hasInfoLine + available := height - chrome - 2 rows := (available + dashboardGap) / (jobCardHeight + dashboardGap) return max(1, rows) * jobGridColumns } @@ -404,14 +432,14 @@ func jobsPerPage(height, extraLines int) int { // visibleJobs returns the agent cards belonging to the current page, the page // index clamped to the pages that exist, and the total number of pages. func (m dashboard) visibleJobs() ([]JobSnapshot, int, int) { - extraLines := 0 - if m.snapshot.WebUIURL != "" { - extraLines++ - } - perPage := jobsPerPage(m.height, extraLines) - if len(m.snapshot.Jobs) > perPage { - // Paging costs one more bottom line, which can cost a whole card row. - perPage = jobsPerPage(m.height, extraLines+1) + hasInfoLine := 0 + if m.snapshot.Identity != "" || m.snapshot.WebUIURL != "" { + hasInfoLine = 1 + } + perPage := jobsPerPage(m.height, hasInfoLine) + if hasInfoLine == 0 && len(m.snapshot.Jobs) > perPage { + // Paging itself adds the info line where none existed before. + perPage = jobsPerPage(m.height, 1) } if len(m.snapshot.Jobs) == 0 { return nil, 0, 0 diff --git a/ui_test.go b/ui_test.go index 57bf1d9..b95ff5a 100644 --- a/ui_test.go +++ b/ui_test.go @@ -38,12 +38,60 @@ func TestDashboardShowsStatusAndTargets(t *testing.T) { func TestDashboardShowsWebDashboardLinkWhenEnabled(t *testing.T) { m := newDashboard() updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) - if view := updated.(dashboard).View(); strings.Contains(view, "web dashboard:") { - t.Fatalf("dashboard showed a web dashboard link while disabled: %s", view) + if view := ansi.Strip(updated.(dashboard).View()); strings.Contains(view, "web:") { + t.Fatalf("dashboard showed a web link while disabled: %s", view) } updated, _ = updated.(dashboard).Update(snapshotMsg(GlorpSnapshot{WebUIURL: "http://localhost:8765"})) - if view := updated.(dashboard).View(); !strings.Contains(view, "web dashboard: http://localhost:8765") { - t.Fatalf("dashboard missing web dashboard link: %s", view) + if view := ansi.Strip(updated.(dashboard).View()); !strings.Contains(view, "web: http://localhost:8765") { + t.Fatalf("dashboard missing web link: %s", view) + } +} + +// TestUnderlineSpanStyleMatchesItsCellBackground checks the style used for +// the web link is underlined and matches the background and foreground of +// whichever status bar cell it renders inside, rather than leaving that span +// unpainted when the parent cell's style resets (issue #647). +func TestUnderlineSpanStyleMatchesItsCellBackground(t *testing.T) { + for i, cell := range statusBars { + style := underlineSpanStyle(i) + if !style.GetUnderline() { + t.Fatalf("underline span style for cell %d is not underlined", i) + } + if style.GetBackground() != cell.GetBackground() || style.GetForeground() != cell.GetForeground() { + t.Fatalf("underline span style for cell %d = bg %q fg %q, want bg %q fg %q matching its status bar cell", + i, style.GetBackground(), style.GetForeground(), cell.GetBackground(), cell.GetForeground()) + } + } +} + +// TestDashboardCombinesPagerAndWebLinkOnOneLineWithNoGap checks the pager +// hint and web link share a single line directly under the counts line, with +// no blank gap line between them, instead of each sitting on its own bare +// line (issue #647). +func TestDashboardCombinesPagerAndWebLinkOnOneLineWithNoGap(t *testing.T) { + m := newDashboard() + updated, _ := m.Update(tea.WindowSizeMsg{Width: 120, Height: 30}) + updated, _ = updated.(dashboard).Update(snapshotMsg(GlorpSnapshot{ + Jobs: pagedJobsSnapshot(6).Jobs, + Concurrency: 6, + WebUIURL: "http://localhost:8765", + })) + view := ansi.Strip(updated.(dashboard).View()) + lines := strings.Split(view, "\n") + countsLine, infoLine := -1, -1 + for i, line := range lines { + if strings.Contains(line, "jobs:") { + countsLine = i + } + if strings.Contains(line, "page 1/") && strings.Contains(line, "web: http://localhost:8765") { + infoLine = i + } + } + if countsLine < 0 || infoLine < 0 { + t.Fatalf("dashboard did not combine the pager hint and web link onto one line: %s", view) + } + if infoLine != countsLine+1 { + t.Fatalf("info line followed the counts line with a gap (counts at %d, info at %d): %s", countsLine, infoLine, view) } } @@ -380,15 +428,27 @@ func TestDashboardShowsQuota(t *testing.T) { } } -func TestDashboardShowsIdentityLeftmostInStatusBar(t *testing.T) { +// TestDashboardShowsIdentityLeftmostOnStatusBarSecondLine checks the instance +// id sits on the line below the job counts/quota line, leftmost of any other +// cell sharing that second line (issue #647). +func TestDashboardShowsIdentityLeftmostOnStatusBarSecondLine(t *testing.T) { m := newDashboard() updated, _ := m.Update(tea.WindowSizeMsg{Width: 100, Height: 30}) - updated, _ = updated.(dashboard).Update(snapshotMsg(GlorpSnapshot{Identity: "BA6B21B5", Quota: "weekly 87% left"})) + updated, _ = updated.(dashboard).Update(snapshotMsg(GlorpSnapshot{ + Identity: "BA6B21B5", Quota: "weekly 87% left", WebUIURL: "http://localhost:8765", + })) view := ansi.Strip(updated.(dashboard).View()) - idIndex := strings.Index(view, "id: BA6B21B5") quotaIndex := strings.Index(view, "quota: weekly 87% left") - if idIndex < 0 || quotaIndex < 0 || idIndex > quotaIndex { - t.Fatalf("dashboard did not show instance id leftmost in the status bar: %s", view) + idIndex := strings.Index(view, "id: BA6B21B5") + webIndex := strings.Index(view, "web: http://localhost:8765") + if quotaIndex < 0 || idIndex < 0 || webIndex < 0 { + t.Fatalf("dashboard missing expected status bar cells: %s", view) + } + if idIndex < quotaIndex { + t.Fatalf("dashboard showed the instance id on the counts line instead of the line below it: %s", view) + } + if idIndex > webIndex { + t.Fatalf("dashboard did not show the instance id leftmost on the status bar's second line: %s", view) } }