Skip to content

feat(i18n): add Spanish translations#1443

Open
thomasbeaudry wants to merge 1 commit into
feat/spanish-mechanismfrom
feat/spanish-translations
Open

feat(i18n): add Spanish translations#1443
thomasbeaudry wants to merge 1 commit into
feat/spanish-mechanismfrom
feat/spanish-translations

Conversation

@thomasbeaudry

Copy link
Copy Markdown
Collaborator

Summary

  • Add es entries to all 11 web app translation JSON files (269 lines added, 0 removed)

This is a pure translation PR — no code changes. Stacked on #1442.

Test plan

  • Only translation JSON files changed
  • Visual spot-check in-app

🤖 Generated with Claude Code

@thomasbeaudry
thomasbeaudry requested a review from joshunrau as a code owner July 23, 2026 23:44
@thomasbeaudry
thomasbeaudry force-pushed the feat/spanish-translations branch 2 times, most recently from 1e38617 to c4065ca Compare July 24, 2026 03:13
@thomasbeaudry
thomasbeaudry force-pushed the feat/spanish-mechanism branch from 30bab68 to 87ab4f0 Compare July 24, 2026 03:45
Add `es` entries to all web app translation JSON files.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@gdevenyi gdevenyi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — Spanish translations

The JSON side is complete and the Spanish reads well. I walked every leaf in all twelve namespace files on this branch: every translation object now has an es key — no gaps, no stray objects missing fr. For a hand-written 269-line translation pass that's a good result, and reviewing it as a standalone PR with no code changes is the right call.

The problem is scope, not quality.

The JSON files are roughly 7% of the app's strings

This codebase deliberately prefers inline translations — AGENTS.md: "prefer inline translations with t({ en: '...', fr: '...' }) unless translation is used multiple times" — and that's where nearly all user-facing text actually lives. On this branch:

count
t({ ... }) calls with an en key in apps/web/src 382 across 51 files
…of those that also have es 2 (both in SaveStatus.tsx, from #1442)
translation-JSON leaves gaining es in this PR 269

So once #1442 and this land, a user who selects Español gets Spanish for shared labels like "Password", "Delete", "Active Group" — and English for essentially every page: the admin settings page, group management, the datahub, session setup, instrument pages, the user account page. The result is a per-string language mosaic, which reads worse to a Spanish speaker than a consistently English UI would.

I'd suggest one of:

  • finish the inline strings before exposing the option (large but mechanical, and t({ en, fr }) call sites are trivially greppable), or
  • land the mechanism and the JSON now, but keep Spanish out of ALL_LANGUAGES until coverage is real, so #1442's plumbing ships without a half-translated user-facing option.

Either way it's worth stating the intended coverage bar in the PR description, because right now "add Spanish translations" reads as complete.

Nothing prevents this from regressing

Adding es to UserConfig.LanguageOptions does not make es a required key — libui types the argument as { [L in Language]?: string } (TranslateFunction in libui/dist/i18n/types.d.ts) and t() falls back to defaultLanguage at runtime. So the compiler will never object to the next t({ en, fr }) anyone writes, and the JSON files can drift back out of sync just as silently.

(Worth flagging because #1439's description asserts the opposite — that declaring es makes it required, which is the stated reason for splitting that stack into three PRs. It doesn't.)

Since the type system won't help, a test would:

// walks every translations/*.json leaf and asserts it has every active language
it.each(Object.keys(translations))('%s has all active languages', ...)

That's cheap, and it's the only thing that will keep this PR's work from decaying. An ESLint rule over t({ ... }) object literals would cover the inline half.

Smaller notes

  • Terminology drift with #1439. common.json translates assignment as "Asignación", while #1439's built-in Spanish email template uses "evaluación" ("Su evaluación de Open Data Capture") for the same concept. Both PRs are in flight; worth agreeing on one term now — the existing French uses "Assignation" in common.json and "évaluation" in #1439, so the same split exists there.
  • Base branch. This targets feat/spanish-mechanism. If #1442 merges normally the base auto-retargets, but if it's squashed this will need rebasing — or the two could simply be one PR, since neither is useful without the other.
  • user.json is {} and untouched, which is correct — just noting I checked rather than missed it.

},
"assignment": {
"en": "Assignment",
"es": "Asignación",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Terminology check across the in-flight PRs: this renders assignment as "Asignación", but #1439's DEFAULT_ASSIGNMENT_EMAIL_TEMPLATE uses "evaluación"subject.es: 'Su evaluación de Open Data Capture', and the body says "Se le ha asignado un cuestionario". A participant would see one word in the app and a different one in the email about the same object.

The same split already exists in French ("Assignation" here vs "évaluation" in #1439's template and in AssignmentEmailForm's "Lien d'évaluation envoyé à …"), so this is worth settling once for both languages while both PRs are still open.

"demo": {
"availableUsers": {
"en": "Available Users",
"es": "Usuarios disponibles",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I verified programmatically that every translation leaf across all twelve translations/*.json files on this branch now carries es — no gaps. Nice.

The thing that will erode it: libui types t()'s argument as { [L in Language]?: string }, so nothing forces a new key to include every language, and t() silently falls back to defaultLanguage. A missing translation is invisible in CI, in review, and at runtime.

A single data-driven test over these files would lock in what this PR achieved:

const LANGUAGES = ['en', 'es', 'fr'] as const;
// walk each namespace; every object with an 'en' key must have all of LANGUAGES

Deriving LANGUAGES from the same tuple #1442 should be exporting from schemas/setup means adding a fourth language automatically fails the test until the JSON is filled in.

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