Repository navigation
[Maintainer] i18n: migrate navbar and top bar - #1169
Conversation
…ale audit - scripts/i18n-literals.ts: shared scanner for user-visible literals, a per-file ratchet (i18n/literal-baseline.json) wired into i18n:check, and report/update commands; exemptions via i18n/literal-allowlist.json or an i18n-ignore comment - scripts/i18n-codemod.ts: rewrites JSX text, UI props, conditional and template copy and notification payloads into i18n._ descriptors with the usePascalI18n hook; module-level and server copy is listed for a person - en-XA pseudo-locale compiled from English, registered by the app outside production, plus a Playwright audit that screenshots surfaces and lists copy that bypassed the catalog - i18n:check accepts AI drafts flagged fuzzy + Review: required (they are never compiled into packs); check --reviewed keeps the sign-off gate
…rface helpers; codemod accepts explicit files
Localizes the project navbar, share panel, app header, the viewport view bar (3D/2D tabs, split, ground mode, more-views menu) and the viewer toolbar with its Display menu. Module option lists become lazy descriptors; scene theme names share one helper with the viewer controls. zh-CN/fr/es/ja drafts are fuzzy and await human review.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 562f425. Configure here.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 562f42581e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| export function AppHeader({ children }: { children?: ReactNode }) { | ||
| const i18n = usePascalI18n() |
There was a problem hiding this comment.
AppHeader is rendered from the server-component home and settings pages, but this module has no 'use client' boundary and now calls usePascalI18n, which is exported from a client-only module and uses React client hooks. Rendering either route will therefore attempt to invoke a client export from the server (or render outside the client provider), causing a server-render failure rather than producing the localized label. Move the localized header into a client component or add an appropriate client boundary.
Useful? React with 👍 / 👎.
| for (const [path, candidates] of scan) { | ||
| total += candidates.length | ||
| const allowed = baseline.files[path] ?? 0 | ||
| if (candidates.length > allowed) { |
There was a problem hiding this comment.
The guard compares only the number of candidates in each file, so a change can delete one legacy literal and introduce a different untranslated literal while keeping the count at or below allowed; i18n:check then passes despite the stated guarantee that a file may never gain hardcoded copy. This is especially easy in files with large baseline allowances. Persist candidate identities (for example, stable text/location fingerprints) and require current candidates to be a subset of the baseline rather than treating the count as a reusable budget.
Useful? React with 👍 / 👎.
…view A translation that is Review: human on the base branch with the same English must stay reviewed. CI checks out the merge commit's first parent for the comparison, and release.yml runs check --reviewed before publish. The review rules are now a pure function so tests use fixtures.
The baseline lists each file's legacy copy by text, so new or swapped copy fails, and CI fails when the baseline gains entries relative to the base. i18n-ignore needs a reason; hyphenated labels and string constants rendered in JSX are copy; toast descriptions count once. The codemod keeps one-line bodies valid and converts explicitly named .ts modules. Enter, Space, Tab and Del leave the allowlist.
…rds; playwright-core 1.64.1
Scene theme names move to a shared helper used by both toolbars. 64 new messages; zh-CN/fr/es/ja drafts are fuzzy and await human review.
A translation that appears without a Source-Hash becomes a fuzzy draft unless a person already marked it Review: human, so pasted or tool-filled copy cannot reach the compiled packs without review.
Server pages render AppHeader, so the usePascalI18n hook there returned a 500 on the home page (cli-smoke).
They can render inside server components, where an inserted hook crashes the request.
A const is flagged only when the nearest declaration of the rendered name is that const string, so shadowing parameters, locals and same-named constants elsewhere in the file no longer swap the copy.
…eaks release.yml runs release-check instead of check --reviewed: it compares the committed catalogs with the last v* tag and fails when a reviewed entry with unchanged English became a draft, or when a compiled pack contains a draft. i18n:check runs the same checks against the PR base in CI. Both write per-locale draft counts to the job summary. Job-level tests run the command against temporary catalog copies.
Moved, split or extracted copy no longer fails: each text the baseline adds
must consume an equal text removed elsewhere, duplicates counted. JSX text is
recorded as it renders (whitespace, full HTML entities via entities,
{' '} joins), constants inside sentences become part of the sentence, and
concatenated copy is a candidate. The allowlist may not gain entries
relative to the base. Named callbacks and async functions are not hook
owners.
…dempotent --vars Copy is encoded so the compiled message renders the original text. Hooks are never placed in named callbacks or async functions, and editor package entry modules without 'use client' are left for a person. Descriptor mode stops at existing descriptors and i18n._ calls.
…ent.before A reviewed ID replaced by a new ID with the same English needs its translation carried over or reviewed again. On push, CI fetches and compares with the commit before the push.
…s for the new guards
Below sm their text is hidden, so both buttons carry the translated aria-label; the navbar-mobile audit surface checks names at 390px.
…rror extract marks a translation whose English changed fuzzy + Review: required and keeps the old Source-Hash; that entry is never shipped, so ordinary CI counts it as awaiting review. A reviewed entry with a stale hash, and every stale entry under --reviewed, still fail.
Releases tag packages @pascal-app/<pkg>@<version>, so the v* lookup found v1.0.3, which has no catalogs, and the check could never fail. The base is now the latest @pascal-app/editor@* tag (the package that ships the catalogs); when that tag predates the catalogs it falls back to origin/main (another commit with catalogs), then to the commit that introduced them, and otherwise fails loudly. Tests run the default lookup in a temporary repository and fail on a reviewed-to-draft change.
- Same-English matches between a removed reviewed ID and a new ID are a
warning (console and job summary), since they may be unrelated additions.
A rename declared with a renamed-from: <old id> descriptor comment is
carried over by i18n:extract and fails CI only if it arrives without the
reviewed translation.
- release-check fails on a --base that cannot be resolved or read.
- The allowlist applies to whole joined sentences; constants inside
concatenations count as copy; parenthesized named callbacks are not hook
owners.
- The codemod encodes adjacent static pieces together, keeps no-break
spaces, encodes text before a placeholder knowing a { follows, decodes
attribute entities, and escapes CR and line separators in emitted strings.
- The baseline format is version 3, since fingerprints now use rendered
text.
A previous-line ignore applies only when that line is a standalone comment, so an exemption written after one literal no longer hides a new literal on the following line.
|
Tooling cross-review at 70f7702: APPROVE I reviewed only this PR's own diff on top of #1167 (53abaae), checked out locally at /tmp. CI is green at the head: quality, ci, cli-smoke, Changed paths and Bugbot all pass.
P2 (doesn't block)
Stacking: this PR contains #1167's head. After #1167 merges, it needs main merged in or a rebase, and a re-check, before it merges. |
What does this PR do?
Localizes the editor's navbar and top bar. Part of #621. Depends on #1167 (codemod, literal guard,
en-XA, and the viewer toolbar, which moved there): this branch merges it, so the diff includes #1167 until that merges.editor-navbar.tsx): the all-projects link, project-name field and rename tooltip, Preview, Share.smthe Preview and Share text is hidden, so both buttons now carry the translatedaria-label.navbar-mobileaudit surface (390 px) checks that every control is named. With the labels removed it fails on both buttons.AppHeaderis rendered by server pages, so the translated link is a small client leaf (app-home-link.tsx) and the shell stays server-safe.view-bar.tsx): the 3D/2D tabs, close/pin/unpin, more-views menu, Split/Swap, and the ground-mode group.labelthey register, becauseEditorViewDescriptoris host API.selectmessage per state.fuzzy+Review: required(AI,AI-Provider: cursor-agent); the UI shows English until a fluent reviewer approves. The literal baseline shrinks; it never grows.navbar,navbar-mobile,share-panel,display-menuandmore-viewssurfaces added toi18n/pseudo-surfaces.ts.How to test
bun run i18n:check: passes and reports 107 entries per locale awaiting review (64 from [Maintainer] i18n: string-extraction codemod + CI literal guard #1167 plus 43 here).bun dev, open/and a project with?lang=en-XA. The home header, navbar, share panel, view bar and viewer toolbar show bracketed, accented text.curl '/?lang=en-XA'returns 200 witharia-label="[Þášçáļ ĥöɱé ~~~]"server-rendered.bun run i18n:pseudo-audit -- navbar navbar-mobile share-panel display-menu more-views: all five PASS, with no untranslated strings and no unnamed controls. I ran this; screenshots below.Follow-ups (not in this PR)
<html lang>(inherited from [Maintainer] i18n: locale registry, formatting and first locales #1148).Screenshots / screen recording
en-XA(accented, ~30 % longer, bracketed), captured byi18n:pseudo-audit. The left build panel, bottom tool dock and camera hints are later surface PRs.Navbar and view bar:
Navbar en-XA
Navbar at 390 px (Preview and Share are named although their text is hidden):
Navbar mobile en-XA
Share panel:
Share panel en-XA
Display menu (one line per row after the width fix):
Display menu en-XA
More views menu:
More views en-XA
Checklist
bun devbun checkto verify)mainbranchTo show artifacts inline, enable in settings.
Note
Low Risk
Presentation-layer string wiring and locale catalogs only; English defaults unchanged and behavior is the same aside from translated copy and accessibility labels.
Reviewed by Cursor Bugbot for commit 96ca6c7. Bugbot is set up for automated code reviews on this repo. Configure here.