Skip to content

Translations: Feat - full coverage in eight languages, plus tooling - #2377

Open
gabrielbazan7 wants to merge 15 commits into
bitpay:developfrom
gabrielbazan7:fix/localization-long-text
Open

gabrielbazan7 wants to merge 15 commits into
bitpay:developfrom
gabrielbazan7:fix/localization-long-text

Conversation

@gabrielbazan7

@gabrielbazan7 gabrielbazan7 commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Dependency upgrade: i18next 21.6.4 → 25.8.14 and react-i18next 12.2.0 → 16.5.4.
Crowdin is gone: translations live in the repo and yarn translation:check runs in
CI, erroring on any missing translation. From here on, a PR adding a user-facing string
must ship all 8 translations or CI blocks it. Workflow in locales/README.md.

@gabrielbazan7 gabrielbazan7 changed the title Fix/localization long text Translations: Feat - full coverage in eight languages, plus tooling Sep 8, 2026
@gabrielbazan7
gabrielbazan7 marked this pull request as ready for review September 8, 2026 16:24

@JohnathanWhite JohnathanWhite left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Automated review by Codex — gpt-5.6-sol, with findings verified locally against this PR's head commit before posting. Anything that didn't hold up was dropped.

Four findings. The locale catalogues themselves checked out clean — details in the summary below.


Locale consistency check (separate from the Codex pass): clean. All 9 catalogues parse, 0 duplicate keys, 0 placeholder mismatches ({{…}}, %s, %1$s) across all 8 translations vs English. Key-set asymmetries are correct CLDR plural categories across 11 plural families (ja/zh _other; es/fr/pt _one/_many/_other; ru _one/_few/_many/_other). Every literal t() key across 977 source files resolves in en.json. Ran the PR's own new checker under the pinned i18next 25.8.14: 0 errors, and its tests pass 7/7.

Comment thread package.json
Comment thread src/navigation/tabs/home/components/Crypto.tsx Outdated
Comment thread package.json
@gabrielbazan7
gabrielbazan7 force-pushed the fix/localization-long-text branch from 2cd4edb to 5c815f1 Compare September 9, 2026 14:49

@JohnathanWhite JohnathanWhite left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: shippable as is.

The i18next 21 -> 25 / react-i18next 12 -> 16 jump is the risky part of this and it looks correctly handled:

  • catalogs use v4 plural suffixes, consistent with v22 moving to Intl.PluralRules
  • intl-pluralrules is required before i18next init
  • interpolation escaping stays disabled for React
  • keySeparator: false / nsSeparator: false protect the literal-sentence keys — which matters a lot here, since keys contain . and :
  • Suspense in react-i18next v16 is a non-issue because resources are bundled synchronously

One follow-up, explicitly not blocking: the new CI gate can't see missing keys.

scripts/check-translations.js reads only locales/<lang>/translation.json (line 186) and derives everything from Object.keys(en) (line 51) — it never scans src/. So it enforces catalog↔catalog consistency, and does that thoroughly: interpolation mismatch, markup balance, brace pairing, plural suffixes, whitespace. But a t('...') whose key is absent from en is invisible to it by construction, and renders as the raw key.

Six live examples today, missing from all 9 catalogs:

  • 1W Gain/Loss, 1M Gain/Loss, 3M Gain/Loss, 1Y Gain/Loss, 5Y Gain/Losssrc/navigation/tabs/home/components/AssetsGainLossDropdown.tsx
  • Ready to Continuesrc/navigation/wallet/screens/InviteCosigners.tsx

All six were present in locales/en on develop and are dropped here, so rendered output is unchanged — English in every locale before and after. No regression, which is why this isn't blocking. But they are user-facing (Home portfolio period selector, cosigner invite screen) and they're precisely the class of gap this PR sets out to close. Worth either restoring the keys or adding a source-scan pass to the checker — i18next-cli extract already knows how to find them.

Nice work on the checker itself, incidentally — the interpolation and markup comparisons are more rigorous than the Crowdin setup they replace.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants