Skip to content

feat: add language switcher#2527

Open
finnar-bin wants to merge 1 commit into
stagefrom
feat/4148-product-localization
Open

feat: add language switcher#2527
finnar-bin wants to merge 1 commit into
stagefrom
feat/4148-product-localization

Conversation

@finnar-bin

Copy link
Copy Markdown
Contributor

Description

Adds a language switcher in the user preferences settings

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Screenshots / Screen recording

image

@finnar-bin finnar-bin self-assigned this Jul 8, 2026
@finnar-bin finnar-bin changed the title Add language switcher feat: add language switcher Jul 8, 2026
Comment thread src/views/accounts/profile/Preference.js
Comment thread src/views/accounts/profile/Preference.js
Comment thread src/views/accounts/profile/Preference.js
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review summary

Small, contained change — a language dropdown on the Preferences page that persists prefs.locale on the user record. Three concerns, left as inline comments:

  1. Labels are BCP‑47 codes, not native names. HI-IN / ZH-CN etc. are hard to recognize without prior knowledge of locale tags. Displaying each language in its own script is the standard pattern.
  2. No consumer for the preference. No i18n library is installed and no code path in src/ reads prefs.locale. If Manager‑UI reads it server‑side that's fine — otherwise the setting will appear broken to users. Worth clarifying in the PR description or gating behind a flag until wiring lands.
  3. No validation of legacy values. If a stored prefs.locale isn't in LOCALES, the <NativeSelect> renders with an unmatched value and React will warn.

Nothing security-sensitive; nothing that touches auth or the CMS fetch path. No performance concerns — six-item static list, one extra piece of state. Approval blockers are (1) and (2); (3) is a nice-to-have hardening.

@finnar-bin finnar-bin force-pushed the feat/4148-product-localization branch from 6d97b2e to b9b6ddc Compare July 8, 2026 01:35
Comment thread src/views/accounts/profile/Preference.js
Comment thread src/views/accounts/profile/Preference.js
Comment thread src/views/accounts/profile/Preference.js
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

Review summary

The change itself is small and safely persists a new locale field into userInfo.prefs without disturbing existing keys (...prefs spread is preserved). A few observations:

Main concern — no consumer for this preference yet. A codebase search turns up no i18n runtime (no useTranslation, no i18n/LanguageProvider, no locale-aware string catalog) — the only locale references in src/ are unrelated (FillerContent, instance-level locale settings for CMS content, etc.). So this switcher lets a user save a locale that nothing currently reads. That's fine if it's a deliberate scaffolding step ahead of the Manager-UI i18n rollout, but worth calling out because:

  • A user changing this today gets no visible effect, which will read as a bug in support tickets.
  • Until a consumer lands, the "App Language" row could reasonably be gated behind a feature flag or omitted.

If there's a paired Manager-UI PR that consumes prefs.locale, linking it in the description would make the intent clear.

Smaller items (left as inline comments):

  • LOCALES naming diverges from the sibling teamOption / instanceOptions (camelCase) constants.
  • Labels display raw upper-cased locale codes (EN-US, HI-IN) rather than language names — awkward for users who don't already read English.
  • 'en-US' default is repeated in the useState init and the useEffect fallback; extract to a shared constant.
  • prefs?.locale || 'en-US'?? would be safer if empty string is ever a legitimate value (unlikely, but conveys intent).

No security or performance concerns.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant