refactor(react): read shared i18n keys instead of hardcoding 27 strings - #5266
Open
sauldom102 wants to merge 1 commit into
Open
refactor(react): read shared i18n keys instead of hardcoding 27 strings#5266sauldom102 wants to merge 1 commit into
sauldom102 wants to merge 1 commit into
Conversation
Twenty-seven call sites hardcoded English that `actions.*`/`navigation.*` already held. The keys were written, then later call sites spelled the text out again, so a consumer translating `actions.close` saw seven buttons stay English. Also corrects the CI hint added in #5262, which was pointing at the wrong keys. It suggested the shallowest key holding the same English, which gave `ai.today` for a date preset, `wizard.previous` for a date navigator and `filters.number.value` for a survey field. Depth is the wrong signal: what matters is whether the namespace is domain-neutral. The hint now separates a shared key ("read this") from a match in another feature's namespace ("borrow only if it is the same feature"), so it can no longer talk anyone into wiring a component to an unrelated domain. No rendered text changes: every key's English is byte-identical to the literal it replaces, and consumers that supply no dictionary (which is all of them today) keep resolving through the built-in defaults. Baseline 158 -> 131. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to #5262. Twenty-seven call sites hardcoded English that
actions.*/navigation.*already held — the keys were written, then later call sites spelled the text out again. A consumer translatingactions.closesaw seven buttons stay English. This wires those sites to the keys, and corrects the CI hint from #5262 that was pointing at the wrong ones.Type of change
Implementation details
refactor: read
actions.*/navigation.*at 27 sites across 22 filesWhich keys, and how many sites each
actions.closenavigation.previousnavigation.nextactions.clearactions.addactions.saveactions.selectAllactions.otheractions.moreactions.deleteThree sites needed more than a swap:
FileListandCalloutInternalhave an early return / throw before their hooks, souseI18n()goes above it rather than becoming a conditional call;BreadcrumbEllipsishad an implicit-return body and needed a block.WidgetUpdateDialog'ssaveLabel = "Save"was a destructuring default, which cannot reach a hook — it now resolves assaveLabel ?? t.actions.save, matching thetitle ?? t.widgets.editParamsTitleidiom one line above it.fix: stop the CI hint suggesting keys from unrelated namespaces
What was wrong with it
ci(react): block newly added untranslated copy #5262 suggested the shallowest key holding the same English. Depth turns out to be the wrong signal — it produced
ai.todayfor a date preset,wizard.previousfor a date navigator,filters.number.valuefor a survey field,chat.backfor a filter picker. Following those suggestions would wire components to unrelated domains, so a product translatingai.settingswould silently change OneDataCollection's Settings button.What matters is whether the namespace is domain-neutral. The hint now reports three states instead of one: a shared key ("read this"), a match in another feature's namespace ("borrow only if it is the same feature"), or nothing ("add a key"). The PR comment splits its tables the same way.
This is also why this PR is 27 sites and not 68. Of the 68 findings whose English exists somewhere in the dictionary, only 27 have a key that is safe to reuse anywhere; the other 41 need either a new key in their own namespace or a deliberate promotion to
actions.*.chore: regenerate the baseline (158 → 131)
test: cover the three-way hint split and the generic-namespace preference
Notes for reviewers
No rendered text changes, and no new translation keys — so no API-surface impact.
Why this cannot break a consumer's tests (checked against the factorial monorepo)
Two things had to hold, and both were verified rather than assumed:
Every key's English is byte-identical to the literal it replaces. This is not automatic — the key index matches case-insensitively, so
"Close"could have matched a key holding"close"and quietly changed rendered text. All 10 keys check out exactly.factorial supplies no dictionary. On
origin/main, across 3951 files importing@factorialco/f0-react, everyI18nProvideroccurrence is in a.spec.tsx— and they are comments explaining that components get mocked to avoid it. SouseI18n()resolves throughdefaultI18nContext, the built-in English.Together: rendered output is unchanged, in production and in tests. factorial specs that query these strings —
name: 'Other actions'inCompensationPlanDetailHeader,getByLabelText('Next')inApplicationPreviewSidePanel,name: 'Clear'inFactorialChatRuntimeProvider— keep passing.Worth knowing for later: two f0 consumers under development already mount
<I18nProvider translations={aiTranslations}>for the mobile AI agent. Once that ships, these strings become app-controlled — which is the point of the work, but it means those specs should query byrole/data-testidbefore the provider lands, not after.Full local verification:
tscclean,oxlintclean (the 3 remainingdisplayNameerrors are pre-existing on main — identical count with this branch stashed), unit suite 7048 passed / 0 failed.Separately spotted and not fixed here:
CarouselPrevious/CarouselNextspread{...props}before settinglabel, so a consumer-passedlabelis silently discarded even thoughButtonInternalPropsadvertises it. That is a behaviour change, so it wants its own PR.🤖 Generated with Claude Code