mirror of
https://github.com/tiennm99/ghglance.git
synced 2026-10-15 22:14:56 +00:00
fix(card): eliminate frame overflows and add release-gate standard (#8)
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.
This commit is contained in:
1 parent
4609688fe2
commit
93880f7ddd
6 files changed
+229
-35
No files matched your search
@@ -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/<theme>/` 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.
|
||||
|
||||
@@ -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 `<path>` 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 `<text>`, `<rect>`, `<circle>`, `<line>`, 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/<theme>/` 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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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, `
|
||||
<text x="%d" y="%d" font-size="9" fill="%s">%s</text>`,
|
||||
x, topPad-4, t.Muted, first.Month().String()[:3])
|
||||
|
||||
+21
-8
@@ -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) {
|
||||
<text x="%d" y="%d" font-size="12" fill="%s" text-anchor="middle">%s</text>`,
|
||||
cx, 95, t.Accent, escapeXML(c.value),
|
||||
cx, 120, t.Text, escapeXML(c.label))
|
||||
if c.end != "" {
|
||||
if c.detail != "" {
|
||||
fmt.Fprintf(&b, `
|
||||
<text x="%d" y="%d" font-size="10" fill="%s" text-anchor="middle">%s</text>`,
|
||||
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.
|
||||
|
||||
@@ -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, `
|
||||
<circle cx="%d" cy="%d" r="4" fill="%s"/>
|
||||
<text x="%d" y="%d" font-size="12" fill="%s">%s</text>`,
|
||||
@@ -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, `
|
||||
<g transform="translate(%d,%.2f) scale(%.3f)" fill="%s">%s</g>
|
||||
<text x="%d" y="%d" font-size="11" font-weight="600" fill="%s" text-anchor="end">%s</text>`,
|
||||
barX+barW+6, float64(y-iconSize+2), scale, t.Muted, iconStar,
|
||||
width-6, y, t.Accent, escapeXML(formatInt(r.Stars)))
|
||||
<text x="%d" y="%d" font-size="12" font-weight="600" fill="%s" text-anchor="end">%s ★</text>`,
|
||||
valueX, y, t.Accent, escapeXML(formatInt(r.Stars)))
|
||||
}
|
||||
|
||||
b.WriteString(footer)
|
||||
|
||||
Reference in new issue
Block a user