Skip to content

fix(react): stop the i18n gate flagging key caps and icon glyphs - #5269

Open
sauldom102 wants to merge 1 commit into
mainfrom
claude/i18n-skip-non-copy-elements
Open

fix(react): stop the i18n gate flagging key caps and icon glyphs#5269
sauldom102 wants to merge 1 commit into
mainfrom
claude/i18n-skip-non-copy-elements

Conversation

@sauldom102

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #5262. Five of the 133 tracked strings are not product copy at all — a key cap, two icon glyphs and the name of a dev-only overlay. Left alone they would sit in the baseline forever, so this removes them: one as a general rule, three as inline markers. Baseline 133 → 128.

Type of change

  • Other: tooling precision fix (plus 3 comment-only source changes)

Implementation details

  • fix: skip text directly inside <kbd>, <code>, <pre>, <samp> and <var>

    Why this is a rule and not three more markers

    F0AiChatTextArea renders <kbd>Enter</kbd> {i18n.ai.clarifyingQuestion.navHint.select}. The cap reads what is printed on the physical key; the sentence beside it is already translated. That is a property of <kbd>, not of these two lines, so the next component to render a shortcut hint should not have to rediscover the marker. The other four elements hold code, sample output and variable names, which are not translated either.

    Only the immediate parent counts. A <kbd> nested inside a translated sentence must not silence the sentence, and prose under a <pre> is a markup bug rather than something to hide. Both are pinned by tests:

    <p><kbd>Enter</kbd> to select, or press <kbd>Esc</kbd></p>
    // → still flags "to select, or press"
  • fix: mark the AD icon glyphs and the XRay overlay title i18n-exempt

    Why these three stay literals

    AudioDescriptionToggleIcons draws AD as <text> inside the SVG — it is part of the icon artwork, the same way CC is on a closed-captions badge, not a label beside it. SVG <text> does legitimately carry copy elsewhere (chart labels), so this cannot be a blanket rule and takes a marker.

    lib/xray.tsx is the dev-only component inspector, rendered behind an enabled flag; XRay is its name.

  • chore: regenerate the baseline (133 → 128)

  • test: cover the element rule, including the two cases that must still be flagged

Notes for reviewers

No behaviour change: the three source edits are JSX comments, and the element rule only affects what the scanner reports. Adds no translation keys, so no API-surface impact.

tsc clean, format clean, 101 script tests pass, and the touched components' suites pass (55 files / 710 tests).

Conflicts with #5266 on untranslated-copy-debt.json only — both regenerate it. Whichever lands second needs pnpm --filter @factorialco/f0-react run check:untranslated-copy --update, which is the whole resolution since the file is generated.

After this and #5266, the remaining 101 findings all need new translation keys, which check-api-surface.ts classifies as breaking (TranslationShape marks every key required). That is a public-API call rather than a mechanical cleanup, so I have not opened PRs for it.

🤖 Generated with Claude Code

Five of the 133 tracked strings are not product copy at all, and would sit in
the baseline forever.

`<kbd>Enter</kbd>` is a key cap: it reads what is printed on the key, and the
sentence around it is already translated separately
(`F0AiChatTextArea` pairs it with `ai.clarifyingQuestion.navHint.select`).
That generalises, so it is a rule rather than a marker: text directly inside
`<kbd>`, `<code>`, `<pre>`, `<samp>` or `<var>` is machine or device text.
Only the immediate parent counts — a `<kbd>` inside a sentence must not
silence the sentence, which the tests pin down.

The other three are one-offs and get inline markers: the "AD" in
AudioDescriptionToggleIcons is a glyph inside the icon artwork, like the CC
badge, and "XRay" names the dev-only inspector overlay.

Baseline 133 -> 128.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauldom102
sauldom102 requested a review from a team as a code owner August 26, 2026 14:48
@github-actions github-actions Bot added fix react Changes affect packages/react labels Aug 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Review policy: Code change

Default rule: any other change needs one approval from f0-devs (rule 4).

Required approvals

Team Why Status
@factorialco/f0-devs Every code change needs a dev approval ⏳ pending
How this was decided
  • PRs touching only sds/ modules require their owners and nothing else.
  • Otherwise, docs-only changes (*.md, *.mdx, *.stories.tsx, anything in __stories__/) → one f0-general approval.
  • Otherwise, feat: titles → one f0-devs and one f0-designers approval. Not a feature? Fix the title prefix.
  • Anything else → one f0-devs approval.
  • Add the needs-design-review label to also request a design approval on any PR.
  • Creating a new sds/ module (new package.yml) additionally requires an f0-general approval.

Policy source: ownership/review-policy.ts · Team members: ownership/teams.yml

@github-actions

Copy link
Copy Markdown
Contributor

✅ No untranslated copy added

Every user-visible string in this PR comes from the i18n layer. Codebase total unchanged at 128.

@github-actions

Copy link
Copy Markdown
Contributor

✅ No New Circular Dependencies

No new circular dependencies detected. Current count: 0

@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

✅ Storybook docs — no pages lost

Every page reachable on main is still reachable here.

Links point at this PR's Storybook build — browse the full Storybook.

Snapshot of the Storybook index (docs pages + stories) compared against main. Non-blocking.

@github-actions

Copy link
Copy Markdown
Contributor

🔍 Visual review for your branch is published 🔍

Here are the links to:

@github-actions

Copy link
Copy Markdown
Contributor

📦 Alpha Package Version Published

Use pnpm i github:factorialco/f0#npm/alpha-pr-5269 to install the package

Use pnpm i github:factorialco/f0#db2ee157392e3a4f9fc05d7ae5eb2cd29e80dc18 to install this specific commit

@github-actions

Copy link
Copy Markdown
Contributor

✅ No breaking public API changes

No public exports were removed, renamed, or had existing props/types changed in a breaking way compared to main.

Comparing f0, experimental and ai against main. Adding components, types, or optional props is safe. This check is non-blocking.

⚠️ Could not analyze component-status (no-base) — a build may have failed; results may be incomplete.

@github-actions

Copy link
Copy Markdown
Contributor

♿ Accessibility (axe) — components changed in this PR

✅ No a11y issues in the stories this PR changed.

Scope: only stories in the files/component folders this PR changed. It can't yet flag downstream ripple from shared-code/token changes, or diff against main (planned: base-vs-head delta).

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for packages/react

Status Category Percentage Covered / Total
🔵 Lines 69% 29118 / 42199
🔵 Statements 67.95% 30802 / 45326
🔵 Functions 61.84% 6950 / 11238
🔵 Branches 61.64% 21805 / 35372
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/react/src/components/F0VideoPlayer/components/AudioDescriptionToggleIcons.tsx 100% 100% 100% 100%
packages/react/src/lib/xray.tsx 27.58% 18.18% 20% 26.92% 35, 40-78, 134-164, 175
Generated in workflow #17534 for commit 6199799 by the Vitest Coverage Report Action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix react Changes affect packages/react

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant