Skip to content

feat(jef-82): add web internationalization support - #253

Closed
mankatcheung wants to merge 6 commits into
mainfrom
feat/jef-82-support-i18n
Closed

mankatcheung wants to merge 6 commits into
mainfrom
feat/jef-82-support-i18n

Conversation

@mankatcheung

@mankatcheung mankatcheung commented Aug 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Adds i18next, react-i18next, and i18next-browser-languagedetector
  • Moves every translation catalog into separate JSON files for en, en-GB, zh-HK, zh-TW, and zh-CN
  • Detects locale from the URL query parameter first, then browser navigator languages, then local storage
  • Keeps the selected locale in the URL as ?locale=... and local storage when changed in settings
  • Expands translated coverage across navigation, settings, root errors, authentication, and common UI labels
  • Adds locale tests for URL detection, persistence, translation output, and document language

Verification

  • Web tests pass: 30 files, 245 tests
  • Monorepo typecheck passes
  • Web lint passes with existing no-explicit-any warnings
  • Monorepo build passes

Closes JEF-82

Summary by CodeRabbit

  • New Features

    • Added multilingual support for English, British English, Simplified Chinese, and Traditional Chinese.
    • Added language selection in profile settings, with locale detected from browser settings, URL, or saved preference.
    • Localised navigation, landing page, authentication, settings, command palette, error messages, and accessibility labels.
    • Updated page language metadata and translated 404 and error screens.
  • Tests

    • Added coverage for locale fallback, URL-based selection, translations, number formatting, and document language updates.

@coderabbitai

coderabbitai Bot commented Aug 6, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@mankatcheung, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 46 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a6825cf2-a242-453a-abdc-3ba952204dbe

📥 Commits

Reviewing files that changed from the base of the PR and between 4490193 and 05c5a38.

📒 Files selected for processing (7)
  • apps/web/src/i18n/en.json
  • apps/web/src/i18n/zh-CN.json
  • apps/web/src/i18n/zh-HK.json
  • apps/web/src/i18n/zh-TW.json
  • apps/web/src/lib/errors.ts
  • apps/web/src/lib/undoToast.ts
  • apps/web/src/routes/_authenticated/dashboard.tsx

Walkthrough

The pull request adds five supported locales, locale detection and persistence, translated messages, locale-aware formatting, document language updates, and translated root, authenticated, settings, landing, shared, login, TOTP, and registration screens.

Changes

Locale support

