From e0e99b57741ae99f4d40c8a1b45a17ff57912f6e Mon Sep 17 00:00:00 2001 From: Tien Nguyen Minh Date: Sun, 19 Apr 2026 09:54:28 +0700 Subject: [PATCH] =?UTF-8?q?fix(card):=20y-axis=20last=20tick=20must=20be?= =?UTF-8?q?=20=E2=89=A5=20data=20max=20(chartH=20overflow)=20(#11)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When niceTicks picks a step that doesn't divide the data max, the last returned tick was the highest step multiple ≤ max. Callers used it as yMax, so any data point > lastTick rendered a bar > chartH that poked above the chart top into the title area. Concrete case from the dracula demo: max=625 with step=100 → ticks [0, 100, 200, 300, 400, 500, 600], yMax=600, bar height for 625 = 110*(625/600) = 114.58 — 4.58 px past the chart top and right against the card title. Fix in niceTicks itself: round the top tick UP to the next step multiple (`last = ceil(max/step) * step`), so 625 yields [..., 600, 700] and the same 625 bar lands at 110*(625/700) ≈ 98.2 px, with a clean 12 px gap to the title. This is the stable answer to title-vs-bar collision: regardless of which weekday (or year, or month, or hour) holds the peak, the chart headroom is built into the axis instead of leaned on per-card. The title auto-shrink from the previous fix still applies — that's for literal text width, an orthogonal problem. Add TestNiceTicksCoversMax covering the cases (625, 99, 101, 7, 49, 999, 1001) that would have silently regressed before. --- internal/card/axis.go | 22 +++++++++++++++++----- internal/card/card_test.go | 20 ++++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/internal/card/axis.go b/internal/card/axis.go index 61fc86bf..5d13fcba 100644 --- a/internal/card/axis.go +++ b/internal/card/axis.go @@ -6,11 +6,17 @@ import ( "strconv" ) -// niceTicks returns evenly-spaced tick values in [0, max] such that the step -// is a 1/2/5/10 × 10^k number and the tick count is roughly targetTicks. +// niceTicks returns evenly-spaced tick values starting at 0 with a step of +// 1/2/5/10 × 10^k and a tick count roughly targetTicks. The last tick is the +// smallest step multiple ≥ max, so callers can safely use it as yMax without +// data points ever exceeding the chart's height. // -// Mirrors d3.scaleLinear().nice() / d3.axisLeft().ticks(n) so charts built -// on top look visually consistent with the d3 reference. +// Mirrors d3.scaleLinear().nice() / d3.axisLeft().ticks(n) so charts built on +// top look visually consistent with the d3 reference. +// +// Example: niceTicks(625, 5) returns [0, 100, 200, 300, 400, 500, 600, 700] +// — note 700 > 625, so a bar of 625 fits at 625/700 ≈ 89% of chart height +// with a clean gap above it. func niceTicks(max float64, targetTicks int) []float64 { if max <= 0 || targetTicks <= 0 { return []float64{0} @@ -30,8 +36,14 @@ func niceTicks(max float64, targetTicks int) []float64 { step = 10 * exp } + // Round the top tick up to the next step multiple so the chart's yMax + // always strictly covers the data. Without this, a data point of 625 + // against a step of 100 would yield a last tick of 600 — bars for 625 + // would render at 104% of chart height and poke into the title area. + last := math.Ceil(max/step) * step + out := []float64{} - for v := 0.0; v <= max+step/1e9; v += step { + for v := 0.0; v <= last+step/1e9; v += step { out = append(out, v) } return out diff --git a/internal/card/card_test.go b/internal/card/card_test.go index baee9681..ef21e3af 100644 --- a/internal/card/card_test.go +++ b/internal/card/card_test.go @@ -158,6 +158,26 @@ func TestFormatInt(t *testing.T) { } } +// TestNiceTicksCoversMax guards the key invariant: the last tick returned +// must be ≥ the requested max, or bar-chart cards render bars taller than +// the chart area and poke into the title. Regression case: max=625 step=100 +// previously gave ticks stopping at 600 and bars at 625/600 = 104 % of +// chartH. +func TestNiceTicksCoversMax(t *testing.T) { + cases := []float64{625, 99, 101, 1, 7, 49, 999, 1000, 1001} + for _, m := range cases { + ticks := niceTicks(m, 5) + if len(ticks) == 0 { + t.Errorf("niceTicks(%v): empty", m) + continue + } + last := ticks[len(ticks)-1] + if last < m { + t.Errorf("niceTicks(%v): last tick %v < max — bars will overflow", m, last) + } + } +} + func TestEscapeXML(t *testing.T) { got := escapeXML(``) want := "<a & "b" 'c'>"