diff --git a/CHANGELOG.md b/CHANGELOG.md index f7d0963..d7a42f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- The report screen now draws a per-day sparkline of the selected range, with + idle days rendered as gaps rather than as low bars (#80). + +### Changed + +- The report is rendered as a real table that sizes itself to the terminal: + the numeric columns are right-aligned and the label column takes the slack, + instead of a fixed 32-column layout that wrapped on a narrow terminal (#66). +- The budget burn-down bars are drawn with block glyphs and colored — green + under budget, red over it — while still showing the true, unclamped + percentage (#80). + ## [1.9.0] - 2026-07-28 ### Added diff --git a/README.it.md b/README.it.md index 073908f..444041e 100644 --- a/README.it.md +++ b/README.it.md @@ -119,6 +119,12 @@ cambiare raggruppamento, riesportare o tornare alla home. Se il token risulta invalido o revocato durante l'uso, la TUI ripropone automaticamente il wizard di setup. +Il report stesso viene disegnato come una tabella dimensionata sul tuo terminale: +quando la finestra si restringe, è la colonna delle etichette a cedere spazio per +prima, così le colonne numeriche e l'importo restano leggibili. Sopra la tabella, una +sparkline giornaliera mostra a colpo d'occhio l'andamento del periodo — un giorno +senza ore registrate appare come un vuoto, non come una barra. + ### Comandi nella TUI Ogni schermata mostra un footer con i tasti che accetta in quel momento, generato diff --git a/README.md b/README.md index 378cdca..075ebd1 100644 --- a/README.md +++ b/README.md @@ -116,6 +116,11 @@ the **range picker**, which offers presets (this month, last month, last 7 days, report you can change the grouping, re-export, or go back home. If the token becomes invalid or is revoked while in use, the TUI automatically re-runs the setup wizard. +The report itself renders as a table sized to your terminal: as the window narrows, +the label column gives up its space first, so the numeric and amount columns keep +their footing. Above the table, a per-day sparkline shows the shape of the period at a +glance — a day with no logged hours renders as a gap, not a bar. + ### TUI commands Every screen shows a footer with the keys it currently accepts, generated directly diff --git a/docs/demo.gif b/docs/demo.gif index 498ca2d..485dfa4 100644 Binary files a/docs/demo.gif and b/docs/demo.gif differ diff --git a/docs/superpowers/plans/2026-07-28-report-dashboard.md b/docs/superpowers/plans/2026-07-28-report-dashboard.md new file mode 100644 index 0000000..ab4d1e5 --- /dev/null +++ b/docs/superpowers/plans/2026-07-28-report-dashboard.md @@ -0,0 +1,1503 @@ +# Report Dashboard Implementation Plan (v1.9, tranche C) + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Collapse the billing editor's four parallel selection indices into one array (#117), replace the hand-built report with a width-aware `lipgloss/table` (#66), and turn the report into a dashboard with a per-day sparkline and a block-glyph budget gauge (#80). + +**Architecture:** Domain stays pure — the per-day series is a new pure function in `internal/report`; everything else is presentation. The table lives in its own file so `report.go` keeps its size, and every choice that `termenv.Ascii` erases (zebra background, over-budget color, total styling) is tested by asserting on the returned `lipgloss.Style`, never on rendered bytes. + +**Tech Stack:** Go 1.26.5, bubbletea v1.3.10, **lipgloss v1.1.0 including its `table` sub-package** (already an indirect part of an existing dependency — `go.mod` must not gain a new module). + +**Spec:** `docs/superpowers/specs/2026-07-28-report-dashboard-design.md` + +## Global Constraints + +- `internal/report` and `internal/duration` stay **pure**: no I/O, no imports of `internal/config` or `internal/clickup`. +- Everything committed to the repo is in **English** — code, identifiers, comments, test messages, UI strings, commit messages. (The spec and this plan are Italian/English docs under `docs/superpowers/`, which is the historical exception.) +- **Conventional Commits.** **Never** add a `Co-Authored-By` trailer to a commit message. +- Before every commit, all five must be clean/green: `gofmt -l .`, `go vet ./...`, `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, `go build ./...`, `go test ./... -race`. +- **No new module in `go.mod`.** `github.com/charmbracelet/lipgloss/table` is a sub-package of the existing `lipgloss` requirement. `bubbles/progress` is deliberately NOT adopted (Task 6 explains why). +- **Demo-mode parity:** `CLICKUP_DEMO=1 clup` must show the table, the sparkline and the gauge. No network. +- Never call the real ClickUp API. +- Golden files are regenerated with `go test ./internal/tui -update`, never hand-edited. +- Prefer `any` over `interface{}`; use `slices`/`maps` where they fit. + +--- + +### Task 1: Collapse the billing editor's four selection indices (#117) + +**This task is a pure refactor. There is no RED step.** The characterization test in Step 1 must pass *before* you change anything — it pins the property the four indices existed to provide. Do not invent a failing test to satisfy a TDD habit; if the test in Step 1 fails before the refactor, you have written it wrong. + +**Files:** +- Modify: `internal/tui/rates.go` (26 occurrences: 4 struct fields, 8 inside `move`, 14 in the Step 6 table) +- Modify: `internal/tui/rates_view.go` (7 occurrences) +- Modify: `internal/tui/listbrowser.go:152` (1 occurrence) +- Test: `internal/tui/rates_test.go` (add one test; change nothing else) + +**Interfaces:** +- Consumes: nothing from earlier tasks. +- Produces: `ratesModel.sel [secCount]int`, replacing the fields `idx`, `memIdx`, `ovIdx`, `ruleIdx`. Later tasks do not touch this screen. + +**Background you need:** `ratesModel` is the billing settings editor. It has four sections (`secLists`, `secMembers`, `secOverrides`, `secRules`, with `secCount` as the count), and `rt.sec` holds the active one. Today each section keeps its own selected-row index in its own field. `rt.draft.idx` is a *different* thing — the override-creation wizard's own picker — and **must not be touched**. + +- [ ] **Step 1: Write the characterization test** + +Add to `internal/tui/rates_test.go`: + +```go +// Each section keeps its own selected row: moving in one section must not +// disturb another, and returning to a section must restore where you were. +// This is the property the four parallel indices existed to provide, and the +// one a single array indexed by section has to preserve. +func TestRatesSelectionIsPerSection(t *testing.T) { + t.Parallel() + m := Model{screen: screenRates, ratesScreen: ratesModel{ + sec: secLists, + rows: []rateRow{{listID: "l1", name: "Website"}, {listID: "l2", name: "Internal"}}, + members: []memberRow{{id: 1, name: "alice"}, {id: 2, name: "bob"}}, + }} + + // Move to the second list, then switch to Members. + mm, _ := m.updateRates(keyMsg("down")) + mm, _ = mm.(Model).updateRates(keyMsg("tab")) + rt := mm.(Model).ratesScreen + if rt.sec != secMembers { + t.Fatalf("tab did not reach the Members section: sec=%v", rt.sec) + } + if got := rt.selected(secMembers); got != 0 { + t.Errorf("Members selection = %d, want 0: a fresh section starts at its first row", got) + } + + // Move inside Members, then come back to Lists. + mm, _ = mm.(Model).updateRates(keyMsg("down")) + mm, _ = mm.(Model).updateRates(keyMsg("shift+tab")) + rt = mm.(Model).ratesScreen + if rt.sec != secLists { + t.Fatalf("shift+tab did not return to the Lists section: sec=%v", rt.sec) + } + if got := rt.selected(secLists); got != 1 { + t.Errorf("Lists selection = %d, want 1: leaving and returning must not reset it", got) + } + if got := rt.selected(secMembers); got != 1 { + t.Errorf("Members selection = %d, want 1: it must survive leaving the section", got) + } +} +``` + +This test calls `rt.selected(sec)`, which does not exist yet. Add it to `internal/tui/rates.go` next to `selCount`, reading the *current* fields, so the test compiles and passes before the refactor: + +```go +// selected returns the selected row of a section. It exists so tests can name +// a section's selection without depending on how the model stores it. +func (rt ratesModel) selected(sec ratesSection) int { + switch sec { + case secMembers: + return rt.memIdx + case secOverrides: + return rt.ovIdx + case secRules: + return rt.ruleIdx + default: + return rt.idx + } +} +``` + +- [ ] **Step 2: Run it — it must PASS** + +Run: `go test ./internal/tui -run TestRatesSelectionIsPerSection -v` +Expected: **PASS**. This is a characterization test on unchanged behaviour. A failure here means the test is wrong, not the code. + +- [ ] **Step 3: Commit the characterization test** + +```bash +git add internal/tui/rates_test.go internal/tui/rates.go +git commit -m "test(tui): pin per-section selection in the billing editor" +``` + +- [ ] **Step 4: Replace the four fields with one array** + +In `internal/tui/rates.go`, in the `ratesModel` struct, delete the fields `idx`, `memIdx`, `ovIdx` and `ruleIdx`, and place this immediately after `sec`: + +```go + // sel is the selected row of each section, indexed by ratesSection. One + // array instead of four parallel fields means move() needs no switch at + // all, and a fifth section is a non-event. Its zero value is identical to + // four zeroed ints, so no golden changes. + sel [secCount]int +``` + +Keep the `rows`, `members` and `overrides` slices exactly where they are. + +Rewrite `selected` to read the array: + +```go +// selected returns the selected row of a section. It exists so tests can name +// a section's selection without depending on how the model stores it. +func (rt ratesModel) selected(sec ratesSection) int { return rt.sel[sec] } +``` + +- [ ] **Step 5: Rewrite `move`** + +Replace the whole of `move` in `internal/tui/rates.go` with: + +```go +// move shifts the selection of the active section by delta, clamped. +func (rt ratesModel) move(delta int) ratesModel { + next := rt.sel[rt.sec] + delta + if next < 0 || next > rt.selCount()-1 { + return rt + } + rt.sel[rt.sec] = next + return rt +} +``` + +**Leave `selCount` exactly as it is.** Its switch is not duplication: the four counts are genuinely different (`len(rt.rows)`, `len(rt.members)`, `len(rt.overrides)+1`, `ruleCount`). + +- [ ] **Step 6: Update every remaining read and write** + +Each of these keeps its surrounding `switch` — the *behaviour* differs per section, only the index lookup changes. (`commit`'s switch is on `rt.edit`, not on the section; it only reads the indices.) Inside a `case secLists:` write `rt.sel[secLists]`, **not** `rt.sel[rt.sec]`: the constant names the invariant, the variable hides it. + +⚠️ **Match by the "Was" text, not by the line number.** The numbers below are measured on the file as it stands *before* Step 1. Step 1 inserts the `selected` method, Step 4 rewrites the struct and Step 5 shrinks `move`, so by the time you reach this step every rates.go line number has shifted by several lines. Two entries look ambiguous — `switch rt.ruleIdx {` appears twice and `id := rt.rows[rt.idx].listID` appears twice — but each pair takes an identical replacement, so matching on text is safe. + +In `internal/tui/rates.go`: + +| Line (pre-Step-1) | Was | Becomes | +|---|---|---| +| 588 | `if rt.ovIdx >= len(rt.overrides) {` | `if rt.sel[secOverrides] >= len(rt.overrides) {` | +| 594 | `switch rt.ruleIdx {` | `switch rt.sel[secRules] {` | +| 644 | `rt.rates[rt.rows[rt.idx].listID] = f` | `rt.rates[rt.rows[rt.sel[secLists]].listID] = f` | +| 648 | `id := rt.rows[rt.idx].listID` | `id := rt.rows[rt.sel[secLists]].listID` | +| 662 | `id := rt.rows[rt.idx].listID` | `id := rt.rows[rt.sel[secLists]].listID` | +| 684 | `rt.memberRates[rt.members[rt.memIdx].id] = f` | `rt.memberRates[rt.members[rt.sel[secMembers]].id] = f` | +| 695 | `rt.ovIdx = indexOfOverride(...)` | `rt.sel[secOverrides] = indexOfOverride(...)` | +| 698 | `rt.overrides[rt.ovIdx].rate = f` | `rt.overrides[rt.sel[secOverrides]].rate = f` | +| 768 | `delete(rt.rates, rt.rows[rt.idx].listID)` | `delete(rt.rates, rt.rows[rt.sel[secLists]].listID)` | +| 772 | `delete(rt.memberRates, rt.members[rt.memIdx].id)` | `delete(rt.memberRates, rt.members[rt.sel[secMembers]].id)` | +| 775 | `if rt.ovIdx < len(rt.overrides) {` | `if rt.sel[secOverrides] < len(rt.overrides) {` | +| 778 | `slices.Delete(rt.overrides, rt.ovIdx, rt.ovIdx+1)` | `slices.Delete(rt.overrides, rt.sel[secOverrides], rt.sel[secOverrides]+1)` | +| 781 | `switch rt.ruleIdx {` | `switch rt.sel[secRules] {` | + +In `internal/tui/rates_view.go`: + +| Line | Was | Becomes | +|---|---|---| +| 54 | `billingRow(th, i == rt.idx, line)` | `billingRow(th, i == rt.sel[secLists], line)` | +| 56 | `sel := rt.rows[rt.idx]` | `sel := rt.rows[rt.sel[secLists]]` | +| 77 | `billingRow(th, i == rt.memIdx, line)` | `billingRow(th, i == rt.sel[secMembers], line)` | +| 79 | `sel := rt.members[rt.memIdx]` | `sel := rt.members[rt.sel[secMembers]]` | +| 95 | `billingRow(th, i == rt.ovIdx, line)` | `billingRow(th, i == rt.sel[secOverrides], line)` | +| 97 | `billingRow(th, rt.ovIdx >= len(rt.overrides), ...)` | `billingRow(th, rt.sel[secOverrides] >= len(rt.overrides), ...)` | +| 134 | `billingRow(th, i == rt.ruleIdx, ...)` | `billingRow(th, i == rt.sel[secRules], ...)` | + +In `internal/tui/listbrowser.go:152`: `rt.idx = found` becomes `rt.sel[secLists] = found`. + +**Do not touch `rt.draft.idx`** anywhere — it is the override wizard's own picker, not a section selection. + +- [ ] **Step 7: Verify nothing is left behind** + +Run: `grep -rn 'rt\.idx\|rt\.memIdx\|rt\.ovIdx\|rt\.ruleIdx' internal/` +Expected: **no output.** + +- [ ] **Step 8: Run the full suite** + +Run: `go build ./... && go test ./internal/tui -race` +Expected: PASS, including all 31 `TestRates*` tests **unchanged** and every golden **byte-identical** (the array's zero value equals four zeroed ints). If a golden changed, you altered behaviour — find out what before continuing. + +- [ ] **Step 9: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/rates.go internal/tui/rates_view.go internal/tui/listbrowser.go +git commit -m "refactor(tui): collapse the billing editor's four selection indices into one array (#117)" +``` + +--- + +### Task 2: Theme tokens for the grid + +**Files:** +- Modify: `internal/tui/theme.go` +- Test: `internal/tui/theme_test.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `palette.Subtle lipgloss.AdaptiveColor`; `theme.Cell`, `theme.Border`, `theme.Zebra` (all `lipgloss.Style`). Task 3 uses all four. + +- [ ] **Step 1: Write the failing tests** + +In `internal/tui/theme_test.go`, add `Subtle` to the adaptive list inside `TestPaletteIsAdaptive`, so its slice literal reads: + +```go + {"Primary", p.Primary}, {"Accent", p.Accent}, + {"Danger", p.Danger}, {"Success", p.Success}, + {"Subtle", p.Subtle}, +``` + +Add `Border` and `Zebra` to `paletteSample`'s slice literal, so it reads: + +```go + {"Title", th.Title}, {"Help", th.Help}, {"Err", th.Err}, + {"OK", th.OK}, {"Accent", th.Accent}, {"Box", th.Box}, + {"Border", th.Border}, {"Zebra", th.Zebra}, +``` + +And add this new test: + +```go +// The zebra style must actually carry a background: it is the only thing that +// distinguishes an odd row, and the package goldens run under termenv.Ascii, +// which strips backgrounds — so nothing else in the suite would notice it +// going missing. +func TestZebraCarriesBackground(t *testing.T) { + t.Parallel() + th := paletteTheme(true) + if th.Zebra.GetBackground() == th.Cell.GetBackground() { + t.Error("Zebra has the same background as a plain cell, so odd rows are indistinguishable") + } +} +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `go test ./internal/tui -run 'TestPaletteIsAdaptive|TestZebraCarriesBackground|TestGoldenPalette' -v` +Expected: FAIL — `p.Subtle`, `th.Border`, `th.Zebra` and `th.Cell` are undefined (compile error). + +- [ ] **Step 3: Add the palette token** + +In `internal/tui/theme.go`, add to the `palette` struct: + +```go + Subtle lipgloss.AdaptiveColor // zebra row background +``` + +and to `defaultPalette`: + +```go + Subtle: lipgloss.AdaptiveColor{Light: "254", Dark: "236"}, +``` + +Extend the doc comment above `defaultPalette` with this paragraph: + +```go +// Subtle is a background, so it is judged by a different rule than the five +// foregrounds above: 236 (#303030) on dark and 254 (#E4E4E4) on light are +// chosen so the DEFAULT FOREGROUND still clears contrast when painted on top +// of them, not so they contrast with the terminal's own background. A zebra +// stripe that swallows its own text is worse than no stripe at all. +``` + +- [ ] **Step 4: Add the three styles** + +In the `theme` struct: + +```go + Cell lipgloss.Style // a plain report-table cell: no color, just the renderer + Border lipgloss.Style // the report table's frame + Zebra lipgloss.Style // alternate report-table row +``` + +In `newTheme`'s returned literal: + +```go + Cell: r.NewStyle(), + Border: r.NewStyle().Foreground(p.Muted), + Zebra: r.NewStyle().Background(p.Subtle), +``` + +`Cell` exists so the table's style function never has to call `lipgloss.NewStyle()`, which would silently render base rows through the *default* renderer while zebra rows go through the injected one. It is the same discipline that kept `help.New()` out of the footer. + +Finally, `theme.go`'s file-level comment above `palette` says a future user-supplied theme "will override these five values". There are now six. Update the sentence. + +- [ ] **Step 5: Run the tests, then regenerate the palette goldens** + +Run: `go test ./internal/tui -run 'TestPaletteIsAdaptive|TestZebraCarriesBackground' -v` +Expected: PASS. + +Then: `go test ./internal/tui -run TestGoldenPalette -update && go test ./internal/tui -run TestGoldenPalette -v` +Expected: PASS. Inspect `internal/tui/testdata/palette_dark.golden` and `palette_light.golden` — each must have gained exactly two lines, `Border` and `Zebra`. + +- [ ] **Step 6: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/theme.go internal/tui/theme_test.go internal/tui/testdata/palette_dark.golden internal/tui/testdata/palette_light.golden +git commit -m "feat(tui): add Subtle palette token and Cell/Border/Zebra styles" +``` + +--- + +### Task 3: The report grid (#66) + +**Files:** +- Create: `internal/tui/report_table.go` +- Create: `internal/tui/report_table_test.go` +- Modify: `internal/tui/report.go` (`reportModel.view`) +- Modify: `internal/tui/app.go` (`screenBody`, the `screenReport` case) +- Modify: `internal/tui/report_test.go`, `internal/tui/golden_test.go` (call sites of `view`) + +**Interfaces:** +- Consumes: `theme.Cell`, `theme.Border`, `theme.Zebra`, `theme.Header`, `theme.OK` (Task 2). +- Produces: + - `reportTable(th theme, r report.Report, width int) string` + - `reportRows(r report.Report) (rows [][]string, firstTotal int)` + - `reportItemWidth(rows [][]string, headers []string, width int) int` + - `reportStyleFunc(th theme, firstTotal int) table.StyleFunc` + - `reportModel.view(th theme, width int) string` — **signature change**; Task 5 changes it no further. + +**Background:** `report.Report` carries `Buckets []Bucket` (each with `Label`, `Hours`, `BilledHours`, `Amounts`), `CurrencySubtotals`, `TotalHours`, `BilledHours`, `TotalAmount`, `DefaultCurrency`. The existing helpers `formatAmounts(amounts, fallback)`, `hoursOf(h)` and `truncate(s, n)` are in `report.go` and must be reused, not reimplemented. + +`lipgloss/table`'s defaults are hostile here and must all be overridden: `borderColumn: true` (draws separators between columns), `wrap: true` (wraps a long cell onto a second line instead of letting us truncate), and `borderStyle: lipgloss.NewStyle()` (the *default* renderer, bypassing the injected one). + +- [ ] **Step 1: Write the failing tests** + +Create `internal/tui/report_table_test.go`: + +```go +package tui + +import ( + "strings" + "testing" + "time" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" + "github.com/marcoarnulfo/clickup-cli/internal/report" +) + +// tableFixture is a two-bucket, single-currency report. +func tableFixture() report.Report { + start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + return report.Report{ + Start: start, End: start.AddDate(0, 1, 0), + Scope: "me", GroupBy: report.GroupByList, DefaultCurrency: "EUR", Timezone: "UTC", + Buckets: []report.Bucket{ + {Label: "Website", Key: "l1", Hours: 12.5, BillableHours: 12.5, BilledHours: 12.5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}, + {Label: "Internal", Key: "l2", Hours: 3, BillableHours: 0, BilledHours: 0, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 0}}}, + }, + CurrencySubtotals: []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 15.5, BillableHours: 12.5, BilledHours: 12.5, Amount: 625}, + }, + TotalHours: 15.5, BillableHours: 12.5, NonBillableHours: 3, BilledHours: 12.5, TotalAmount: 625, + } +} + +// The table must never be wider than the terminal. help's arithmetic taught us +// that "approximately fits" is not a property — this asserts the real thing. +// +// The widths start at 60 because below roughly 48 the Item column hits its +// 12-column floor and the table deliberately overflows instead of shrinking +// into illegibility; TestReportTableFloorsRatherThanVanishing covers that end. +func TestReportTableNeverExceedsWidth(t *testing.T) { + t.Parallel() + r := tableFixture() + r.Buckets[0].Label = "A very long list name that will not fit anywhere near a narrow terminal" + for _, width := range []int{60, 80, 100, 120} { + out := reportTable(testTheme(true), r, width) + for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("width %d: line is %d columns:\n%s", width, w, line) + } + } + } +} + +// Under about 48 columns there is not enough room for a 12-column label plus +// the numeric columns, and the table overflows rather than cutting labels down +// to nothing. Pinning it makes the trade-off deliberate instead of accidental. +func TestReportTableFloorsRatherThanVanishing(t *testing.T) { + t.Parallel() + out := reportTable(testTheme(true), tableFixture(), 30) + if !strings.Contains(out, "Website") { + t.Errorf("the label column collapsed past usefulness at 30 columns:\n%s", out) + } +} + +// The Item column is the only one allowed to lose: truncating an amount would +// hide money, and the numeric columns are the point of having a grid. +func TestReportItemWidthGivesSlackToTheLabel(t *testing.T) { + t.Parallel() + headers := []string{"Item", "Hours", "Billed", "Amount"} + rows, _ := reportRows(tableFixture()) + rows[0][0] = strings.Repeat("x", 200) // a label longer than any terminal + + narrow := reportItemWidth(rows, headers, 60) + wide := reportItemWidth(rows, headers, 120) + if narrow >= wide { + t.Errorf("item width did not grow with the terminal: 60 -> %d, 120 -> %d", narrow, wide) + } + if narrow < 12 { + t.Errorf("item width %d fell below the 12-column floor", narrow) + } +} + +// width <= 0 is the first render, before the terminal has sent WindowSizeMsg. +// It must reproduce the fixed 32-column layout the report had before #66. +func TestReportItemWidthZeroIsNaturalWidth(t *testing.T) { + t.Parallel() + headers := []string{"Item", "Hours", "Billed", "Amount"} + rows, _ := reportRows(tableFixture()) + rows[0][0] = strings.Repeat("x", 200) + if got := reportItemWidth(rows, headers, 0); got != 32 { + t.Errorf("natural item width = %d, want 32", got) + } +} + +// A long label must be cut short, never wrapped onto a second line: one bucket +// is one row. lipgloss/table wraps by default, so this pins Wrap(false). +func TestReportTableTruncatesRatherThanWraps(t *testing.T) { + t.Parallel() + r := tableFixture() + r.Buckets[0].Label = strings.Repeat("long ", 40) + out := reportTable(testTheme(true), r, 60) + // top border, header, header separator, 2 buckets, TOTAL, bottom border. + if n := len(strings.Split(strings.TrimRight(out, "\n"), "\n")); n != 7 { + t.Errorf("table has %d lines, want 7 — a cell wrapped instead of truncating:\n%s", n, out) + } +} + +// The multi-currency total block: a TOTAL row with an empty Amount cell, +// followed by one subtotal row per currency. +func TestReportRowsMultiCurrencyTotals(t *testing.T) { + t.Parallel() + r := tableFixture() + r.CurrencySubtotals = []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 12.5, BilledHours: 12.5, Amount: 625}, + {Currency: "USD", Hours: 3, BilledHours: 3, Amount: 150}, + } + rows, firstTotal := reportRows(r) + if firstTotal != 2 { + t.Fatalf("firstTotal = %d, want 2 (the two buckets come first)", firstTotal) + } + if len(rows) != 5 { + t.Fatalf("got %d rows, want 5 (2 buckets + TOTAL + 2 subtotals)", len(rows)) + } + if rows[2][0] != "TOTAL" || rows[2][3] != "" { + t.Errorf("TOTAL row = %q, want an empty Amount cell when the report is multi-currency", rows[2]) + } + if !strings.Contains(rows[3][0], "subtotal EUR") || !strings.Contains(rows[4][0], "subtotal USD") { + t.Errorf("subtotal rows = %q, %q", rows[3][0], rows[4][0]) + } +} + +// TestReportStyleFunc is the ONLY check on the grid's colors. The package +// goldens run under termenv.Ascii, which strips backgrounds and bold, so a +// broken zebra or an uncolored TOTAL would leave every golden byte-identical. +// It therefore asserts on the Style the function returns, not on rendered text. +func TestReportStyleFunc(t *testing.T) { + t.Parallel() + th := paletteTheme(true) // real colors, so backgrounds are comparable + const firstTotal = 2 + f := reportStyleFunc(th, firstTotal) + + if got := f(table.HeaderRow, 0); !got.GetBold() { + t.Error("header row is not bold") + } + if f(0, 0).GetBackground() == th.Zebra.GetBackground() { + t.Error("row 0 is striped; zebra must start on the second row") + } + if f(1, 0).GetBackground() != th.Zebra.GetBackground() { + t.Error("row 1 is not striped") + } + if f(firstTotal, 0).GetBackground() == th.Zebra.GetBackground() { + t.Error("the TOTAL row is striped; totals are not part of the zebra sequence") + } + if f(firstTotal, 0).GetForeground() != th.OK.GetForeground() { + t.Error("the TOTAL row does not carry the OK color") + } + if !f(firstTotal, 0).GetBold() { + t.Error("the TOTAL row is not bold") + } + if f(0, 0).GetAlign() != lipgloss.Left { + t.Error("the Item column is not left-aligned") + } + for col := 1; col <= 3; col++ { + if f(0, col).GetAlign() != lipgloss.Right { + t.Errorf("column %d is not right-aligned", col) + } + } +} +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `go test ./internal/tui -run 'TestReport(Table|ItemWidth|Rows|StyleFunc)' -v` +Expected: FAIL — compile error, none of the four functions exist. + +- [ ] **Step 3: Write the table** + +Create `internal/tui/report_table.go`: + +```go +package tui + +import ( + "fmt" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" + "github.com/marcoarnulfo/clickup-cli/internal/report" +) + +// The report grid's fixed dimensions. Slack is taken from and given to the +// Item column alone: the numeric columns are the reason to have a grid, and an +// amount must never be truncated because a truncated amount hides money. +const ( + reportNumWidth = 8 // reserved for each of Hours and Billed + reportMinItemWidth = 12 // below this the labels stop being labels + reportNatItemWidth = 32 // the fixed width the report had before #66 + // reportTableChrome is the border and padding the table spends on itself: + // two outer borders (column separators are off) plus two padding columns + // on each of the four columns. + reportTableChrome = 2 + 4*2 +) + +var reportHeaders = []string{"Item", "Hours", "Billed", "Amount"} + +// reportRows renders the report as table cells and reports the index at which +// the total rows begin. Bucket rows come first, then either a single TOTAL row +// (one currency) or a TOTAL row with an empty Amount followed by one subtotal +// row per currency. +// +// Per-bucket Amounts are indicative: PerDay rounding can drift a few cents +// from the subtotals at a fine grouping. CurrencySubtotals is authoritative +// and is never re-derived from the bucket rows. +func reportRows(r report.Report) ([][]string, int) { + rows := make([][]string, 0, len(r.Buckets)+len(r.CurrencySubtotals)+1) + for _, b := range r.Buckets { + rows = append(rows, []string{ + b.Label, + fmt.Sprintf("%.2f", b.Hours), + fmt.Sprintf("%.2f", b.BilledHours), + formatAmounts(b.Amounts, r.DefaultCurrency), + }) + } + firstTotal := len(rows) + + if len(r.CurrencySubtotals) <= 1 { + rows = append(rows, []string{ + "TOTAL", + fmt.Sprintf("%.2f", r.TotalHours), + fmt.Sprintf("%.2f", r.BilledHours), + fmt.Sprintf("%.2f %s", r.TotalAmount, r.DefaultCurrency), + }) + return rows, firstTotal + } + + rows = append(rows, []string{ + "TOTAL", + fmt.Sprintf("%.2f", r.TotalHours), + fmt.Sprintf("%.2f", r.BilledHours), + "", + }) + for _, s := range r.CurrencySubtotals { + rows = append(rows, []string{ + " subtotal " + s.Currency, + fmt.Sprintf("%.2f", s.Hours), + fmt.Sprintf("%.2f", s.BilledHours), + fmt.Sprintf("%.2f %s", s.Amount, s.Currency), + }) + } + return rows, firstTotal +} + +// reportItemWidth splits width between the fixed numeric columns and the Item +// column. It never stretches Item past the longest label (empty space is not a +// feature) and never shrinks it below reportMinItemWidth — unless the labels +// themselves are shorter than that floor. +// +// Reserving reportNumWidth for Hours and Billed is a worst case: real values +// are shorter, so the table often renders narrower than the terminal. Narrower +// is fine; wider is the bug this arithmetic exists to prevent. +func reportItemWidth(rows [][]string, headers []string, width int) int { + maxLabel := lipgloss.Width(headers[0]) + amount := lipgloss.Width(headers[3]) + for _, row := range rows { + maxLabel = max(maxLabel, lipgloss.Width(row[0])) + amount = max(amount, lipgloss.Width(row[3])) + } + if width <= 0 { + return min(maxLabel, reportNatItemWidth) + } + floor := min(reportMinItemWidth, maxLabel) + item := width - reportTableChrome - 2*reportNumWidth - amount + return max(floor, min(maxLabel, item)) +} + +// reportStyleFunc decides every per-cell style: alignment, the header, the +// zebra stripe and the totals. +// +// It is a separate function because a golden cannot see what it does. TestMain +// pins termenv.Ascii, which strips backgrounds and bold, so a broken stripe or +// an uncolored TOTAL leaves every golden byte-identical. The test asserts on +// the Style this returns. +// +// It never calls lipgloss.NewStyle(): that would build on the DEFAULT +// renderer, so plain rows and themed rows would render through different +// renderers. th.Cell is the plain style on the right one. +func reportStyleFunc(th theme, firstTotal int) table.StyleFunc { + return func(row, col int) lipgloss.Style { + st := th.Cell + switch { + case row == table.HeaderRow: + st = th.Header + case row >= firstTotal: + st = th.OK + if row == firstTotal { + st = st.Bold(true) // the TOTAL line, not the per-currency subtotals + } + case row%2 == 1: + st = th.Zebra + } + st = st.Padding(0, 1) + if col > 0 { + st = st.Align(lipgloss.Right) + } + return st + } +} + +// reportTable renders the buckets and the totals as one table sized to width. +// width <= 0 means natural width: the first render, before the terminal has +// sent its WindowSizeMsg. +// +// Three lipgloss/table defaults are wrong for this grid and are all turned +// off: BorderColumn draws separators between columns, Wrap sends a long label +// onto a second line instead of letting truncate cut it, and the zero +// BorderStyle renders the frame through the default renderer. +// +// Sizing is implicit and Table.Width is deliberately NOT called. With +// BorderColumn off, the library's resizer counts only the separators between +// columns, so it believes the frame costs nothing, while String clips the +// result with MaxWidth — the table would render two columns too wide and then +// have its right border sliced off. Pre-formatted numeric cells, labels +// truncated to itemW and Padding(0, 1) from the style function give the exact +// widths this arithmetic assumes. +func reportTable(th theme, r report.Report, width int) string { + rows, firstTotal := reportRows(r) + itemW := reportItemWidth(rows, reportHeaders, width) + for i := range rows { + rows[i][0] = truncate(rows[i][0], itemW) + } + return table.New(). + Border(lipgloss.RoundedBorder()). + BorderStyle(th.Border). + BorderColumn(false). + BorderHeader(true). + Wrap(false). + Headers(reportHeaders...). + Rows(rows...). + StyleFunc(reportStyleFunc(th, firstTotal)). + String() +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/tui -run 'TestReport(Table|ItemWidth|Rows|StyleFunc)' -v` +Expected: PASS. + +- [ ] **Step 5: Wire the table into the report view** + +In `internal/tui/report.go`, replace the body of `reportModel.view` from the `header :=` line down to (and including) the `return` with: + +```go + body := reportTable(th, r, width) + if len(r.Buckets) == 0 { + body = th.Box.Render("No hours to show.") + } + // The billable split is a note, not a row of data: inside the table it + // would take the zebra stripe and be split across columns. + split := th.Help.Render(fmt.Sprintf(" billable %s · non-billable %s", + hoursOf(r.BillableHours), hoursOf(r.NonBillableHours))) + return title + "\n\n" + summary + "\n\n" + body + "\n" + split +``` + +Change the signature to `func (rm reportModel) view(th theme, width int) string`. Delete the now-unused local variables (`header`, `rows`, `total`) — `staticcheck` will name any you miss. Keep `title` and `summary` exactly as they are. + +- [ ] **Step 6: Update the call sites** + +In `internal/tui/app.go`, the `screenReport` case of `screenBody` becomes: + +```go + case screenReport: + return m.rep.view(m.theme, m.width) +``` + +In the tests, add a width argument to every `view(testTheme(true))` call on a `reportModel`. There are **four**: +- `internal/tui/golden_test.go:120` → `newReport(goldenReport(), "").view(testTheme(true), 80)` +- `internal/tui/report_test.go:104` and `:122` → `.view(testTheme(true), 80)` +- `internal/tui/report_test.go:157` → `mm.rep.view(testTheme(true), 80)` (inside `TestReportViewShowsSummaryAndBillableSplit`) + +Run `go vet ./...` — **not** `go build ./...`, which does not compile `_test.go` files and would report a clean tree while the tests are broken — and fix any call site it names. + +- [ ] **Step 7: Regenerate the report golden and add two width goldens** + +In `internal/tui/golden_test.go`, replace `TestGoldenReport` with: + +```go +func TestGoldenReport(t *testing.T) { + t.Parallel() + // One bucket gets a label long enough that the three widths actually + // diverge. goldenReport's own labels are 8 columns, so without this all + // three goldens would be byte-identical and would pin nothing about width. + // At 60 the label is cut to 24 columns, at 80 to 44, and at 120 it fits + // whole — which is also the case that proves the column does not stretch + // past its content. + r := goldenReport() + r.Buckets[0].Label = "Website — landing page redesign and checkout hardening" + + for _, tc := range []struct { + name string + width int + }{{"report_narrow", 60}, {"report", 80}, {"report_wide", 120}} { + golden(t, tc.name, newReport(r, "").view(testTheme(true), tc.width)) + } +} + +// A multi-currency report renders a TOTAL row with an empty Amount cell plus +// one subtotal row per currency — the widest shape the total block takes. +func TestGoldenReportMultiCurrency(t *testing.T) { + t.Parallel() + r := goldenReport() + r.Buckets = append(r.Buckets, report.Bucket{ + Label: "Mobile", Key: "l3", Hours: 4, BillableHours: 4, BilledHours: 4, + Amounts: []report.CurrencyAmount{{Currency: "USD", Amount: 200}}, + }) + r.CurrencySubtotals = []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 15.5, BillableHours: 12.5, BilledHours: 12.5, Amount: 625}, + {Currency: "USD", Hours: 4, BillableHours: 4, BilledHours: 4, Amount: 200}, + } + r.TotalHours, r.BilledHours = 19.5, 16.5 + golden(t, "report_multicurrency", newReport(r, "").view(testTheme(true), 80)) +} +``` + +Run: `go test ./internal/tui -run 'TestGoldenReport' -update && go test ./internal/tui -run 'TestGoldenReport' -v` +Expected: PASS. + +**Then read all four golden files.** Check by eye: the header sits above a separator line, `Hours`/`Billed`/`Amount` are right-aligned and incolumnated (the old layout did not align `Amount` at all), `TOTAL` is present, the billable split sits *below* the border, and `report_narrow` is not wider than 60 columns. **The three width goldens must differ from each other** — the long label is cut at 60 and 80 and whole at 120. If they came out identical, `reportItemWidth` is not being consulted. + +- [ ] **Step 8: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/report_table.go internal/tui/report_table_test.go internal/tui/report.go internal/tui/app.go internal/tui/golden_test.go internal/tui/report_test.go internal/tui/testdata/ +git commit -m "feat(tui): render the report as a width-aware lipgloss table (#66)" +``` + +--- + +### Task 4: `report.DailyHours` (#80, domain half) + +**Files:** +- Create: `internal/report/daily.go` +- Create: `internal/report/daily_test.go` + +**Interfaces:** +- Consumes: nothing. +- Produces: `report.DailyHours(entries []TimeEntry, start, end time.Time, loc *time.Location) []float64` — one element per day in `[start, end)`. Task 5 consumes it. + +**Background:** `internal/report` is **pure** — no I/O, no imports outside the standard library and this module's pure packages. `TimeEntry` has `Start time.Time` and `Duration time.Duration`. `aggregate.go` already defines `dayFormat`, and `period.go` defines `midnightIn(t, loc)` and `normLoc(loc)` — reuse them; do not write a second day-truncation. + +A day must be advanced with `AddDate(0, 0, 1)` and identified by its formatted key, **never** by adding 24 hours: across a daylight-saving transition a day is 23 or 25 hours long, and second-arithmetic silently drops or duplicates one. + +- [ ] **Step 1: Write the failing tests** + +Create `internal/report/daily_test.go`: + +```go +package report + +import ( + "testing" + "time" +) + +func TestDailyHours(t *testing.T) { + t.Parallel() + utc := time.UTC + day := func(y int, m time.Month, d int) time.Time { + return time.Date(y, m, d, 0, 0, 0, 0, utc) + } + entry := func(start time.Time, h float64) TimeEntry { + return TimeEntry{Start: start, Duration: time.Duration(h * float64(time.Hour))} + } + + for _, tc := range []struct { + name string + entries []TimeEntry + start, end time.Time + want []float64 + }{ + { + name: "empty range yields nil", + start: day(2026, time.July, 5), end: day(2026, time.July, 5), + want: nil, + }, + { + name: "end before start yields nil", + start: day(2026, time.July, 5), end: day(2026, time.July, 1), + want: nil, + }, + { + name: "no entries still yields one zero per day", + start: day(2026, time.July, 1), end: day(2026, time.July, 4), + want: []float64{0, 0, 0}, + }, + { + name: "idle days stay zero rather than disappearing", + entries: []TimeEntry{ + entry(day(2026, time.July, 1).Add(9*time.Hour), 2), + entry(day(2026, time.July, 3).Add(9*time.Hour), 1.5), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 4), + want: []float64{2, 0, 1.5}, + }, + { + name: "several entries on one day are summed", + entries: []TimeEntry{ + entry(day(2026, time.July, 2).Add(9*time.Hour), 2), + entry(day(2026, time.July, 2).Add(14*time.Hour), 3), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{0, 5}, + }, + { + name: "entries outside the range are ignored", + entries: []TimeEntry{ + entry(day(2026, time.June, 30).Add(9*time.Hour), 8), + entry(day(2026, time.July, 2).Add(9*time.Hour), 1), + entry(day(2026, time.July, 9).Add(9*time.Hour), 8), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{0, 1}, + }, + { + name: "an entry is credited to the day it starts on", + entries: []TimeEntry{ + // Starts at 23:00 on the 1st and runs three hours into the 2nd. + entry(day(2026, time.July, 1).Add(23*time.Hour), 3), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{3, 0}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := DailyHours(tc.entries, tc.start, tc.end, utc) + if len(got) != len(tc.want) { + t.Fatalf("got %d days %v, want %d days %v", len(got), got, len(tc.want), tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("day %d = %v, want %v (full series %v)", i, got[i], tc.want[i], got) + } + } + }) + } +} + +// A day is not 24 hours across a DST transition. Rome springs forward on +// 2026-03-29, making that day 23 hours long: a series built by adding 24h +// would drift into the next day and mis-credit every entry after it. +func TestDailyHoursSurvivesDST(t *testing.T) { + t.Parallel() + rome, err := time.LoadLocation("Europe/Rome") + if err != nil { + t.Skipf("tzdata unavailable: %v", err) + } + start := time.Date(2026, time.March, 28, 0, 0, 0, 0, rome) + end := time.Date(2026, time.March, 31, 0, 0, 0, 0, rome) + entries := []TimeEntry{ + {Start: time.Date(2026, time.March, 28, 10, 0, 0, 0, rome), Duration: time.Hour}, + {Start: time.Date(2026, time.March, 29, 10, 0, 0, 0, rome), Duration: 2 * time.Hour}, + {Start: time.Date(2026, time.March, 30, 10, 0, 0, 0, rome), Duration: 3 * time.Hour}, + } + got := DailyHours(entries, start, end, rome) + want := []float64{1, 2, 3} + if len(got) != len(want) { + t.Fatalf("got %d days %v, want 3", len(got), got) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("day %d = %v, want %v (full series %v)", i, got[i], want[i], got) + } + } +} +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `go test ./internal/report -run TestDailyHours -v` +Expected: FAIL — `undefined: DailyHours`. + +- [ ] **Step 3: Implement it** + +Create `internal/report/daily.go`: + +```go +package report + +import "time" + +// DailyHours returns one element per day in [start, end): the total hours of +// the entries that STARTED that day, read in loc. It returns nil when the +// range is empty or inverted. +// +// It counts ALL hours, billable and not — the same total Bucket.Hours carries. +// The series answers "when did I work", not "when did I bill"; the billable +// share has its own line under the report table. +// +// The hours of an entry land entirely on its start day, which is the same rule +// groupKeys uses for GroupByDay — the two views cannot disagree about which +// day an overnight entry belongs to. +// +// This exists instead of reusing GroupByDay buckets because Build creates no +// bucket for a day with no entries. A series built from buckets would close +// the gaps, drawing a full month of work for three days of it. +// +// Days are advanced with AddDate and identified by their formatted key, never +// by adding 24 hours: across a daylight-saving transition a day is 23 or 25 +// hours long, and second-arithmetic would silently drop or duplicate one. +func DailyHours(entries []TimeEntry, start, end time.Time, loc *time.Location) []float64 { + loc = normLoc(loc) + day := midnightIn(start, loc) + last := midnightIn(end, loc) + if !day.Before(last) { + return nil + } + + index := map[string]int{} + var out []float64 + for d := day; d.Before(last); d = d.AddDate(0, 0, 1) { + index[d.Format(dayFormat)] = len(out) + out = append(out, 0) + } + + for _, e := range entries { + if i, ok := index[e.Start.In(loc).Format(dayFormat)]; ok { + out[i] += e.Duration.Hours() + } + } + return out +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/report -run TestDailyHours -v` +Expected: PASS, both tests and all seven sub-cases. + +- [ ] **Step 5: Confirm the package is still pure** + +Run: `go list -deps ./internal/report | grep 'clickup-cli'` +Expected: exactly two lines — `.../internal/duration` (imported by `model.go` for `duration.RoundMode`) and `.../internal/report` itself, since `-deps` includes the root package. **Neither `internal/config` nor `internal/clickup` may appear**; that is the whole assertion. Anything else in the list is a purity break. + +- [ ] **Step 6: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/report/daily.go internal/report/daily_test.go +git commit -m "feat(report): add DailyHours, a zero-filled per-day series (#80)" +``` + +--- + +### Task 5: The sparkline (#80, presentation half) + +**Files:** +- Create: `internal/tui/sparkline.go` +- Create: `internal/tui/sparkline_test.go` +- Modify: `internal/tui/report.go` (`reportModel`, `newReport`, `view`, and the new `Model.dailySeries` helper — it goes here, next to `memberFilterNote`, not in app.go) +- Modify: `internal/tui/app.go` (the `newReport` call at line ~689) +- Modify: `internal/tui/rates.go:869`, `internal/tui/report.go:73` and `:195` (the other `newReport` call sites) +- Modify: `internal/tui/golden_test.go`, `internal/tui/report_test.go`, `internal/tui/log_test.go` (test call sites) + +**Interfaces:** +- Consumes: `report.DailyHours` (Task 4); `reportModel.view(th, width)` (Task 3). +- Produces: + - `sparkline(values []float64, maxCells int) string` + - `newReport(r report.Report, note string, daily []float64) reportModel` — **signature change** + - `(m Model) dailySeries() []float64` + +**Background:** `reportModel` holds only a `report.Report` and a note; it has no entries, which is why the series is passed in rather than computed inside. The `Model` has `visibleEntries()`, `currentRange()` and `loc`. Do **not** add a `Daily` field to `report.Report`: that would change a domain type and the CLI's JSON output for a rendering need. + +- [ ] **Step 1: Write the failing tests** + +Create `internal/tui/sparkline_test.go`: + +```go +package tui + +import ( + "strings" + "testing" +) + +func TestSparkline(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + values []float64 + maxCells int + want string + }{ + {"empty", nil, 10, ""}, + {"all zero renders as gaps", []float64{0, 0, 0}, 10, " "}, + {"a single value is full height", []float64{5}, 10, "█"}, + {"equal values are all full height", []float64{4, 4, 4}, 10, "███"}, + {"an idle day is a gap, not a low bar", []float64{8, 0, 8}, 10, "█ █"}, + {"the maximum is full and the minimum non-zero is lowest", []float64{1, 8}, 10, "▁█"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := sparkline(tc.values, tc.maxCells); got != tc.want { + t.Errorf("sparkline(%v, %d) = %q, want %q", tc.values, tc.maxCells, got, tc.want) + } + }) + } +} + +// A custom range of a year is 365 days and cannot be drawn one cell per day. +// Resampling keeps the line inside the terminal. +func TestSparklineResamples(t *testing.T) { + t.Parallel() + values := make([]float64, 365) + for i := range values { + values[i] = float64(i) + } + got := sparkline(values, 30) + if n := len([]rune(got)); n != 30 { + t.Errorf("got %d cells, want 30", n) + } + // The series rises monotonically, so the resampled line must too. + runes := []rune(got) + for i := 1; i < len(runes); i++ { + if strings.IndexRune(sparkLevels, runes[i]) < strings.IndexRune(sparkLevels, runes[i-1]) { + t.Errorf("cell %d (%q) is lower than cell %d (%q) in a rising series: %q", + i, runes[i], i-1, runes[i-1], got) + } + } +} + +// Fewer values than cells must not be stretched: 5 days is 5 cells. +func TestSparklineDoesNotStretch(t *testing.T) { + t.Parallel() + if got := sparkline([]float64{1, 2, 3, 4, 5}, 40); len([]rune(got)) != 5 { + t.Errorf("sparkline of 5 values = %q (%d cells), want 5 cells", got, len([]rune(got))) + } +} +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `go test ./internal/tui -run TestSparkline -v` +Expected: FAIL — `undefined: sparkline`, `undefined: sparkLevels`. + +- [ ] **Step 3: Implement it** + +Create `internal/tui/sparkline.go`: + +```go +package tui + +import "math" + +// sparkLevels are the eight bar heights, lowest first. Index 0 is the lowest +// NON-ZERO level: an exact zero renders as a space instead, because a day with +// no work must read as a gap and ▁ would read as a little work. +const sparkLevels = "▁▂▃▄▅▆▇█" + +// sparkline renders values as block glyphs, one cell per value, resampled to +// at most maxCells when there are more values than cells. +// +// Heights are relative to the largest value in the series, so the shape shows +// the period's rhythm rather than an absolute scale. +func sparkline(values []float64, maxCells int) string { + if len(values) == 0 { + return "" + } + if maxCells > 0 && len(values) > maxCells { + values = resample(values, maxCells) + } + + peak := 0.0 + for _, v := range values { + peak = max(peak, v) + } + + levels := []rune(sparkLevels) + out := make([]rune, 0, len(values)) + for _, v := range values { + if v <= 0 || peak == 0 { + out = append(out, ' ') + continue + } + lvl := int(math.Ceil(v / peak * float64(len(levels)))) + out = append(out, levels[min(max(lvl, 1), len(levels))-1]) + } + return string(out) +} + +// resample averages values into k contiguous buckets. Bucket i covers +// values[i*n/k : (i+1)*n/k], which distributes the remainder deterministically +// and leaves no bucket empty as long as k <= n. +func resample(values []float64, k int) []float64 { + n := len(values) + out := make([]float64, k) + for i := range out { + lo, hi := i*n/k, (i+1)*n/k + if hi <= lo { + hi = lo + 1 + } + sum := 0.0 + for _, v := range values[lo:hi] { + sum += v + } + out[i] = sum / float64(hi-lo) + } + return out +} +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/tui -run TestSparkline -v` +Expected: PASS. + +- [ ] **Step 5: Carry the series into the report model** + +In `internal/tui/report.go`, extend the model and its constructor: + +```go +type reportModel struct { + r report.Report + note string + daily []float64 // one entry per day of the range; see report.DailyHours +} + +func newReport(r report.Report, note string, daily []float64) reportModel { + return reportModel{r: r, note: note, daily: daily} +} +``` + +Add the `Model` helper, next to `memberFilterNote`: + +```go +// dailySeries is the per-day hours of the visible entries over the current +// range. It lives on the Model because reportModel has no entries — the report +// is a rendering of an already-aggregated value. +func (m Model) dailySeries() []float64 { + start, end := m.currentRange() + return report.DailyHours(m.visibleEntries(), start, end, m.loc) +} +``` + +- [ ] **Step 6: Render it** + +In `reportModel.view`, insert the sparkline between `summary` and `body`. Replace the `return` line with: + +```go + out := title + "\n\n" + summary + "\n\n" + if line := rm.sparkView(th, width); line != "" { + out += line + "\n\n" + } + return out + body + "\n" + split +``` + +and add: + +```go +// sparkLabel is appended to the sparkline. Its width plus a margin is what +// sparkView reserves, and it guarantees the line never ends in the space a +// zero-hours day renders as. +const sparkLabel = " hours/day" + +// sparkView renders the per-day sparkline, or "" when there is nothing worth +// drawing: a range of one day is a single cell, and a report with no buckets +// already says so in its body. +func (rm reportModel) sparkView(th theme, width int) string { + if len(rm.daily) < 2 || len(rm.r.Buckets) == 0 { + return "" + } + cells := 31 // the longest month, used until the terminal reports its width + if width > 0 { + cells = max(1, width-len(sparkLabel)-2) + } + return th.Accent.Render(sparkline(rm.daily, cells)) + th.Help.Render(sparkLabel) +} +``` + +- [ ] **Step 7: Update the four production call sites** + +Each already has the range and the entries in scope, so all four become the same shape: + +- `internal/tui/app.go:689` → `m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries())` +- `internal/tui/rates.go:869` → same +- `internal/tui/report.go:73` → same +- `internal/tui/report.go:195` (`applyReport`) → same + +Then add `nil` as the third argument at these five test call sites — `log_test.go:51`, `report_test.go:57`, `:73`, `:104`, `:122` — and run `go vet ./...` to catch any that moved. Use `go vet`, **not** `go build ./...`: `go build` does not compile `_test.go` files, so it would report a clean tree while every test call site is still broken. The golden tests are the exception: Step 8 gives them a real series instead of `nil`. + +- [ ] **Step 8: Give the goldens a real series and regenerate** + +In `internal/tui/golden_test.go`, add this fixture next to `goldenReport`: + +```go +// goldenDaily is a fixed per-day series with two idle days, so the golden +// pins both a bar and a gap. +func goldenDaily() []float64 { + return []float64{3.5, 0, 1.25, 4, 0, 2, 6} +} +``` + +and pass `goldenDaily()` instead of `nil` in `TestGoldenReport` and `TestGoldenReportMultiCurrency`. + +Run: `go test ./internal/tui -run TestGoldenReport -update && go test ./internal/tui -race` +Expected: PASS. + +**Read `internal/tui/testdata/report.golden`.** The sparkline must sit between the summary and the table, show gaps where the series is zero, and end with `hours/day`. + +- [ ] **Step 9: Check demo mode by hand** + +Run: `go run ./cmd/clup` with `CLICKUP_DEMO=1`, press Enter to generate the report. +Expected: a sparkline with visible gaps, a bordered table with aligned numbers, and a footer. The fixture entries fall on days **2, 3, 5, 6, 7, 9 and 10** of the range, so days 1, 4 and 8 are gaps. Day 2 is the non-billable sprint-planning session and **must** appear as a bar: the series counts all hours, not only billable ones. + +- [ ] **Step 10: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/sparkline.go internal/tui/sparkline_test.go internal/tui/report.go internal/tui/app.go internal/tui/rates.go internal/tui/golden_test.go internal/tui/report_test.go internal/tui/log_test.go internal/tui/testdata/ +git commit -m "feat(tui): draw a per-day sparkline on the report (#80)" +``` + +--- + +### Task 6: The budget gauge (#80, gauge half) + +**Files:** +- Modify: `internal/tui/budget.go` +- Modify: `internal/tui/budget_test.go` + +**Interfaces:** +- Consumes: `theme.OK`, `theme.Err`, `theme.Help` (all pre-existing). +- Produces: `renderBudgetBar(th theme, percentUsed float64) string` — **signature change**. + +**Background — why not `bubbles/progress`:** #80's checkbox names it, and it is the wrong tool here. `progress.percentageView` runs `math.Max(0, math.Min(1, percent))` *before* formatting, so a budget burned to 130% renders as `100%` — it hides the single fact this screen exists to show. (`progress.New()` also reads `termenv.ColorProfile()`, the real terminal rather than the injected renderer, which would force the color profile onto the `theme` for one caller.) The existing pure function already gets this right and only needs better glyphs and color. + +- [ ] **Step 1: Write the failing tests** + +In `internal/tui/budget_test.go`, replace `TestRenderBudgetBarClampsFillNotPercent` with: + +```go +// A list over its budget must still be visible in the number even though the +// bar itself caps its fill at 100%. This is exactly what bubbles/progress +// cannot do — it clamps the percentage to 100 before formatting it — and it is +// why this screen keeps its own bar. +func TestRenderBudgetBarClampsFillNotPercent(t *testing.T) { + t.Parallel() + out := renderBudgetBar(testTheme(true), 150) + if !strings.Contains(out, "150%") { + t.Errorf("renderBudgetBar(150) = %q, want the unclamped 150%% in the label", out) + } + if full := strings.Repeat(string(gaugeFull), budgetBarWidth); !strings.Contains(out, full) { + t.Errorf("renderBudgetBar(150) = %q, want a fully filled bar", out) + } +} + +// Over budget is the state the view exists to surface, so it must not look the +// same as a healthy one. The goldens run under termenv.Ascii, which strips the +// color entirely, so this asserts on the style rather than the output. +func TestBudgetBarColorsOverBudget(t *testing.T) { + t.Parallel() + th := paletteTheme(true) // real colors, so foregrounds are comparable + under := budgetFillStyle(th, 60) + over := budgetFillStyle(th, 130) + if under.GetForeground() != th.OK.GetForeground() { + t.Error("a bar under budget is not drawn in the OK color") + } + if over.GetForeground() != th.Err.GetForeground() { + t.Error("a bar over budget is not drawn in the Err color") + } +} +``` + +And in `TestBudgetViewRendersProgressBar`, replace the `#`/`-` assertion with: + +```go + if !strings.ContainsRune(out, gaugeFull) || !strings.ContainsRune(out, gaugeEmpty) { + t.Errorf("expected a bar built from %q and %q; got:\n%s", gaugeFull, gaugeEmpty, out) + } +``` + +- [ ] **Step 2: Run them to verify they fail** + +Run: `go test ./internal/tui -run 'TestRenderBudgetBar|TestBudgetBarColors|TestBudgetViewRendersProgressBar' -v` +Expected: FAIL — `gaugeFull`, `gaugeEmpty` and `budgetFillStyle` are undefined, and `renderBudgetBar` takes one argument. + +- [ ] **Step 3: Implement** + +In `internal/tui/budget.go`, replace `renderBudgetBar` and add the two helpers: + +```go +// The gauge's glyphs. Blocks rather than '#' and '-': at any font size they +// read as a filled bar instead of as text. +const ( + gaugeFull = '█' + gaugeEmpty = '░' +) + +// budgetFillStyle is the color of the filled part of a gauge. Over budget is +// the state this screen exists to surface, so it must not look like a healthy +// one. It is a named function because the package goldens run under +// termenv.Ascii, which strips the color: asserting on the style is the only +// way to test the choice. +func budgetFillStyle(th theme, percentUsed float64) lipgloss.Style { + if percentUsed > 100 { + return th.Err + } + return th.OK +} + +// renderBudgetBar renders a percent-used value as a fixed-width gauge, e.g. +// "████████████░░░░░░░░ 60%". The fill is clamped to [0, 100] — a list can run +// over budget, but the bar cannot render past full — while the percentage in +// the label is shown UNCLAMPED, so an over-100% burn stays visible in the +// number. That asymmetry is the whole point: bubbles/progress clamps the +// number too, which is why this screen does not use it. +func renderBudgetBar(th theme, percentUsed float64) string { + fillPct := min(max(percentUsed, 0), 100) + filled := int(fillPct / 100 * budgetBarWidth) + bar := budgetFillStyle(th, percentUsed).Render(strings.Repeat(string(gaugeFull), filled)) + + th.Help.Render(strings.Repeat(string(gaugeEmpty), budgetBarWidth-filled)) + return fmt.Sprintf("%s %.0f%%", bar, percentUsed) +} +``` + +Add `"github.com/charmbracelet/lipgloss"` to the imports. The clamp uses the +built-in `min`/`max`, so no `math` import is needed. + +In `budgetModel.view`, the call becomes `renderBudgetBar(th, l.PercentUsed)`, and the format string loses the brackets that used to wrap the bar: + +```go + rows.WriteString(fmt.Sprintf("%-24s %s %.2f / %.2f %s (remaining %.2f)\n", + truncate(l.ListName, 24), renderBudgetBar(th, l.PercentUsed), l.Billed, l.Budget, l.Currency, l.Remaining)) +``` + +- [ ] **Step 4: Run the tests to verify they pass** + +Run: `go test ./internal/tui -run 'TestBudget|TestRenderBudgetBar' -v` +Expected: PASS. + +- [ ] **Step 5: Regenerate the budget golden** + +Run: `go test ./internal/tui -run TestGoldenBudget -update && go test ./internal/tui -race` +Expected: PASS. + +**Read `internal/tui/testdata/budget.golden`** — the bar must be built from `█` and `░`, and the percentage must still be there. + +- [ ] **Step 6: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add internal/tui/budget.go internal/tui/budget_test.go internal/tui/testdata/budget.golden +git commit -m "feat(tui): draw the budget burn-down as a block gauge (#80)" +``` + +--- + +### Task 7: Documentation and closing + +**Files:** +- Modify: `CHANGELOG.md` +- Modify: `docs/demo.gif` (regenerated, not hand-edited) + +**Interfaces:** +- Consumes: everything from Tasks 1-6. +- Produces: nothing code depends on. + +- [ ] **Step 1: Add the CHANGELOG entries** + +Under `## [Unreleased]` in `CHANGELOG.md`, in the existing `### Added` / `### Changed` subsections (create them if this is the first entry since the last release): + +```markdown +### Added + +- The report screen now draws a per-day sparkline of the selected range, with + idle days rendered as gaps rather than as low bars (#80). + +### Changed + +- The report is rendered as a real table that sizes itself to the terminal: + the numeric columns are right-aligned and the label column takes the slack, + instead of a fixed 32-column layout that wrapped on a narrow terminal (#66). +- The budget burn-down bars are drawn with block glyphs and colored — green + under budget, red over it — while still showing the true, unclamped + percentage (#80). +``` + +- [ ] **Step 2: Regenerate the demo GIF** + +The report's appearance changed, so `docs/demo.gif` — embedded in both READMEs — is stale. + +Run: `vhs docs/demo.tape` +Expected: `docs/demo.gif` is rewritten. If `vhs` is not installed (`brew install vhs`), stop and report this step as blocked rather than committing a stale GIF. + +**Then look at the result.** Extract a frame from the report section and check the table renders with its border and aligned columns, and that the sparkline is visible. + +- [ ] **Step 3: Full gate and commit** + +```bash +gofmt -l . && go vet ./... && go run honnef.co/go/tools/cmd/staticcheck@latest ./... && go build ./... && go test ./... -race +git add CHANGELOG.md docs/demo.gif +git commit -m "docs: record the report dashboard and refresh the demo GIF" +``` + +- [ ] **Step 4: Report what still needs a human** + +These are for the controller to do after the branch merges — they are **not** commits: + +1. **Open an issue** for the budget screen's width: its rendered line is 94 columns and wraps on an 80-column terminal. This is pre-existing (it predates this tranche) and out of scope here, but this tranche touched that exact line's glyphs and colors, so it must not be left unrecorded. Bilingual body, `area:tui` + `area:billing`. +2. **Close #117** with a note that the four indices are now `sel [secCount]int` and that `selCount`'s switch was deliberately kept, since its four counts genuinely differ. +3. **Close #66**, noting that column separators and row separators are off by design and that `TOTAL` is distinguished by weight and color because `lipgloss/table`'s `BorderRow` is all-or-nothing. +4. **Close #80**, noting that `bubbles/progress` was evaluated and rejected: it clamps the percentage to 100 before formatting, which would hide exactly the over-budget state the view exists to show. + +--- + +## Self-Review + +**Spec coverage:** + +| Spec section | Task | +|---|---| +| §6.1 `sel [secCount]int` | 1 | +| §6.2 table, column arithmetic, borders, `Wrap(false)` | 3 | +| §6.2 `Subtle`/`Border`/`Zebra` (+ `Cell`) | 2 | +| §6.3 `DailyHours` | 4 | +| §6.3 `sparkline`, `dailySeries`, `newReport` signature | 5 | +| §6.4 gauge | 6 | +| §7.1 characterization test | 1, Step 1 | +| §7.2 pure tests | 4 Step 1, 5 Step 1 | +| §7.3 width test | 3 Step 1 | +| §7.4 style-function tests | 3 Step 1, 6 Step 1 | +| §7.5 goldens | 3 Step 7, 5 Step 8, 6 Step 5 | +| §8 execution order | Tasks 1-7 in order | +| §5 issue for the budget width | 7 Step 4 | + +**Deviation from the spec, recorded deliberately:** §6.2 lists two new theme +styles (`Border`, `Zebra`); this plan adds a third, `Cell`. Without it +`reportStyleFunc` would have to call `lipgloss.NewStyle()` for unstyled rows, +building them on the *default* renderer while themed rows use the injected one +— the discipline tranche B2 established when it refused `help.New()`. The +spec's §6.2 arithmetic and §7 test list are unaffected. + +**Type consistency:** `reportModel.view(th theme, width int)` is introduced in +Task 3 and unchanged by Task 5. `newReport` gains its third parameter in +Task 5 only. `renderBudgetBar` gains `th` in Task 6 only. `sparkLevels`, +`gaugeFull` and `gaugeEmpty` are referenced by tests in the same task that +defines them. diff --git a/docs/superpowers/specs/2026-07-28-report-dashboard-design.md b/docs/superpowers/specs/2026-07-28-report-dashboard-design.md new file mode 100644 index 0000000..fbe8cea --- /dev/null +++ b/docs/superpowers/specs/2026-07-28-report-dashboard-design.md @@ -0,0 +1,484 @@ +# Report dashboard: griglia, sparkline e gauge (v1.9, tranche C) — Design + +> Spec della quarta tranche della milestone **v1.9 TUI design system**. +> Copre #117, #66 e #80. Ramo: `feat/report-dashboard`. + +## 1. Obiettivo + +Tre cose, in quest'ordine: + +1. **#117** — collassare i quattro indici di selezione paralleli dell'editor di + fatturazione in un solo array indicizzato per sezione. Refactor puro, + nessun cambio di comportamento. +2. **#66** — sostituire il report costruito a mano con `fmt.Sprintf` con una + `lipgloss/table` vera: header, righe zebra, colonne numeriche allineate a + destra, TOTAL colorato, **larghezza adattiva al terminale**. +3. **#80** — trasformare il report in un cruscotto: una sparkline delle ore per + giorno sul report, e un gauge a blocchi al posto della barra ASCII nella + schermata Budget. + +L'esito visibile: il report smette di essere una colonna fissa a 32 caratteri +che va a capo su un terminale stretto e spreca spazio su uno largo, e guadagna +un colpo d'occhio sull'andamento del periodo. + +## 2. Perché #117 va per primo + +Lo dice la sua stessa issue: entrambe le review della #116 hanno riconosciuto la +duplicazione e l'hanno rimandata alla v1.9 «perché è un refactor meccanico di +uno schermo che il design system ristrutturerà comunque», e il reviewer +whole-branch ha aggiunto che gli switch a 4 vie di `move`/`selCount` sono «già +al limite della manutenibilità», quindi va affrontato **per primo** quando la +v1.9 tocca quello schermo. + +La v1.9 quello schermo l'ha già toccato tre volte (tranche A gli ha dato il +tema, B1 la keymap, B2 il footer e la nota sulla tariffa zero) senza mai fare il +refactor. Questa è l'ultima tranche che lo tocca: o adesso o l'issue resta +aperta a milestone chiusa. + +## 3. Stato attuale (misurato, non supposto) + +### 3.1 `internal/tui/rates.go` + +- `ratesModel` porta quattro campi di selezione: `idx` (Liste), `memIdx` + (Membri), `ovIdx` (Override), `ruleIdx` (Regole). +- `move` fa **due switch**, uno per leggere l'indice della sezione attiva (a 3 + vie: `secLists` è gestito dall'inizializzatore `cur := rt.idx`) e uno a 4 vie + per riscriverlo. È l'unica funzione la cui duplicazione sia interamente + eliminabile. +- `selCount`, `startEdit` e `clearSelected` fanno anch'esse uno switch sulla + sezione, ma **il loro switch non è duplicazione**: i conteggi sono + genuinamente diversi (`len(rows)`, `len(members)`, `len(overrides)+1`, + `ruleCount`) e i comportamenti di modifica/cancellazione divergono per + sezione. In quelle funzioni cambia solo la *lettura* dell'indice. +- `commit` **non** fa uno switch sulla sezione: il suo switch è a 8 vie su + `rt.edit` (`editKind`). Legge i quattro indici, e basta. La issue #117 dice + altro; la issue si sbaglia. +- `rates_view.go` legge i quattro campi in **sette occorrenze** distribuite + sulle quattro sezioni (righe 54 e 56 per le Liste, 77 e 79 per i Membri, 95 e + 97 per gli Override, 134 per le Regole). +- **C'è uno scrittore esterno al file**: `listbrowser.go:152` fa `rt.idx = found` + quando il browser delle liste restituisce una lista alla sezione Liste. +- **Nessun test tocca i quattro campi direttamente.** I 31 `TestRates*` di + `rates_test.go` guidano tutti da messaggi di tastiera. La suite è quindi una + rete di sicurezza vera, non una che verifica l'implementazione: è per questo + che il refactor si può fare senza riscriverla. + +### 3.2 `internal/tui/report.go` + +- `reportModel.view` costruisce l'header e ogni riga con `fmt.Sprintf` a + larghezze fisse: `%-32s %8s %8s %s`. Le etichette si troncano a 32 rune. +- La colonna `Amount` è l'ultima e non è allineata: `%s` la lascia crescere a + destra, quindi `625.00 EUR` e `0.00 EUR` non si incolonnano (visibile nel + golden attuale). +- Il blocco totali, i sottototali per valuta e la nota + `billable … · non-billable …` finiscono **dentro** `th.Box` insieme alle + righe, quindi la nota oggi sta dentro il bordo. +- Larghezza risultante: **63 colonne** di contenuto più il bordo di `th.Box`. +- `reportModel` porta solo `report.Report` e una nota. **Non ha le entry**, il + che vincola da dove può arrivare la serie giornaliera (§6.3). +- `newReport` ha **4 siti di produzione** (`app.go:689`, `rates.go:869`, + `report.go:73`, `report.go:195`) e 6 siti nei test. + +### 3.3 `internal/tui/budget.go` + +- `renderBudgetBar(percentUsed float64) string` produce `[####----] 60%` con + glifi ASCII, riempimento clampato a `[0, 100]` e **percentuale non clampata** + — e il commento dice che è deliberato, perché sforare il budget è + esattamente ciò che quella vista deve far vedere. +- La funzione non riceve il tema: la barra è monocroma. +- La riga di budget renderizzata è larga **94 colonne** (golden + `budget.golden`), quindi già oggi va a capo su un terminale da 80. Fuori + ambito, §9. + +### 3.4 Vincoli dell'ambiente di test + +- `TestMain` fissa `lipgloss.SetColorProfile(termenv.Ascii)`, quindi **i golden + non contengono ANSI**. Ne discende che i golden **non possono vedere**: + - le righe zebra (uno sfondo); + - il colore rosso di uno sforamento di budget; + - quale stile porta la riga TOTAL. + + Questa non è una limitazione da aggirare: i golden restano la rete per il + *layout*, e ciò che è invisibile ad Ascii richiede test diretti sugli stili + (§7). Lo stesso ragionamento della tranche A, dove i golden non potevano + vedere due ruoli di stile scambiati. + +### 3.5 Cosa fanno davvero le librerie + +Verificato leggendo il sorgente delle versioni bloccate in `go.mod`, non la +documentazione: + +- **`lipgloss/table` v1.1.0** — `table.New()` lascia `borderStyle` al suo valore + zero, e uno `lipgloss.Style` zero renderizza attraverso **il renderer di + default**: va iniettato con `BorderStyle`, altrimenti è la stessa falla che la + tranche B2 ha evitato non usando `help.New()`. I default sono + `borderColumn: true` (separatori fra + colonne) e **`wrap: true`** — quest'ultimo manderebbe a capo un'etichetta + lunga invece di troncarla, spezzando l'invariante "una riga per bucket". + Entrambi vanno spenti. +- **`bubbles/progress` v1.0.0** — `percentageView` fa + `math.Max(0, math.Min(1, percent))` **prima** di formattare: un budget + sforato al 130% si legge `100%`. Inoltre `New()` prende + `termenv.ColorProfile()`, cioè il terminale vero e non il renderer iniettato, + quindi userebbe richiedere l'esposizione del profilo colore sul `theme`. La + libreria **non viene adottata**: §6.4. + +## 4. Decisioni prese + +| Domanda | Decisione | +|---|---| +| Dove va il gauge, dato che sommare budget in valute diverse è vietato dalla visione di progetto (multi-valuta a subtotali, no FX)? | Sparkline sul report, gauge nella schermata Budget: ogni `BudgetLine` porta già la sua valuta, quindi il problema FX non si pone e non c'è nessuno stato condizionale da specificare. | +| Quanto la tabella si adatta al terminale? | Adattiva: le colonne numeriche restano fisse, tutto lo spazio in più o in meno va alla colonna `Item`. | +| Come rendiamo il gauge? | Con la nostra funzione pura, potenziata — non con `bubbles/progress`, che clampa il numero che la vista esiste per mostrare. | +| Come facciamo le righe zebra? | Sfondo alternato da un nuovo token `Subtle` della palette. | + +## 5. Fuori ambito + +- **`bubbles/progress`**: motivato in §6.4. La checkbox della #80 va chiusa con + un commento che spiega perché. +- **La larghezza della schermata Budget.** La sua riga è larga 94 colonne e va + a capo a 80. È un difetto **preesistente**, non introdotto qui: questa + tranche cambia i glifi e i colori di quella riga, non la sua struttura. Va + aperta una issue dedicata prima di chiudere la tranche — non basta scriverlo + qui, perché una spec non è un tracker. +- **La riorganizzazione di `rates.go`** oltre la #117. Il file è a 914 righe, + ma la #117 chiede un refactor puro e nient'altro. +- **`report.Report.Daily`**: la serie giornaliera *non* entra nel tipo di + dominio né nell'output JSON del CLI (§6.3). + +## 6. Disegno + +### 6.1 #117 — `sel [secCount]int` + +`ratesModel` perde `idx`, `memIdx`, `ovIdx`, `ruleIdx` e guadagna: + +```go +// sel is the selected row of each section, indexed by ratesSection. One array +// instead of four fields means move() needs no switch at all, and a fifth +// section is a non-event. +sel [secCount]int +``` + +`move` diventa: + +```go +func (rt ratesModel) move(delta int) ratesModel { + next := rt.sel[rt.sec] + delta + if next < 0 || next > rt.selCount()-1 { + return rt + } + rt.sel[rt.sec] = next + return rt +} +``` + +Ogni altra lettura diventa esplicita nella sezione in cui si trova — +`rt.rows[rt.sel[secLists]]`, `rt.members[rt.sel[secMembers]]`, +`rt.sel[secOverrides]`, `rt.sel[secRules]` — **non** `rt.sel[rt.sec]`: dentro un +`case secLists:` la costante nomina l'invariante, la variabile la nasconde. + +`listbrowser.go:152` diventa `rt.sel[secLists] = found`. + +**Il valore zero dell'array è identico a quattro `int` a zero**, quindi ogni +golden esistente resta byte per byte lo stesso e i 31 test di `rates_test.go` +non si toccano. + +**Nota per chi implementa: qui non esiste un RED.** È un refactor puro: il test +nuovo (§7.1) deve passare *prima* del refactor, perché fissa la proprietà per +cui i quattro indici esistevano. Un implementatore a cui si dice «TDD RED→GREEN» +su un refactor si inventa un test rotto per poterlo far diventare verde. + +### 6.2 #66 — la tabella + +Nuovo file `internal/tui/report_table.go`, così `report.go` resta sulle sue +~270 righe invece di gonfiarsi: + +```go +// reportTable renders the report's buckets and totals as one table, sized to +// width. width <= 0 means "natural width" (the first render, before the +// terminal has sent its WindowSizeMsg). +func reportTable(th theme, r report.Report, width int) string +``` + +**Aritmetica delle colonne** — deve essere esatta, o due implementatori +scrivono due tabelle diverse: + +- `Hours` e `Billed`: larghezza fissa **8**. +- `Amount`: la più lunga delle stringhe effettivamente renderizzate. A cedere + per prima è sempre l'etichetta. + + > **Emendamento (review finale whole-branch).** Questa regola diceva + > originariamente «**senza tetto**: troncare un importo nasconde soldi», ed + > era sbagliata. `GroupByTotal` è il raggruppamento del primo caricamento e il + > suo unico bucket porta **tutte** le valute dello workspace: con tre valute + > la tabella sforava già a 60 colonne, con cinque anche a 100. La colonna + > `Amount` ha quindi un tetto, e viene troncata **come ultima risorsa**, solo + > quando `Item` è già al suo pavimento. + > + > Il ragionamento originale era sbagliato nella premessa, non nella + > conclusione: un importo *per bucket* è **indicativo**, come dice già il + > commento di `reportRows` — l'arrotondamento `PerDay` può scostarsi di + > qualche centesimo dai sottototali su un raggruppamento fine. La cifra + > autorevole è `CurrencySubtotals`, che ha righe proprie più in basso nella + > stessa tabella. Troncare un indicativo non nasconde soldi: l'esatto è + > quattro righe sotto. +- Overhead: **2** bordi verticali (solo esterni: i separatori fra colonne sono + spenti, vedi sotto) + 4 colonne × 2 di padding = **10**. La costante è una + previsione sul comportamento di `lipgloss/table`, non un dato: l'invariante + vincolante è la *larghezza renderizzata*, ed è il test §7.3 a stabilirla. Se + il conteggio reale risulta diverso, si corregge la costante, non il test. +- `itemW = width - 10 - 8 - 8 - amountW`, dove `maxLabel` è la più lunga fra le + etichette dei bucket **e** quelle dei totali (`TOTAL`, ` subtotal EUR`), poi: + - non si allarga sopra `maxLabel`: niente spazio vuoto stirato; + - non si stringe sotto 12: a quel punto si accetta lo sforamento, perché a 30 + colonne non c'è niente da salvare. + - **Quando `maxLabel` è minore di 12 vince il tetto, non il pavimento.** Non è + un caso di scuola: `GroupByTotal` è il raggruppamento del primo caricamento + e le sue etichette sono `Total` e `TOTAL`, cioè `maxLabel = 5`. Un + `clamp(x, 12, 5)` è un intervallo invertito, e prendere il pavimento per + primo stirerebbe la colonna a 12 contraddicendo la riga sopra. La formula è + quindi `max(min(12, maxLabel), min(maxLabel, disponibile))`. +- `width <= 0` → `itemW = min(maxLabel, 32)`. È la stessa **regola** di prima + della #66 (un tetto a 32 colonne), non lo stesso rendering: oggi la colonna è + larga 32 fisse anche quando le etichette sono corte, mentre questa si adatta + al contenuto. Il primo render cambierà aspetto, ed è voluto. + +Le etichette si troncano a `itemW` con la `truncate` esistente, che taglia per +rune e non per byte. + +**Il dimensionamento è implicito, e `Table.Width()` non va chiamata.** Le +colonne si dimensionano da sole sul contenuto: celle numeriche già formattate, +etichette già troncate a `itemW`, `Padding(0, 1)` applicato a ogni cella dalla +`StyleFunc`. `Table.Width(w)` è una trappola con questa configurazione: in +`resizing.go`, `totalHorizontalBorder()` conta **solo i separatori fra colonne**, +quindi con `BorderColumn(false)` restituisce 0 e ignora i due bordi esterni, +mentre `String()` taglia l'output con `MaxWidth(t.width)`. Il risultato è una +tabella renderizzata a `w+2` e poi amputata a `w`: **il bordo destro sparisce**. +Il suo resizer inoltre restringe le colonne più larghe senza riguardo per le +larghezze fisse (che onora solo via `style.GetWidth()`, dove `Style.Width` +include il padding, quindi «8» darebbe 6 colonne di contenuto). + +**Configurazione della tabella:** + +- `Border(lipgloss.RoundedBorder())`, `BorderStyle(th.Border)` — iniettato, + §3.5. +- `BorderColumn(false)`: nessun separatore verticale fra colonne, come il + layout attuale. +- `BorderHeader(true)`: una riga di separazione sotto l'header. È l'unico + separatore orizzontale che si ottiene, perché `BorderRow` è tutto-o-niente. +- `Wrap(false)`: il troncamento lo facciamo noi, §3.5. +- Allineamento: `Item` a sinistra, `Hours`/`Billed`/`Amount` a destra. + +**Righe:** i bucket, poi il blocco totali, **nella stessa tabella** — è tutto il +punto dell'allineamento. Il blocco totali è una riga sola quando il report è a +valuta singola, altrimenti una riga `TOTAL` con la cella `Amount` vuota seguita +da un sottototale per valuta. Il TOTAL si distingue per grassetto e colore, non +per una linea, perché `lipgloss/table` non sa disegnare un separatore su una +riga sola. + +**La nota `billable … · non-billable …` esce dal bordo** e va sotto la tabella. +Oggi sta dentro `th.Box` insieme ai dati; non è una riga di dati, e dentro una +tabella si prenderebbe la zebratura e la divisione in colonne. Questo cambia il +golden ed è voluto. + +`th.Box` esce dal report — il bordo lo disegna la tabella — e resta per il caso +vuoto (`No hours to show.`) e per tutte le altre schermate. + +**Firma:** `reportModel.view(th theme, width int)`. È la prima view del progetto +a ricevere la larghezza; `screenBody` le passa `m.width`, lo stesso valore che +il footer già usa. + +**Il tema** guadagna due stili e la palette un token: + +```go +// palette +Subtle lipgloss.AdaptiveColor // zebra row background + +// theme +Border lipgloss.Style // the report table's frame: Muted foreground +Zebra lipgloss.Style // alternate row background +``` + +`Subtle` va commentato come gli altri cinque token, ma il criterio è diverso: +236 su scuro e 254 su chiaro sono scelti perché il **foreground di default +continui a passare il contrasto sopra di essi**, non contro il fondo del +terminale. + +### 6.3 #80 — la sparkline + +La serie giornaliera è dominio, quindi sta in `internal/report/daily.go`, puro: + +```go +// DailyHours returns one element per day in [start, end), each the total hours +// of the entries that started that day. +func DailyHours(entries []TimeEntry, start, end time.Time, loc *time.Location) []float64 +``` + +Regole, tutte vincolanti: + +- Le ore di un'entry sono accreditate **al giorno in cui l'entry inizia**, + letto in `loc` — la stessa regola di `groupKeys` per `GroupByDay`, così le + due viste non possono divergere. +- **Si contano tutte le ore, fatturabili e non**, coerentemente con + `Bucket.Hours` e con l'etichetta `hours/day`. Non è ovvio e va detto: la + sparkline risponde a «quando ho lavorato», non a «quando ho fatturato», e la + quota fatturabile ha già la sua riga sotto la tabella. +- I giorni si iterano per `AddDate(0, 0, 1)` sulla mezzanotte in `loc` e si + indicizzano per chiave formattata, **mai** per aritmetica sui secondi: con + l'ora legale un giorno non dura 24 ore. +- Le entry il cui giorno cade fuori da `[start, end)` si ignorano. +- `end <= start` → `nil`. + +Il riempimento a zero è il motivo per cui non si riusano i bucket: +`GroupByDay` **non crea un bucket per i giorni vuoti**, quindi una sparkline +costruita sui bucket comprimerebbe le pause e mostrerebbe un mese pieno per tre +giorni di lavoro. + +Il rendering è presentazione e sta separato, in `internal/tui/sparkline.go`: + +```go +// sparkline renders values as block glyphs, resampled to at most maxCells. +func sparkline(values []float64, maxCells int) string +``` + +- Zero → **spazio**. Una pausa deve leggersi come una pausa; `▁` direbbe «un + po' di lavoro». La sparkline è sempre seguita da un'etichetta, quindi nessuno + spazio finisce a fine riga. +- Non-zero → `▁▂▃▄▅▆▇█`, livello `clamp(ceil(v/max*8), 1, 8)`. +- Oltre `maxCells`: bucket contigui `values[i*n/k : (i+1)*n/k]` con `k = maxCells`, + media aritmetica di ciascuno. Aritmetica intera, resto distribuito in modo + deterministico. Serve davvero: un range custom di un anno sono 365 celle. +- `values` vuoto → stringa vuota. + +**Da dove arrivano i valori.** `reportModel` non ha le entry (§3.2), quindi la +serie le viene passata: `newReport(r report.Report, note string, daily []float64)`. +I 4 siti di produzione la calcolano da un helper sul Model, che ha già range, +entry visibili e `loc`: + +```go +func (m Model) dailySeries() []float64 +``` + +**Non** si aggiunge un campo `Daily` a `report.Report`: caricherebbe il tipo di +dominio di un campo che esiste solo per una vista. (L'output JSON del CLI non +c'entra: `export.JSON` serializza una sua `jsonReport`, non `report.Report`, +quindi un campo in più lì non lo toccherebbe comunque.) + +La sparkline compare sotto la riga di summary, **solo** se il range copre ≥ 2 +giorni e il report ha almeno un bucket, seguita dall'etichetta attenuata +` hours/day`. Le celle disponibili sono `maxCells = max(1, width - 12)` — dodici +è la lunghezza dell'etichetta più un margine — e **31 quando `width <= 0`**, che +è il mese più lungo. + +La demo la esercita senza modifiche a `demo.go`: le entry fixture cadono nei +giorni **2, 3, 5, 6, 7, 9 e 10** del range, quindi i buchi si vedono. Il giorno +2 è la sessione di sprint planning non fatturabile, e comparirà come barra +proprio perché la serie conta tutte le ore. + +### 6.4 #80 — il gauge + +`renderBudgetBar` cambia firma e glifi: + +```go +func renderBudgetBar(th theme, percentUsed float64) string +``` + +- Glifi `█` (pieno) e `░` (vuoto) al posto di `#` e `-`. +- Riempimento clampato a `[0, 100]`, **percentuale reale e non clampata** — + comportamento invariato rispetto a oggi, ed è il motivo per cui + `bubbles/progress` non va bene: clampa il numero a 100 e nasconde lo + sforamento. +- Pieno colorato `th.OK` fino al 100%, `th.Err` oltre. Vuoto `th.Help`. + +Il resto della schermata Budget non si tocca. + +## 7. Come si verifica + +Il criterio guida: **un golden verde non dimostra niente su ciò che `termenv.Ascii` +cancella** (§3.4). Ogni scelta invisibile ai golden ha un test diretto. + +### 7.1 #117 + +- I 31 `TestRates*` esistenti, **invariati**: sono la rete di sicurezza. +- Un test nuovo, `TestRatesSelectionIsPerSection`: scendi di una riga nelle + Liste, `tab` ai Membri, verifica che i Membri partano da 0, torna alle Liste e + verifica che la riga selezionata sia ancora quella. **Passa prima e dopo il + refactor**: caratterizza la proprietà per cui i quattro indici esistevano. +- I golden restano invariati (valore zero identico). + +### 7.2 Puri + +- `report.DailyHours`, table-driven: riempimento a zero, entry fuori range, un + giorno con più entry, attraversamento di un cambio d'ora legale, `end <= start`, + slice vuota. +- `sparkline`, table-driven: vuota, tutti zero, un solo valore, valori uguali, + ricampionamento quando `n > maxCells`, mappatura dei livelli agli estremi. Su + quest'ultimo punto la formula va letta prima di scrivere il caso: con + `clamp(ceil(v/max*8), 1, 8)` il minimo non-zero dà `▁` **solo se è ≤ max/8** — + per `{7, 8}` dà `▇`. Il caso di test va costruito rispettando il vincolo. + +### 7.3 Larghezza + +`TestReportTableWidth`, table-driven su 0/40/60/80/120: nessuna riga supera la +larghezza richiesta (quando questa sta sopra il pavimento), l'etichetta è la +prima a cedere, le colonne numeriche non si restringono mai. + +### 7.4 Ciò che i golden non vedono + +- `TestReportTableStyleFunc`: asserisce sullo `Style` **restituito** dalla + `StyleFunc`, non sull'output renderizzato — parità delle righe zebra, righe + totali con lo stile dei totali e non zebrate, header con lo stile dell'header. +- `TestBudgetBarOverBudget`: verifica che oltre il 100% lo stile del pieno sia + `th.Err` e non `th.OK`. Il *numero* (130%) lo vede già il golden; il colore no. + +### 7.4-bis Test esistenti che il cambio di firma rompe + +Due test di `budget_test.go` vanno riscritti, non solo ricompilati: +`TestRenderBudgetBarClampsFillNotPercent` (riga 38) chiama +`renderBudgetBar(150)` con un argomento solo e asserisce +`strings.Repeat("#", budgetBarWidth)`, e `TestBudgetViewRendersProgressBar` +(riga 24) asserisce che la barra sia fatta di `#` e `-`. Entrambi vanno portati +alla nuova firma e ai nuovi glifi. + +I due golden della palette (`palette_dark`, `palette_light`) cambiano perché +`paletteSample` in `theme_test.go` va esteso con i token nuovi — altrimenti +`Border` e `Zebra` non comparirebbero nell'unico posto della suite che cattura +le sequenze di colore reali. Va aggiornato anche il commento in cima a +`theme.go`, che oggi dice che un tema utente sovrascriverà «questi cinque +valori». + +### 7.5 Golden + +- `report` rigenerato, più `report_narrow` (60) e `report_wide` (120). +- Un golden multi-valuta, per fissare la riga `TOTAL` con la cella `Amount` + vuota e i sottototali. +- `budget` rigenerato con i glifi nuovi. + +## 8. Ordine di esecuzione + +1. **#117** — il refactor degli indici. Primo, come chiede la sua issue. +2. **Token del tema** — `Subtle` nella palette, `Border` e `Zebra` negli stili. +3. **La tabella** — `report_table.go`, cablata in `report.go` con la larghezza. +4. **`report.DailyHours`** — puro, con i suoi test. +5. **La sparkline** — `sparkline.go`, cablata nella view del report. +6. **Il gauge** — `renderBudgetBar` con tema e glifi a blocchi. +7. **Chiusura** — CHANGELOG, rigenerazione di `docs/demo.gif` (il report cambia + aspetto, quindi la GIF del README nasce stale), issue sulla larghezza della + schermata Budget, commenti di chiusura su #117/#66/#80. + +## 9. Vincoli globali + +- `internal/report` e `internal/duration` restano **puri**: nessun I/O, nessuna + dipendenza da `config` o `clickup`. `DailyHours` rispetta il vincolo. +- Tutto ciò che vive nel repo è in **inglese** — codice, commenti, messaggi di + test, stringhe UI, messaggi di commit. Questa spec è in italiano per + l'eccezione storica di `docs/superpowers/`. +- **Conventional Commits**, e **mai** un trailer `Co-Authored-By`. +- Prima di ogni commit: `gofmt -l .`, `go vet ./...`, + `go run honnef.co/go/tools/cmd/staticcheck@latest ./...`, `go build ./...`, + `go test ./... -race`, tutti puliti. +- **Parità in demo mode** per ogni funzionalità: `CLICKUP_DEMO=1 clup` deve + mostrare tabella, sparkline e gauge senza toccare la rete. +- Nessuna chiamata reale all'API ClickUp durante lo sviluppo. +- Nessuna nuova dipendenza in `go.mod`: `lipgloss/table` è un sottopacchetto di + una dipendenza già presente, e `bubbles/progress` non viene adottato. diff --git a/internal/report/daily.go b/internal/report/daily.go new file mode 100644 index 0000000..336d660 --- /dev/null +++ b/internal/report/daily.go @@ -0,0 +1,45 @@ +package report + +import "time" + +// DailyHours returns one element per day in [start, end): the total hours of +// the entries that STARTED that day, read in loc. It returns nil when the +// range is empty or inverted. +// +// It counts ALL hours, billable and not — the same total Bucket.Hours carries. +// The series answers "when did I work", not "when did I bill"; the billable +// share has its own line under the report table. +// +// The hours of an entry land entirely on its start day, which is the same rule +// groupKeys uses for GroupByDay — the two views cannot disagree about which +// day an overnight entry belongs to. +// +// This exists instead of reusing GroupByDay buckets because Build creates no +// bucket for a day with no entries. A series built from buckets would close +// the gaps, drawing a full month of work for three days of it. +// +// Days are advanced with AddDate and identified by their formatted key, never +// by adding 24 hours: across a daylight-saving transition a day is 23 or 25 +// hours long, and second-arithmetic would silently drop or duplicate one. +func DailyHours(entries []TimeEntry, start, end time.Time, loc *time.Location) []float64 { + loc = normLoc(loc) + day := midnightIn(start, loc) + last := midnightIn(end, loc) + if !day.Before(last) { + return nil + } + + index := map[string]int{} + var out []float64 + for d := day; d.Before(last); d = d.AddDate(0, 0, 1) { + index[d.Format(dayFormat)] = len(out) + out = append(out, 0) + } + + for _, e := range entries { + if i, ok := index[e.Start.In(loc).Format(dayFormat)]; ok { + out[i] += e.Duration.Hours() + } + } + return out +} diff --git a/internal/report/daily_test.go b/internal/report/daily_test.go new file mode 100644 index 0000000..9e0ddaa --- /dev/null +++ b/internal/report/daily_test.go @@ -0,0 +1,120 @@ +package report + +import ( + "testing" + "time" +) + +func TestDailyHours(t *testing.T) { + t.Parallel() + utc := time.UTC + day := func(y int, m time.Month, d int) time.Time { + return time.Date(y, m, d, 0, 0, 0, 0, utc) + } + entry := func(start time.Time, h float64) TimeEntry { + return TimeEntry{Start: start, Duration: time.Duration(h * float64(time.Hour))} + } + + for _, tc := range []struct { + name string + entries []TimeEntry + start, end time.Time + want []float64 + }{ + { + name: "empty range yields nil", + start: day(2026, time.July, 5), end: day(2026, time.July, 5), + want: nil, + }, + { + name: "end before start yields nil", + start: day(2026, time.July, 5), end: day(2026, time.July, 1), + want: nil, + }, + { + name: "no entries still yields one zero per day", + start: day(2026, time.July, 1), end: day(2026, time.July, 4), + want: []float64{0, 0, 0}, + }, + { + name: "idle days stay zero rather than disappearing", + entries: []TimeEntry{ + entry(day(2026, time.July, 1).Add(9*time.Hour), 2), + entry(day(2026, time.July, 3).Add(9*time.Hour), 1.5), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 4), + want: []float64{2, 0, 1.5}, + }, + { + name: "several entries on one day are summed", + entries: []TimeEntry{ + entry(day(2026, time.July, 2).Add(9*time.Hour), 2), + entry(day(2026, time.July, 2).Add(14*time.Hour), 3), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{0, 5}, + }, + { + name: "entries outside the range are ignored", + entries: []TimeEntry{ + entry(day(2026, time.June, 30).Add(9*time.Hour), 8), + entry(day(2026, time.July, 2).Add(9*time.Hour), 1), + entry(day(2026, time.July, 9).Add(9*time.Hour), 8), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{0, 1}, + }, + { + name: "an entry is credited to the day it starts on", + entries: []TimeEntry{ + // Starts at 23:00 on the 1st and runs three hours into the 2nd. + entry(day(2026, time.July, 1).Add(23*time.Hour), 3), + }, + start: day(2026, time.July, 1), end: day(2026, time.July, 3), + want: []float64{3, 0}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got := DailyHours(tc.entries, tc.start, tc.end, utc) + if len(got) != len(tc.want) { + t.Fatalf("got %d days %v, want %d days %v", len(got), got, len(tc.want), tc.want) + } + for i := range tc.want { + if got[i] != tc.want[i] { + t.Errorf("day %d = %v, want %v (full series %v)", i, got[i], tc.want[i], got) + } + } + }) + } +} + +// A day is not 24 hours across a DST transition. Rome falls back on +// 2026-10-25, making that day 25 hours long: a series built by adding 24h +// real-world hours would produce a duplicate calendar-day key and lose the +// third day entirely. Advancing by AddDate(0,0,1) correctly handles both +// 23-hour (spring) and 25-hour (autumn) transitions. +func TestDailyHoursSurvivesDST(t *testing.T) { + t.Parallel() + rome, err := time.LoadLocation("Europe/Rome") + if err != nil { + t.Skipf("tzdata unavailable: %v", err) + } + start := time.Date(2026, time.October, 24, 0, 0, 0, 0, rome) + end := time.Date(2026, time.October, 27, 0, 0, 0, 0, rome) + entries := []TimeEntry{ + {Start: time.Date(2026, time.October, 24, 10, 0, 0, 0, rome), Duration: time.Hour}, + {Start: time.Date(2026, time.October, 25, 10, 0, 0, 0, rome), Duration: 2 * time.Hour}, + {Start: time.Date(2026, time.October, 26, 10, 0, 0, 0, rome), Duration: 3 * time.Hour}, + } + got := DailyHours(entries, start, end, rome) + want := []float64{1, 2, 3} + if len(got) != len(want) { + t.Fatalf("got %d days %v, want 3", len(got), got) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("day %d = %v, want %v (full series %v)", i, got[i], want[i], got) + } + } +} diff --git a/internal/tui/app.go b/internal/tui/app.go index 9780d48..2d12895 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -686,7 +686,7 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.report = report.Build(m.visibleEntries(), groupBy, p, start, end, m.loc) m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote()) + m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) // Report is reached three ways (Home enter, Report's own reload, and // the logDone reload) and all three arrive here: re-rooting rather // than replacing makes every arrival converge on nav == [Home], which @@ -922,7 +922,7 @@ func (m Model) screenBody() string { case screenLoading: return m.theme.Title.Render("Loading hours…") case screenReport: - return m.rep.view(m.theme) + return m.rep.view(m.theme, m.width) case screenExport: return m.export.view(m.theme) case screenRates: diff --git a/internal/tui/budget.go b/internal/tui/budget.go index 0485e14..89ac3ed 100644 --- a/internal/tui/budget.go +++ b/internal/tui/budget.go @@ -6,6 +6,7 @@ import ( "github.com/charmbracelet/bubbles/key" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/marcoarnulfo/clickup-cli/internal/report" ) @@ -27,26 +28,40 @@ func (m Model) updateBudget(msg tea.KeyMsg) (tea.Model, tea.Cmd) { return m, nil } -// budgetBarWidth is the text progress bar's width in characters, not counting -// the surrounding brackets. +// budgetBarWidth is the gauge's width in characters. const budgetBarWidth = 20 -// renderBudgetBar renders a percent-used value as a fixed-width text bar, -// e.g. "[############--------] 60%". The bar's fill is clamped to [0, 100] -// (a list can run over budget, but the bar itself can't render past full); -// the percentage in the label is shown unclamped so an over-100% burn is -// still visible in the number. -func renderBudgetBar(percentUsed float64) string { - fillPct := percentUsed - if fillPct < 0 { - fillPct = 0 - } - if fillPct > 100 { - fillPct = 100 +// The gauge's glyphs. Blocks rather than '#' and '-': at any font size they +// read as a filled bar instead of as text. +const ( + gaugeFull = '█' + gaugeEmpty = '░' +) + +// budgetFillStyle is the color of the filled part of a gauge. Over budget is +// the state this screen exists to surface, so it must not look like a healthy +// one. It is a named function because the package goldens run under +// termenv.Ascii, which strips the color: asserting on the style is the only +// way to test the choice. +func budgetFillStyle(th theme, percentUsed float64) lipgloss.Style { + if percentUsed > 100 { + return th.Err } + return th.OK +} + +// renderBudgetBar renders a percent-used value as a fixed-width gauge, e.g. +// "████████████░░░░░░░░ 60%". The fill is clamped to [0, 100] — a list can run +// over budget, but the bar cannot render past full — while the percentage in +// the label is shown UNCLAMPED, so an over-100% burn stays visible in the +// number. That asymmetry is the whole point: bubbles/progress clamps the +// number too, which is why this screen does not use it. +func renderBudgetBar(th theme, percentUsed float64) string { + fillPct := min(max(percentUsed, 0), 100) filled := int(fillPct / 100 * budgetBarWidth) - bar := strings.Repeat("#", filled) + strings.Repeat("-", budgetBarWidth-filled) - return fmt.Sprintf("[%s] %.0f%%", bar, percentUsed) + bar := budgetFillStyle(th, percentUsed).Render(strings.Repeat(string(gaugeFull), filled)) + + th.Help.Render(strings.Repeat(string(gaugeEmpty), budgetBarWidth-filled)) + return fmt.Sprintf("%s %.0f%%", bar, percentUsed) } func (bm budgetModel) view(th theme) string { @@ -57,7 +72,7 @@ func (bm budgetModel) view(th theme) string { var rows strings.Builder for _, l := range bm.lines { rows.WriteString(fmt.Sprintf("%-24s %s %.2f / %.2f %s (remaining %.2f)\n", - truncate(l.ListName, 24), renderBudgetBar(l.PercentUsed), l.Billed, l.Budget, l.Currency, l.Remaining)) + truncate(l.ListName, 24), renderBudgetBar(th, l.PercentUsed), l.Billed, l.Budget, l.Currency, l.Remaining)) } body := th.Box.Render(strings.TrimRight(rows.String(), "\n")) return title + "\n\n" + body diff --git a/internal/tui/budget_test.go b/internal/tui/budget_test.go index 6df2d0c..f34cc77 100644 --- a/internal/tui/budget_test.go +++ b/internal/tui/budget_test.go @@ -21,8 +21,8 @@ func TestBudgetViewRendersProgressBar(t *testing.T) { t.Errorf("budget view missing %q; got:\n%s", want, out) } } - if !strings.Contains(out, "#") || !strings.Contains(out, "-") { - t.Error("expected a text progress bar built from '#' and '-'") + if !strings.ContainsRune(out, gaugeFull) || !strings.ContainsRune(out, gaugeEmpty) { + t.Errorf("expected a bar built from %q and %q; got:\n%s", gaugeFull, gaugeEmpty, out) } } @@ -33,19 +33,37 @@ func TestBudgetViewEmptyShowsMessage(t *testing.T) { } } -// A list that is over its budget must still be visible in the number even -// though the bar itself caps its fill at 100%. +// A list over its budget must still be visible in the number even though the +// bar itself caps its fill at 100%. This is exactly what bubbles/progress +// cannot do — it clamps the percentage to 100 before formatting it — and it is +// why this screen keeps its own bar. func TestRenderBudgetBarClampsFillNotPercent(t *testing.T) { - out := renderBudgetBar(150) + t.Parallel() + out := renderBudgetBar(testTheme(true), 150) if !strings.Contains(out, "150%") { t.Errorf("renderBudgetBar(150) = %q, want the unclamped 150%% in the label", out) } - full := strings.Repeat("#", budgetBarWidth) - if !strings.Contains(out, full) { + if full := strings.Repeat(string(gaugeFull), budgetBarWidth); !strings.Contains(out, full) { t.Errorf("renderBudgetBar(150) = %q, want a fully filled bar", out) } } +// Over budget is the state the view exists to surface, so it must not look the +// same as a healthy one. The goldens run under termenv.Ascii, which strips the +// color entirely, so this asserts on the style rather than the output. +func TestBudgetBarColorsOverBudget(t *testing.T) { + t.Parallel() + th := paletteTheme(true) // real colors, so foregrounds are comparable + under := budgetFillStyle(th, 60) + over := budgetFillStyle(th, 130) + if under.GetForeground() != th.OK.GetForeground() { + t.Error("a bar under budget is not drawn in the OK color") + } + if over.GetForeground() != th.Err.GetForeground() { + t.Error("a bar over budget is not drawn in the Err color") + } +} + // TestBudgetKeyLabels pins the exact label set budget.go's updateBudget // accepts today (every case label, verbatim), plus q — handled globally in // app.go, in no case clause of budget.go itself. A dropped label is caught by diff --git a/internal/tui/golden_test.go b/internal/tui/golden_test.go index 842ce76..42e679f 100644 --- a/internal/tui/golden_test.go +++ b/internal/tui/golden_test.go @@ -87,6 +87,23 @@ func goldenReport() report.Report { } } +// goldenDaily is a fixed per-day series with two idle days, so the golden +// pins both a bar and a gap. +func goldenDaily() []float64 { + return []float64{3.5, 0, 1.25, 4, 0, 2, 6} +} + +// goldenDailyPartialMonth is a 31-day series with 10 days of data followed by +// 21 zero-hours days — the shape of the default view (the current month, +// viewed partway through) that goldenDaily() never depicts, since its own +// 7-value series ends non-zero. This is the exact shape that let the +// "hours/day" label drift 21 columns from the chart before it was moved to a +// prefix (visible in the shipped docs/demo.gif). +func goldenDailyPartialMonth() []float64 { + days := []float64{3.5, 0, 1.25, 4, 0, 2, 6, 1, 5, 2.5} // 10 days worked + return append(days, make([]float64, 21)...) // the rest of July, not yet worked +} + // goldenEntries is the fixed entry set the browser, filters and rates screens // render from. func goldenEntries() []report.TimeEntry { @@ -117,7 +134,54 @@ func TestGoldenHomeWithNotices(t *testing.T) { func TestGoldenReport(t *testing.T) { t.Parallel() - golden(t, "report", newReport(goldenReport(), "").view(testTheme(true))) + // One bucket gets a label long enough that the three widths actually + // diverge. goldenReport's own labels are 8 columns, so without this all + // three goldens would be byte-identical and would pin nothing about width. + // At 60 the label is cut to 24 columns, at 80 to 44, and at 120 it fits + // whole — which is also the case that proves the column does not stretch + // past its content. + r := goldenReport() + r.Buckets[0].Label = "Website — landing page redesign and checkout hardening" + + for _, tc := range []struct { + name string + width int + }{{"report_narrow", 60}, {"report", 80}, {"report_wide", 120}} { + golden(t, tc.name, newReport(r, "", goldenDaily()).view(testTheme(true), tc.width)) + } +} + +// A multi-currency report renders a TOTAL row with an empty Amount cell plus +// one subtotal row per currency — the widest shape the total block takes. +func TestGoldenReportMultiCurrency(t *testing.T) { + t.Parallel() + r := goldenReport() + r.Buckets = append(r.Buckets, report.Bucket{ + Label: "Mobile", Key: "l3", Hours: 4, BillableHours: 4, BilledHours: 4, + Amounts: []report.CurrencyAmount{{Currency: "USD", Amount: 200}}, + }) + r.CurrencySubtotals = []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 15.5, BillableHours: 12.5, BilledHours: 12.5, Amount: 625}, + {Currency: "USD", Hours: 4, BillableHours: 4, BilledHours: 4, Amount: 200}, + } + // goldenReport's own BillableHours (12.5, from Website only) must grow by + // Mobile's 4 billable hours too, or the split line below the table + // contradicts the TOTAL row above it (a 19.5h total next to a 12.5h + // billable + 3h non-billable split, which only adds up to 15.5h). + // NonBillableHours is unchanged: Mobile is fully billable, so the 3h from + // Internal is still the whole non-billable side. + r.TotalHours, r.BillableHours, r.BilledHours = 19.5, 16.5, 16.5 + golden(t, "report_multicurrency", newReport(r, "", goldenDaily()).view(testTheme(true), 80)) +} + +// The default view: the current month, viewed partway through, so the daily +// series ends in a run of zero-hours days. goldenDaily() never depicted this +// shape (its own series ends non-zero) — it is exactly the shape that let the +// sparkline's "hours/day" label drift away from the chart before the label +// became a prefix. +func TestGoldenReportPartialMonth(t *testing.T) { + t.Parallel() + golden(t, "report_partial_month", newReport(goldenReport(), "", goldenDailyPartialMonth()).view(testTheme(true), 80)) } func TestGoldenExport(t *testing.T) { diff --git a/internal/tui/listbrowser.go b/internal/tui/listbrowser.go index d0857eb..07fcaba 100644 --- a/internal/tui/listbrowser.go +++ b/internal/tui/listbrowser.go @@ -149,7 +149,7 @@ func (m *Model) selectBrowsedList(id, name string) tea.Cmd { rt.rows = append(rt.rows, rateRow{listID: id, name: name}) found = len(rt.rows) - 1 } - rt.idx = found + rt.sel[secLists] = found rt.sec = secLists // the browsed list is a row of the Lists section m.ratesScreen = rt *m = m.pop() diff --git a/internal/tui/log_test.go b/internal/tui/log_test.go index 374c6d0..583ef9b 100644 --- a/internal/tui/log_test.go +++ b/internal/tui/log_test.go @@ -48,7 +48,7 @@ func newTestModelOnReport() Model { panic(err) // fixed test config, never expected to fail } m.report = report.Build(m.entries, report.GroupByTotal, p, start, end, nil) - m.rep = newReport(m.report, "") + m.rep = newReport(m.report, "", nil) return m } diff --git a/internal/tui/rates.go b/internal/tui/rates.go index 7c3c311..265f6c6 100644 --- a/internal/tui/rates.go +++ b/internal/tui/rates.go @@ -107,13 +107,15 @@ type overrideRow struct { type ratesModel struct { sec ratesSection + // sel is the selected row of each section, indexed by ratesSection. One + // array instead of four parallel fields means move() needs no switch at + // all, and a fifth section is a non-event. Its zero value is identical to + // four zeroed ints, so no golden changes. + sel [secCount]int + rows []rateRow // lists - idx int // selection in the Lists section members []memberRow - memIdx int overrides []overrideRow - ovIdx int - ruleIdx int editing bool edit editKind @@ -525,6 +527,10 @@ func (rt ratesModel) startDraft() ratesModel { return rt } +// selected returns the selected row of a section. It exists so tests can name +// a section's selection without depending on how the model stores it. +func (rt ratesModel) selected(sec ratesSection) int { return rt.sel[sec] } + // selCount is the number of selectable rows in the active section (the // Overrides section has one extra row: "new override"). func (rt ratesModel) selCount() int { @@ -542,29 +548,11 @@ func (rt ratesModel) selCount() int { // move shifts the selection of the active section by delta, clamped. func (rt ratesModel) move(delta int) ratesModel { - cur := rt.idx - switch rt.sec { - case secMembers: - cur = rt.memIdx - case secOverrides: - cur = rt.ovIdx - case secRules: - cur = rt.ruleIdx - } - next := cur + delta + next := rt.sel[rt.sec] + delta if next < 0 || next > rt.selCount()-1 { return rt } - switch rt.sec { - case secLists: - rt.idx = next - case secMembers: - rt.memIdx = next - case secOverrides: - rt.ovIdx = next - case secRules: - rt.ruleIdx = next - } + rt.sel[rt.sec] = next return rt } @@ -585,13 +573,13 @@ func (rt ratesModel) startEdit() ratesModel { rt.editing, rt.edit = true, editMemberRate rt.input = newNumberInput("member rate (Esc to cancel)") case secOverrides: - if rt.ovIdx >= len(rt.overrides) { + if rt.sel[secOverrides] >= len(rt.overrides) { return rt.startDraft() } rt.editing, rt.edit = true, editOverrideRate rt.input = newNumberInput("override rate (Esc to cancel)") case secRules: - switch rt.ruleIdx { + switch rt.sel[secRules] { case ruleDefaultCurrency: rt.editing, rt.edit = true, editDefaultCurrency rt.input = newTextInput("default currency (e.g. EUR)") @@ -641,11 +629,11 @@ func (rt ratesModel) commit(v string) ratesModel { rt.msg = badRate return rt } - rt.rates[rt.rows[rt.idx].listID] = f + rt.rates[rt.rows[rt.sel[secLists]].listID] = f return done() case editListCurrency: - id := rt.rows[rt.idx].listID + id := rt.rows[rt.sel[secLists]].listID if v == "" { delete(rt.currencies, id) return done() @@ -659,7 +647,7 @@ func (rt ratesModel) commit(v string) ratesModel { return done() case editListBudget: - id := rt.rows[rt.idx].listID + id := rt.rows[rt.sel[secLists]].listID if v == "" { delete(rt.budgets, id) return done() @@ -681,7 +669,7 @@ func (rt ratesModel) commit(v string) ratesModel { rt.msg = badRate return rt } - rt.memberRates[rt.members[rt.memIdx].id] = f + rt.memberRates[rt.members[rt.sel[secMembers]].id] = f return done() case editOverrideRate: @@ -692,10 +680,10 @@ func (rt ratesModel) commit(v string) ratesModel { } if rt.draft.active { rt.overrides = upsertOverride(rt.overrides, overrideRow{listID: rt.draft.listID, member: rt.draft.member, rate: f}) - rt.ovIdx = indexOfOverride(rt.overrides, rt.draft.listID, rt.draft.member) + rt.sel[secOverrides] = indexOfOverride(rt.overrides, rt.draft.listID, rt.draft.member) rt.draft = overrideDraft{} } else { - rt.overrides[rt.ovIdx].rate = f + rt.overrides[rt.sel[secOverrides]].rate = f } return done() @@ -765,20 +753,20 @@ func (rt ratesModel) clearSelected() ratesModel { switch rt.sec { case secLists: if len(rt.rows) > 0 { - delete(rt.rates, rt.rows[rt.idx].listID) // revert to the default rate + delete(rt.rates, rt.rows[rt.sel[secLists]].listID) // revert to the default rate } case secMembers: if len(rt.members) > 0 { - delete(rt.memberRates, rt.members[rt.memIdx].id) + delete(rt.memberRates, rt.members[rt.sel[secMembers]].id) } case secOverrides: - if rt.ovIdx < len(rt.overrides) { + if rt.sel[secOverrides] < len(rt.overrides) { // The selection stays valid: after the delete the freed index is // either another override or the trailing "new override" row. - rt.overrides = slices.Delete(rt.overrides, rt.ovIdx, rt.ovIdx+1) + rt.overrides = slices.Delete(rt.overrides, rt.sel[secOverrides], rt.sel[secOverrides]+1) } case secRules: - switch rt.ruleIdx { + switch rt.sel[secRules] { case ruleDefaultCurrency: rt.defCur = "" case ruleIncrement: @@ -866,7 +854,7 @@ func (m Model) saveRates(rt ratesModel) (tea.Model, tea.Cmd) { start, end := m.currentRange() m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote()) + m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) m = m.pop() return m, nil } diff --git a/internal/tui/rates_test.go b/internal/tui/rates_test.go index 7a3559d..a2582c8 100644 --- a/internal/tui/rates_test.go +++ b/internal/tui/rates_test.go @@ -704,6 +704,44 @@ func TestRatesDraftPickMemberKeyLabels(t *testing.T) { } } +// Each section keeps its own selected row: moving in one section must not +// disturb another, and returning to a section must restore where you were. +// This is the property the four parallel indices existed to provide, and the +// one a single array indexed by section has to preserve. +func TestRatesSelectionIsPerSection(t *testing.T) { + t.Parallel() + m := Model{screen: screenRates, ratesScreen: ratesModel{ + sec: secLists, + rows: []rateRow{{listID: "l1", name: "Website"}, {listID: "l2", name: "Internal"}}, + members: []memberRow{{id: 1, name: "alice"}, {id: 2, name: "bob"}}, + }} + + // Move to the second list, then switch to Members. + mm, _ := m.updateRates(keyMsg("down")) + mm, _ = mm.(Model).updateRates(keyMsg("tab")) + rt := mm.(Model).ratesScreen + if rt.sec != secMembers { + t.Fatalf("tab did not reach the Members section: sec=%v", rt.sec) + } + if got := rt.selected(secMembers); got != 0 { + t.Errorf("Members selection = %d, want 0: a fresh section starts at its first row", got) + } + + // Move inside Members, then come back to Lists. + mm, _ = mm.(Model).updateRates(keyMsg("down")) + mm, _ = mm.(Model).updateRates(keyMsg("shift+tab")) + rt = mm.(Model).ratesScreen + if rt.sec != secLists { + t.Fatalf("shift+tab did not return to the Lists section: sec=%v", rt.sec) + } + if got := rt.selected(secLists); got != 1 { + t.Errorf("Lists selection = %d, want 1: leaving and returning must not reset it", got) + } + if got := rt.selected(secMembers); got != 1 { + t.Errorf("Members selection = %d, want 1: it must survive leaving the section", got) + } +} + // TestRatesDraftRateStepKeyLabels pins the draft's third step: by the time // draft.step is draftRate, rt.editing is already true (rates.go's "enter" on // draftPickMember sets both), so this step's labels come from the SAME diff --git a/internal/tui/rates_view.go b/internal/tui/rates_view.go index bbd1c50..839d4f7 100644 --- a/internal/tui/rates_view.go +++ b/internal/tui/rates_view.go @@ -51,9 +51,9 @@ func (rt ratesModel) listsView(th theme) string { bud, hasBud := rt.budgets[r.listID] line := fmt.Sprintf("%-24s %10.2f %-5s %10s %s", truncate(r.name, 24), rate, rt.effectiveCurrency(r.listID), moneyOrDash(bud, hasBud), tag) - b += billingRow(th, i == rt.idx, line) + b += billingRow(th, i == rt.sel[secLists], line) } - sel := rt.rows[rt.idx] + sel := rt.rows[rt.sel[secLists]] note := fmt.Sprintf("Effective for %s: %.2f %s", truncate(sel.name, 24), rt.rateFor(sel.listID), rt.effectiveCurrency(sel.listID)) if n := rt.pairsForList(sel.listID); n > 0 { note += fmt.Sprintf(" · %d (list,member) override(s) take precedence here", n) @@ -74,9 +74,9 @@ func (rt ratesModel) membersView(th theme) string { rate, tag = v, "member rate" } line := fmt.Sprintf("%-30s %10.2f %s", truncate(fmt.Sprintf("%s (%d)", mr.name, mr.id), 30), rate, tag) - b += billingRow(th, i == rt.memIdx, line) + b += billingRow(th, i == rt.sel[secMembers], line) } - sel := rt.members[rt.memIdx] + sel := rt.members[rt.sel[secMembers]] note := "A member rate wins over any per-list rate, on every list." if n := rt.listsForMember(sel.id); n > 0 { note = fmt.Sprintf("%s is overridden on %d list(s) by a (list,member) rate.", truncate(sel.name, 24), n) @@ -92,9 +92,9 @@ func (rt ratesModel) overridesView(th theme) string { truncate(rt.listName(o.listID), 20), truncate(fmt.Sprintf("%s (%d)", rt.memberName(o.member), o.member), 22), o.rate, below, src) - b += billingRow(th, i == rt.ovIdx, line) + b += billingRow(th, i == rt.sel[secOverrides], line) } - b += billingRow(th, rt.ovIdx >= len(rt.overrides), "+ new (list,member) override") + b += billingRow(th, rt.sel[secOverrides] >= len(rt.overrides), "+ new (list,member) override") if len(rt.overrides) == 0 { b += "\n" + th.Help.Render("No (list,member) overrides — the most specific level of the precedence.") + "\n" } @@ -131,7 +131,7 @@ func (rt ratesModel) rulesView(th theme) string { } b := "" for i, f := range fields { - b += billingRow(th, i == rt.ruleIdx, fmt.Sprintf("%-22s %s", f[0], f[1])) + b += billingRow(th, i == rt.sel[secRules], fmt.Sprintf("%-22s %s", f[0], f[1])) } return b + "\n" + th.Help.Render("The default currency and rounding rule apply to every list without its own currency.") + "\n" } diff --git a/internal/tui/report.go b/internal/tui/report.go index fb9fa72..08139ec 100644 --- a/internal/tui/report.go +++ b/internal/tui/report.go @@ -7,6 +7,7 @@ import ( "github.com/charmbracelet/bubbles/key" tea "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" "github.com/marcoarnulfo/clickup-cli/internal/duration" "github.com/marcoarnulfo/clickup-cli/internal/export" "github.com/marcoarnulfo/clickup-cli/internal/report" @@ -14,11 +15,14 @@ import ( ) type reportModel struct { - r report.Report - note string + r report.Report + note string + daily []float64 // one entry per day of the range; see report.DailyHours } -func newReport(r report.Report, note string) reportModel { return reportModel{r: r, note: note} } +func newReport(r report.Report, note string, daily []float64) reportModel { + return reportModel{r: r, note: note, daily: daily} +} // nextGroupBy cycles total -> task -> list -> day -> tag -> [member] -> total. // The member grouping is only offered for the team scope. @@ -56,6 +60,14 @@ func (m Model) memberFilterNote() string { return fmt.Sprintf(" (%d/%d members)", k, n) } +// dailySeries is the per-day hours of the visible entries over the current +// range. It lives on the Model because reportModel has no entries — the report +// is a rendering of an already-aggregated value. +func (m Model) dailySeries() []float64 { + start, end := m.currentRange() + return report.DailyHours(m.visibleEntries(), start, end, m.loc) +} + func (m Model) updateReport(msg tea.KeyMsg) (tea.Model, tea.Cmd) { k := keysFor(m) switch { @@ -70,7 +82,7 @@ func (m Model) updateReport(msg tea.KeyMsg) (tea.Model, tea.Cmd) { if p, ok := m.pricingOrErr(); ok { m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote()) + m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) } case key.Matches(msg, k.ChangeRange): m = m.pop() @@ -192,30 +204,17 @@ func (m *Model) applyReport() bool { start, end := m.currentRange() m.report = report.Build(m.visibleEntries(), g, p, start, end, m.loc) m.report.Scope = m.scope - m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote()) + m.rep = newReport(m.report, m.memberFilterNote()+m.filteredNote(), m.dailySeries()) return true } -// formatAmounts renders a bucket's per-currency amounts on one line, e.g. -// "150.00 EUR + 90.00 USD". An empty slice renders as a zero in fallback. -func formatAmounts(amounts []report.CurrencyAmount, fallback string) string { - if len(amounts) == 0 { - return fmt.Sprintf("%.2f %s", 0.0, fallback) - } - parts := make([]string, 0, len(amounts)) - for _, a := range amounts { - parts = append(parts, fmt.Sprintf("%.2f %s", a.Amount, a.Currency)) - } - return strings.Join(parts, " + ") -} - // hoursOf renders hours the same way export.SummaryLine does, via // duration.FormatHours — one hours formatter shared across TUI and export. func hoursOf(h float64) string { return duration.FormatHours(time.Duration(h * float64(time.Hour))) } -func (rm reportModel) view(th theme) string { +func (rm reportModel) view(th theme, width int) string { r := rm.r // Timezone is surfaced here (#83): with no configured `timezone` it reads // "Local" (time.Local.String()), not a portable IANA name — accepted, see @@ -225,42 +224,76 @@ func (rm reportModel) view(th theme) string { report.PeriodLabel(r.Start, r.End), r.Scope, rm.note, r.GroupBy, r.Timezone)) summary := th.Accent.Render(export.SummaryLine(r)) - header := th.Header.Render( - fmt.Sprintf("%-32s %8s %8s %s", "Item", "Hours", "Billed", "Amount")) - rows := header + "\n" - for _, b := range r.Buckets { - rows += fmt.Sprintf("%-32s %8.2f %8.2f %s\n", - truncate(b.Label, 32), b.Hours, b.BilledHours, formatAmounts(b.Amounts, r.DefaultCurrency)) - } - - // Totals: one line when the report is single-currency, otherwise a TOTAL - // hours line plus one authoritative subtotal line per currency (no FX). - // Per-bucket Amounts are indicative only (PerDay rounding can drift a few - // cents from these subtotals with a finer grouping) — CurrencySubtotals - // below is the authoritative total, never re-derived from the bucket rows. - var total string - if len(r.CurrencySubtotals) <= 1 { - total = th.OK.Render(fmt.Sprintf("%-32s %8.2f %8.2f %.2f %s", - "TOTAL", r.TotalHours, r.BilledHours, r.TotalAmount, r.DefaultCurrency)) + var body string + if len(r.Buckets) == 0 { + body = th.Box.Render("No hours to show.") } else { - total = th.OK.Render(fmt.Sprintf("%-32s %8.2f %8.2f", "TOTAL", r.TotalHours, r.BilledHours)) - for _, s := range r.CurrencySubtotals { - total += "\n" + th.OK.Render(fmt.Sprintf("%-32s %8.2f %8.2f %.2f %s", - " subtotal "+s.Currency, s.Hours, s.BilledHours, s.Amount, s.Currency)) - } + body = reportTable(th, r, width) + } + // The billable split is a note, not a row of data: inside the table it + // would take the zebra stripe and be split across columns. + split := th.Help.Render(fmt.Sprintf(" billable %s · non-billable %s", + hoursOf(r.BillableHours), hoursOf(r.NonBillableHours))) + out := title + "\n\n" + summary + "\n\n" + if line := rm.sparkView(th, width); line != "" { + out += line + "\n\n" } - total += "\n" + th.Help.Render(fmt.Sprintf(" billable %s · non-billable %s", hoursOf(r.BillableHours), hoursOf(r.NonBillableHours))) + return out + body + "\n" + split +} - body := th.Box.Render(rows + total) - if len(r.Buckets) == 0 { - body = th.Box.Render("No hours to show.") +// sparkLabel prefixes the sparkline. It leads rather than trails so the chart +// always reads left-to-right from a fixed origin: DailyHours zero-fills the +// whole range and a zero renders as a space (see sparkline.go), so a label +// appended after the chart drifts as many columns as there are idle days at +// the end of the range — visibly so for the current month, viewed partway +// through, which is most of the trailing days. Its width plus a margin is +// what sparkView reserves for the chart. +const sparkLabel = "hours/day " + +// sparkView renders the per-day sparkline, or "" when there is nothing worth +// drawing: a range of one day is a single cell, and a report with no buckets +// already says so in its body. +func (rm reportModel) sparkView(th theme, width int) string { + if len(rm.daily) < 2 || len(rm.r.Buckets) == 0 { + return "" + } + cells := 31 // the longest month, used until the terminal reports its width + if width > 0 { + cells = max(1, width-lipgloss.Width(sparkLabel)-2) } - return title + "\n\n" + summary + "\n\n" + body + // Trailing idle days render as trailing spaces (see sparkline.go); with + // the label leading rather than trailing, those spaces would otherwise + // dangle at the end of the line for no reader to see. The trim runs on + // the label and chart ASSEMBLED, not on the chart alone: when every day + // in the range is idle, sparkline itself is all spaces, and trimming only + // the chart would leave the label's OWN trailing separator space dangling + // instead ("hours/day" followed by one space and then nothing — the exact + // defect this trim exists to prevent, just moved one segment to the + // left). A chart with no ink still renders (see the doc comment above: + // only "no buckets" and "fewer than two days" skip the line entirely) — + // zero hours on every day of the range is itself the information, the + // same way one idle day among several already renders as a blank cell + // rather than vanishing. + raw := strings.TrimRight(sparkLabel+sparkline(rm.daily, cells), " ") + cut := min(len(sparkLabel), len(raw)) + label, chart := raw[:cut], raw[cut:] + return th.Help.Render(label) + th.Accent.Render(chart) } // truncate shortens to n runes (not bytes), to avoid breaking UTF-8 characters -// in task names with accents or emoji. +// in task names with accents or emoji. An empty s always yields "", whatever +// n is: there is nothing to truncate, so there is nothing to mark as cut. +// Otherwise n <= 1 always yields "…": r[:n-1] would otherwise index +// negatively (a panic) for n <= 0, and the whole package now relies on that +// guard rather than on each call site keeping n >= some floor itself (see +// report_table.go's shaveToWidth for why that matters). func truncate(s string, n int) string { + if s == "" { + return "" + } + if n <= 1 { + return "…" + } r := []rune(s) if len(r) <= n { return s diff --git a/internal/tui/report_table.go b/internal/tui/report_table.go new file mode 100644 index 0000000..0803747 --- /dev/null +++ b/internal/tui/report_table.go @@ -0,0 +1,259 @@ +package tui + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" + "github.com/marcoarnulfo/clickup-cli/internal/report" +) + +// The report grid's fixed dimensions. Slack is taken from and given to the +// Item column first: the numeric columns are the reason to have a grid, so +// Item gives up its space before they do. Amount is the one exception, and +// only as a last resort — see reportAmountWidth for why that is safe. +const ( + reportNumWidth = 8 // reserved for each of Hours and Billed + reportMinItemWidth = 12 // below this the labels stop being labels + reportNatItemWidth = 32 // the fixed width the report had before #66 + // reportTableChrome is the border and padding the table spends on itself: + // two outer borders (column separators are off) plus two padding columns + // on each of the four columns. + reportTableChrome = 2 + 4*2 +) + +var reportHeaders = []string{"Item", "Hours", "Billed", "Amount"} + +// reportRows renders the report as table cells and reports the index at which +// the total rows begin. Bucket rows come first, then either a single TOTAL row +// (one currency) or a TOTAL row with an empty Amount followed by one subtotal +// row per currency. +// +// Per-bucket Amounts are indicative: PerDay rounding can drift a few cents +// from the subtotals at a fine grouping. CurrencySubtotals is authoritative +// and is never re-derived from the bucket rows. +func reportRows(r report.Report) ([][]string, int) { + rows := make([][]string, 0, len(r.Buckets)+len(r.CurrencySubtotals)+1) + for _, b := range r.Buckets { + rows = append(rows, []string{ + b.Label, + fmt.Sprintf("%.2f", b.Hours), + fmt.Sprintf("%.2f", b.BilledHours), + formatAmounts(b.Amounts, r.DefaultCurrency), + }) + } + firstTotal := len(rows) + + if len(r.CurrencySubtotals) <= 1 { + rows = append(rows, []string{ + "TOTAL", + fmt.Sprintf("%.2f", r.TotalHours), + fmt.Sprintf("%.2f", r.BilledHours), + fmt.Sprintf("%.2f %s", r.TotalAmount, r.DefaultCurrency), + }) + return rows, firstTotal + } + + rows = append(rows, []string{ + "TOTAL", + fmt.Sprintf("%.2f", r.TotalHours), + fmt.Sprintf("%.2f", r.BilledHours), + "", + }) + for _, s := range r.CurrencySubtotals { + rows = append(rows, []string{ + " subtotal " + s.Currency, + fmt.Sprintf("%.2f", s.Hours), + fmt.Sprintf("%.2f", s.BilledHours), + fmt.Sprintf("%.2f %s", s.Amount, s.Currency), + }) + } + return rows, firstTotal +} + +// formatAmounts renders a bucket's per-currency amounts on one line, e.g. +// "150.00 EUR + 90.00 USD". An empty slice renders as a zero in fallback. +func formatAmounts(amounts []report.CurrencyAmount, fallback string) string { + if len(amounts) == 0 { + return fmt.Sprintf("%.2f %s", 0.0, fallback) + } + parts := make([]string, 0, len(amounts)) + for _, a := range amounts { + parts = append(parts, fmt.Sprintf("%.2f %s", a.Amount, a.Currency)) + } + return strings.Join(parts, " + ") +} + +// reportItemWidth splits width between the fixed numeric columns and the Item +// column. It never stretches Item past the longest label (empty space is not a +// feature) and never shrinks it below reportMinItemWidth — unless the labels +// themselves are shorter than that floor. +// +// Reserving reportNumWidth for Hours and Billed is a worst case: real values +// are shorter, so the table often renders narrower than the terminal. Narrower +// is fine; wider is the bug this arithmetic exists to prevent — but this +// function alone cannot promise it: it assumes Amount keeps its natural, +// untruncated width, and a multi-currency bucket's Amount can blow well past +// whatever is left once Item is already at its floor. reportAmountWidth is the +// other half of that promise: it is what actually claws space back from +// Amount, as a last resort, once Item has none left to give. +func reportItemWidth(rows [][]string, width int) int { + maxLabel := lipgloss.Width(reportHeaders[0]) + amount := lipgloss.Width(reportHeaders[3]) + for _, row := range rows { + maxLabel = max(maxLabel, lipgloss.Width(row[0])) + amount = max(amount, lipgloss.Width(row[3])) + } + if width <= 0 { + return min(maxLabel, reportNatItemWidth) + } + floor := min(reportMinItemWidth, maxLabel) + item := width - reportTableChrome - 2*reportNumWidth - amount + return max(floor, min(maxLabel, item)) +} + +// reportAmountWidth is reportItemWidth's counterpart for the Amount column. +// Item yields first — it is the point of having a grid, so it gives up all +// the slack reportItemWidth is willing to give before Amount loses a single +// column. Only once Item is already at its floor and the row would still +// overflow does this claw space back from Amount, as a last resort. +// +// That resort mostly targets a per-bucket Amount, which is indicative only: +// reportRows' own doc comment already says so — PerDay rounding can drift a +// few cents from the subtotals at a fine grouping, and CurrencySubtotals is +// authoritative. Those authoritative per-currency figures do render as their +// own rows at the bottom of the same table, at full precision — but amountW +// is one column width shared by every row, subtotals included, so a subtotal +// cell truncates exactly like a bucket cell once its natural width no longer +// fits. That only happens below about 52 columns (a 3-currency subtotal like +// "1234567.00 EUR" needs 14 of its own); at 60 columns and above — the width +// this file's own tests treat as the table's design range +// (TestReportTableNeverExceedsWidth starts there) — every subtotal fits +// whole. Below roughly 48 columns the Item column has already hit its +// reportMinItemWidth floor and the whole table is in the degraded regime that +// floor exists to accept; a truncated subtotal in that band is one more +// symptom of the same accepted trade-off, not a new one. +// +// width <= 0 is the first render, before the terminal has sent its +// WindowSizeMsg: nothing is sized against it yet, so Amount stays at its +// natural width, same as Item. Otherwise this never stretches Amount past +// what the content needs, only ever shrinks it — down to 1, never lower: +// truncate's own guard (n <= 1) is what makes that safe, not a floor here. +func reportAmountWidth(rows [][]string, width, itemW int) int { + natural := lipgloss.Width(reportHeaders[3]) + for _, row := range rows { + natural = max(natural, lipgloss.Width(row[3])) + } + if width <= 0 { + return natural + } + budget := width - reportTableChrome - 2*reportNumWidth - itemW + return max(1, min(natural, budget)) +} + +// reportStyleFunc decides every per-cell style: alignment, the header, the +// zebra stripe and the totals. +// +// It is a separate function because a golden cannot see what it does. TestMain +// pins termenv.Ascii, which strips backgrounds and bold, so a broken stripe or +// an uncolored TOTAL leaves every golden byte-identical. The test asserts on +// the Style this returns. +// +// It never calls lipgloss.NewStyle(): that would build on the DEFAULT +// renderer, so plain rows and themed rows would render through different +// renderers. th.Cell is the plain style on the right one. +func reportStyleFunc(th theme, firstTotal int) table.StyleFunc { + return func(row, col int) lipgloss.Style { + st := th.Cell + switch { + case row == table.HeaderRow: + st = th.Header + case row >= firstTotal: + st = th.OK + if row == firstTotal { + st = st.Bold(true) // the TOTAL line, not the per-currency subtotals + } + case row%2 == 1: + st = th.Zebra + } + st = st.Padding(0, 1) + if col > 0 { + st = st.Align(lipgloss.Right) + } + return st + } +} + +// reportTable renders the buckets and the totals as one table sized to width. +// width <= 0 means natural width: the first render, before the terminal has +// sent its WindowSizeMsg. +// +// Three lipgloss/table defaults are wrong for this grid and are all turned +// off: BorderColumn draws separators between columns, Wrap sends a long label +// onto a second line instead of letting truncate cut it, and the zero +// BorderStyle renders the frame through the default renderer. +// +// Sizing is implicit and Table.Width is deliberately NOT called. With +// BorderColumn off, the library's resizer counts only the separators between +// columns, so it believes the frame costs nothing, while String clips the +// result with MaxWidth — the table would render two columns too wide and then +// have its right border sliced off. Pre-formatted numeric cells, labels +// truncated to itemW and Padding(0, 1) from the style function give the exact +// widths this arithmetic assumes. +func reportTable(th theme, r report.Report, width int) string { + rows, firstTotal := reportRows(r) + itemW := reportItemWidth(rows, width) + amountW := reportAmountWidth(rows, width, itemW) + for i := range rows { + rows[i][0] = shaveToWidth(rows[i][0], itemW) + rows[i][3] = shaveToWidth(rows[i][3], amountW) + } + return table.New(). + Border(lipgloss.RoundedBorder()). + BorderStyle(th.Border). + BorderColumn(false). + BorderHeader(true). + Wrap(false). + Headers(reportHeaders...). + Rows(rows...). + StyleFunc(reportStyleFunc(th, firstTotal)). + String() +} + +// shaveToWidth truncates s until it renders at no more than w display +// columns. truncate cuts by RUNE count (it never breaks a multi-byte UTF-8 +// character), but the widths this file computes (reportItemWidth, +// reportAmountWidth) and lipgloss/table's own resizer all measure in DISPLAY +// columns (lipgloss.Width). The two agree for ASCII but not for double-width +// runes (CJK, emoji) or a multi-currency Amount like "12345.00 EUR + 6789.00 +// USD + 4321.00 GBP": a value truncated to w runes can still render at up to +// 2w columns, which is exactly the "table wider than the terminal" bug this +// whole file exists to prevent. Bucket labels are ClickUp list/task names and +// a wide workspace can carry any number of currencies, so both are reachable, +// not theoretical. Shave one more rune at a time until the rendered width +// actually fits. +// +// Safe for w >= 1, without the caller keeping its own floor: truncate itself +// guards n <= 1. reportItemWidth's own floor (reportMinItemWidth, itself +// bounded below by "Item"'s own width) keeps its w at 4 or above, so the loop +// never gets near that guard for the Item column; reportAmountWidth has no +// equivalent floor and can legitimately hand back 1 — exactly the case the +// guard exists for. +// +// w <= 0 is NOT safe and the caller must not pass it: truncate(s, 0) (like +// truncate(s, 1)) returns "…", which is 1 column wide, so "> w" stays true +// forever and the loop never returns. Both of today's callers keep w at 1 or +// above (see above), so this is unreached, not merely untested — the guard +// below exists so a future caller that breaks that invariant gets an empty +// string back instead of a hang with no diagnosis. +func shaveToWidth(s string, w int) string { + if w <= 0 { + return "" + } + s = truncate(s, w) + for lipgloss.Width(s) > w { + s = truncate(s, len([]rune(s))-1) + } + return s +} diff --git a/internal/tui/report_table_test.go b/internal/tui/report_table_test.go new file mode 100644 index 0000000..7056b7f --- /dev/null +++ b/internal/tui/report_table_test.go @@ -0,0 +1,222 @@ +package tui + +import ( + "strings" + "testing" + "time" + + "github.com/charmbracelet/lipgloss" + "github.com/charmbracelet/lipgloss/table" + "github.com/marcoarnulfo/clickup-cli/internal/report" +) + +// tableFixture is a two-bucket, single-currency report. +func tableFixture() report.Report { + start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + return report.Report{ + Start: start, End: start.AddDate(0, 1, 0), + Scope: "me", GroupBy: report.GroupByList, DefaultCurrency: "EUR", Timezone: "UTC", + Buckets: []report.Bucket{ + {Label: "Website", Key: "l1", Hours: 12.5, BillableHours: 12.5, BilledHours: 12.5, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 625}}}, + {Label: "Internal", Key: "l2", Hours: 3, BillableHours: 0, BilledHours: 0, + Amounts: []report.CurrencyAmount{{Currency: "EUR", Amount: 0}}}, + }, + CurrencySubtotals: []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 15.5, BillableHours: 12.5, BilledHours: 12.5, Amount: 625}, + }, + TotalHours: 15.5, BillableHours: 12.5, NonBillableHours: 3, BilledHours: 12.5, TotalAmount: 625, + } +} + +// The table must never be wider than the terminal. help's arithmetic taught us +// that "approximately fits" is not a property — this asserts the real thing. +// +// The widths start at 60 because below roughly 48 the Item column hits its +// 12-column floor and the table deliberately overflows instead of shrinking +// into illegibility; TestReportTableFloorsRatherThanVanishing covers that end. +// +// The second label is all double-width runes (🚀). truncate cuts by rune +// count, but reportItemWidth's budget and lipgloss/table's own resizer both +// measure in display columns — without reportTable's second truncation pass +// this label alone renders at up to 2x its rune-counted width and blows the +// terminal budget. An all-ASCII label can never exercise that mismatch. +// +// The threeCurrencies/fiveCurrencies buckets exercise the equivalent mismatch +// on the Amount column: formatAmounts joins every currency onto one line +// ("12345.00 EUR + 6789.00 USD + ..."), which a GroupByTotal report — the +// default first-load grouping — produces from its single bucket for every +// currency in the workspace. Before Amount was allowed to truncate too, 3 +// currencies overflowed at width 60 and 5 currencies overflowed even at 100. +func TestReportTableNeverExceedsWidth(t *testing.T) { + t.Parallel() + threeCurrencies := []report.CurrencyAmount{ + {Currency: "EUR", Amount: 12345}, + {Currency: "USD", Amount: 6789}, + {Currency: "GBP", Amount: 4321}, + } + fiveCurrencies := []report.CurrencyAmount{ + {Currency: "EUR", Amount: 12345}, + {Currency: "USD", Amount: 6789}, + {Currency: "GBP", Amount: 4321}, + {Currency: "CHF", Amount: 999}, + {Currency: "JPY", Amount: 50000}, + } + for _, label := range []string{ + "A very long list name that will not fit anywhere near a narrow terminal", + strings.Repeat("🚀", 40), + } { + r := tableFixture() + r.Buckets[0].Label = label + r.Buckets = append(r.Buckets, + report.Bucket{Label: "Consulting", Key: "l3", Hours: 5, BilledHours: 5, Amounts: threeCurrencies}, + report.Bucket{Label: "Global Ops", Key: "l4", Hours: 2, BilledHours: 2, Amounts: fiveCurrencies}, + ) + for _, width := range []int{60, 80, 100, 120} { + out := reportTable(testTheme(true), r, width) + for _, line := range strings.Split(strings.TrimRight(out, "\n"), "\n") { + if w := lipgloss.Width(line); w > width { + t.Errorf("label %q, width %d: line is %d columns:\n%s", label, width, w, line) + } + } + } + } +} + +// At width 30 the numeric-column arithmetic alone goes negative, and +// reportItemWidth clamps to what the labels actually need instead of +// collapsing to nothing. tableFixture's longest label ("Internal", 8 columns) +// is itself under reportMinItemWidth, so this exercises the "cap wins over +// the floor" branch — the table overflows width 30 rather than truncating a +// short label down to illegibility. TestReportItemWidthGivesSlackToTheLabel +// covers the floor itself, with a label long enough for it to bind. +func TestReportTableFloorsRatherThanVanishing(t *testing.T) { + t.Parallel() + out := reportTable(testTheme(true), tableFixture(), 30) + if !strings.Contains(out, "Website") { + t.Errorf("the label column collapsed past usefulness at 30 columns:\n%s", out) + } +} + +// The Item column is the one that gives up its space first: the numeric +// columns are the point of having a grid, so Item shrinks before they do. +// Amount only gives up space once Item is already at its floor and the row +// would still overflow (TestReportTableNeverExceedsWidth's multi-currency +// cases), which this fixture's short, single-currency Amounts never trigger. +func TestReportItemWidthGivesSlackToTheLabel(t *testing.T) { + t.Parallel() + rows, _ := reportRows(tableFixture()) + rows[0][0] = strings.Repeat("x", 200) // a label longer than any terminal + + narrow := reportItemWidth(rows, 60) + wide := reportItemWidth(rows, 120) + if narrow >= wide { + t.Errorf("item width did not grow with the terminal: 60 -> %d, 120 -> %d", narrow, wide) + } + if narrow < 12 { + t.Errorf("item width %d fell below the 12-column floor", narrow) + } + // At width 30 the numeric-column budget alone is negative (see + // TestReportTableFloorsRatherThanVanishing), so with a label this long the + // result must be exactly reportMinItemWidth, not some other value that + // merely happens to sit in [8, 24] — this is the case where the floor + // itself, not the label length, decides the answer. + if got := reportItemWidth(rows, 30); got != reportMinItemWidth { + t.Errorf("reportItemWidth(_, 30) = %d, want the %d-column floor", got, reportMinItemWidth) + } +} + +// width <= 0 is the first render, before the terminal has sent WindowSizeMsg. +// It must reproduce the fixed 32-column layout the report had before #66. +func TestReportItemWidthZeroIsNaturalWidth(t *testing.T) { + t.Parallel() + rows, _ := reportRows(tableFixture()) + rows[0][0] = strings.Repeat("x", 200) + if got := reportItemWidth(rows, 0); got != 32 { + t.Errorf("natural item width = %d, want 32", got) + } +} + +// A long label must render as one row, never spread across several lines. +// This does not actually exercise Wrap(false): reportTable never calls +// Table.Width, so every column is sized to exactly its own content's width +// (see resizing.go) and lipgloss/table has no spare width to wrap a cell +// into regardless of the Wrap setting — verified empirically, the rendered +// output is byte-identical with and without Wrap(false). Wrap(false) is kept +// as insurance against a future change that starts calling Width(); what this +// test actually pins is the real invariant: one bucket is one row. +func TestReportTableTruncatesRatherThanWraps(t *testing.T) { + t.Parallel() + r := tableFixture() + r.Buckets[0].Label = strings.Repeat("long ", 40) + out := reportTable(testTheme(true), r, 60) + // top border, header, header separator, 2 buckets, TOTAL, bottom border. + if n := len(strings.Split(strings.TrimRight(out, "\n"), "\n")); n != 7 { + t.Errorf("table has %d lines, want 7 — a cell wrapped instead of truncating:\n%s", n, out) + } +} + +// The multi-currency total block: a TOTAL row with an empty Amount cell, +// followed by one subtotal row per currency. +func TestReportRowsMultiCurrencyTotals(t *testing.T) { + t.Parallel() + r := tableFixture() + r.CurrencySubtotals = []report.CurrencySubtotal{ + {Currency: "EUR", Hours: 12.5, BilledHours: 12.5, Amount: 625}, + {Currency: "USD", Hours: 3, BilledHours: 3, Amount: 150}, + } + rows, firstTotal := reportRows(r) + if firstTotal != 2 { + t.Fatalf("firstTotal = %d, want 2 (the two buckets come first)", firstTotal) + } + if len(rows) != 5 { + t.Fatalf("got %d rows, want 5 (2 buckets + TOTAL + 2 subtotals)", len(rows)) + } + if rows[2][0] != "TOTAL" || rows[2][3] != "" { + t.Errorf("TOTAL row = %q, want an empty Amount cell when the report is multi-currency", rows[2]) + } + if !strings.Contains(rows[3][0], "subtotal EUR") || !strings.Contains(rows[4][0], "subtotal USD") { + t.Errorf("subtotal rows = %q, %q", rows[3][0], rows[4][0]) + } +} + +// TestReportStyleFunc is the ONLY check on the grid's colors. The package +// goldens run under termenv.Ascii, which strips backgrounds and bold, so a +// broken zebra or an uncolored TOTAL would leave every golden byte-identical. +// It therefore asserts on the Style the function returns, not on rendered text. +func TestReportStyleFunc(t *testing.T) { + t.Parallel() + th := paletteTheme(true) // real colors, so backgrounds are comparable + const firstTotal = 2 + f := reportStyleFunc(th, firstTotal) + + if got := f(table.HeaderRow, 0); !got.GetBold() { + t.Error("header row is not bold") + } + if f(0, 0).GetBackground() == th.Zebra.GetBackground() { + t.Error("row 0 is striped; zebra must start on the second row") + } + if f(1, 0).GetBackground() != th.Zebra.GetBackground() { + t.Error("row 1 is not striped") + } + if f(firstTotal, 0).GetBackground() == th.Zebra.GetBackground() { + t.Error("the TOTAL row is striped; totals are not part of the zebra sequence") + } + if f(firstTotal, 0).GetForeground() != th.OK.GetForeground() { + t.Error("the TOTAL row does not carry the OK color") + } + if !f(firstTotal, 0).GetBold() { + t.Error("the TOTAL row is not bold") + } + if f(firstTotal+1, 0).GetBold() { + t.Error("a per-currency subtotal row is bold; only the TOTAL row should be") + } + if f(0, 0).GetAlign() != lipgloss.Left { + t.Error("the Item column is not left-aligned") + } + for col := 1; col <= 3; col++ { + if f(0, col).GetAlign() != lipgloss.Right { + t.Errorf("column %d is not right-aligned", col) + } + } +} diff --git a/internal/tui/report_test.go b/internal/tui/report_test.go index 1aa9167..ce086b0 100644 --- a/internal/tui/report_test.go +++ b/internal/tui/report_test.go @@ -54,7 +54,7 @@ func TestNextGroupByMeSkipsMember(t *testing.T) { func TestReportCycleGroupByTeamViaUpdate(t *testing.T) { m := Model{scope: "team", screen: screenReport, now: time.Now} m.report = report.Report{GroupBy: report.GroupByDay} - m.rep = newReport(m.report, "") + m.rep = newReport(m.report, "", nil) u, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("g")}) m = u.(Model) if m.report.GroupBy != report.GroupByTag { @@ -70,7 +70,7 @@ func TestReportCycleGroupByWithBadRoundingRoutesToErrorScreen(t *testing.T) { cfg.Billing.Rounding.Increment = "not-a-duration" m := Model{cfg: cfg, scope: "me", screen: screenReport, now: time.Now} m.report = report.Report{GroupBy: report.GroupByTotal} - m.rep = newReport(m.report, "") + m.rep = newReport(m.report, "", nil) u, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("g")}) mm := u.(Model) if mm.screen != screenError { @@ -101,7 +101,7 @@ func TestReportViewRendersPerCurrencyAmounts(t *testing.T) { }, TotalHours: 4, BillableHours: 3, NonBillableHours: 1, BilledHours: 3, } - out := newReport(r, "").view(testTheme(true)) + out := newReport(r, "", nil).view(testTheme(true), 80) for _, want := range []string{"200.00 EUR", "100.00 USD", "subtotal EUR", "subtotal USD", "non-billable"} { if !strings.Contains(out, want) { t.Errorf("view missing %q; got:\n%s", want, out) @@ -119,7 +119,7 @@ func TestReportViewSingleCurrencyShowsOneTotal(t *testing.T) { CurrencySubtotals: []report.CurrencySubtotal{{Currency: "EUR", Hours: 2, BillableHours: 2, BilledHours: 2, Amount: 100}}, TotalHours: 2, BillableHours: 2, BilledHours: 2, TotalAmount: 100, } - out := newReport(r, "").view(testTheme(true)) + out := newReport(r, "", nil).view(testTheme(true), 80) if strings.Contains(out, "subtotal") { t.Errorf("single-currency report should not list subtotals; got:\n%s", out) } @@ -128,6 +128,28 @@ func TestReportViewSingleCurrencyShowsOneTotal(t *testing.T) { } } +// TestReportViewEmptyReportShowsNoHoursAndSplit pins the empty-report shape: +// with no buckets, reportTable is never called (the table would have nothing +// to size against) and "No hours to show." renders instead. The billable +// split line, previously folded inside the box and so absent for an empty +// report, is now always rendered below it — nothing else in the suite exercises +// the zero-bucket path, so this is the only place that would catch either +// regressing. +func TestReportViewEmptyReportShowsNoHoursAndSplit(t *testing.T) { + start := time.Date(2026, time.July, 1, 0, 0, 0, 0, time.UTC) + r := report.Report{ + Start: start, End: start.AddDate(0, 1, 0), Scope: "me", GroupBy: report.GroupByTotal, + DefaultCurrency: "EUR", + } + out := newReport(r, "", nil).view(testTheme(true), 80) + if !strings.Contains(out, "No hours to show.") { + t.Errorf("empty report view missing \"No hours to show.\"; got:\n%s", out) + } + if !strings.Contains(out, "billable 0.00h · non-billable 0.00h") { + t.Errorf("empty report view missing the billable split line; got:\n%s", out) + } +} + // TestReportViewShowsSummaryAndBillableSplit drives Update to build a report // from a mixed-currency fixture and asserts the rendered view carries the // shared export.SummaryLine ("N billing lines · Xh · amounts"), an explicit @@ -154,7 +176,7 @@ func TestReportViewShowsSummaryAndBillableSplit(t *testing.T) { t.Fatalf("screen = %v, want screenReport (err: %v)", mm.screen, mm.err) } // billable: 2h @ 100 EUR/h = 200 EUR, 1h @ 90 USD/h = 90 USD. - out := mm.rep.view(testTheme(true)) + out := mm.rep.view(testTheme(true), 80) for _, want := range []string{ "2 billing lines · 3.00h · 200.00 EUR, 90.00 USD", // export.SummaryLine (Lines counts only billable units) "subtotal EUR", "200.00 EUR", @@ -277,3 +299,56 @@ func TestMemberFilterNotePartial(t *testing.T) { t.Errorf("memberFilterNote = %q, want ' (2/3 members)'", got) } } + +// TestSparkViewLabelLeadsWithTrailingZeroDays pins the fix for the label +// detaching from the chart: the series ends in three idle days (zero-fills, +// rendered as spaces — see sparkline.go), the shape of the default view (the +// current month, viewed partway through) that goldenDaily() never exercised +// because its own series ends non-zero. Before this fix, the label was +// appended after the chart and the trailing spaces were never trimmed, so it +// drifted as many columns as there were trailing idle days — visibly so in +// the shipped docs/demo.gif. With the label leading and the trailing spaces +// trimmed, it now sits immediately before the first cell every time. +func TestSparkViewLabelLeadsWithTrailingZeroDays(t *testing.T) { + t.Parallel() + rm := newReport(report.Report{Buckets: []report.Bucket{{Label: "x"}}}, "", + []float64{3.5, 0, 1.25, 4, 0, 0, 0}) + if got, want := rm.sparkView(testTheme(true), 0), "hours/day ▇ ▃█"; got != want { + t.Errorf("sparkView = %q, want %q", got, want) + } +} + +// TestSparkViewAllZeroSeriesHasNoTrailingWhitespace covers a series with +// buckets but zero hours on every day of the range: sparkline itself renders +// as all spaces (no ink anywhere), and the old fix only trimmed the chart, so +// the label's own trailing separator space ("hours/day ") was left dangling +// with nothing after it — the same defect the trim exists to prevent, just +// moved one segment left. The line still renders (a chart with no ink is +// itself the information, same as one idle day among several rendering as a +// blank cell rather than vanishing); it must simply carry no trailing space. +func TestSparkViewAllZeroSeriesHasNoTrailingWhitespace(t *testing.T) { + t.Parallel() + rm := newReport(report.Report{Buckets: []report.Bucket{{Label: "x"}}}, "", + []float64{0, 0, 0, 0, 0, 0, 0}) + got := rm.sparkView(testTheme(true), 0) + if got != strings.TrimRight(got, " ") { + t.Errorf("sparkView = %q, has trailing whitespace", got) + } + if got != "hours/day" { + t.Errorf("sparkView = %q, want the label alone with no separator space", got) + } +} + +// TestTruncateEmptyStringAlwaysEmpty covers the deliberately empty Amount +// cell in the multi-currency TOTAL row (see reportRows): with the n <= 1 +// guard alone, truncate("", n) for n <= 1 returned "…" — an ellipsis standing +// in for nothing, rendered whenever that cell's column shrinks to 1 or less. +// An empty input has nothing to mark as cut, whatever n is. +func TestTruncateEmptyStringAlwaysEmpty(t *testing.T) { + t.Parallel() + for _, n := range []int{0, 1, 5} { + if got := truncate("", n); got != "" { + t.Errorf("truncate(%q, %d) = %q, want %q", "", n, got, "") + } + } +} diff --git a/internal/tui/sparkline.go b/internal/tui/sparkline.go new file mode 100644 index 0000000..0124fa7 --- /dev/null +++ b/internal/tui/sparkline.go @@ -0,0 +1,59 @@ +package tui + +import "math" + +// sparkLevels are the eight bar heights, lowest first. Index 0 is the lowest +// NON-ZERO level: an exact zero renders as a space instead, because a day with +// no work must read as a gap and ▁ would read as a little work. +const sparkLevels = "▁▂▃▄▅▆▇█" + +// sparkline renders values as block glyphs, one cell per value, resampled to +// at most maxCells when there are more values than cells. +// +// Heights are relative to the largest value in the series, so the shape shows +// the period's rhythm rather than an absolute scale. +func sparkline(values []float64, maxCells int) string { + if len(values) == 0 { + return "" + } + if maxCells > 0 && len(values) > maxCells { + values = resample(values, maxCells) + } + + peak := 0.0 + for _, v := range values { + peak = max(peak, v) + } + + levels := []rune(sparkLevels) + out := make([]rune, 0, len(values)) + for _, v := range values { + if v <= 0 || peak == 0 { + out = append(out, ' ') + continue + } + lvl := int(math.Ceil(v / peak * float64(len(levels)))) + out = append(out, levels[min(max(lvl, 1), len(levels))-1]) + } + return string(out) +} + +// resample averages values into k contiguous buckets. Bucket i covers +// values[i*n/k : (i+1)*n/k], which distributes the remainder deterministically +// and leaves no bucket empty as long as k <= n. +func resample(values []float64, k int) []float64 { + n := len(values) + out := make([]float64, k) + for i := range out { + lo, hi := i*n/k, (i+1)*n/k + if hi <= lo { + hi = lo + 1 + } + sum := 0.0 + for _, v := range values[lo:hi] { + sum += v + } + out[i] = sum / float64(hi-lo) + } + return out +} diff --git a/internal/tui/sparkline_test.go b/internal/tui/sparkline_test.go new file mode 100644 index 0000000..26eb816 --- /dev/null +++ b/internal/tui/sparkline_test.go @@ -0,0 +1,60 @@ +package tui + +import ( + "strings" + "testing" +) + +func TestSparkline(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + values []float64 + maxCells int + want string + }{ + {"empty", nil, 10, ""}, + {"all zero renders as gaps", []float64{0, 0, 0}, 10, " "}, + {"a single value is full height", []float64{5}, 10, "█"}, + {"equal values are all full height", []float64{4, 4, 4}, 10, "███"}, + {"an idle day is a gap, not a low bar", []float64{8, 0, 8}, 10, "█ █"}, + {"the maximum is full and the minimum non-zero is lowest", []float64{1, 8}, 10, "▁█"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := sparkline(tc.values, tc.maxCells); got != tc.want { + t.Errorf("sparkline(%v, %d) = %q, want %q", tc.values, tc.maxCells, got, tc.want) + } + }) + } +} + +// A custom range of a year is 365 days and cannot be drawn one cell per day. +// Resampling keeps the line inside the terminal. +func TestSparklineResamples(t *testing.T) { + t.Parallel() + values := make([]float64, 365) + for i := range values { + values[i] = float64(i) + } + got := sparkline(values, 30) + if n := len([]rune(got)); n != 30 { + t.Errorf("got %d cells, want 30", n) + } + // The series rises monotonically, so the resampled line must too. + runes := []rune(got) + for i := 1; i < len(runes); i++ { + if strings.IndexRune(sparkLevels, runes[i]) < strings.IndexRune(sparkLevels, runes[i-1]) { + t.Errorf("cell %d (%q) is lower than cell %d (%q) in a rising series: %q", + i, runes[i], i-1, runes[i-1], got) + } + } +} + +// Fewer values than cells must not be stretched: 5 days is 5 cells. +func TestSparklineDoesNotStretch(t *testing.T) { + t.Parallel() + if got := sparkline([]float64{1, 2, 3, 4, 5}, 40); len([]rune(got)) != 5 { + t.Errorf("sparkline of 5 values = %q (%d cells), want 5 cells", got, len([]rune(got))) + } +} diff --git a/internal/tui/testdata/budget.golden b/internal/tui/testdata/budget.golden index 24d0917..37eec25 100644 --- a/internal/tui/testdata/budget.golden +++ b/internal/tui/testdata/budget.golden @@ -1,6 +1,6 @@ Budget burn-down -╭──────────────────────────────────────────────────────────────────────────────────────────────╮ -│ Website [############--------] 62% 625.00 / 1000.00 EUR (remaining 375.00) │ -╰──────────────────────────────────────────────────────────────────────────────────────────────╯ \ No newline at end of file +╭────────────────────────────────────────────────────────────────────────────────────────────╮ +│ Website ████████████░░░░░░░░ 62% 625.00 / 1000.00 EUR (remaining 375.00) │ +╰────────────────────────────────────────────────────────────────────────────────────────────╯ \ No newline at end of file diff --git a/internal/tui/testdata/palette_dark.golden b/internal/tui/testdata/palette_dark.golden index fd0db12..fe403e2 100644 --- a/internal/tui/testdata/palette_dark.golden +++ b/internal/tui/testdata/palette_dark.golden @@ -7,3 +7,5 @@ Accent Accent Box ╭─────╮ │ Box │ ╰─────╯ +Border Border +Zebra Zebra diff --git a/internal/tui/testdata/palette_light.golden b/internal/tui/testdata/palette_light.golden index f49480f..603d99e 100644 --- a/internal/tui/testdata/palette_light.golden +++ b/internal/tui/testdata/palette_light.golden @@ -7,3 +7,5 @@ Accent Accent Box ╭─────╮ │ Box │ ╰─────╯ +Border Border +Zebra Zebra diff --git a/internal/tui/testdata/report.golden b/internal/tui/testdata/report.golden index bc825f2..2ef5536 100644 --- a/internal/tui/testdata/report.golden +++ b/internal/tui/testdata/report.golden @@ -3,10 +3,13 @@ Report July 2026 — scope me — grouped by list — tz UTC 0 billing lines · 12.50h · 625.00 EUR -╭───────────────────────────────────────────────────────────────╮ -│ Item Hours Billed Amount │ -│ Website 12.50 12.50 625.00 EUR │ -│ Internal 3.00 0.00 0.00 EUR │ -│ TOTAL 15.50 12.50 625.00 EUR │ -│ billable 12.50h · non-billable 3.00h │ -╰───────────────────────────────────────────────────────────────╯ \ No newline at end of file +hours/day ▅ ▂▆ ▃█ + +╭─────────────────────────────────────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├─────────────────────────────────────────────────────────────────────────┤ +│ Website — landing page redesign and checkou… 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰─────────────────────────────────────────────────────────────────────────╯ + billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_multicurrency.golden b/internal/tui/testdata/report_multicurrency.golden new file mode 100644 index 0000000..a103b5f --- /dev/null +++ b/internal/tui/testdata/report_multicurrency.golden @@ -0,0 +1,18 @@ +Report July 2026 — scope me — grouped by list — tz UTC + + +0 billing lines · 16.50h · 625.00 EUR, 200.00 USD + +hours/day ▅ ▂▆ ▃█ + +╭───────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├───────────────────────────────────────────┤ +│ Website 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ Mobile 4.00 4.00 200.00 USD │ +│ TOTAL 19.50 16.50 │ +│ subtotal EUR 15.50 12.50 625.00 EUR │ +│ subtotal USD 4.00 4.00 200.00 USD │ +╰───────────────────────────────────────────╯ + billable 16.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_narrow.golden b/internal/tui/testdata/report_narrow.golden new file mode 100644 index 0000000..0ef5409 --- /dev/null +++ b/internal/tui/testdata/report_narrow.golden @@ -0,0 +1,15 @@ +Report July 2026 — scope me — grouped by list — tz UTC + + +0 billing lines · 12.50h · 625.00 EUR + +hours/day ▅ ▂▆ ▃█ + +╭─────────────────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├─────────────────────────────────────────────────────┤ +│ Website — landing page … 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰─────────────────────────────────────────────────────╯ + billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_partial_month.golden b/internal/tui/testdata/report_partial_month.golden new file mode 100644 index 0000000..d16d97e --- /dev/null +++ b/internal/tui/testdata/report_partial_month.golden @@ -0,0 +1,15 @@ +Report July 2026 — scope me — grouped by list — tz UTC + + +0 billing lines · 12.50h · 625.00 EUR + +hours/day ▅ ▂▆ ▃█▂▇▄ + +╭─────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├─────────────────────────────────────┤ +│ Website 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰─────────────────────────────────────╯ + billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/testdata/report_wide.golden b/internal/tui/testdata/report_wide.golden new file mode 100644 index 0000000..e63388a --- /dev/null +++ b/internal/tui/testdata/report_wide.golden @@ -0,0 +1,15 @@ +Report July 2026 — scope me — grouped by list — tz UTC + + +0 billing lines · 12.50h · 625.00 EUR + +hours/day ▅ ▂▆ ▃█ + +╭───────────────────────────────────────────────────────────────────────────────────╮ +│ Item Hours Billed Amount │ +├───────────────────────────────────────────────────────────────────────────────────┤ +│ Website — landing page redesign and checkout hardening 12.50 12.50 625.00 EUR │ +│ Internal 3.00 0.00 0.00 EUR │ +│ TOTAL 15.50 12.50 625.00 EUR │ +╰───────────────────────────────────────────────────────────────────────────────────╯ + billable 12.50h · non-billable 3.00h \ No newline at end of file diff --git a/internal/tui/theme.go b/internal/tui/theme.go index 7fc92bc..5247230 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -4,13 +4,14 @@ import "github.com/charmbracelet/lipgloss" // palette holds the semantic colors the whole TUI is built from. It is the // only place a color literal belongs: styles derive from these, and a future -// user-supplied theme (#82) will override these five values, not the styles. +// user-supplied theme (#82) will override these six values, not the styles. type palette struct { Primary lipgloss.AdaptiveColor // titles and headings Accent lipgloss.AdaptiveColor // selection, highlighted values Muted lipgloss.AdaptiveColor // help lines and secondary text Danger lipgloss.AdaptiveColor // errors Success lipgloss.AdaptiveColor // confirmations + Subtle lipgloss.AdaptiveColor // zebra row background } // theme is the styled surface the views render through. It travels as an @@ -28,6 +29,9 @@ type theme struct { Accent lipgloss.Style Box lipgloss.Style Header lipgloss.Style // bold, uncolored: the report's column header row + Cell lipgloss.Style // a plain report-table cell: no color, just the renderer + Border lipgloss.Style // the report table's frame + Zebra lipgloss.Style // alternate report-table row } // newTheme builds the styles for a palette on a specific renderer. Production @@ -49,6 +53,9 @@ func newTheme(r *lipgloss.Renderer, p palette) theme { Accent: r.NewStyle().Foreground(p.Accent), Box: r.NewStyle().Border(lipgloss.RoundedBorder()).Padding(0, 1), Header: r.NewStyle().Bold(true), + Cell: r.NewStyle(), + Border: r.NewStyle().Foreground(p.Muted), + Zebra: r.NewStyle().Background(p.Subtle), } } @@ -64,6 +71,12 @@ func newTheme(r *lipgloss.Renderer, p palette) theme { // // Muted (240, #585858) is left alone: it already clears 7:1 on white. Adaptive // means legible on both backgrounds, not different on both. +// +// Subtle is a background, so it is judged by a different rule than the five +// foregrounds above: 236 (#303030) on dark and 254 (#E4E4E4) on light are +// chosen so the DEFAULT FOREGROUND still clears contrast when painted on top +// of them, not so they contrast with the terminal's own background. A zebra +// stripe that swallows its own text is worse than no stripe at all. func defaultPalette() palette { return palette{ Primary: lipgloss.AdaptiveColor{Light: "127", Dark: "205"}, @@ -71,5 +84,6 @@ func defaultPalette() palette { Muted: lipgloss.AdaptiveColor{Light: "240", Dark: "240"}, Danger: lipgloss.AdaptiveColor{Light: "124", Dark: "196"}, Success: lipgloss.AdaptiveColor{Light: "28", Dark: "42"}, + Subtle: lipgloss.AdaptiveColor{Light: "254", Dark: "236"}, } } diff --git a/internal/tui/theme_test.go b/internal/tui/theme_test.go index 85c3a29..e3ddc2d 100644 --- a/internal/tui/theme_test.go +++ b/internal/tui/theme_test.go @@ -72,6 +72,7 @@ func paletteSample(th theme) string { }{ {"Title", th.Title}, {"Help", th.Help}, {"Err", th.Err}, {"OK", th.OK}, {"Accent", th.Accent}, {"Box", th.Box}, + {"Border", th.Border}, {"Zebra", th.Zebra}, } { fmt.Fprintf(&b, "%-8s%s\n", row.name, row.st.Render(row.name)) } @@ -100,6 +101,7 @@ func TestPaletteIsAdaptive(t *testing.T) { }{ {"Primary", p.Primary}, {"Accent", p.Accent}, {"Danger", p.Danger}, {"Success", p.Success}, + {"Subtle", p.Subtle}, } { if tc.c.Light == tc.c.Dark { t.Errorf("%s is not adaptive: Light == Dark == %q", tc.name, tc.c.Dark) @@ -143,6 +145,18 @@ func TestNoColorProducesNoEscapes(t *testing.T) { } } +// The zebra style must actually carry a background: it is the only thing that +// distinguishes an odd row, and the package goldens run under termenv.Ascii, +// which strips backgrounds — so nothing else in the suite would notice it +// going missing. +func TestZebraCarriesBackground(t *testing.T) { + t.Parallel() + th := paletteTheme(true) + if th.Zebra.GetBackground() == th.Cell.GetBackground() { + t.Error("Zebra has the same background as a plain cell, so odd rows are indistinguishable") + } +} + // CLICOLOR_FORCE is the force-color variable termenv implements (FORCE_COLOR, // the npm convention, is deliberately not supported). func TestCliColorForceKeepsColor(t *testing.T) {