Layer / File(s) Summary
Locale catalogue and provider
apps/web/src/lib/i18n.tsx, apps/web/src/i18n/*.json, apps/web/package.json
The i18n module adds locale detection, persistence, formatting, React context services, document language updates, and pre-hydration initialisation. Translation catalogues and runtime dependencies support five locales.
Root document and error screens
apps/web/src/routes/__root.tsx
The root route provides locale context, sets the active document language, injects the initialisation script, and translates not-found and route-error messages.
Authenticated navigation and language settings
apps/web/src/routes/_authenticated/route.tsx, apps/web/src/routes/_authenticated/settings/route.tsx, apps/web/src/routes/_authenticated/settings/profile.tsx
Authenticated navigation and settings labels use translations. The profile page adds a language selector that updates the active locale.
Landing, authentication, and shared components
apps/web/src/routes/index.tsx, apps/web/src/routes/login.tsx, apps/web/src/routes/register.tsx, apps/web/src/components/*.tsx
Landing, login, TOTP, registration, command palette, error state, and OAuth text use translation keys.
Locale behaviour tests
apps/web/src/__tests__/lib/i18n.test.tsx
Tests cover English fallback, URL-selected zh-CN, translated dashboard output, and document language updates.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant LocaleProvider
  participant SettingsProfilePage
  participant RootDocument
  participant TranslatedScreens
  Browser->>LocaleProvider: provide URL, storage, or browser locale
  SettingsProfilePage->>LocaleProvider: select locale
  LocaleProvider->>RootDocument: update document language and URL
  LocaleProvider->>TranslatedScreens: provide translated UI and formatting
Loading

Possibly related PRs

Poem

A rabbit selects a language with care,
Menus and messages appear everywhere.
The document language follows the choice,
Tests confirm each translated voice.
Five locales now answer the call.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding web internationalisation support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/jef-82-support-i18n

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/src/lib/i18n.tsx`:
- Around line 123-154: Update detectLocale to wrap localStorage.getItem in
try/catch, continuing through navigator.languages and ultimately the English
fallback when storage is unavailable. Also wrap the localStorage.setItem call in
LocaleProvider’s locale useEffect with try/catch so blocked storage cannot crash
persistence or locale initialization.
- Around line 148-154: Update LocaleProvider and its locale initialization so
the first render uses a stable server-compatible locale rather than
detectLocale(). After hydration, detect and apply the browser or stored locale,
then persist it only after detection completes; preserve the existing
document.documentElement.lang update and avoid locale-dependent hydration
mismatches.

In `@apps/web/src/routes/_authenticated/route.tsx`:
- Around line 123-130: Complete navigation localisation in
apps/web/src/routes/_authenticated/route.tsx at lines 123-130 by reusing
translated labels for desktop and mobile navigation, settings entries, and
sign-out controls, adding an applications translation if required. Update the
settings heading in apps/web/src/routes/_authenticated/settings/route.tsx at
lines 33-38 to render through t('nav.settings').
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b2c77854-9418-4125-b6cc-afab3cc90671

📥 Commits

Reviewing files that changed from the base of the PR and between e729f91 and 46c3d6e.

📒 Files selected for processing (6)
  • apps/web/src/__tests__/lib/i18n.test.tsx
  • apps/web/src/lib/i18n.tsx
  • apps/web/src/routes/__root.tsx
  • apps/web/src/routes/_authenticated/route.tsx
  • apps/web/src/routes/_authenticated/settings/profile.tsx
  • apps/web/src/routes/_authenticated/settings/route.tsx

Comment thread apps/web/src/lib/i18n.tsx Outdated
Comment on lines +123 to +154
function detectLocale(): Locale {
if (typeof window === 'undefined') return 'en';
const stored = localStorage.getItem(LOCALE_STORAGE_KEY);
if (isLocale(stored)) return stored;
for (const language of navigator.languages) {
if (isLocale(language)) return language;
if (language.startsWith('zh-HK')) return 'zh-HK';
if (language.startsWith('zh-TW')) return 'zh-TW';
if (language.startsWith('zh')) return 'zh-CN';
if (language.startsWith('en-GB')) return 'en-GB';
if (language.startsWith('en')) return 'en';
}
return 'en';
}

interface LocaleContextValue {
locale: Locale;
setLocale: (locale: Locale) => void;
t: (key: string) => string;
formatDate: (value: Date | string | number, options?: Intl.DateTimeFormatOptions) => string;
formatNumber: (value: number, options?: Intl.NumberFormatOptions) => string;
}

const LocaleContext = createContext<LocaleContextValue | null>(null);

export function LocaleProvider({ children }: { children: React.ReactNode }) {
const [locale, setLocaleState] = useState<Locale>(detectLocale);

useEffect(() => {
document.documentElement.lang = locale;
localStorage.setItem(LOCALE_STORAGE_KEY, locale);
}, [locale]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate i18n file =="
fd -a 'i18n\.(tsx|ts)$' . | sed 's#^\./##'

echo "== Outline =="
ast-grep outline apps/web/src/lib/i18n.tsx --view expanded || true

echo "== Relevant file section =="
cat -n apps/web/src/lib/i18n.tsx | sed -n '1,220p'

echo "== Locate LocaleProvider usages and startup script references =="
rg -n "LocaleProvider|LOCALE_STORAGE_KEY|i18n|window\.localStorage|localStorage\.(get|set)Item" apps/web/src -S || true

echo "== Package/typescript info =="
if [ -f package.json ]; then cat package.json | sed -n '1,120p'; fi
if [ -f apps/web/package.json ]; then cat apps/web/package.json | sed -n '1,140p'; fi

Repository: mankatcheung/job-finder

Length of output: 15120


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Root route relevant section =="
cat -n apps/web/src/routes/__root.tsx | sed -n '70,105p'

echo "== Runtime storage access model probe =="
node --input-type=module <<'JS'
for (const model of ['unavailable', 'blocked']) {
  const ns = model === 'unavailable'
    ? {}
    : { localStorage: { getItem() { throw new TypeError('Blocked') }, setItem() { throw new TypeError('Blocked') } } };
  const detectLocale = function (ns) {
    const LOCALE_STORAGE_KEY = 'locale';
    if (typeof ns.window === 'undefined') return 'en';
    const stored = ns.localStorage.getItem(LOCALE_STORAGE_KEY);
    if (stored && ['en','en-GB','zh-HK','zh-TW','zh-CN'].includes(stored)) return stored;
    for (const language of ns.navigator.languages) {
      if (['en','en-GB','zh-HK','zh-TW','zh-CN'].includes(language)) return language;
      if (language.startsWith('zh-HK')) return 'zh-HK';
      if (language.startsWith('zh-TW')) return 'zh-TW';
      if (language.startsWith('zh')) return 'zh-CN';
      if (language.startsWith('en-GB')) return 'en-GB';
      if (language.startsWith('en')) return 'en';
    }
    return 'en';
  };
  console.log(model, 'detectLocale throws:', (() => { try { detectLocale(ns); return false } catch { return true } })());
}
JS

echo "== Web Storage error handling documentation snippet =="
python3 - <<'PY'
# Deterministic model of current unguarded storage access: a throw from getItem throws before navigator fallback;
# a throw from setItem throws from the effect and is not caught by LOCALE_INIT_SCRIPT.
cases = {
    "detectLocale.getItemThrows": "throws before navigator fallback, falls back to en-GB if navigator.language can throw; storage falls back to en",
    "localeEffect.setItemThrows": "throws during effect; root catches",
}
for k, v in cases.items():
    print(f"{k}: {v}")
PY

Repository: mankatcheung/job-finder

Length of output: 2098


Handle blocked Web Storage in locale detection and persistence.

localStorage.getItem() can throw when storage is unavailable or blocked, and detectLocale() does not fall back to browser preferences in that case. localStorage.setItem() in the useEffect can also throw. Wrap storage reads and writes with try/catch; on failure, keep using browser preferences or the English default instead of crashing locale initialisation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/lib/i18n.tsx` around lines 123 - 154, Update detectLocale to
wrap localStorage.getItem in try/catch, continuing through navigator.languages
and ultimately the English fallback when storage is unavailable. Also wrap the
localStorage.setItem call in LocaleProvider’s locale useEffect with try/catch so
blocked storage cannot crash persistence or locale initialization.

Comment thread apps/web/src/lib/i18n.tsx
Comment on lines +123 to +130
const localizedMainNav = MAIN_NAV.map((item) => ({
...item,
label: t(`nav.${item.label.toLowerCase()}`),
}));
const localizedSettingsNav = SETTINGS_NAV.map((item) => ({
...item,
label: t(`settings.${item.label.toLowerCase()}`),
}));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Complete localisation of navigation labels.

The selected locale does not update every navigation label. Literal labels remain in the mobile bottom navigation, settings entries, and sign-out controls. The settings page heading also remains English.

  • apps/web/src/routes/_authenticated/route.tsx#L123-L130: Use translated labels for every desktop and mobile navigation representation. Add a short applications translation if the bottom navigation needs one.
  • apps/web/src/routes/_authenticated/settings/route.tsx#L33-L38: Render the heading with t('nav.settings').
📍 Affects 2 files
  • apps/web/src/routes/_authenticated/route.tsx#L123-L130 (this comment)
  • apps/web/src/routes/_authenticated/settings/route.tsx#L33-L38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/routes/_authenticated/route.tsx` around lines 123 - 130,
Complete navigation localisation in apps/web/src/routes/_authenticated/route.tsx
at lines 123-130 by reusing translated labels for desktop and mobile navigation,
settings entries, and sign-out controls, adding an applications translation if
required. Update the settings heading in
apps/web/src/routes/_authenticated/settings/route.tsx at lines 33-38 to render
through t('nav.settings').

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/src/routes/login.tsx`:
- Around line 220-230: Complete authentication localization in
apps/web/src/routes/login.tsx at lines 220-230 by adding catalogue keys and
replacing the remaining TOTP validation message, error text, verification
control, and back control labels with t(...) calls. Also update
apps/web/src/routes/register.tsx at line 108 to localize the success heading,
verification copy, confirmation label, submit-state text, and validation
messages using catalogue keys and t(...).
- Around line 101-107: Update the prompt rendering at
apps/web/src/routes/login.tsx lines 101-107 and 156-160, and
apps/web/src/routes/register.tsx lines 125-139, to use a single translated
message for each sentence rather than concatenating separate translation
fragments around inline Link elements. Add or reuse translation keys that
support the link interpolation while preserving the existing destinations and
styling, including the registration and sign-in links.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c25bd6de-7919-4e74-bec6-770c1442b035

📥 Commits

Reviewing files that changed from the base of the PR and between 46c3d6e and 8403da0.

📒 Files selected for processing (8)
  • apps/web/src/i18n/en-GB.json
  • apps/web/src/i18n/en.json
  • apps/web/src/i18n/zh-CN.json
  • apps/web/src/i18n/zh-HK.json
  • apps/web/src/i18n/zh-TW.json
  • apps/web/src/lib/i18n.tsx
  • apps/web/src/routes/login.tsx
  • apps/web/src/routes/register.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/src/lib/i18n.tsx

Comment on lines +101 to +107
<h1 className="text-2xl font-bold text-gray-900 dark:text-gray-100">
{t('auth.signIn')}
</h1>
<p className="mt-1 text-sm text-gray-500 dark:text-gray-400">
Don&apos;t have an account?{' '}
{t('auth.noAccount')}{' '}
<Link to="/register" className="text-blue-600 hover:underline">
Register
{t('auth.register')}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep translated sentences intact around inline links.

Both routes split sentences into independent fragments and insert spaces around links. This prevents locale-specific word order and produces incorrect CJK spacing.

  • apps/web/src/routes/login.tsx#L101-L107: render the no-account prompt as one translated message.
  • apps/web/src/routes/login.tsx#L156-L160: render the no-account error as one translated message.
  • apps/web/src/routes/register.tsx#L125-L139: replace the hard-coded account prompt and separate auth.signIn fragment with one translated message.
📍 Affects 2 files
  • apps/web/src/routes/login.tsx#L101-L107 (this comment)
  • apps/web/src/routes/login.tsx#L156-L160
  • apps/web/src/routes/register.tsx#L125-L139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/routes/login.tsx` around lines 101 - 107, Update the prompt
rendering at apps/web/src/routes/login.tsx lines 101-107 and 156-160, and
apps/web/src/routes/register.tsx lines 125-139, to use a single translated
message for each sentence rather than concatenating separate translation
fragments around inline Link elements. Add or reuse translation keys that
support the link interpolation while preserving the existing destinations and
styling, including the registration and sign-in links.

Comment on lines +220 to +230
{t('auth.twoFactor')}
</h1>
<p className="mt-1 text-sm text-gray-500 dark:text-gray-400">
Enter the 6-digit code from your authenticator app, or one of your backup codes.
{t('auth.twoFactorHelp')}
</p>
</div>

<form onSubmit={handleSubmit(onSubmit)} className="space-y-4">
<div>
<label className="block text-sm font-medium text-gray-700 dark:text-gray-300 mb-1">
Code
{t('auth.code')}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Complete authentication localisation in both routes.

The PR translates only part of each authentication flow. Add catalogue keys and use t(...) for the remaining validation, error, success, and action text.

  • apps/web/src/routes/login.tsx#L220-L230: translate the TOTP validation message and the error, verification, and back controls.
  • apps/web/src/routes/register.tsx#L108-L108: translate the success heading, verification copy, confirmation label, submit states, and validation messages.
📍 Affects 2 files
  • apps/web/src/routes/login.tsx#L220-L230 (this comment)
  • apps/web/src/routes/register.tsx#L108-L108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/routes/login.tsx` around lines 220 - 230, Complete
authentication localization in apps/web/src/routes/login.tsx at lines 220-230 by
adding catalogue keys and replacing the remaining TOTP validation message, error
text, verification control, and back control labels with t(...) calls. Also
update apps/web/src/routes/register.tsx at line 108 to localize the success
heading, verification copy, confirmation label, submit-state text, and
validation messages using catalogue keys and t(...).

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/web/src/lib/i18n.tsx`:
- Around line 62-67: Update getInitialLocale to return a server-stable default
locale for both server and initial browser rendering, rather than reading the
URL, localStorage, or i18next.language during initialization. Move the
query/storage/browser locale detection into the post-hydration logic covering
the related lines, then apply the normalized locale after hydration so the
initial translated output remains consistent.
- Around line 62-67: Update getInitialLocale to apply the supported-locale
policy in URL, browser language, then stored locale order, validating each
candidate through the existing normalization logic. In apps/web/src/lib/i18n.tsx
lines 62-67, prefer the query locale, then browser language, then localStorage;
in lines 161-161, mirror that same validated selection before assigning
document.documentElement.lang so unsupported values are never written.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d75005a1-1389-4251-8c6a-266b1483e82b

📥 Commits

Reviewing files that changed from the base of the PR and between 8403da0 and 4490193.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • apps/web/package.json
  • apps/web/src/__tests__/lib/i18n.test.tsx
  • apps/web/src/components/CommandPalette.tsx
  • apps/web/src/components/ErrorState.tsx
  • apps/web/src/components/OAuthButtons.tsx
  • apps/web/src/i18n/en.json
  • apps/web/src/lib/i18n.tsx
  • apps/web/src/routes/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/web/src/tests/lib/i18n.test.tsx

Comment thread apps/web/src/lib/i18n.tsx
Comment on lines +62 to +67
function getInitialLocale(): Locale {
if (typeof window === 'undefined') return normalizeLocale(i18next.language);
const queryLocale = new URLSearchParams(window.location.search).get(LOCALE_QUERY_KEY);
return normalizeLocale(
queryLocale ?? localStorage.getItem(LOCALE_STORAGE_KEY) ?? i18next.language,
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Avoid browser-specific locale output during hydration.

getInitialLocale() returns en during server rendering but can return zh-CN during the first browser render. A URL or stored locale can therefore make translated content differ before hydration completes. Initialise with a server-stable locale, then detect and apply the browser locale after hydration.

Also applies to: 85-101

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/lib/i18n.tsx` around lines 62 - 67, Update getInitialLocale to
return a server-stable default locale for both server and initial browser
rendering, rather than reading the URL, localStorage, or i18next.language during
initialization. Move the query/storage/browser locale detection into the
post-hydration logic covering the related lines, then apply the normalized
locale after hydration so the initial translated output remains consistent.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use one supported-locale selection policy.

Both implementations prioritise local storage over browser languages. This conflicts with the configured detector order and the stated URL, browser, then storage priority. The startup script also writes an unvalidated query or storage value to document.documentElement.lang.

  • apps/web/src/lib/i18n.tsx#L62-L67: select and normalise locales using URL, browser language, then stored locale.
  • apps/web/src/lib/i18n.tsx#L161-L161: mirror the same supported-locale validation and source order before setting the document language.
📍 Affects 1 file
  • apps/web/src/lib/i18n.tsx#L62-L67 (this comment)
  • apps/web/src/lib/i18n.tsx#L161-L161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/web/src/lib/i18n.tsx` around lines 62 - 67, Update getInitialLocale to
apply the supported-locale policy in URL, browser language, then stored locale
order, validating each candidate through the existing normalization logic. In
apps/web/src/lib/i18n.tsx lines 62-67, prefer the query locale, then browser
language, then localStorage; in lines 161-161, mirror that same validated
selection before assigning document.documentElement.lang so unsupported values
are never written.

@mankatcheung

Copy link
Copy Markdown
Owner Author

Closing as stale — this has been sitting unmerged since Aug 6 and predates the Job Finder → Trakwyn rebrand (branding, cookie names, package scope all changed since). Re-implementing JEF-82 fresh off current main rather than rebasing this.

mankatcheung added a commit that referenced this pull request Aug 17, 2026
* feat(jef-82): web internationalization, phase 1 (nav/auth/settings/landing)

Adds i18next + react-i18next + i18next-browser-languagedetector, with
5 flat locale catalogs (en, en-GB, zh-HK, zh-TW, zh-CN) at 100% key
parity, and wires them into nav, auth (login/register/TOTP), the
landing page, root 404/error boundaries, the command palette, error
messages, and the settings language switcher. Locale detection order:
URL ?locale= -> navigator.languages -> localStorage, persisted back to
both the URL and localStorage on change.

Re-implemented fresh rather than reviving the closed #253 (stale,
predates the Job Finder -> Trakwyn rebrand) -- reused its i18next setup
and translation catalogs as a seed, backfilled the key coverage gap
those catalogs had (only en.json carried the full key set; the other
four were missing landing.*, commands.*, and several errors.*/common.*
keys), and reapplied everything onto the current cookie-only-auth
component structure rather than merging two divergent branches.

