feat(web): add Ukrainian localization and extract hardcoded copy#6312
Conversation
github-actions
Bot
added
no-rust-required
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
One prose-wrapping nit in the new test code; otherwise no dashboard data-layer, query-key, or mutation-invalidation violations found. The i18n architecture (fallback resolution, key coverage tests, locale registration) looks correct.
Generated by Claude Code
| // Every locale must resolve to the full EN shape after fallback. Raw locale | ||
| // objects may stay partial; getTranslation is the runtime contract. |
There was a problem hiding this comment.
CLAUDE.md prose-wrapping rule: "Break only at sentence boundaries. One sentence = one line, regardless of length."
The line break here falls mid-sentence — "Raw locale" ends one line, "objects may stay partial…" continues on the next. Should be two separate lines, one sentence each:
// Every locale must resolve to the full EN shape after fallback.
// Raw locale objects may stay partial; getTranslation is the runtime contract.Generated by Claude Code
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
Three issues found in this pass.
Missing CHANGELOG entry. The other PRs landing in [Unreleased] this cycle (e.g. #6315, #6316) each add a bullet under the relevant section. This PR adds no entry. Please add one under ### Added or ### Changed in ## [Unreleased] (one sentence per line, with (@pavver) attribution per the pre-commit hook requirement).
DeployPage.tsx — troubleshooting answer formatting regression (see inline comment). The answers previously contained <code>/<em>/<br /> nodes for readable code snippets; converting them to plain strings drops that formatting for all locales including English.
MetricsPage.tsx — fragile split on a literal URL substring (see inline comment). copy.desc.split('/api/registry/click')[1] silently truncates if any future locale omits the exact substring.
Generated by Claude Code
There was a problem hiding this comment.
The troubleshootingItems answers were converted from JSX (which contained <code>, <em>, and <br /> nodes) to plain strings in the i18n table.
This is a regression for English users as well: the formatted code snippets (e.g. flyctl tokens org <your-org-name>) are now rendered as unstyled monospace-less text.
Before merging, the render site needs to either:
(a) parse the answer strings as Markdown/HTML so the formatting survives, or
(b) keep the JSX structure for the answers and only translate the surrounding text strings, or
(c) document that the formatting loss is intentional and acceptable.
Generated by Claude Code
There was a problem hiding this comment.
The metrics description is rendered by splitting on the literal substring '/api/registry/click':
{copy.desc.replace('/api/registry/click', '')}
<code ...>/api/registry/click</code>
{copy.desc.split('/api/registry/click')[1]}split(...)[1] is undefined (silently renders as nothing) if any locale's metrics.desc omits or abbreviates that exact substring.
The current Ukrainian value includes it verbatim, so it passes today, but this is a silent breakage waiting for the next translator.
A more robust approach is to store the URL separately (e.g. a {endpoint} interpolation slot) and assemble the JSX in the component, so translators never need to replicate the raw URL.
Generated by Claude Code
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
Thanks for the localization pass — the getTranslation English-fallback design is sound and the key extraction is clean. One user-visible rendering regression needs to be fixed before merge, plus two incomplete locale lists and an efficiency issue. Details below, most important first.
Blocking
1. web/src/pages/MetricsPage.tsx:62 — .replace() renders the whole description plus a duplicated tail
{copy.desc.replace('/api/registry/click', '')}
<code>/api/registry/click</code>
{copy.desc.split('/api/registry/click')[1]}copy.desc.replace('/api/registry/click', '') returns the entire sentence with the path removed, not the prefix before it.
With the EN string "Aggregate counts … recorded via the /api/registry/click worker endpoint." the paragraph renders as:
Aggregate counts … recorded via the worker endpoint. + /api/registry/click + worker endpoint.
So the full sentence shows once, the <code> lands at the end, and worker endpoint. is duplicated (uk has the same problem — the path sits at the end of its sentence).
The first segment should be copy.desc.split('/api/registry/click')[0].
Should fix in this PR
2. web/src/components/SearchDialog.tsx:223 — paste-prefix locale set is missing uk
const langs = new Set(['zh', 'zh-TW', 'ja', 'ko', 'de', 'es', 'pl'])This set already lists pl but not the new uk.
Pasting a https://librefang.ai/uk/skills/<id> registry URL into the search box no longer strips the uk prefix, so CATEGORIES.includes('uk') is false and the direct-navigate shortcut silently fails for Ukrainian URLs.
3. web/src/i18n.ts:2923 — getTranslation deep-merges the whole tree on every render with no cache
For any non-English locale, getTranslation rebuilds the entire merged translation object on every call via mergeObject(english, selected).
It is called directly from hot-path components — BackToTop re-renders on every scroll, SiteHeader on every menu/lang/feature toggle — so each re-render clones the full tree.
It also defeats SearchDialog's useMemo(..., [t]): because t is a fresh object each render for non-EN users, anchorHits/itemHits recompute every frame.
A module-level per-locale cache fixes both (EN already early-returns, so it is unaffected).
Optional / note
4. web/index.html:45 — pre-hydration <html lang> and localized meta do not cover uk (or pl)
The inline detection script only handles zh-TW/zh/de/ja/ko/es, and the metaDesc table the same.
Non-JS crawlers and link unfurls on /uk therefore see <html lang="en"> and the English description; App.tsx corrects documentElement.lang after hydration, so this is SEO/preview-only.
pl already has the same gap so it predates this PR, but since this change adds a new selectable locale and /uk routing it would be good to wire uk (and pl) into the SEO path here too.
github-actions
Bot
added
area/docs
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
Two issues worth resolving before merge — see inline notes. The Ukrainian translation work itself looks thorough; these are architectural questions about ErrorBoundary and the partial-locale contract, not translation quality issues.
Generated by Claude Code
There was a problem hiding this comment.
ErrorBoundary is a class component, so it cannot call hooks and has no access to the active locale. The file hardcodes translations.en!.errorBoundary!, which means non-English users always see English error text — the new errorBoundary i18n keys are dead for every locale other than English.
Two ways to fix:
Option A (minimal): Accept English-only error boundaries and add a comment so the next reader understands why: // Class component cannot read hooks; always falls back to English. Convert to a functional wrapper to localize.
Option B (correct): Wrap with a thin functional ErrorBoundaryWrapper that reads the locale via useStore (or however the rest of the app resolves it), resolves the errorBoundary strings, and passes them as props into the class component's render(). This is the standard pattern for class components that need hook-provided values.
Generated by Claude Code
There was a problem hiding this comment.
The test 'Ukrainian has no missing raw keys vs en' asserts that translations.uk (the raw partial object) has zero missing keys relative to English. But the comment immediately below it says: "Raw locale objects may stay partial; getTranslation is the runtime contract."
These two contradict each other. Either:
-
The design allows partial locales — in which case the test should be deleted or rewritten to assert that
getTranslation('uk')(the merged result) has full EN coverage, not the raw object. Otherwise this test will fail for any future language that legitimately omits keys it inherits from English. -
Every locale must be 100% complete — in which case remove the "may stay partial" comment, and the test is correct.
Pick one contract and make the test and the comment agree. The current state encodes contradictory promises for the next contributor adding a new locale.
Generated by Claude Code
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
One reachability issue in the new code: web/src/components/ErrorBoundary.tsx imports translations directly and always reads translations.en!.errorBoundary!, bypassing getTranslation entirely. The Ukrainian translations you added for errorBoundary (lines +1199–1204 in uk.json) are unreachable through this class component — it will always render English error text regardless of the active locale.
Since ErrorBoundary is a class component it cannot use hooks, so there is no lang state available in the usual way. The simplest fix is to read the locale from document.documentElement.lang (or a module-level export of the resolved locale) in getDerivedStateFromError / render, and pass it to getTranslation. Alternatively, convert to a functional wrapper that wraps the class with a locale-aware shell.
Everything else in the PR looks good — locale parity across en/uk for all new keys is complete, and the getTranslation fallback logic is correct.
Generated by Claude Code
Web i18n Review NotesSummaryThis follow-up addresses the review feedback around the website localization layer. What ChangedThe website now separates translator-authored locale data from runtime translation lookup.
The strict Ukrainian raw-key completeness check was removed from the regular unit test suite. Translator AuditA new command was added: cd web
pnpm i18n:audit ukThe audit compares There is also an aggregate mode: pnpm i18n:audit --allThat mode lists every non-English locale that is not raw-key complete. Why This ShapeThe review surfaced two valid but different requirements:
Keeping those checks in one automatic test made the contract confusing. The new split keeps both requirements:
This also prevents English spread from masking missing Ukrainian keys. VerificationThe follow-up was verified with: cd web
pnpm lint
pnpm test
pnpm i18n:audit ukThe Ukrainian audit currently reports: |
houko
left a comment
houko
left a comment
There was a problem hiding this comment.
All five blocking items from the previous review are resolved as of 96290b26, and a couple of the architectural notes too. Verified against the current branch:
- MetricsPage
.replace()duplication → fixed. Now renders{copy.descPrefix}<code>{copy.endpoint}</code>{copy.descSuffix}with the three slots supplied per-locale. No duplicated tail. - ErrorBoundary locale bypass → fixed. A functional wrapper now reads
useAppStore(s => s.lang)and passesgetTranslation(lang).errorBoundaryinto the class component as a prop, so Ukrainian error text is reachable. Cleaner than thedocument.documentElement.langapproach I suggested. - CHANGELOG entry → added under
## [Unreleased] → ### Changedwith(@pavver)attribution. - DeployPage formatting regression → fixed. Replaced the plain strings with a structured
RichTextPart[][]model that restores<code>/<em>/<br>for all locales (EN included) withoutdangerouslySetInnerHTML. - Incomplete locale lists + efficiency → fixed.
ukadded to the SearchDialog paste-prefix set and to the index.html lang/meta SEO paths (pltoo);getTranslationnow memoizes merged locales in a module-level cache, which also fixes theuseMemo([t])reference thrash.
The i18n test contract (assert on the merged getTranslation result, raw locale may stay partial) is now self-consistent. No new issues introduced.
Approving.
github-actions
Bot
added
ready-for-review
2 participants
Summary
This PR adds Ukrainian localization support for the public web UI and moves the hardcoded web copy covered in this pass into the shared i18n system.
Changes
ukas a selectable web locale with/ukroute support.getTranslation(lang)so every locale falls back toenwhen a key is missing.Commit split
feat(web): add Ukrainian localization/ukdetection and URL prefix handling.refactor(web): move hardcoded text into i18nScope note
This is not a claim that every hardcoded string in
web/has been exhaustively audited.The PR covers the web pages and components touched in this localization pass.
A follow-up should add automated hardcoded-string, missing-key, and dead-key checks so future cleanup can be exhaustive and repeatable.
Verification
Ran:
pnpm lint pnpm test pnpm build pnpm test:e2eResults:
Notes:
pnpm buildandpnpm test:e2ewere run outside the sandbox becausetsxneeds an IPC pipe that the sandbox blocks.web/public/feed.xml,web/public/registry.json,web/test-results/) were not included in the commits.