From af20be8a7aa0e2b43a0c6a464126f565a34a570a Mon Sep 17 00:00:00 2001 From: Tien Nguyen Minh Date: Sun, 19 Apr 2026 11:00:02 +0700 Subject: [PATCH] fix(card): donut legend caps at 7 rows (Other inclusive); add heatmap gutter (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups: 1. "Top 7 languages" counts rows the reader sees, not "7 named + Other". Revert collapseOther to the original semantic: when the input has more than n entries, keep in[:n-1] named and fold the tail into a single "Other" row — 7 rows total with topN=7. Input with ≤7 real languages still passes through unchanged. Test and adversarial-profile updated to pin the cap (6 named + Other, Shell/Kotlin/Java collapse). 2. Contributions heatmap grid was touching the card right edge (leftPad 22 + 53*6 = 340 exactly). Shrink cells from 5×5 to 4×4 and bump leftPad to 30, giving 45 px of right gutter. Grid no longer reads as bleeding into the frame border; weekday label column also gets a few more pixels of breathing room on the left. --- docs/design-guidelines.md | 4 ++-- internal/card/card_test.go | 15 +++++++-------- internal/card/contributions_heatmap.go | 9 +++++---- internal/card/donut_chart.go | 18 ++++++++---------- 4 files changed, 22 insertions(+), 24 deletions(-) diff --git a/docs/design-guidelines.md b/docs/design-guidelines.md index 03c985ad..97bc4e6b 100644 --- a/docs/design-guidelines.md +++ b/docs/design-guidelines.md @@ -50,7 +50,7 @@ Cap rows at what fits: up to 7 rows per card. Stats splits commits into lifetime | Donut centre | `(250, 110)` | | Outer radius | 55 | | Inner radius | 30 | -| Top-N entries shown | Up to 7 named languages, plus an "Other" row when the tail is non-zero (8 rows max) | +| Top-N entries shown | 7 rows max, "Other" inclusive (6 named + "Other" when there's a tail, up to 7 named when there isn't) | | Slice stroke | `theme.Background`, 1.5 px (gap between slices) | | Legend origin | `(20, 55)` | | Legend row height | 20 px | @@ -78,7 +78,7 @@ When there's **exactly one slice** (one language at 100%), the renderer emits tw | Metric | Value | | --- | --- | | Grid | 7 rows × 53 columns (Sunday → Saturday, oldest week → newest) | -| Cell size | 5 × 5 px, 1 px gap (exact fit: `leftPad(22) + 53 × 6 = 340`) | +| Cell size | 4 × 4 px, 1 px gap. Grid footprint = `53 × 5 = 265 px`; `leftPad(30) + 265 = 295 px`, leaving a 45 px right gutter so the grid doesn't bleed into the frame border | | Cell colour | 5-bucket ramp `mixHex(Background, Accent, k/4)` for `k ∈ 0..4` — no dedicated ramp field on the theme schema | | Weekday labels | Mon / Wed / Fri only, right-anchored in the `leftPad` gutter | | Month labels | Printed above the first week where a 1st-of-month day falls; skipped when `x > width − 20` so `Dec` / `Apr` can't spill past the frame | diff --git a/internal/card/card_test.go b/internal/card/card_test.go index e35f2d3f..585e6a50 100644 --- a/internal/card/card_test.go +++ b/internal/card/card_test.go @@ -141,11 +141,10 @@ func TestDonutEmpty(t *testing.T) { } } -// TestDonutTopSevenPlusOther confirms the "7 named + Other" contract: when -// the input has more than 7 languages, the legend shows all 7 real entries -// followed by a single "Other" bucket. Regression guard against anyone -// sliding topN back to "N-1 real + Other" packing. -func TestDonutTopSevenPlusOther(t *testing.T) { +// TestDonutMaxSevenRows confirms the legend caps at 7 rows (Other inclusive): +// 9 input languages collapse to the top 6 named + "Other". Regression guard +// against anyone flipping the collapse semantic back to "7 named + Other". +func TestDonutMaxSevenRows(t *testing.T) { th, _ := theme.Lookup("dracula") stats := []github.LangStat{ {Name: "Go", Color: "#00ADD8", Value: 100}, @@ -159,13 +158,13 @@ func TestDonutTopSevenPlusOther(t *testing.T) { {Name: "Java", Color: "#b07219", Value: 20}, } svg := string(renderDonutCard("Test", stats, th)) - for _, want := range []string{"Go", "TypeScript", "Python", "Rust", "JavaScript", "HTML", "Shell", "Other"} { + for _, want := range []string{"Go", "TypeScript", "Python", "Rust", "JavaScript", "HTML", "Other"} { if !strings.Contains(svg, ">"+want+" ") { t.Errorf("expected legend row for %q; not found in:\n%s", want, svg) } } - // Kotlin + Java spill into Other (they must NOT appear as named rows). - for _, dropped := range []string{">Kotlin ", ">Java "} { + // Shell, Kotlin, Java are outside the top 6 and roll into Other. + for _, dropped := range []string{">Shell ", ">Kotlin ", ">Java "} { if strings.Contains(svg, dropped) { t.Errorf("expected %q to be collapsed into Other, but it renders:\n%s", dropped, svg) } diff --git a/internal/card/contributions_heatmap.go b/internal/card/contributions_heatmap.go index d707c57d..ae7feaa3 100644 --- a/internal/card/contributions_heatmap.go +++ b/internal/card/contributions_heatmap.go @@ -22,15 +22,16 @@ func (contributionsHeatmapCard) SVG(p *github.Profile, t theme.Theme) ([]byte, e // theme.Accent in four intensity buckets so every palette inherits a usable // heatmap without a separate color ramp in the theme schema. // -// Geometry is sized so 53 weeks fit inside the 340 px frame: -// leftPad (22) + 53*(cellSize+cellGap)=53*6=318 → grid ends at x=340. +// Geometry: 4 px cells + 1 px gap = 5 px per week column × 53 weeks = 265 px. +// Left pad 30 (weekday labels), right pad 45 — both well clear of the frame +// so the grid doesn't read as "bleeding" off the card. func renderHeatmap(title string, days []github.DailyContribution, t theme.Theme) []byte { const ( width = 340 height = 200 - cellSize = 5 + cellSize = 4 cellGap = 1 - leftPad = 22 + leftPad = 30 topPad = 62 ) diff --git a/internal/card/donut_chart.go b/internal/card/donut_chart.go index 2726992f..71535c25 100644 --- a/internal/card/donut_chart.go +++ b/internal/card/donut_chart.go @@ -99,23 +99,21 @@ func polar(cx, cy float64, r, angle float64) (float64, float64) { return cx + r*math.Cos(angle), cy + r*math.Sin(angle) } -// collapseOther returns the top n named entries, optionally followed by an -// "Other" row summing everything past that. "Top N" means N actual languages -// — the Other row is a bonus when there's a non-zero tail, not one of the N. -// When the input fits in N entries the caller gets the slice back as-is. +// collapseOther caps the legend at n rows. If there are more than n inputs +// it keeps the top (n-1) named slices and folds everything past that into a +// single "Other" row — so n counts TOTAL rows (Other inclusive), matching +// what readers count when they glance at the legend. func collapseOther(in []github.LangStat, n int) []github.LangStat { if len(in) <= n { return in } - out := make([]github.LangStat, 0, n+1) - out = append(out, in[:n]...) + out := make([]github.LangStat, 0, n) + out = append(out, in[:n-1]...) var rest int64 - for _, s := range in[n:] { + for _, s := range in[n-1:] { rest += s.Value } - if rest > 0 { - out = append(out, github.LangStat{Name: "Other", Value: rest}) - } + out = append(out, github.LangStat{Name: "Other", Value: rest}) return out }