From 8a17e9ed218f34ace1e10e1aca4f19a11a39a3b4 Mon Sep 17 00:00:00 2001 From: m5r Date: Fri, 21 Aug 2026 23:33:32 +0200 Subject: [PATCH] admin: fix dashboard phantom scroll, drop last-failure column from upstream table the chart's screen-reader table wore srOnly directly; overflow and height do not apply to a table box, so it laid out 1200px tall below the page while clip-path hid the paint. wrap it in a hidden div, which clips properly and keeps the table role. failure detail is the diagnostics page's job since milestone 27; the column and the now dead formatAge go. --- .../features/dashboard/DashboardPage.test.tsx | 1 - .../dashboard/TimeseriesChart.test.tsx | 51 +++++++++++++++++++ .../features/dashboard/TimeseriesChart.tsx | 46 +++++++++-------- .../dashboard/UpstreamHealthTable.test.tsx | 35 ++++--------- .../dashboard/UpstreamHealthTable.tsx | 23 ++------- admin/src/lib/format.test.ts | 24 ++++----- admin/src/lib/format.ts | 18 ++----- admin/src/ui/styles.ts | 10 +++- 8 files changed, 111 insertions(+), 97 deletions(-) create mode 100644 admin/src/features/dashboard/TimeseriesChart.test.tsx diff --git a/admin/src/features/dashboard/DashboardPage.test.tsx b/admin/src/features/dashboard/DashboardPage.test.tsx index 2a755ca..ac5b8ca 100644 --- a/admin/src/features/dashboard/DashboardPage.test.tsx +++ b/admin/src/features/dashboard/DashboardPage.test.tsx @@ -193,7 +193,6 @@ test("dashboard renders stats, chart, disk card, upstream table and health banne expect(screen.getByText("https://dns.example/dns-query")).toBeTruthy(); expect(screen.getByText("90.0%")).toBeTruthy(); expect(screen.getByText("100.0%")).toBeTruthy(); - expect(screen.getByText("timeout · 3h ago")).toBeTruthy(); expect(screen.getByText("1/2 available")).toBeTruthy(); }); diff --git a/admin/src/features/dashboard/TimeseriesChart.test.tsx b/admin/src/features/dashboard/TimeseriesChart.test.tsx new file mode 100644 index 0000000..5ab7f22 --- /dev/null +++ b/admin/src/features/dashboard/TimeseriesChart.test.tsx @@ -0,0 +1,51 @@ +import { render, screen, within } from "@testing-library/react"; +import * as stylex from "@stylexjs/stylex"; +import type { StatsTimeseries } from "@/lib/types"; +import { styles as shared } from "@/ui/styles"; +import TimeseriesChart from "./TimeseriesChart"; + +const SINCE = 1_700_000_000; + +function timeseries(bucketCount: number): StatsTimeseries { + return { + period: "24h", + since: SINCE, + until: SINCE + bucketCount * 1800, + bucket_seconds: 1800, + buckets: Array.from({ length: bucketCount }, (_, i) => ({ + ts: SINCE + i * 1800, + queries: i + 1, + blocked: 1, + cached: 1, + })), + }; +} + +/** The element wearing the shared hidden style, found by its compiled classes. */ +function hiddenElement(container: HTMLElement): Element | null { + const classes = stylex.props(shared.srOnly).className?.split(" ").filter(Boolean) ?? []; + expect(classes.length).toBeGreaterThan(0); + return container.querySelector(classes.map((name) => `.${name}`).join("")); +} + +test("the data table is the SVG's accessible equivalent", () => { + render(); + + expect(screen.getByRole("img", { name: /queries over time/i })).toBeTruthy(); + const table = screen.getByRole("table", { name: "Queries per time bucket" }); + expect(within(table).getAllByRole("row").length).toBe(4); +}); + +/** + * `overflow` does not apply to a table box and `height` on one is a minimum, so + * the hidden style has to sit on a block container wrapping the table. Worn by + * the table itself it clips the paint but not the layout, and 48 invisible rows + * push the document's scroll height a screen past the app shell. + */ +test("the hidden data table is clipped by a block wrapper, not by the table itself", () => { + const { container } = render(); + + const hidden = hiddenElement(container); + expect(hidden?.tagName).toBe("DIV"); + expect(hidden?.querySelector("table")).not.toBeNull(); +}); diff --git a/admin/src/features/dashboard/TimeseriesChart.tsx b/admin/src/features/dashboard/TimeseriesChart.tsx index fec64bb..92705e3 100644 --- a/admin/src/features/dashboard/TimeseriesChart.tsx +++ b/admin/src/features/dashboard/TimeseriesChart.tsx @@ -292,29 +292,31 @@ export default function TimeseriesChart({ data }: { data: StatsTimeseries }) { ))} - - - - - - - - - - - - - {layout.bars.map((bar) => ( - - - - - - +
+
Queries per time bucket
TimeQueriesBlockedCachedOther
{formatTime(bar.bucket.ts)}{bar.bucket.queries}{bar.bucket.blocked}{bar.bucket.cached}{bar.other}
+ + + + + + + + - ))} - -
Queries per time bucket
TimeQueriesBlockedCachedOther
+ + + {layout.bars.map((bar) => ( + + {formatTime(bar.bucket.ts)} + {bar.bucket.queries} + {bar.bucket.blocked} + {bar.bucket.cached} + {bar.other} + + ))} + + + ); } diff --git a/admin/src/features/dashboard/UpstreamHealthTable.test.tsx b/admin/src/features/dashboard/UpstreamHealthTable.test.tsx index 8283613..0b081e7 100644 --- a/admin/src/features/dashboard/UpstreamHealthTable.test.tsx +++ b/admin/src/features/dashboard/UpstreamHealthTable.test.tsx @@ -4,14 +4,6 @@ import UpstreamHealthTable from "./UpstreamHealthTable"; const NOW_S = 1_700_000_000; -beforeEach(() => { - vi.spyOn(Date, "now").mockReturnValue(NOW_S * 1000); -}); - -afterEach(() => { - vi.restoreAllMocks(); -}); - const ZERO: UpstreamPeriodStats = { attempts: 0, successes: 0, @@ -27,7 +19,6 @@ function period(overrides: Partial = {}): UpstreamPeriodSta successes: 90, failures: 10, success_rate: 0.9, - // 3h30m ago, far from a unit boundary. last_failure_at: NOW_S - 12_600, last_failure_error: "Timeout", ...overrides, @@ -69,7 +60,7 @@ test("the ranged columns sit under a header naming the selected period", () => { renderTable([entry()]); expect(screen.getByRole("columnheader", { name: "Selected period · 24h" })).toBeTruthy(); - for (const name of ["Upstream", "Status now", "Attempts", "Failures", "Success rate", "Last failure"]) { + for (const name of ["Upstream", "Status now", "Attempts", "Failures", "Success rate"]) { expect(screen.getByRole("columnheader", { name })).toBeTruthy(); } @@ -78,6 +69,14 @@ test("the ranged columns sit under a header naming the selected period", () => { expect(screen.queryByRole("columnheader", { name: "Available" })).toBeNull(); }); +test("failure detail is the Diagnostics page's job; the card never shows it", () => { + renderTable([entry()]); + + expect(screen.queryByRole("columnheader", { name: "Last failure" })).toBeNull(); + expect(screen.queryByText(/Timeout/)).toBeNull(); + expect(screen.queryByText(/ago$/)).toBeNull(); +}); + test("status now is one word from live state, not from the window", () => { renderTable([ entry({ url: "https://a.example/dns-query" }), @@ -90,20 +89,7 @@ test("status now is one word from live state, not from the window", () => { expect(within(rowOf("https://c.example/dns-query")).getByText("Disabled")).toBeTruthy(); }); -test("last failure pairs the error name with its age, em-dash when the window holds none", () => { - renderTable([ - entry({ url: "https://a.example/dns-query" }), - entry({ - url: "https://b.example/dns-query", - period: period({ last_failure_at: null, last_failure_error: null }), - }), - ]); - - expect(within(rowOf("https://a.example/dns-query")).getByText("Timeout · 3h ago")).toBeTruthy(); - expect(within(rowOf("https://b.example/dns-query")).getByText("—")).toBeTruthy(); -}); - -test("a window with no attempts renders em-dashes and never a perfect rate", () => { +test("a window with no attempts renders an em-dash and never a perfect rate", () => { renderTable([entry({ period: ZERO })]); const cells = within(rowOf("https://dns.example/dns-query")).getAllByRole("cell"); @@ -113,7 +99,6 @@ test("a window with no attempts renders em-dashes and never a perfect rate", () "0", "0", "—", - "—", ]); expect(screen.queryByText("100.0%")).toBeNull(); expect(screen.queryByText("0.0%")).toBeNull(); diff --git a/admin/src/features/dashboard/UpstreamHealthTable.tsx b/admin/src/features/dashboard/UpstreamHealthTable.tsx index 5646b60..9250dc4 100644 --- a/admin/src/features/dashboard/UpstreamHealthTable.tsx +++ b/admin/src/features/dashboard/UpstreamHealthTable.tsx @@ -1,5 +1,4 @@ import * as stylex from "@stylexjs/stylex"; -import { formatAge } from "@/lib/format"; import type { UpstreamHealth, UpstreamHealthEntry, UpstreamPeriodStats } from "@/lib/types"; import { styles as shared } from "@/ui/styles"; import { colors } from "@/ui/tokens.stylex"; @@ -137,19 +136,11 @@ function successRate(period: UpstreamPeriodStats): string { } /** - * The age is formatted once, when the row renders; nothing here ticks. It is - * measured against the browser's clock rather than the response's `until`, so a - * cached response ages visibly instead of freezing at the moment it was served. + * The dashboard answers availability only. Failure detail — what failed, when, + * and how often — is the Diagnostics page's job, so `last_failure_at` and + * `last_failure_error` are read there rather than repeated in this row. */ -function lastFailure(period: UpstreamPeriodStats, nowSeconds: number): string { - if (period.last_failure_at === null) return "—"; - const age = formatAge(Math.max(0, nowSeconds - period.last_failure_at)); - const error = period.last_failure_error; - return error === null || error === "" ? age : `${error} · ${age}`; -} - export default function UpstreamHealthTable({ health }: { health: UpstreamHealth }) { - const nowSeconds = Math.floor(Date.now() / 1000); const idle = health.upstreams.length > 0 && health.upstreams.every(({ period }) => period.attempts === 0); return ( @@ -168,7 +159,7 @@ export default function UpstreamHealthTable({ health }: { health: UpstreamHealth - + Selected period · {health.period} @@ -188,9 +179,6 @@ export default function UpstreamHealthTable({ health }: { health: UpstreamHealth Success rate - - Last failure - @@ -220,9 +208,6 @@ export default function UpstreamHealthTable({ health }: { health: UpstreamHealth {successRate(upstream.period)} - - {lastFailure(upstream.period, nowSeconds)} - ); })} diff --git a/admin/src/lib/format.test.ts b/admin/src/lib/format.test.ts index 2dbddb9..5252db1 100644 --- a/admin/src/lib/format.test.ts +++ b/admin/src/lib/format.test.ts @@ -1,4 +1,4 @@ -import { formatAge, formatBytes, formatDuration, formatMicros, formatTime } from "@/lib/format"; +import { formatBytes, formatDuration, formatMicros, formatTime } from "@/lib/format"; test("formatTime renders unix seconds in the given locale and zone", () => { // 2024-01-01T00:00:00Z; ICU emits U+202F before AM/PM in recent Node. @@ -15,23 +15,19 @@ test("formatBytes humanizes with binary units", () => { expect(formatBytes(2 * 1024 ** 4)).toBe("2.0 TiB"); }); -test("formatAge steps up a unit at each boundary and truncates", () => { - expect(formatAge(0)).toBe("0s ago"); - expect(formatAge(59)).toBe("59s ago"); - expect(formatAge(60)).toBe("1m ago"); - expect(formatAge(3599)).toBe("59m ago"); - expect(formatAge(3600)).toBe("1h ago"); - expect(formatAge(10800)).toBe("3h ago"); - expect(formatAge(86399)).toBe("23h ago"); - expect(formatAge(86400)).toBe("1d ago"); - expect(formatAge(400000)).toBe("4d ago"); -}); - -test("formatDuration is the same span without the 'ago', and never negative", () => { +test("formatDuration steps up a unit at each boundary and truncates", () => { expect(formatDuration(0)).toBe("0s"); expect(formatDuration(59)).toBe("59s"); + expect(formatDuration(60)).toBe("1m"); + expect(formatDuration(3599)).toBe("59m"); expect(formatDuration(3600)).toBe("1h"); + expect(formatDuration(10800)).toBe("3h"); + expect(formatDuration(86399)).toBe("23h"); expect(formatDuration(86400)).toBe("1d"); + expect(formatDuration(400000)).toBe("4d"); +}); + +test("formatDuration is never negative", () => { // Clock skew between the server's timestamps and the browser's clock. expect(formatDuration(-5)).toBe("0s"); }); diff --git a/admin/src/lib/format.ts b/admin/src/lib/format.ts index 815d193..ebccd33 100644 --- a/admin/src/lib/format.ts +++ b/admin/src/lib/format.ts @@ -28,21 +28,9 @@ const AGE_UNITS = [ ] as const; /** - * Seconds of elapsed time → a coarse "3h ago". Truncating and single-unit on - * purpose: this labels a snapshot the caller renders once, so a reader must not - * take it for a live count. Nothing re-renders it as it ages. - */ -export function formatAge(seconds: number): string { - for (const unit of AGE_UNITS) { - if (seconds >= unit.seconds) return `${Math.floor(seconds / unit.seconds)}${unit.suffix} ago`; - } - return `${Math.floor(seconds)}s ago`; -} - -/** - * Seconds of elapsed time → a coarse "3h", the same single truncated unit as - * `formatAge` without the "ago". For a span the caller labels itself, as in - * "active for 3h". A negative span reads "0s": clock skew is not a duration. + * Seconds of elapsed time → a coarse "3h". Truncating and single-unit on + * purpose, for a span the caller labels itself, as in "active for 3h". A + * negative span reads "0s": clock skew is not a duration. */ export function formatDuration(seconds: number): string { for (const unit of AGE_UNITS) { diff --git a/admin/src/ui/styles.ts b/admin/src/ui/styles.ts index 84a2409..32c10aa 100644 --- a/admin/src/ui/styles.ts +++ b/admin/src/ui/styles.ts @@ -193,7 +193,15 @@ export const styles = stylex.create({ mono: { fontFamily: "ui-monospace, SFMono-Regular, Menlo, Monaco, Consolas, monospace", }, - /** Visible to a screen reader only; the element keeps its place in the a11y tree. */ + /** + * Visible to a screen reader only; the element keeps its place in the a11y tree. + * + * Apply it to a block container. `overflow` has no effect on a table box, and + * `height` on one is a minimum, so a `` wearing this still lays out at + * its full content height and pushes the page's scrollable overflow past the + * app shell — invisible, because `clip-path` still hides the paint. Wrap the + * table in a hidden `
` instead of hiding the table itself. + */ srOnly: { position: "absolute", width: 1,