Repository navigation
Compose chart axis labels with the locale, not with string glue - #83
Merged
Merged
Conversation
The x-axis composed its label by gluing a translated month name onto a day
number, which hardcodes a Latin month-first shape and a space separator. A
lang=zh viewer read "7月 16" for a day bucket, where zh wants 7月16日, and
"7月 26" for a monthly bucket, which reads as the 26th of July rather than
July 2026. The same glue put the month first for sv and id, whose locales
order the day first ("16 juli", "16 Jul").
fmtLabel now builds a UTC Date from the bucket label and formats it with
Intl.DateTimeFormat, the call the tooltip formatter (fmtBucketDate) and the
server-side formatBucketLabel already make. The month branch carries a full
year: a two-digit year is not an option, because zh ICU renders it
"26年7月", which reads as the year 26.
The shape of the label now picks the branch, not the range, which drops the
range list that had to agree with the bucketing in click-repository.ts. The
month.* translation keys are left in place: the client no longer reads them,
but zh.ts is in flight in another change, and dropping keys from one locale
table at a time fails the key-parity test.
New client-chart-labels.test.ts extracts the shipped function from
adminClientScript() output and runs it per language. The chunk is evaluated
with no t() in scope, so a return to the glue throws instead of rendering a
wrong label. Mutating the year option or dropping the day option fails the
suite.
Follow-up to the review of #77, whose locale-aware day and month labels this builds on. - Hourly labels glued "HH:00" and keyed off range === '24h'. They now come from Intl like the tooltip, so en reads "09:00 AM" on both and id "09.00". The label's shape picks the format; fmtLabel takes no range. - A daily or weekly axis that crosses a year boundary printed "Oct 20" for 2025 and "Oct 5" for 2026 with nothing to tell them apart. axisLabels() now adds the year to the first label and wherever the year changes. - Full-year monthly labels ("Sep 2026", "2026年9月", "sep. 2026") overlapped the final "Today" label 30 units away. axisLabels() estimates label widths for the 9px monospace axis and drops the neighbor that would collide with today. - bucketDate() parses a bucket label once for the axis and the tooltip. The server-side formatBucketLabel stays separate: the client ships as a generated script string and cannot import it. - The Intl formatters are built once per page, like numberFormatter. Tests iterate SUPPORTED_LANGUAGES, load the client script once per language, and replace the weekly-vs-daily comparison that could not fail with assertions on the weekly label's month and day.
The chart axis formats month names with Intl now, so nothing reads the twelve month.* keys. They still shipped in every admin page's translation table, and translators kept maintaining names the UI never showed. #71 has landed, so all four locales drop them together and the key-parity test in i18n.test.ts stays green.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
shrtnr | 0c810c6 | Nov 02 2026, 01:06 PM |
DennisAlund
commented
Nov 2, 2026
DennisAlund
left a comment
DennisAlund
left a comment
Member
Author
There was a problem hiding this comment.
Code review (xhigh). 12 findings, inline below. No high-severity bug: the year marks and the today guard behave as described for en, id, sv and zh across every range shape, including New Year crossings. The two worth fixing first: Intl.DateTimeFormat#format throws on an Invalid Date where the old path degraded to text, and labels dropped by the collision guard keep their emphasized dot.
Checked by running the shipped axisLabels under Node for every range shape and language. client-chart-labels.test.ts and i18n.test.ts pass (42 tests).
Addresses the review on #83. - Intl.DateTimeFormat#format throws on an invalid date, where the old toLocaleDateString path printed "Invalid Date". bucketDate() now returns null for a label in no known shape, and the formatters show it as it arrived, so the rest of the analytics panel still renders. - The label cadence counts back from the final bucket, so today always sits a full interval from its neighbor. The overlap guard stays for wide year labels, such as zh "2025年12月20日" beside "12月21日". - Emphasized dots follow the kept labels. A label dropped for overlap used to leave its dot behind. - The axis hour uses hour: 'numeric' ("1:00 PM", "9.00"). The tooltip keeps 2-digit to match the bundle chart's server-rendered tooltip, which big-chart.test.ts pins. - The date formatters are built on first use, so admin pages without a chart skip them. AXIS_FONT_SIZE feeds both the <text> elements and the width estimate. - axisLabels reads the x positions from pts instead of a copied array. The tests now run the real renderTimeline() with each locale's translation table and read the SVG. That drops the copied chart geometry and checks each locale's real today text. It also covers the regular-label drop branch, the dot alignment and the lazy formatters.
DennisAlund
added a commit
that referenced
this pull request
Nov 3, 2026
…tches Patch release for the fixes merged since 0.43.0: the chart axis formats labels with Intl instead of string glue (#83), the admin's Chinese pages declare lang="zh-Hans" (#84), and hono and zod pick up published advisory fixes (#82) alongside the dependency upgrade (#81). The spec embeds info.version, so the bump refreshes x-spec-hash in all three SDKs. The emitted spec differs from 0.43.0 in info.version only: spec change does not affect SDK surface; bump hash to record review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Takes over #77 by @myml, with the review findings on it addressed. The first commit is #77's commit cherry-picked unchanged, authored by the contributor.
What #77 fixed
The x-axis glued a translated month name onto a day number. zh read
7月 16for a day and7月 26for July 2026, and sv and id got the month first.fmtLabelnow formats throughIntl.DateTimeFormat, and monthly labels carry the full year.Review findings addressed
HH:00and keyed offrange09:00 AMin en,09.00in id). The label's shape picks the format.fmtLabeltakes no range.axisLabels()adds the year on the first label and wherever the year changes, only when the axis crosses a year.TodayaxisLabels()estimates label widths (0.6em Latin, 1em CJK, 9px mono) and drops the neighbor that would overlapToday.month.*keys deadSUPPORTED_LANGUAGES.bucketDate()serves both the axis and the tooltip. The server-sideformatBucketLabelstays separate: the client ships as a generated script string and cannot import it.BUCKET_FORMATSbuilt once per page, likenumberFormatter.Second review round
Addressed in
0c810c6: invalid bucket labels no longer throw, emphasized dots follow the kept labels, the label cadence counts back from today, the axis hour uses the locale's own form, the formatters build on first use, andAXIS_FONT_SIZEfeeds the width estimate. The tests now run the realrenderTimeline()with each locale's translation table.Tests
yarn test: 109 files, 1701 tests pass.tsc --noEmit: clean.yarn e2e: 38 pass.client-chart-labels.test.ts.Supersedes #77.