Repository navigation
Compose chart axis labels with the locale, not with string glue #77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,114 @@ | ||
| // Copyright 2026 Oddbit (https://oddbit.id) | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| // The admin client's chart x-axis labels. fmtLabel() composes a canonical | ||
| // bucket label ("YYYY-MM-DD HH" hourly, "YYYY-MM-DD" daily and weekly, | ||
| // "YYYY-MM" monthly) into the text the axis shows, and that composition is a | ||
| // locale decision: which of the month and the day comes first, what separates | ||
| // them, and whether the day or the year carries a counter. | ||
| // | ||
| // These tests cut the shipped function out of adminClientScript() and run it | ||
| // with UI_LANG set, the same way the other client.ts tests do. The chunk is | ||
| // evaluated with no t() in scope, so a version that went back to gluing | ||
| // translated month names onto day numbers throws here rather than rendering a | ||
| // wrong label. | ||
| // | ||
| // Assertions name the components that carry meaning (month, day, year) and | ||
| // the order they appear in, not the whole string: ICU punctuation has shifted | ||
| // between runtime versions. The server-side sibling, formatBucketLabel, is | ||
| // pinned the same way in big-chart.test.ts. | ||
| import { describe, expect, it } from "vitest"; | ||
| import { adminClientScript } from "../../client"; | ||
| import type { Translations } from "../../i18n/types"; | ||
| import { extractTopLevelChunk } from "../client-script"; | ||
|
|
||
| function loadFmtLabel(lang: string) { | ||
| const script = adminClientScript("1.0.0", {} as unknown as Translations); | ||
| const code = [ | ||
| `var UI_LANG = ${JSON.stringify(lang)};`, | ||
| extractTopLevelChunk(script, /^function fmtLabel\(/), | ||
| "return fmtLabel;", | ||
| ].join("\n"); | ||
| // eslint-disable-next-line @typescript-eslint/no-implied-eval | ||
| const factory = new Function(code) as () => (label: string, range: string) => string; | ||
| return factory(); | ||
| } | ||
|
|
||
| describe("client.ts fmtLabel", () => { | ||
| it("formats an hourly bucket as the hour", () => { | ||
| expect(loadFmtLabel("en")("2026-07-16 09", "24h")).toBe("09:00"); | ||
| }); | ||
|
|
||
| it("puts the month first for en", () => { | ||
| const out = loadFmtLabel("en")("2026-07-16", "7d"); | ||
| expect(out).toContain("Jul"); | ||
| expect(out).toContain("16"); | ||
| expect(out.indexOf("16")).toBeGreaterThan(out.search(/[A-Za-z]/)); | ||
| }); | ||
|
|
||
| it("puts the day first for sv and id", () => { | ||
| for (const lang of ["sv", "id"]) { | ||
| const out = loadFmtLabel(lang)("2026-07-16", "7d"); | ||
| expect(out, `${lang} must carry the day`).toContain("16"); | ||
| expect(out.indexOf("16"), `${lang} puts the day before the month`).toBeLessThan( | ||
| out.search(/[A-Za-z]/), | ||
| ); | ||
| } | ||
| }); | ||
|
|
||
| it("uses the Chinese month and day counter for zh", () => { | ||
| // "日" is the day counter zh ICU appends. Gluing the translated month name | ||
| // onto the day number produced "7月 16", which never carried one. | ||
| const out = loadFmtLabel("zh")("2026-07-16", "7d"); | ||
| expect(out).toContain("7月"); | ||
| expect(out).toContain("16"); | ||
| expect(out).toContain("日"); | ||
| expect(out).not.toBe("7月 16"); | ||
| }); | ||
|
|
||
| it("spells the year out on a monthly bucket", () => { | ||
| // The glued form rendered a two-digit year, so a zh viewer read "7月 26" | ||
| // as the 26th of July. A two-digit year is not an option either: zh ICU | ||
| // renders it "26年7月", which reads as the year 26. | ||
| const out = loadFmtLabel("zh")("2026-07", "all"); | ||
| expect(out).toContain("2026"); | ||
| expect(out).toContain("7月"); | ||
| expect(out).not.toContain("NaN"); | ||
| }); | ||
|
|
||
| it("keeps a monthly bucket distinct from the month's first day", () => { | ||
| // A monthly label is built on the 1st. Falling through to the daily shape | ||
| // would render that day instead, which is the bug this pins. | ||
| const fmt = loadFmtLabel("zh"); | ||
| expect(fmt("2026-07", "all")).not.toBe(fmt("2026-07-01", "all")); | ||
| }); | ||
|
|
||
| it("treats a weekly bucket label like a daily one", () => { | ||
| // The 1y range and a long all range bucket by week and still emit | ||
| // YYYY-MM-DD, so the range must not pick the shape, only the label can. | ||
| const fmt = loadFmtLabel("en"); | ||
| expect(fmt("2026-07-13", "1y")).toBe(fmt("2026-07-13", "all")); | ||
| }); | ||
|
|
||
| it("renders no NaN, undefined or empty label for any bucket shape or language", () => { | ||
| const cases: Array<[string, string]> = [ | ||
| ["2026-07-16 09", "24h"], | ||
| ["2026-07-16", "7d"], | ||
| ["2026-07-16", "30d"], | ||
| ["2026-07-16", "90d"], | ||
| ["2026-07-13", "1y"], | ||
| ["2026-07-16", "all"], | ||
| ["2026-07-13", "all"], | ||
| ["2026-07", "all"], | ||
| ]; | ||
| for (const lang of ["en", "id", "sv", "zh"]) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Locale sweep hardcodes Scenario: adding a fifth language (e.g. ja) to |
||
| for (const [label, range] of cases) { | ||
| const out = loadFmtLabel(lang)(label, range); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| const where = `${lang} ${range} ${label}`; | ||
| expect(out, where).not.toContain("NaN"); | ||
| expect(out, where).not.toContain("undefined"); | ||
| expect(out.trim(), where).not.toBe(""); | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -958,24 +958,28 @@ function fmtAvgPerDay(totalClicks, range, createdAt) { | |
| return String(Math.round(avg)); | ||
| } | ||
|
|
||
| var MONTH_KEYS = ['month.jan','month.feb','month.mar','month.apr','month.may','month.jun','month.jul','month.aug','month.sep','month.oct','month.nov','month.dec']; | ||
| function monthName(m) { return t(MONTH_KEYS[m - 1]); } | ||
|
|
||
| // The chart's x-axis label for one bucket. Bucket labels are canonical and | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Impact: 48 dead strings per admin page, and translators keep maintaining month names the UI never shows (sv |
||
| // UTC: "YYYY-MM-DD HH" for hourly, "YYYY-MM-DD" for daily and weekly, and | ||
| // "YYYY-MM" for monthly. | ||
| // | ||
| // The label is composed by Intl rather than by gluing a translated month name | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment breaks the CLAUDE.md writing rules. "Active voice by default. Passive only when actor is unknown or irrelevant." and "Split sentences with more than two clauses." Line 965 reads "The label is composed by Intl rather than by gluing…": passive with Intl named as actor. Lines 965 to 969 form one sentence of four or more clauses, and lines 969 to 971 chain three more. |
||
| // onto a day number, because which of the two comes first, and what sits | ||
| // between them, are locale decisions: en puts the month first ("Jul 16"), sv | ||
| // and id put the day first ("16 juli", "16 Jul"), and zh needs a day counter | ||
| // ("7月16日"). The glue rendered a monthly bucket as "7月 26", which reads as | ||
| // the 26th of July rather than July 2026, and a two-digit year is not the fix | ||
| // either, because zh renders that as "26年7月", the year 26. | ||
| function fmtLabel(label, range) { | ||
| if (range === '24h') { | ||
| // "YYYY-MM-DD HH" → "HH:00" | ||
| return label.slice(11) + ':00'; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hourly axis label still glues strings and keys off Scenario: 24h range, lang=en: axis shows Suggest: detect hourly labels by the space, as |
||
| } | ||
| if (range === '7d' || range === '30d' || range === '90d' || range === '1y') { | ||
| var parts = label.split('-'); | ||
| return monthName(parseInt(parts[1], 10)) + ' ' + parseInt(parts[2], 10); | ||
| } | ||
| // "all": label can be YYYY-MM-DD (daily/weekly) or YYYY-MM (monthly) | ||
| var parts = label.split('-'); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bucket-label parsing duplicated. Risk: a fix for one label shape lands in the axis or the tooltip but not both. Suggest extracting a |
||
| if (parts.length === 3) { | ||
| return monthName(parseInt(parts[1], 10)) + ' ' + parseInt(parts[2], 10); | ||
| } | ||
| return monthName(parseInt(parts[1], 10)) + ' ' + parts[0].slice(2); | ||
| var monthly = parts.length === 2; | ||
| var day = monthly ? 1 : parseInt(parts[2], 10); | ||
| var dt = new Date(Date.UTC(parseInt(parts[0], 10), parseInt(parts[1], 10) - 1, day)); | ||
| if (monthly) return dt.toLocaleDateString(UI_LANG, { year: 'numeric', month: 'short', timeZone: 'UTC' }); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Longer monthly labels now collide with the final Scenario:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Formatter rebuilt per label. |
||
| return dt.toLocaleDateString(UI_LANG, { month: 'short', day: 'numeric', timeZone: 'UTC' }); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Daily/weekly labels omit the year across a year boundary. Same ambiguity the PR fixed for monthly buckets. Scenario: 1y range on 2026-10-20 (53 weekly buckets, dotInterval 5) shows Suggest: add |
||
| } | ||
|
|
||
| // Humanizes a canonical bucket label ("YYYY-MM", "YYYY-MM-DD", or | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"treats a weekly bucket label like a daily one" cannot fail.
fmtLabelignoresrangeunless it is'24h', sofmt(x,'1y') === fmt(x,'all')always holds, and the test never checks that a weekly label shows the day.Scenario: a change that renders YYYY-MM-DD labels without the day, or as month+year, keeps this green because both sides change together. Hardening: assert the output contains
13andJul, and compare against a known daily label.