From 93880f7ddd4784ce01c135529967b6a18517c766 Mon Sep 17 00:00:00 2001 From: Tien Nguyen Minh Date: Sun, 19 Apr 2026 09:27:17 +0700 Subject: [PATCH] fix(card): eliminate frame overflows and add release-gate standard (#8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three cards overflowed the 340×200 frame for realistic profile data: - contributions-heatmap: the classic case — 53 weeks at 9px cellSize + 2px gap pushed the grid out to x≈611. Shrink to cellSize=5, cellGap=1 so leftPad(22) + 53*6 = 340 (exact fit). Drop month labels within 20 px of the right edge so "Dec"/"Apr" can't stick past the frame. - streak: the third column rendered "N / M" at font-size 28, centered at x=282. For 4+ digit totals (e.g. 584 / 3031) the text extended to x≈347. Refactor to show the active-days integer by itself in the big slot and push "of N total (P%)" into the small detail line that the other two columns already use. - top-starred-repos: the per-row star icon sat at x=306 while the right- anchored number ended at x=334, so 5+ digit star counts collided with the icon. Drop the icon (card title already says "Top Starred Repos"), emit the count as "N ★", right-anchor at x=334 with a 6 px safety gap. Add a new TestCardsFitFrame stress test that renders every card against an adversarial profile (10-digit counts, 40-char names, 20 active years, 53-week span) and asserts every positional attribute stays inside the frame. This is the automated half of the new "fit-the-frame invariant" added to docs/design-guidelines.md + a pre-release review checklist in docs/code-standards.md. Bug reports will still surface text-overflow cases that the coordinate check can't see (a text-anchor="middle" element has a single x attribute but renders outward), so the docs also spell out the human-review step: render dracula against tiny/typical/adversarial fixtures before release. --- docs/code-standards.md | 11 +++ docs/design-guidelines.md | 36 +++++++ internal/card/card_test.go | 131 +++++++++++++++++++++++++ internal/card/contributions_heatmap.go | 22 +++-- internal/card/streak.go | 29 ++++-- internal/card/top_starred_repos.go | 35 +++---- 6 files changed, 229 insertions(+), 35 deletions(-) diff --git a/docs/code-standards.md b/docs/code-standards.md index a4641c9e..98ea6be5 100644 --- a/docs/code-standards.md +++ b/docs/code-standards.md @@ -84,6 +84,17 @@ go build ./... All three must pass. If a test is failing, fix the test before committing — don't skip. +## Card review checklist (pre-release) + +In addition to the compile/test gate above, any change that touches files under `internal/card/` **must** be reviewed against `docs/design-guidelines.md` → "Fit-the-frame invariant" before merging. Specifically: + +- Render the dracula theme against the **tiny / typical / adversarial** profile fixtures and visually verify nothing overflows or overlaps. +- No hard-coded dimensions that only work for the reviewer's own profile (names, digit counts, year spans). +- Every right-anchored value or text-anchor="middle" element fits inside its column / safety margin. +- The CI-built `demo//` gallery is the canonical "dracula on the author's profile" view; don't rely on a local run alone. + +This is a hard gate: a card that overflows on a realistic profile does not ship, even if tests pass. + ## Dependency policy - **Stdlib only.** `go.mod` lists no `require` entries. diff --git a/docs/design-guidelines.md b/docs/design-guidelines.md index bb899d3c..89f1da8e 100644 --- a/docs/design-guidelines.md +++ b/docs/design-guidelines.md @@ -98,6 +98,42 @@ Missing months in the `[first, last]` range are inserted as zero-count rows to k Add new icons by copying the `` from Octicons and appending to `icons.go`. Keep them to the same 16×16 viewBox so the existing scale math applies. +## Fit-the-frame invariant (MUST hold before release) + +Every card MUST render entirely inside the 340 × 200 frame **for every profile the card can encounter**, not just the author's. That means: + +| Thing that varies | Worst case to design for | +| --- | --- | +| Star counts, commit counts, streak lengths, active days | 10-digit formatted integer (e.g. `1,234,567,890`) | +| Repo / language / company / location names | 40+ char strings with CJK / emoji | +| Number of active years | 20+ years (contribution history can start in 2008) | +| Contribution calendar weeks | 53 — not 52 — when the window spans a year transition | + +Concrete rules this implies: + +- **Reserve columns.** When a card has `N` equal-width columns, treat `340 / N` as the hard limit for each column's widest element. No centered text can be wider than its column. +- **Right-anchored values** (stats rows, top-starred bars) must leave a safety margin. Right edge ≤ `width − 6`; do not place an icon to the right of a right-anchored number (they collide on multi-digit values). +- **Long strings get truncated, not wrapped.** Use `truncateName` (top-starred) or pick a layout that allows clipping via `text-overflow` semantics. Never let a wide string push a later element off-screen. +- **Variable-count grids** (heatmap 7×N, by-year N bars) must compute cell size from the container width, not the other way round. Don't hardcode a cell size that only works for the author's profile. +- **Month / year tick labels** within `~20 px` of the right edge must be skipped (they read past the frame otherwise). + +### Review checklist (before merging any card change) + +Render the **dracula** theme against at least three profiles or synthetic fixtures: + +1. **Tiny**: a brand-new account with 0 commits, 0 stars, 1 repo. +2. **Typical**: the author's profile (`tiennm99` via `demo/` regeneration). +3. **Adversarial**: seven-digit commit counts, 40-char repo / company / location names, 20 active years, 53-week span. A small synthetic `*.json` fixture is fine; it doesn't need a token. + +Then open every affected SVG at 1× and 2× zoom and verify: + +- [ ] No ``, ``, ``, ``, or path coordinate exceeds `x=340` or `y=200`, or falls below `x=0` / `y=0`. +- [ ] No two elements overlap in a way that makes either unreadable. +- [ ] Right-anchored numbers don't collide with icons, swatches, or bars. +- [ ] Peak-vs-dim highlighting still reads at a glance (dracula `Background → Accent` contrast is fine; light themes like `github` or `nord_bright` need a separate check). + +The `demo//` gallery auto-regenerates on every push to `main`; use the last CI run as the dracula reference, and stress-test the adversarial case locally before pushing. + ## Accessibility - Contrast is the theme author's responsibility — we don't validate at runtime. diff --git a/internal/card/card_test.go b/internal/card/card_test.go index 84c90ab8..b8d36c10 100644 --- a/internal/card/card_test.go +++ b/internal/card/card_test.go @@ -3,6 +3,8 @@ package card import ( "os" "path/filepath" + "regexp" + "strconv" "strings" "testing" "time" @@ -163,3 +165,132 @@ func TestEscapeXML(t *testing.T) { t.Errorf("escapeXML=%q want %q", got, want) } } + +// TestCardsFitFrame renders every card against an adversarial profile and +// asserts every positional attribute stays inside the 340×200 frame. This is +// the automated half of the "fit-the-frame invariant" from +// docs/design-guidelines.md — a guard against future card changes that would +// silently overflow only for non-author profiles. +func TestCardsFitFrame(t *testing.T) { + p := adversarialProfile() + th, _ := theme.Lookup("dracula") + dir := t.TempDir() + if err := RenderAll(p, th, dir); err != nil { + t.Fatalf("RenderAll: %v", err) + } + + entries, err := os.ReadDir(filepath.Join(dir, "dracula")) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + + for _, e := range entries { + data, err := os.ReadFile(filepath.Join(dir, "dracula", e.Name())) + if err != nil { + t.Fatal(err) + } + assertInFrame(t, e.Name(), string(data)) + } +} + +// attrCoord captures the numeric value of any positional SVG attribute that +// could push content outside the frame. We ignore path `d` attributes — their +// coordinates are always clamped by the chart geometry, and the regex would +// be fragile against the Catmull-Rom Bezier output. +var attrCoord = regexp.MustCompile(`(?:x|y|x1|y1|x2|y2|cx|cy)="(-?\d+(?:\.\d+)?)"`) + +func assertInFrame(t *testing.T, name, svg string) { + t.Helper() + const ( + maxX = 340 + maxY = 200 + ) + for _, m := range attrCoord.FindAllStringSubmatch(svg, -1) { + v, err := strconv.ParseFloat(m[1], 64) + if err != nil { + continue + } + // Distinguish x-ish vs y-ish by the first attr char. + isX := strings.HasPrefix(m[0], "x") || strings.HasPrefix(m[0], "cx") + limit := float64(maxY) + if isX { + limit = float64(maxX) + } + if v < -1 || v > limit+0.5 { + t.Errorf("%s: attribute %q value %v outside frame (limit %v)", name, m[0], v, limit) + } + } +} + +// adversarialProfile exercises every card against the worst-case inputs a +// real user might have: huge counts, long names, 20 active years, 53-week +// calendar. Kept alongside the stress test so updates stay colocated. +func adversarialProfile() *github.Profile { + p := &github.Profile{ + Login: "user-with-a-very-long-login-name", + Name: "A Very Long Display Name That Keeps Going", + Company: "A-Company-With-An-Unusually-Long-Name Pty Ltd", + Location: "A Place With A Name That Is Way Too Long To Fit", + Website: "https://example-with-a-very-long-domain.example.com/profile", + Followers: 1_234_567, + Following: 98_765, + PublicRepos: 4_321, + TotalStars: 10_000_000, + TotalCommits: 123_456, + TotalCommitsAllTime: 9_876_543, + TotalPRs: 12_345, + TotalIssues: 6_789, + TotalReviews: 54_321, + TotalContributedTo: 777, + TotalContributionsLastYear: 200_000, + CreatedAt: time.Date(2008, 1, 1, 0, 0, 0, 0, time.UTC), + ReposByLanguage: []github.LangStat{ + {Name: "JavaScript", Color: "#f1e05a", Value: 1234}, + {Name: "TypeScript", Color: "#3178c6", Value: 999}, + {Name: "Go", Color: "#00ADD8", Value: 500}, + {Name: "Rust", Color: "#dea584", Value: 321}, + {Name: "Python", Color: "#3572A5", Value: 200}, + }, + CommitsByLanguage: []github.LangStat{ + {Name: "JavaScript", Color: "#f1e05a", Value: 1_000_000}, + {Name: "Rust", Color: "#dea584", Value: 500_000}, + }, + CommitsByLanguageAllTime: []github.LangStat{ + {Name: "JavaScript", Color: "#f1e05a", Value: 5_000_000}, + {Name: "Java", Color: "#b07219", Value: 2_000_000}, + }, + } + for i := range p.Productive { + p.Productive[i] = 9999 + p.ProductiveAllTime[i] = 999_999 + } + for i := range p.Weekday { + p.Weekday[i] = 9999 + p.WeekdayAllTime[i] = 999_999 + } + + // Top repos with very long names — truncateName must kick in. + for i := 0; i < 8; i++ { + p.TopRepos = append(p.TopRepos, github.RepoInfo{ + Owner: "user", + Name: "a-repository-with-an-absurdly-long-slug-" + strings.Repeat("x", 20), + Stars: 1_000_000 - i*123_456, + PrimaryLanguage: "TypeScript", + PrimaryColor: "#3178c6", + }) + } + + // 20-year history ending today so the heatmap sees exactly 53 weeks and + // the by-year card gets 20 bars. + base := time.Date(time.Now().Year()-20, 1, 1, 0, 0, 0, 0, time.UTC) + for i := 0; i < 365*20+5; i++ { + p.DailyContributionsAllTime = append(p.DailyContributionsAllTime, + github.DailyContribution{Date: base.AddDate(0, 0, i), Count: i % 17}) + } + yearStart := time.Now().AddDate(-1, 0, 0) + for i := 0; i < 371; i++ { + p.DailyContributions = append(p.DailyContributions, + github.DailyContribution{Date: yearStart.AddDate(0, 0, i), Count: i % 23}) + } + return p +} diff --git a/internal/card/contributions_heatmap.go b/internal/card/contributions_heatmap.go index 8d5d3375..d707c57d 100644 --- a/internal/card/contributions_heatmap.go +++ b/internal/card/contributions_heatmap.go @@ -21,15 +21,17 @@ func (contributionsHeatmapCard) SVG(p *github.Profile, t theme.Theme) ([]byte, e // bottom, oldest week on the left. Cell color mixes theme.Background with // 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. func renderHeatmap(title string, days []github.DailyContribution, t theme.Theme) []byte { const ( - width = 340 - height = 200 - cellSize = 9 - cellGap = 2 - leftPad = 28 - topPad = 55 - dayLabelDX = 22 // where weekday labels anchor (right of grid start) + width = 340 + height = 200 + cellSize = 5 + cellGap = 1 + leftPad = 22 + topPad = 62 ) var b strings.Builder @@ -70,6 +72,9 @@ func renderHeatmap(title string, days []github.DailyContribution, t theme.Theme) // Month labels across the top. We print each month the first time its // first day appears in a week column, skipping consecutive duplicates. + // Labels within ~20 px of the right edge are dropped so a trailing "Dec" + // or "Apr" can't extend past the card frame. + const monthLabelMaxX = width - 20 lastMonth := time.Month(0) for w := 0; w < weeks; w++ { first := cells[w*7].Date @@ -81,6 +86,9 @@ func renderHeatmap(title string, days []github.DailyContribution, t theme.Theme) } lastMonth = first.Month() x := leftPad + w*(cellSize+cellGap) + if x > monthLabelMaxX { + continue + } fmt.Fprintf(&b, ` %s`, x, topPad-4, t.Muted, first.Month().String()[:3]) diff --git a/internal/card/streak.go b/internal/card/streak.go index 2a34afe6..b8ce5fc3 100644 --- a/internal/card/streak.go +++ b/internal/card/streak.go @@ -25,16 +25,18 @@ func (streakCard) SVG(p *github.Profile, t theme.Theme) ([]byte, error) { b.WriteString(header(width, height, t.Background, t.Stroke, t.StrokeOpacity, t.Title, "Streak")) // Three large stat columns (current / longest / active-days) side by side, - // each with a big number on top and a smaller label underneath. Mirrors - // the classic "streak" card layout so embedders recognise it instantly. + // each with a big number on top, a label underneath, and a small detail + // line. Keeping the big number to a single formatted integer per column + // means no column can overflow regardless of magnitude (formatInt adds + // thousands separators and even 10-digit counts fit ≤113 px at 28 px). cols := []struct { - value string - label string - end string // optional date annotation beneath the label + value string + label string + detail string }{ {formatInt(stats.Current), "Current streak", streakRange(stats.CurrentStart, stats.CurrentEnd)}, {formatInt(stats.Longest), "Longest streak", streakRange(stats.LongestStart, stats.LongestEnd)}, - {fmt.Sprintf("%d / %d", stats.Active, stats.Total), "Active days", ""}, + {formatInt(stats.Active), "Active days", activeDaysDetail(stats.Active, stats.Total)}, } colW := width / len(cols) for i, c := range cols { @@ -44,10 +46,10 @@ func (streakCard) SVG(p *github.Profile, t theme.Theme) ([]byte, error) { %s`, cx, 95, t.Accent, escapeXML(c.value), cx, 120, t.Text, escapeXML(c.label)) - if c.end != "" { + if c.detail != "" { fmt.Fprintf(&b, ` %s`, - cx, 140, t.Muted, escapeXML(c.end)) + cx, 140, t.Muted, escapeXML(c.detail)) } } @@ -113,6 +115,17 @@ func computeStreak(days []github.DailyContribution) streakStats { return s } +// activeDaysDetail renders the "/ total" denominator as a small sub-line so +// the big number in the column stays a single formatted integer — that way +// a user with 10,000+ active days never squeezes against the column edges. +func activeDaysDetail(active, total int) string { + if total <= 0 { + return "" + } + pct := 100 * active / total + return fmt.Sprintf("of %s total (%d%%)", formatInt(total), pct) +} + // streakRange formats the open/close dates of a streak as "Mon 2 — Wed 11" // when both are present. Returns "" when the streak is zero-length so the // card renders cleanly. diff --git a/internal/card/top_starred_repos.go b/internal/card/top_starred_repos.go index edae83ef..c21cd6d3 100644 --- a/internal/card/top_starred_repos.go +++ b/internal/card/top_starred_repos.go @@ -18,16 +18,16 @@ const maxTopRepoRows = 5 func (topStarredReposCard) SVG(p *github.Profile, t theme.Theme) ([]byte, error) { const ( - width = 340 - height = 200 - rowX = 20 - rowY0 = 60 - rowDY = 22 - iconSize = 12 - barX = 160 - barW = 140 // max bar width; the top repo fills this - barH = 10 - nameMax = 18 // truncate long repo names at this many characters + width = 340 + height = 200 + rowX = 20 + rowY0 = 60 + rowDY = 22 + barX = 150 + barW = 120 // max bar width; the top repo fills this + barH = 10 + valueX = 334 // right-aligned anchor for the star count + nameMax = 17 // truncate long repo names at this many characters ) repos := ownedNonForkRepos(p.TopRepos) @@ -50,20 +50,17 @@ func (topStarredReposCard) SVG(p *github.Profile, t theme.Theme) ([]byte, error) maxStars = 1 } - scale := float64(iconSize) / 16.0 + // Row layout: language swatch + name on the left, a proportional bar in + // the middle, star count right-anchored at valueX. The card title already + // says "Top Starred Repos", so the per-row star icon would just be noise. for i, r := range repos { y := rowY0 + i*rowDY - lang := r.PrimaryLanguage - if lang == "" { - lang = "—" - } langColor := r.PrimaryColor if langColor == "" { langColor = t.Accent } name := truncateName(r.Name, nameMax) - // Language swatch + repo name on the left; horizontal bar + star count on the right. fmt.Fprintf(&b, ` %s`, @@ -78,10 +75,8 @@ func (topStarredReposCard) SVG(p *github.Profile, t theme.Theme) ([]byte, error) barX, y-barH+2, bw, barH, t.Accent) fmt.Fprintf(&b, ` - %s - %s`, - barX+barW+6, float64(y-iconSize+2), scale, t.Muted, iconStar, - width-6, y, t.Accent, escapeXML(formatInt(r.Stars))) + %s ★`, + valueX, y, t.Accent, escapeXML(formatInt(r.Stars))) } b.WriteString(footer)