Detection tooling for the remaining surfaces: eslint-plugin-i18next's
no-literal-string rule, warn-only, mode: jsx-text-only, scoped to
apps/web/src. Produces 412 warnings across the untranslated surfaces
(applications/documents/offers CRUD, analytics, assistant, settings/
experience, etc.) -- the concrete phase 2/3 checklist. Two of those
412 are expected false positives (the "Trakwyn" brand name in the
header/footer logo, which shouldn't be translated).

Known simplification flagged for follow-up, not fixed here: OAuthButtons
concatenates a translated verb (t('auth.signIn')) with a translated
"with Google"/"with GitHub" suffix -- grammatically fine in English,
but doesn't produce a natural sentence in the Chinese locales. AI-
generated translations throughout should get native-speaker review
before this ships to real users.

* feat(jef-82): i18n phase 2a - applications list/board/CRUD forms

Localizes the applications list page (search, filters, bulk actions,
empty states), the Kanban board, and the new/edit application forms.
Adds ~55 new keys (applications.*, applicationForm.*) at full 5-locale
parity.

Fixes a real bug in i18n.tsx surfaced by this work: useLocale()'s t()
was typed as (key: string) => string with no way to pass interpolation
values, and neither implementation forwarded a second argument to
i18next even if one had been passed. Every {{count}}/{{term}}/etc.
placeholder wired up in this phase would have rendered literally
instead of being substituted. Added a TranslateOptions parameter and
forward it through both the context and useTranslation() fallback
paths; added a regression test proving substitution and count-based
plural-form selection both work now.

Also fixes a detection-methodology blind spot beyond what the lint
rule catches: StatusBadge and the board's column headers render
`{status}` as a JS expression, not a JSX text literal, so
eslint-plugin-i18next's jsx-text-only mode can't see it -- yet it's
untranslated everywhere applications are listed. Wired both to
`t(`status.${status}`)`, with StatusBadge defaulting to the raw value
via i18next's `defaultValue` option if a future status has no
translation yet, rather than leaking the raw "status.foo" key to the
user.

Also discovered: form field labels/placeholders passed as string props
(e.g. <Field label="Company *">) are equally invisible to
jsx-text-only mode -- same blind spot as aria-label, just not
mentioned explicitly before. Translated the labels in the two files
touched this pass; placeholders (e.g. "Acme Corp", "$120k-$160k")
deliberately left as illustrative example text, not translated.

Updated three existing tests (StatusBadge, ApplicationsPage, BoardPage)
whose assertions depended on the previously-untranslated raw status
strings.

* feat(jef-82): i18n phase 2b - application detail page

Localizes the application detail/view page: section tabs, info-item
labels (salary/applied/follow-up/description, reusing the form labels
for location/source/tags/job URL), star/delete button titles, note
CRUD (add/save/cancel, placeholder, delete-toast), and the offers
comparison panel. Adds ~26 new keys (applicationDetail.*) at full
5-locale parity, reusing applications.*/applicationForm.*/common.*
keys wherever the exact same phrase already existed elsewhere.

* feat(jef-82): localize documents, contacts, interviews, and offer form tabs

Wires DocumentsTab, ContactsTab, InterviewsTab, and OfferForm through
useLocale()'s t(), adding documents.*, contacts.*, interviews.*, and
offerForm.* keys to all 5 locale catalogs (en, en-GB, zh-HK, zh-TW, zh-CN).

Beyond what eslint-plugin-i18next's jsx-text-only lint rule flags, this
also fixes two categories of hardcoded text it can't see:
- Dynamic enum-driven text rendered as a JS expression rather than a JSX
  literal: document type badges, interview round type/outcome badges.
- String-literal props: FormLabel/placeholder/aria-label/title text across
  all four files.

Placeholder example data (names, emails, phone number formats, currency
codes) is deliberately left untranslated as illustrative example values,
consistent with prior phases.

Updates InterviewsTab.test.tsx assertions for the now-capitalized/
localized status/type badge text.

eslint-plugin-i18next warning count: 374 -> 334.

* feat(jef-82): localize the security settings page

Wires SettingsSecurityPage through useLocale()'s t(), adding ~85
security.* keys to all 5 locale catalogs. Converts shared.ts's
OAUTH_PROVIDER_LABEL and SECURITY_EVENT_LABEL maps and describeDevice()
from hardcoded English strings to translation-key lookups done at the
call site (describeDevice now returns a DeviceKind key rather than text),
since shared.ts has no hook access to call t() itself.

Fixes a real bug found while wiring this up: useLocale()'s no-provider
fallback path (used by most component tests, and by any real page that
somehow renders outside LocaleProvider) built a brand new object -- with
a new `t` closure -- on every render, unlike the LocaleProvider path
which memoizes via useMemo. Adding `t` to a useEffect dependency array
(the correct fix for the exhaustive-deps lint warning here) exposed this:
the effect re-fired on every render instead of once on mount, and in the
security-activity fetch specifically this caused a duplicate error
message once a later mock rejection landed. Fixed at the root by wrapping
the fallback in the same useMemo pattern as the provider path, verified
with a standalone referential-stability test before wiring `t` into the
effect's deps.

eslint-plugin-i18next warning count: 334 -> 290.

* feat(jef-82): localize the experience settings page

Wires SettingsExperiencePage (work experience/education/skills CRUD)
through useLocale()'s t(), adding experience.* keys plus reusable
common.add/common.edit to all 5 locale catalogs.

Beyond the lint-flagged JSX text, fixes a dynamic-text blind spot: the
skill proficiency chip rendered the raw lowercase enum value
(skill.proficiency, e.g. "beginner") instead of the capitalized label
shown in the <Select> options -- a pre-existing display bug, not just an
i18n gap. Now resolves through the same t(`experience.${value}`,
{ defaultValue }) pattern used for StatusBadge/document types elsewhere.

eslint-plugin-i18next warning count: 290 -> 254.

* feat(jef-82): localize the integrations settings page

Wires SettingsIntegrationsPage (AI provider API keys, custom AI
instructions, API tokens, share links) through useLocale()'s t(), adding
integrations.* keys plus reusable common.copy/common.done to all 5
locale catalogs.

Left LLM_PROVIDER_LABEL/LLM_PROVIDER_OPTIONS in shared.ts untranslated on
purpose -- their values are provider brand names (OpenAI, Anthropic
(Claude), DeepSeek, NVIDIA NIM, etc.), the same category of proper noun
already left alone for currency codes in OfferForm.

eslint-plugin-i18next warning count: 254 -> 227.

* feat(jef-82): localize resume match and notifications settings pages

Wires ResumeMatchTab and SettingsNotificationsPage through useLocale()'s
t(), adding resumeMatch.* and notifications.* keys to all 5 locale
catalogs. en-GB uses "CV" per the existing convention (tabResumeMatch,
landing copy) rather than "resume".

eslint-plugin-i18next warning count: 227 -> 201.

* feat(jef-82): localize the data settings page

Wires SettingsDataPage (export/import/delete account) through
useLocale()'s t(), adding data.* keys to all 5 locale catalogs.

Replaces the hand-rolled `count === 1 ? '' : 's'` pluralization in the
import-result summary with i18next's native count-based plural key
selection (data.importedApplications_one/_other,
data.andNotes_one/_other, etc.) -- the same pattern already used for
applications.deleted_one/_other. Chinese catalogs only carry the
_other form per the established convention (Chinese plural resolution
never selects _one).

eslint-plugin-i18next warning count: 201 -> 189.

* feat(jef-82): localize the dashboard page

Wires DashboardPage through useLocale()'s t(), adding dashboard.* keys
and reusing status.applied/interviewing/offered, applicationDetail.
followUpLabel, and applications.noApplicationsYet where the copy is
identical. Moves UPCOMING_EVENT_LABEL and statItems (both keyed dynamic
label lookups -- the StatusBadge/document-type blind spot pattern again)
inside the component so they can call t().

Uses i18next's count-based plural keys for the weekly-goal streak
message (dashboard.streakWeeks_one/_other) instead of a one-off ternary.

eslint-plugin-i18next warning count: 189 -> 178.

* feat(jef-82): localize the offer comparison page

Wires the offers/compare route through useLocale()'s t(), adding
offerCompare.* keys to all 5 locale catalogs and reusing
offerForm.equityLabel/benefitsLabel for the matching table headers.

eslint-plugin-i18next warning count: 178 -> 167.

* feat(jef-82): localize the new document draft page

Wires documents/new through useLocale()'s t(), adding documentDraft.*
keys and reusing documents.cover_letter/documents.resume from
DocumentsTab for the type toggle, placeholder, and default-title
fallback -- same phrases, same keys.

eslint-plugin-i18next warning count: 167 -> 157.

* feat(jef-82): localize the TOTP step of the login page

TotpStep is a separate function component from LoginPage and wasn't
wired to useLocale() -- LoginPage itself was already fully localized.
All text reuses existing auth.* keys already in the catalogs (no new
keys needed).

Leaves the "Trakwyn" wordmark literal (2 remaining warnings) -- it's the
product's own brand name, same category as currency codes and LLM
provider names left untranslated elsewhere.

eslint-plugin-i18next warning count: 157 -> 150.

* feat(jef-82): localize the profile settings page

Wires SettingsProfilePage through useLocale()'s t(), adding profile.*
keys to all 5 locale catalogs. Moves THEME_OPTIONS inside the component
(module scope couldn't call t()) and reuses documents.uploading /
security.remove where the phrase is identical.

eslint-plugin-i18next warning count: 150 -> 141.

* feat(jef-82): localize the interview round analytics panel

Wires InterviewRoundAnalyticsPanel through useLocale()'s t(), reusing
interviews.phone/technical/onsite/hr/other for the per-type labels
instead of a duplicate TYPE_LABEL map. Replaces the hand-rolled
singular/plural ternaries for offer/rejection sample-size text with
i18next's _one/_other plural keys.

eslint-plugin-i18next warning count: 141 -> 133.

* feat(jef-82): localize shared summary, company briefing, cover letter, and JD import panel

Wires SharedSummaryPage, CompanyBriefingTab, CoverLetterTab, and
JdImportPanel through useLocale()'s t(), adding sharedSummary.*,
companyBriefing.*, coverLetter.*, and jdImport.* keys to all 5 locale
catalogs. Reuses status.* for SharedSummaryPage's STATUS_LABEL map (an
exact duplicate of the existing catalog), and reuses
resumeMatch.addApiKeyPrefix/accountSettingsLinkText/addApiKeySuffix for
the "add your AI API key" message, which is repeated verbatim across
three AI-feature panels.

eslint-plugin-i18next warning count: 133 -> 105.

* feat(jef-82): localize reset password, channel analytics, offers, and assistant pages

Wires ResetPasswordPage, ApplicationChannelAnalyticsPanel, the offers
index route, and AssistantPage through useLocale()'s t(), adding
resetPassword.*, channelAnalytics.*, offers.*, and assistant.* keys.
Reuses security.newPasswordLabel/confirmNewPasswordLabel,
auth.backToSignIn, interviewAnalytics.smallSample,
offerCompare.loadingOffers/bonusSuffix, and integrations.providerLabel
where the phrase already exists verbatim. Moves SUGGESTED_QUESTIONS
inside AssistantPage (module scope couldn't call t()) and replaces its
manual plural ternary with channelAnalytics.appsCount_one/_other.

eslint-plugin-i18next warning count: 105 -> 81.

* feat(jef-82): localize confirm-backup-email, document outcomes, notification inbox, response time, chat, and step-up reauth

Wires ConfirmBackupEmailPage, DocumentVersionOutcomesPanel,
NotificationInboxButton/Panel, ResponseTimeAnalyticsPanel,
ChatConversationView, and useStepUpReauth's dialog through useLocale()'s
t(), adding confirmBackupEmail.*, documentOutcomes.*, notificationInbox.*,
responseTime.*, chat.*, and stepUpReauth.* keys, plus a reusable
common.close. Reuses documents.resume, interviewAnalytics.smallSample/
median, status.*, auth.password/backToSignIn/verifying,
applications.selectAll/deselectAll/selectedCount, security.confirm, and
resumeMatch.addApiKeyPrefix/accountSettingsLinkText/addApiKeySuffix
where the phrase already exists.

Kept documentOutcomes.coverLetterLabel ("Cover letter", lowercase l) as
a separate key from documents.cover_letter ("Cover Letter") since an
existing test asserts the exact original casing.

timeAgo() in -notification-inbox.tsx is a module-scope pure function
that can't call a hook, so it now takes `t` as a parameter instead.
Same fix applied to LOADING_MESSAGES in ChatConversationView (moved
inside the component) -- and while there, extracted a fixed
LOADING_MESSAGE_COUNT constant so the rotation's setInterval doesn't
need the translated (and therefore per-render-new) array in its
dependency array.

eslint-plugin-i18next warning count: 81 -> 51.

* feat(jef-82): localize forgot/verify/confirm-email pages, document draft editor, and conversation history

Wires ForgotPasswordPage, VerifyEmailPage, confirm-email-change,
DocumentDraftEditPage, ConversationHistoryPage, and
ChatDockConversationPicker through useLocale()'s t(), adding
forgotPassword.*, verifyEmail.*, documentDraftEdit.*,
conversationHistory.*, and confirmEmailChange.* keys. Reuses
auth.email/backToSignIn, security.sending, confirmBackupEmail.verifying,
documents.cover_letter/resume, common.delete, and
applicationForm.saving where the phrase already exists.

The assistant module's shared timeAgo() (in -shared.ts, used by both
ConversationHistoryPage and the chat dock picker) is a plain exported
function, not a hook -- same constraint as -notification-inbox.tsx's
timeAgo earlier -- so it now takes `t` as a parameter too, reusing the
same notificationInbox.justNow/minutesAgo/hoursAgo/daysAgo keys rather
than duplicating them under a second namespace.

eslint-plugin-i18next warning count: 51 -> 29.

* feat(jef-82): finish the JEF-82 i18n sweep across the remaining app shell and analytics tail

Wires the last untranslated files through useLocale()'s t():
ShortcutCheatSheet, AnalyticsPage, ChatDockFloatingWindow/Footer,
AuthenticatedLayout, OfferAnalyticsPanel, calendar.tsx,
DocumentPreviewModal, HealthScorePanel, and the settings layout's nav.
Adds shortcuts.*, analyticsPage.*, chatDock.*, authenticatedLayout.*,
offerAnalytics.*, calendarPage.*, documentPreview.*, and
healthScore.applicationHealth, plus one-off fallback-error keys in
RegisterPage. Reuses status.*, applicationDetail.followUpLabel,
dashboard.eventInterview/statTotal, applications.title,
settings.profile/experience/security/integrations/notifications/data,
nav.settings/calendar/analytics, assistant.*, common.close, and
shortcuts.showShortcuts wherever the phrase is identical, rather than
duplicating.

Most of these fixes go beyond what eslint-plugin-i18next's
jsx-text-only rule flags -- module-scope label maps/arrays (SETTINGS_NAV,
VIEW_MODES, WEEKDAY_LABELS, EVENT_LABEL) duplicating existing catalog
entries, and prop-based text (aria-label, title, alt) -- found by
reading each file rather than by lint output alone. calendar.tsx's
weekday abbreviations now come from Intl.DateTimeFormat(locale, {
weekday: 'short' }) instead of a hardcoded English array, so they're
locale-correct rather than translated English strings. The stage-funnel
chart's Y-axis now runs status values through a tickFormatter so the
axis labels aren't raw enum strings either.

eslint-plugin-i18next warning count: 29 -> 10. All 10 remaining are
deliberate: 7 are the "Trakwyn" wordmark (brand name, never localized),
and the last is a keyboard-shortcut symbol ("+ /") that isn't
language-dependent text.
@mankatcheung
mankatcheung deleted the feat/jef-82-support-i18n branch August 17, 2026 21:20
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