Skip to content

feat(i18n): translatable Attribute column labels (batch 5)#1156

Open
JamieRuderman wants to merge 1 commit into
i18n/extract-devicelistfrom
i18n/extract-columns
Open

feat(i18n): translatable Attribute column labels (batch 5)#1156
JamieRuderman wants to merge 1 commit into
i18n/extract-devicelistfrom
i18n/extract-columns

Conversation

@JamieRuderman

Copy link
Copy Markdown
Member

Summary

Batch 5 — makes the device/service/connection table column labels translatable. This is the architectural piece deferred from the device-list chrome batch: the Attribute registry in Attributes.tsx is a module-level static array (~54 string labels) evaluated once at import, so labels can't use the t() hook.

54 columns.* keys, translated to ja/de/es. Stacked on i18n/extract-devicelist (#1155).

Approach (the option chosen: label → id-keyed getter)

  • Attribute.label / Attribute.help are now getters that resolve i18n.t('columns.<id>') / columns.<id>_help at access (render) time, keyed by the attribute's existing id, with the original English string as defaultValue. Backing fields _label / _help hold what the constructor passes.
  • React-node labels pass through unchanged (e.g. the graph-column headers <DeviceGraphColumn />).
  • Fallback is safe: any id without a catalog entry renders its English default — so the many other Attribute-based registries (File/Job/Product/Customer) keep working in English until a later batch, with no code change needed.
  • Verified no id collisions: those other registries use prefixed ids (customer*, job*, product*, script*), so id-keying is unambiguous.

Catalog mechanics

  • Keys are dynamic (columns.${id}), so the i18next-parser can't discover them — they're hand-maintained in the catalogs (same pattern as the cognito dynamic keys). Documented in i18next-parser.config.js; keepRemoved: true means an extract run leaves them intact (verified).
  • The mock placeholder attributes (categoryA–E, statusA–E) are intentionally not keyed — they fall back to English.

Deferred (follow-up)

Hardcoded English still lives inside value functions in this file — connection-type strings ("Peer to Peer", "Inactive", "Local Proxy", …), plus "Yes"/"No", "This system", "since refresh". Those render via inline i18n.t() (no architecture change) and are a clean separate batch.

Verification

  • i18n:check passes; tsc clean; production build passes.
  • Extraction run confirmed the dynamic columns.* keys survive (not pruned).
  • Runtime getter check: labels resolve per language (Name→名前/Name/Nombre, External IP→外部IP/Externe IP/IP externa, …), and an unknown id correctly falls back to its English default.

🤖 Generated with Claude Code

@JamieRuderman

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 694eebe795

ℹ️ 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".

Comment on lines +53 to +54
return typeof this._label === 'string'
? i18n.t(`columns.${this.id}`, { defaultValue: this._label })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve intentionally empty attribute labels

Skip translation when _label is empty. The enterprise-license actions column constructs an attribute with id: 'actions' and label: '' (AdminEnterpriseLicensesListPage.tsx:84-88), but there is no columns.actions entry and i18n/index.ts configures returnEmptyString: false; consequently i18next rejects the empty default and renders the literal key columns.actions in a header that was intentionally blank.

Useful? React with 👍 / 👎.

Comment on lines +52 to +55
get label(): string | React.ReactNode {
return typeof this._label === 'string'
? i18n.t(`columns.${this.id}`, { defaultValue: this._label })
: this._label

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Subscribe label renderers to catalog loading

Ensure components rendering this getter subscribe to i18next updates. On startup with a saved non-English language, ui.setLanguage starts but does not await the lazy changeLanguage load, so headers can first access this getter while only English is available; GridListHeaderTitle, DataDisplay, and ColumnsDrawer do not use useTranslation, and calling i18n.t() directly does not register a React subscription, leaving the English fallback displayed after the locale chunk finishes until an unrelated rerender or remount occurs.

Useful? React with 👍 / 👎.

Comment on lines +53 to +54
return typeof this._label === 'string'
? i18n.t(`columns.${this.id}`, { defaultValue: this._label })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Limit column translations to the intended registries

Avoid applying columns.<id> entries to every Attribute instance. Several admin registries use unprefixed IDs that collide with this catalog—for example, admin users/admins and enterprise licenses define id: 'created' with label Created, while this change resolves it as columns.created (Created date), and admin users' license is similarly translated—so those otherwise untranslated admin tables now acquire unintended, partially translated or renamed headers instead of retaining their supplied defaults.

Useful? React with 👍 / 👎.

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.

1 participant