From 92b6f6ecf9c969391fc5997eb86b2bf91d888246 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 8 Sep 2026 16:10:51 +0000 Subject: [PATCH] Graph: the y-axis minimum belongs to the plot, not the time axis Closes #69. With both `axis` and `timeAxis`, the minimum was written at the bottom row -- which the time axis had already claimed. The two labels ended up side by side with nothing between them, so a chart rendered "$0" and "08-10" as "$008-10". It is visible today in the CoinPay frame on hqtui.com/apps. The minimum marks the bottom of the *plot*, and a time axis takes that row away, so it belongs one row higher whenever one is present. Decided before the labels are written rather than after, because by then the surface has already been subdivided. Fixed in the TypeScript reference and in every port that reimplements the widget: Rust, Go, Python, Zig and C++. The COBOL port renders through the TypeScript library and uses neither option, so it inherits the fix rather than needing one. ## Why no fixture caught it Because none covered the combination. `graph-axis` and `graph-timeaxis` each existed alone, and `timeAxis` appears nowhere in the demo either, so regenerating produced no drift at all. A `graph-axis-timeaxis` scene is added, which is what now holds the five ports to this. The conformance harnesses earned their keep on the way: each refused the new fixture outright rather than silently skipping it, which is the whole point of matching scenes by name. 238 pass under Bun and Node; Rust, Go, Python, Zig and C++ suites green, each comparing 54 widget scenes over 5584 cells. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017Df2FNu5DhinMV2soRz3cy --- packages/hqtui/src/widgets/meters.ts | 13 +- packages/hqtui/test/graph-axis.test.ts | 66 +++++ ports/conformance/fixtures/widgets.json | 248 ++++++++++++++++++ ports/conformance/generate.ts | Bin 42612 -> 43030 bytes ports/cpp/src/widgets.cpp | 12 +- ports/cpp/tests/conformance_widgets.cpp | 12 + ports/go/conformance_widgets_test.go | 10 + ports/go/widgets_meters.go | 11 +- ports/python/hqtui/widgets/meters.py | 10 +- .../python/tests/test_conformance_widgets.py | 12 + ports/rust/src/widgets/meters.rs | 14 +- ports/rust/tests/conformance_widgets.rs | 10 + ports/zig/src/conformance_widgets.zig | 9 + ports/zig/src/widgets/meters.zig | 11 +- 14 files changed, 431 insertions(+), 7 deletions(-) create mode 100644 packages/hqtui/test/graph-axis.test.ts diff --git a/packages/hqtui/src/widgets/meters.ts b/packages/hqtui/src/widgets/meters.ts index 6633702..2928e4b 100644 --- a/packages/hqtui/src/widgets/meters.ts +++ b/packages/hqtui/src/widgets/meters.ts @@ -160,6 +160,16 @@ export function drawGraph(surface: Surface, options: GraphOptions): void { let plotSurface = surface; const axisColor = options.axisColor ?? theme.muted; + /** + * Whether a row at the bottom belongs to the time axis rather than the plot. + * + * Decided before the y-axis labels are written, because the minimum marks the + * bottom of the *plot* and the time axis takes that row away. Writing it at + * `height - 1` regardless put it on the time-axis row, immediately left of the + * first time label and against it: "$0" and "08-10" rendered as "$008-10". + */ + const timeAxisRow = Boolean(options.timeAxis) && surface.height > 2; + if (options.axis) { // Match the window the plot itself will use, so the labels stay truthful. const columns = (options.mode ?? "braille") === "braille" ? surface.width * 2 : surface.width; @@ -172,7 +182,8 @@ export function drawGraph(surface: Surface, options: GraphOptions): void { const labelWidth = Math.max(stringWidth(format(max)), stringWidth(format(min))) + 1; surface.text(0, 0, fit(format(max), labelWidth, "right"), { fg: axisColor }); if (surface.height > 1) { - surface.text(0, surface.height - 1, fit(format(min), labelWidth, "right"), { fg: axisColor }); + const bottom = timeAxisRow ? surface.height - 2 : surface.height - 1; + surface.text(0, bottom, fit(format(min), labelWidth, "right"), { fg: axisColor }); } plotSurface = surface.sub(labelWidth, 0, surface.width - labelWidth, surface.height); } diff --git a/packages/hqtui/test/graph-axis.test.ts b/packages/hqtui/test/graph-axis.test.ts new file mode 100644 index 0000000..3c66eb4 --- /dev/null +++ b/packages/hqtui/test/graph-axis.test.ts @@ -0,0 +1,66 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { renderToText } from "../src/testing.ts"; + +/** + * The y-axis minimum marks the bottom of the plot. When a time axis is present + * it takes the last row, so the minimum belongs one row above it — otherwise + * the two labels sit side by side on the same row and read as one number. + */ +const VALUES = [0, 20, 45, 30, 80, 65, 90, 40, 55, 70]; +const TIME = ["08-10", "08-14", "08-18", "08-22", "08-26"]; + +function frame(options: Record, width = 60, height = 10): string[] { + return renderToText(({ ui }) => ui.graph({ values: VALUES, ...options }), { width, height }) + .split("\n"); +} + +test("the y-axis minimum sits on the plot's bottom row, not the time axis", () => { + const lines = frame({ axis: true, min: 0, max: 100, timeAxis: TIME }); + const axisRow = lines[lines.length - 2] ?? ""; + const timeRow = lines[lines.length - 1] ?? ""; + + assert.match(axisRow, /^\s*0/, "the minimum is one row above the time axis"); + assert.match(timeRow, /08-10/, "the time axis has the bottom row to itself"); + // The bug: the minimum written immediately left of the first time label, so + // the row read "008-10". A digit directly against the label is the signature; + // `0*` would have matched the correct output too, since it allows none. + assert.doesNotMatch(timeRow, /\d08-10/, "no y-axis label fused onto the time axis"); +}); + +test("without a time axis the minimum keeps the bottom row", () => { + const lines = frame({ axis: true, min: 0, max: 100 }); + assert.match(lines[lines.length - 1] ?? "", /^\s*0/); +}); + +test("the maximum stays on the top row either way", () => { + assert.match(frame({ axis: true, min: 0, max: 100 })[0] ?? "", /100/); + assert.match(frame({ axis: true, min: 0, max: 100, timeAxis: TIME })[0] ?? "", /100/); +}); + +test("a formatted axis does not fuse either", () => { + const lines = frame({ + axis: true, + min: 0, + max: 6000, + timeAxis: TIME, + axisFormat: (v: number) => `$${Math.round(v / 1000)}K`, + }); + const timeRow = lines[lines.length - 1] ?? ""; + assert.match(lines[lines.length - 2] ?? "", /\$0K/); + // This is the exact shape seen on hqtui.com/apps: "$008-10". + assert.doesNotMatch(timeRow, /\$0K?08-10/); + assert.match(timeRow, /08-10/); +}); + +test("a two-row graph with a time axis does not lose the minimum off-surface", () => { + // Too short to give the time axis its own row above the minimum; the point is + // that it renders rather than writing outside the surface. + assert.doesNotThrow(() => frame({ axis: true, min: 0, max: 100, timeAxis: TIME }, 40, 2)); + assert.doesNotThrow(() => frame({ axis: true, min: 0, max: 100, timeAxis: TIME }, 40, 3)); +}); + +test("the time axis alone still owns the bottom row", () => { + const lines = frame({ timeAxis: TIME }); + assert.match(lines[lines.length - 1] ?? "", /08-10/); +}); diff --git a/ports/conformance/fixtures/widgets.json b/ports/conformance/fixtures/widgets.json index 728e373..7421db6 100644 --- a/ports/conformance/fixtures/widgets.json +++ b/ports/conformance/fixtures/widgets.json @@ -11075,6 +11075,254 @@ ] } }, + { + "name": "graph-axis-timeaxis", + "width": 34, + "height": 8, + "result": { + "width": 34, + "height": 8, + "chars": [ + [ + 1, + 32 + ], + [ + 1, + 49 + ], + [ + 2, + 48 + ], + [ + 203, + 32 + ], + [ + 1, + 48 + ], + [ + 1, + 10276 + ], + [ + 1, + 10258 + ], + [ + 1, + 10274 + ], + [ + 1, + 10276 + ], + [ + 1, + 10258 + ], + [ + 1, + 10274 + ], + [ + 1, + 10276 + ], + [ + 3, + 10258 + ], + [ + 1, + 10276 + ], + [ + 1, + 10432 + ], + [ + 1, + 10276 + ], + [ + 1, + 10260 + ], + [ + 1, + 10258 + ], + [ + 1, + 10276 + ], + [ + 1, + 10260 + ], + [ + 2, + 10258 + ], + [ + 1, + 10274 + ], + [ + 1, + 10276 + ], + [ + 1, + 10258 + ], + [ + 1, + 10274 + ], + [ + 1, + 10276 + ], + [ + 1, + 10260 + ], + [ + 1, + 10274 + ], + [ + 1, + 10372 + ], + [ + 1, + 10336 + ], + [ + 1, + 10276 + ], + [ + 1, + 10258 + ], + [ + 4, + 32 + ], + [ + 1, + 54 + ], + [ + 1, + 48 + ], + [ + 1, + 115 + ], + [ + 12, + 32 + ], + [ + 1, + 51 + ], + [ + 1, + 48 + ], + [ + 1, + 115 + ], + [ + 10, + 32 + ], + [ + 1, + 48 + ], + [ + 1, + 115 + ] + ], + "fg": [ + [ + 4, + 22702973 + ], + [ + 200, + 29806811 + ], + [ + 4, + 22702973 + ], + [ + 30, + 22587135 + ], + [ + 4, + 29806811 + ], + [ + 3, + 22702973 + ], + [ + 12, + 29806811 + ], + [ + 3, + 22702973 + ], + [ + 10, + 29806811 + ], + [ + 2, + 22702973 + ] + ], + "bg": [ + [ + 272, + 17106698 + ] + ], + "attrs": [ + [ + 272, + 0 + ] + ], + "clusters": [], + "text": [ + " 100 ", + " ", + " ", + " ", + " ", + " ", + " 0⠤⠒⠢⠤⠒⠢⠤⠒⠒⠒⠤⣀⠤⠔⠒⠤⠔⠒⠒⠢⠤⠒⠢⠤⠔⠢⢄⡠⠤⠒", + " 60s 30s 0s" + ] + } + }, { "name": "sparkline-widget", "width": 30, diff --git a/ports/conformance/generate.ts b/ports/conformance/generate.ts index 6fa824d97323da0a35ed6eb77c201a095d82ad0d..434d296c4756e88fd414e676e67ffa85089f3084 100644 GIT binary patch delta 382 zcmX|+u}Z{15QgEpv$3=OSuCzd?s5ns*D3f2Vj&iL$y|0J*_GXm$rXAQ<_Q8mhamVA z7Cwns2)0gg=WQ`N|I9xhGd~}rm(S5_*=}YtT-cxxY)(Ouo8nTY3bph|?L9flAPZ|K zLaj76w1=`a*ot*58Vb9KqP2Q*H9; zfb%ICnuy%VJ2fmaj{BYZJG$%VN%!6!UJXW|uPqQ= 1000 ? number(std::floor(v / 100 + .5) / 10) + "k" : number(v); }; + // Whether the bottom row belongs to the time axis rather than the plot. + // Decided before the y-axis labels are written: the minimum marks the bottom + // of the *plot*, and the time axis takes that row away. Writing it at + // height - 1 regardless put it against the first time label, so "$0" and + // "08-10" rendered as "$008-10". + const bool time_axis_row = !o.time_axis.empty() && s.rect().height > 2; if (o.axis) { double hi = o.max.value_or(highFor(s.rect().width * mult)); int lw = std::max(width(label(hi)), width(label(o.min))) + 1; text(s, 0, 0, fit(label(hi), lw, HQ_RIGHT), t.muted); - if (s.rect().height > 1) - text(s, 0, s.rect().height - 1, fit(label(o.min), lw, HQ_RIGHT), t.muted); + if (s.rect().height > 1) { + const int bottom = time_axis_row ? s.rect().height - 2 : s.rect().height - 1; + text(s, 0, bottom, fit(label(o.min), lw, HQ_RIGHT), t.muted); + } s = s.sub({lw, 0, std::max(0, s.rect().width - lw), s.rect().height}); } // The time axis costs the bottom row, and the plot gets what is left. diff --git a/ports/cpp/tests/conformance_widgets.cpp b/ports/cpp/tests/conformance_widgets.cpp index e51aa93..dbaaf84 100644 --- a/ports/cpp/tests/conformance_widgets.cpp +++ b/ports/cpp/tests/conformance_widgets.cpp @@ -300,6 +300,18 @@ bool draw_scene(const std::string &name, Surface s) { draw_graph(s, g); return true; } + // Both axes together. Each was covered alone, which is how the y-axis minimum + // came to be drawn onto the time-axis row with no fixture noticing. + if (name == "graph-axis-timeaxis") { + Graph g; + g.series = {{kSeries, 0, "", false}}; + g.axis = true; + g.min = 0; + g.max = 100; + g.time_axis = {"60s", "30s", "0s"}; + draw_graph(s, g); + return true; + } if (name == "table-scrollbar") { Table table; table.columns = {{"#", -1, 1, 0, HQ_RIGHT}, {"VALUE"}}; diff --git a/ports/go/conformance_widgets_test.go b/ports/go/conformance_widgets_test.go index 16b105d..cc71bce 100644 --- a/ports/go/conformance_widgets_test.go +++ b/ports/go/conformance_widgets_test.go @@ -104,6 +104,16 @@ func drawWidgetScene(t *testing.T, name string, s Surface) { }) case "graph-timeaxis": DrawGraph(s, GraphOptions{Values: series(), TimeAxis: []string{"60s", "30s", "0s"}}) + // Both axes together. Each was covered alone, which is how the y-axis + // minimum came to be drawn onto the time-axis row with no fixture noticing. + case "graph-axis-timeaxis": + axisMin, axisMax := 0.0, 100.0 + DrawGraph(s, GraphOptions{ + Values: series(), + Axis: true, + Plot: PlotOptions{Min: &axisMin, Max: &axisMax}, + TimeAxis: []string{"60s", "30s", "0s"}, + }) case "sparkline-widget": DrawSparkline(s, SparklineWidgetOptions{Values: series(), Label: "net", Text: "1.2M"}) case "table": diff --git a/ports/go/widgets_meters.go b/ports/go/widgets_meters.go index 2c21ec8..cab4209 100644 --- a/ports/go/widgets_meters.go +++ b/ports/go/widgets_meters.go @@ -230,6 +230,8 @@ func DrawGraph(s Surface, o GraphOptions) { } plotSurface := s + // Decided before the y-axis labels are written; see the minimum below. + timeAxisRow := len(o.TimeAxis) > 0 && s.Height() > 2 axisColor := theme.Muted if o.AxisColor != nil { axisColor = *o.AxisColor @@ -269,7 +271,14 @@ func DrawGraph(s Surface, o GraphOptions) { labelWidth := max(StringWidth(format(maxV)), StringWidth(format(minV))) + 1 s.Text(0, 0, Fit(format(maxV), labelWidth, AlignRight), TextOptions{Fg: &axisColor}) if s.Height() > 1 { - s.Text(0, s.Height()-1, Fit(format(minV), labelWidth, AlignRight), + // The minimum marks the bottom of the plot, and a time axis takes + // that row away. Writing it at Height()-1 regardless put it against + // the first time label, so "$0" and "08-10" rendered as "$008-10". + bottom := s.Height() - 1 + if timeAxisRow { + bottom = s.Height() - 2 + } + s.Text(0, bottom, Fit(format(minV), labelWidth, AlignRight), TextOptions{Fg: &axisColor}) } plotSurface = s.Sub(labelWidth, 0, max(0, s.Width()-labelWidth), s.Height()) diff --git a/ports/python/hqtui/widgets/meters.py b/ports/python/hqtui/widgets/meters.py index 4e77105..5f5c525 100644 --- a/ports/python/hqtui/widgets/meters.py +++ b/ports/python/hqtui/widgets/meters.py @@ -259,6 +259,13 @@ def draw_graph(surface: Surface, options: GraphOptions) -> None: plot_surface = surface axis_color = options.axis_color if options.axis_color is not None else theme.muted + # Whether the bottom row belongs to the time axis rather than the plot. + # Decided before the y-axis labels are written: the minimum marks the bottom + # of the *plot*, and the time axis takes that row away. Writing it at + # height - 1 regardless put it against the first time label, so "$0" and + # "08-10" rendered as "$008-10". + time_axis_row = bool(options.time_axis) and surface.height > 2 + if options.axis: # Match the window the plot itself will use, so the labels stay truthful. columns = surface.width * 2 if options.plot.mode == FillMode.BRAILLE else surface.width @@ -277,8 +284,9 @@ def draw_graph(surface: Surface, options: GraphOptions) -> None: 0, 0, fit(fmt(maximum), label_width, Align.RIGHT), TextOptions(fg=axis_color) ) if surface.height > 1: + bottom = surface.height - 2 if time_axis_row else surface.height - 1 surface.text( - 0, surface.height - 1, fit(fmt(minimum), label_width, Align.RIGHT), + 0, bottom, fit(fmt(minimum), label_width, Align.RIGHT), TextOptions(fg=axis_color), ) plot_surface = surface.sub( diff --git a/ports/python/tests/test_conformance_widgets.py b/ports/python/tests/test_conformance_widgets.py index 789fd45..cd89678 100644 --- a/ports/python/tests/test_conformance_widgets.py +++ b/ports/python/tests/test_conformance_widgets.py @@ -134,6 +134,18 @@ def draw_scene(case, name: str, s: Surface) -> None: ) elif name == "graph-timeaxis": w.draw_graph(s, w.GraphOptions(values=SERIES, time_axis=["60s", "30s", "0s"])) + # Both axes together. Each was covered alone, which is how the y-axis + # minimum came to be drawn onto the time-axis row with no fixture noticing. + elif name == "graph-axis-timeaxis": + w.draw_graph( + s, + w.GraphOptions( + values=SERIES, + axis=True, + plot=PlotOptions(min=0, max=100), + time_axis=["60s", "30s", "0s"], + ), + ) elif name == "sparkline-widget": w.draw_sparkline( s, w.SparklineWidgetOptions(values=SERIES, label="net", text="1.2M") diff --git a/ports/rust/src/widgets/meters.rs b/ports/rust/src/widgets/meters.rs index e1ba340..b5e31d1 100644 --- a/ports/rust/src/widgets/meters.rs +++ b/ports/rust/src/widgets/meters.rs @@ -318,6 +318,13 @@ pub fn draw_graph(surface: &Surface, options: &GraphOptions) { let mut plot_surface = surface.clone(); let axis_color = options.axis_color.unwrap_or(theme.muted); + // Whether the bottom row belongs to the time axis rather than the plot. + // Decided before the y-axis labels are written: the minimum marks the + // bottom of the *plot*, and the time axis takes that row away. Writing it + // at `height - 1` regardless put it against the first time label, so "$0" + // and "08-10" rendered as "$008-10". + let time_axis_row = options.time_axis.is_some() && surface.height() > 2; + if options.axis { // Match the window the plot itself will use, so the labels stay truthful. let columns = if options.plot.mode.unwrap_or(FillMode::Braille) == FillMode::Braille { @@ -348,9 +355,14 @@ pub fn draw_graph(surface: &Surface, options: &GraphOptions) { &TextOptions::new().fg(axis_color), ); if surface.height() > 1 { + let bottom = if time_axis_row { + surface.height() as isize - 2 + } else { + surface.height() as isize - 1 + }; surface.text( 0, - surface.height() as isize - 1, + bottom, &fit(&format(min), label_width, Align::Right), &TextOptions::new().fg(axis_color), ); diff --git a/ports/rust/tests/conformance_widgets.rs b/ports/rust/tests/conformance_widgets.rs index cc2c9b0..b8d1c6f 100644 --- a/ports/rust/tests/conformance_widgets.rs +++ b/ports/rust/tests/conformance_widgets.rs @@ -161,6 +161,16 @@ fn draw_scene(name: &str, s: &Surface) { ..GraphOptions::new(series()) }, ), + // Both axes together. Each was covered alone, which is how the y-axis + // minimum came to be drawn onto the time-axis row without any fixture + // noticing. + "graph-axis-timeaxis" => { + let mut options = GraphOptions::new(series()).with_axis(); + options.plot.min = Some(0.0); + options.plot.max = Some(100.0); + options.time_axis = Some(vec!["60s".into(), "30s".into(), "0s".into()]); + draw_graph(s, &options) + } "sparkline-widget" => draw_sparkline( s, &SparklineWidgetOptions { diff --git a/ports/zig/src/conformance_widgets.zig b/ports/zig/src/conformance_widgets.zig index 8f99762..5e86f27 100644 --- a/ports/zig/src/conformance_widgets.zig +++ b/ports/zig/src/conformance_widgets.zig @@ -115,6 +115,15 @@ fn drawScene(allocator: std.mem.Allocator, name: []const u8, s: Surface) !void { .values = &series, .time_axis = &.{ "60s", "30s", "0s" }, }); + } else if (eq(u8, name, "graph-axis-timeaxis")) { + // Both axes together. Each was covered alone, which is how the y-axis + // minimum came to be drawn onto the time-axis row unnoticed. + try w.drawGraph(allocator, s, .{ + .values = &series, + .axis = true, + .plot = .{ .min = 0, .max = 100 }, + .time_axis = &.{ "60s", "30s", "0s" }, + }); } else if (eq(u8, name, "sparkline-widget")) { w.drawSparkline(s, .{ .values = &series, .label = "net", .text = "1.2M" }); } else if (eq(u8, name, "table")) { diff --git a/ports/zig/src/widgets/meters.zig b/ports/zig/src/widgets/meters.zig index 8abbe1e..a36db04 100644 --- a/ports/zig/src/widgets/meters.zig +++ b/ports/zig/src/widgets/meters.zig @@ -263,6 +263,8 @@ pub fn drawGraph(allocator: std.mem.Allocator, s: Surface, options: GraphOptions const series = options.series orelse &single; var plot_surface = s; + // Decided before the y-axis labels are written; see the minimum below. + const time_axis_row = options.time_axis.len > 0 and s.height() > 2; const axis_color = options.axis_color orelse theme.muted; if (options.axis) { @@ -298,9 +300,16 @@ pub fn drawGraph(allocator: std.mem.Allocator, s: Surface, options: GraphOptions _ = s.text(0, 0, unicode.fit(&padded, max_label, label_width, .right), .{ .fg = axis_color }); if (s.height() > 1) { var padded_min: [64]u8 = undefined; + // The minimum marks the bottom of the plot, and a time axis takes + // that row away. Writing it at height - 1 regardless put it against + // the first time label, so "$0" and "08-10" rendered as "$008-10". + const bottom: isize = if (time_axis_row) + @intCast(s.height() - 2) + else + @intCast(s.height() - 1); _ = s.text( 0, - @intCast(s.height() - 1), + bottom, unicode.fit(&padded_min, min_label, label_width, .right), .{ .fg = axis_color }, );