feat: renderer i18n rollout with locale selector#123
Conversation
Reviewer's GuideImplements renderer-wide internationalization with a persistent language selector (EN/ES/FR/DE), wiring i18next/react-i18next into the app shell, localizing major UI surfaces (tabs, config, source, processed, file tree, dark mode, errors), and adding locale settings utilities plus tests to enforce key parity and locale persistence in Playwright E2E. Sequence diagram for locale change via LanguageSelectorsequenceDiagram
actor User
participant LanguageSelector
participant I18n as I18nextInstance
participant LocalStorage
participant ReactApp
User->>LanguageSelector: change select value
LanguageSelector->>LanguageSelector: handleLanguageChange(event)
LanguageSelector->>I18n: changeLanguage(nextLocale)
I18n-->>LanguageSelector: Promise resolved
LanguageSelector->>LocalStorage: setItem(LOCALE_STORAGE_KEY, nextLocale)
I18n-->>ReactApp: notify language changed
ReactApp->>ReactApp: re-render components
ReactApp->>User: UI text shown in selected locale
Sequence diagram for translated error handling with AppProvider and ErrorBannersequenceDiagram
participant AppProvider
participant ShowError as showError
participant AppState as AppErrorState
participant ErrorBanner
participant T as useTranslation_t
participant User
AppProvider->>ShowError: showError({translationKey})
ShowError->>AppState: setAppError({translationKey, translationOptions, message})
AppState-->>ErrorBanner: appError updated
ErrorBanner->>T: t(appError.translationKey, appError.translationOptions)
T-->>ErrorBanner: translatedMessage
ErrorBanner-->>User: render error banner with translated text
User->>ErrorBanner: click dismiss
ErrorBanner->>AppProvider: dismissError()
AppProvider->>AppState: setAppError(null)
Updated class diagram for renderer i18n settings and LanguageSelectorclassDiagram
class SupportedLocale {
<<type>>
}
class Settings {
<<module>>
+SUPPORTED_LOCALES : SupportedLocale[]
+DEFAULT_LOCALE : SupportedLocale
+LOCALE_STORAGE_KEY : string
+isSupportedLocale(value string | null | undefined) bool
+getInitialLocale() SupportedLocale
+persistLocale(locale SupportedLocale) void
}
class LanguageSelector {
+LanguageSelector()
-handleLanguageChange(event ReactChangeEventHTMLSelectElement) void
}
class I18nInstance {
<<i18next>>
+isInitialized : boolean
+language : string
+resolvedLanguage : string
+use(plugin InitReactI18next) I18nInstance
+init(options I18nInitOptions) void
+changeLanguage(locale SupportedLocale) Promise
+t(key string, options object) string
}
class I18nConfig {
<<module>>
-resources : object
+i18n : I18nInstance
}
class AppError {
+message : string
+translationKey : string
+translationOptions : Record~string, string | number~
+timestamp : number
}
class AppProvider {
+appError : AppError
+showError(error string) void
+showError(error ErrorPayload) void
+dismissError() void
}
class ErrorPayload {
+translationKey : string
+translationOptions : Record~string, string | number~
+message : string
}
class ErrorBanner {
+ErrorBanner()
}
class ReactComponent {
<<react-i18next_hook>>
+useTranslation() TranslationHook
}
class TranslationHook {
+t(key string, options object) string
+i18n : I18nInstance
}
Settings --> SupportedLocale : defines
Settings ..> I18nConfig : used_by
I18nConfig --> I18nInstance : configures
LanguageSelector ..|> ReactComponent : uses_useTranslation
LanguageSelector --> TranslationHook : obtains
LanguageSelector --> Settings : reads_supported_locales
LanguageSelector --> I18nInstance : calls_changeLanguage
LanguageSelector --> Settings : calls_persistLocale
AppProvider --> AppError : manages
AppProvider --> ErrorPayload : accepts
ErrorBanner ..|> ReactComponent : uses_useTranslation
ErrorBanner --> AppProvider : reads_appError
ErrorBanner --> TranslationHook : uses_t
ErrorBanner --> AppError : renders
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
📝 WalkthroughWalkthroughThis PR introduces comprehensive internationalization (i18n) support across the renderer UI layer, adding language selection, locale persistence, and translated strings for English, Spanish, French, and German. Supporting infrastructure includes locale management, i18n initialization, and test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LanguageSelector as LanguageSelector<br/>Component
participant i18n as i18next Instance
participant localStorage as Browser<br/>Storage
participant UIComponents as UI Components<br/>(App, ConfigTab, etc.)
User->>LanguageSelector: Select language (e.g., 'es')
activate LanguageSelector
LanguageSelector->>i18n: changeLanguage('es')
activate i18n
i18n->>i18n: Load locale resources
i18n-->>LanguageSelector: Language updated
deactivate i18n
LanguageSelector->>localStorage: persistLocale('es')
activate localStorage
localStorage->>localStorage: Store app.locale = 'es'
deactivate localStorage
LanguageSelector-->>UIComponents: Trigger re-render (i18n context change)
deactivate LanguageSelector
activate UIComponents
UIComponents->>i18n: t('translation.key')
i18n-->>UIComponents: Return translated string
UIComponents->>UIComponents: Render with Spanish text
deactivate UIComponents
Note over User,UIComponents: On page reload:
User->>User: Page reload
activate UIComponents
UIComponents->>localStorage: getInitialLocale()
activate localStorage
localStorage-->>UIComponents: Retrieve 'es' from storage
deactivate localStorage
UIComponents->>i18n: changeLanguage('es')
activate i18n
i18n-->>UIComponents: Load Spanish resources
deactivate i18n
UIComponents->>UIComponents: Render with persisted locale
deactivate UIComponents
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.16.0)package.json┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/renderer/components/App.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m src/renderer/components/ConfigTab.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m �[1m Loading rules from local config...�[0m
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. Comment |
Summary of ChangesHello @Mehdi-Bl, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's user experience by introducing full internationalization support. Users can now select their preferred language from English, Spanish, French, or German, and this preference will be remembered. The changes involve integrating a new localization framework, updating numerous UI components to use localized strings, and adding new components for language selection. This foundational work makes the application accessible to a broader audience and streamlines future language additions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Dependency Review✅ No vulnerabilities or license issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches. Re-running this action after a short time may resolve the issue. See the documentation for more information and troubleshooting advice. Scanned Files
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully introduces internationalization to the renderer with support for English, Spanish, French, and German. The implementation uses i18next and react-i18next effectively, covering most UI text, validation messages, and error banners. The addition of a persistent LanguageSelector and comprehensive tests ensures a solid foundation for multi-language support. However, there are a few areas where localization is incomplete and some minor issues in the updated documentation and error handling reactivity that should be addressed to improve consistency and maintainability.
|
Review Summary by QodoAdd internationalization (i18n) support with multi-language locale selector
WalkthroughsDescription• Add i18n support with EN/ES/FR/DE locale bundles and persistent language selector • Localize all UI text across tabs, config, source, processed, file-tree, and error messages • Implement locale persistence via localStorage with automatic browser language detection • Add locale key parity tests and E2E locale persistence coverage • Update AGENTS.md with planning docs and code structure guidelines Diagramflowchart LR
A["i18next + react-i18next"] --> B["Locale Settings<br/>EN/ES/FR/DE"]
B --> C["Language Selector<br/>Component"]
C --> D["localStorage<br/>Persistence"]
D --> E["UI Localization<br/>All Components"]
E --> F["Error Messages<br/>Translated"]
B --> G["Locale Parity<br/>Tests"]
B --> H["E2E Locale<br/>Tests"]
File Changes1. src/renderer/i18n/index.ts
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
AppContext.tsxyou’re callingi18n.t(...)insidehandleAnalyze/handleReanalyzewithout importing thei18ninstance in that file, which will cause a runtime reference error; add the appropriate import fromsrc/renderer/i18n. - You now import the i18n bootstrap file in both
index.tsxandApp.tsx(./i18n/../i18n); even with theisInitializedguard it would be cleaner to initialize i18n only once in the renderer entrypoint and drop the second import to avoid redundant side effects.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `AppContext.tsx` you’re calling `i18n.t(...)` inside `handleAnalyze`/`handleReanalyze` without importing the `i18n` instance in that file, which will cause a runtime reference error; add the appropriate import from `src/renderer/i18n`.
- You now import the i18n bootstrap file in both `index.tsx` and `App.tsx` (`./i18n` / `../i18n`); even with the `isInitialized` guard it would be cleaner to initialize i18n only once in the renderer entrypoint and drop the second import to avoid redundant side effects.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/renderer/components/ConfigTab.tsx`:
- Around line 408-413: Replace the hardcoded English fallback 'Unknown error'
with a translated fallback using the i18n function t inside the
setProviderTestResult call: when computing the message for setProviderTestResult
(in the catch block where t('config.connectionTestFailed', ...) is used), use
error instanceof Error ? error.message : t('config.unknownError') (or your
project's existing "unknown error" key, e.g. 'errors.unknown' or
'common.unknownError') so the fallback is localized; update the translation key
if necessary.
🧹 Nitpick comments (10)
src/renderer/context/AppContext.tsx (1)
484-484: Inconsistent error translation strategy: eageri18n.t()vs. deferredtranslationKey.Lines 484 and 543 use
i18n.t('errors.electronApiUnavailable')to bake a translated string into theErrormessage at throw time, while all other error sites pass{ translationKey: '...' }toshowErrorfor deferred/reactive translation. Since these thrown errors are caught and re-displayed with a generic key (e.g.,errors.processingFailed), the eager-translated message only appears inconsole.error— meaning console logs become locale-dependent, which can hinder debugging.Consider using a fixed English string for the thrown
Error(since it's only for console/developer diagnostics) and keepingtranslationKeyfor user-facing display:♻️ Suggested change
- throw new Error(i18n.t('errors.electronApiUnavailable')); + throw new Error('Electron API is not available');Apply the same at both occurrences (Lines 484 and 543).
Also applies to: 543-543
tests/setup.ts (2)
2-3: Redundant side-effect import.Line 2 (
import '../src/renderer/i18n') is a side-effect-only import, but line 3 (import i18n from '../src/renderer/i18n') already triggers the same module initialization. The side-effect import on line 2 can be removed.♻️ Suggested fix
import '@testing-library/jest-dom'; -import '../src/renderer/i18n'; import i18n from '../src/renderer/i18n';
5-8: Consider importingLOCALE_STORAGE_KEYinstead of hardcoding'app.locale'.The storage key is defined as a constant in
src/renderer/i18n/settings.ts. Hardcoding it here creates a silent breakage risk if the key is ever renamed.♻️ Suggested fix
+import { LOCALE_STORAGE_KEY } from '../src/renderer/i18n/settings'; + beforeEach(async () => { - window.localStorage.setItem('app.locale', 'en'); + window.localStorage.setItem(LOCALE_STORAGE_KEY, 'en'); await i18n.changeLanguage('en'); });tests/unit/i18n/locales-parity.test.ts (1)
8-42: Consider extending parity checks to validate placeholder consistency.The current test verifies key parity but not that dynamic placeholders (e.g.,
{{name}},{{selected}},{{total}}) are present in all locale translations where the English source uses them. A missing placeholder would silently render raw{{name}}text to the user.This could be a follow-up enhancement — extract
{{...}}tokens from English values and assert their presence in each locale's corresponding value.tests/e2e/electron-process-flow.spec.ts (1)
332-344: Solid e2e coverage for locale persistence.The test properly validates the full round-trip: default → change → reload → verify persistence. One minor consideration:
After
page.reload()on line 340, you wait fordomcontentloadedbut don't wait for the app heading or any other readiness signal before asserting. If i18n initialization is async, the assertions on lines 342-343 could be flaky. The existing fixture (line 253) waits for the heading — consider adding a similar wait here for robustness.Suggested improvement
await page.reload(); await page.waitForLoadState('domcontentloaded'); + await expect(page.getByRole('heading', { name: 'AI Code Fusion' })).toBeVisible(); await expect(page.getByTestId('language-selector')).toHaveValue('es'); await expect(page.getByRole('tab', { name: 'Inicio' })).toBeVisible();tests/unit/components/language-selector.test.tsx (1)
25-38: Spy cleanup could be missed on test failure.If the
waitForassertion on line 31 throws,setItemSpy.mockRestore()on line 37 is never reached, potentially leaking the spy into subsequent tests. Consider usingafterEachor placing the restore in afinallyblock.Safer spy cleanup
test('changes locale and persists it to localStorage', async () => { const setItemSpy = jest.spyOn(Storage.prototype, 'setItem'); - render(<LanguageSelector />); - - fireEvent.change(screen.getByTestId('language-selector'), { target: { value: 'es' } }); - - await waitFor(() => { - expect(i18n.resolvedLanguage?.startsWith('es') || i18n.language.startsWith('es')).toBe(true); - expect(setItemSpy).toHaveBeenCalledWith(LOCALE_STORAGE_KEY, 'es'); - expect(screen.getByLabelText('Idioma')).toBeInTheDocument(); - }); - - setItemSpy.mockRestore(); + try { + render(<LanguageSelector />); + + fireEvent.change(screen.getByTestId('language-selector'), { target: { value: 'es' } }); + + await waitFor(() => { + expect(i18n.resolvedLanguage?.startsWith('es') || i18n.language.startsWith('es')).toBe(true); + expect(setItemSpy).toHaveBeenCalledWith(LOCALE_STORAGE_KEY, 'es'); + expect(screen.getByLabelText('Idioma')).toBeInTheDocument(); + }); + } finally { + setItemSpy.mockRestore(); + } });src/renderer/components/LanguageSelector.tsx (1)
18-22: Missing error handling forchangeLanguage.If
i18n.changeLanguagerejects, the unhandled promise will surface as an uncaught error. Consider wrapping in try/catch so the locale isn't persisted on failure and the user gets feedback.Suggested fix
- const handleLanguageChange = async (event: React.ChangeEvent<HTMLSelectElement>) => { - const nextLocale = event.target.value as SupportedLocale; - await i18n.changeLanguage(nextLocale); - persistLocale(nextLocale); - }; + const handleLanguageChange = async (event: React.ChangeEvent<HTMLSelectElement>) => { + const nextLocale = event.target.value as SupportedLocale; + try { + await i18n.changeLanguage(nextLocale); + persistLocale(nextLocale); + } catch (error) { + console.error('Failed to change language:', error); + } + };src/renderer/i18n/index.ts (1)
10-15: Resources map must be kept in sync withSUPPORTED_LOCALESmanually.If a locale is added to
SUPPORTED_LOCALESinsettings.tsbut not to thisresourcesobject, it will silently have no translations. The locale parity tests should catch missing keys but won't catch a completely missing locale resource import. Consider deriving the resources map programmatically or adding a static assertion.src/renderer/components/FileTree.tsx (1)
44-44:useTranslationin every recursive tree item — note the performance trade-off.Each
FileTreeItemComponentinstance subscribes to i18n change events viauseTranslation(). On a language switch with a large expanded tree (thousands of nodes), all items re-render simultaneously. This is acceptable since language changes are infrequent, but worth being aware of if tree performance becomes a concern.src/renderer/components/App.tsx (1)
6-6: Remove the redundant i18n side-effect import.Since
src/renderer/index.tsxalready imports and initializes i18n before rendering the App component, the import on line 6 is unnecessary. The i18n module has a guard (if (!i18n.isInitialized)) that prevents re-initialization, making this import here redundant and worth removing for clarity.
| setProviderTestResult({ | ||
| ok: false, | ||
| message: `Connection test failed: ${ | ||
| error instanceof Error ? error.message : 'Unknown error' | ||
| }`, | ||
| message: t('config.connectionTestFailed', { | ||
| message: error instanceof Error ? error.message : 'Unknown error', | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Untranslated fallback string 'Unknown error' in catch block.
The error message fallback 'Unknown error' on line 411 is hardcoded in English while all other strings are translated.
Suggested fix
- message: t('config.connectionTestFailed', {
- message: error instanceof Error ? error.message : 'Unknown error',
- }),
+ message: t('config.connectionTestFailed', {
+ message: error instanceof Error ? error.message : t('common.unknownError'),
+ }),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setProviderTestResult({ | |
| ok: false, | |
| message: `Connection test failed: ${ | |
| error instanceof Error ? error.message : 'Unknown error' | |
| }`, | |
| message: t('config.connectionTestFailed', { | |
| message: error instanceof Error ? error.message : 'Unknown error', | |
| }), | |
| }); | |
| setProviderTestResult({ | |
| ok: false, | |
| message: t('config.connectionTestFailed', { | |
| message: error instanceof Error ? error.message : t('common.unknownError'), | |
| }), | |
| }); |
🤖 Prompt for AI Agents
In `@src/renderer/components/ConfigTab.tsx` around lines 408 - 413, Replace the
hardcoded English fallback 'Unknown error' with a translated fallback using the
i18n function t inside the setProviderTestResult call: when computing the
message for setProviderTestResult (in the catch block where
t('config.connectionTestFailed', ...) is used), use error instanceof Error ?
error.message : t('config.unknownError') (or your project's existing "unknown
error" key, e.g. 'errors.unknown' or 'common.unknownError') so the fallback is
localized; update the translation key if necessary.
Code Review by Qodo
1. connectionTestFailed exposes error.message
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| message: t('config.connectionTestFailed', { | ||
| message: error instanceof Error ? error.message : 'Unknown error', | ||
| }), |
There was a problem hiding this comment.
1. connectiontestfailed exposes error.message 📘 Rule violation ⛨ Security
The provider connection test failure message interpolates and displays the raw caught error.message to the user, which can leak internal/system details. This violates the requirement that user-facing errors remain generic with details only in internal logs.
Agent Prompt
## Issue description
The provider connection test error UI currently interpolates `error.message` into the user-facing string, which can leak internal details.
## Issue Context
Compliance requires user-facing errors to be generic, while detailed error information (including stack traces / internal messages) should only go to internal logs.
## Fix Focus Areas
- src/renderer/components/ConfigTab.tsx[410-412]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const getInitialLocale = (): SupportedLocale => { | ||
| if (typeof window === 'undefined') { | ||
| return DEFAULT_LOCALE; | ||
| } | ||
|
|
||
| const storedLocale = window.localStorage.getItem(LOCALE_STORAGE_KEY); | ||
| if (storedLocale) { | ||
| const normalizedStoredLocale = normalizeLocale(storedLocale); | ||
| if (normalizedStoredLocale) { | ||
| return normalizedStoredLocale; | ||
| } | ||
| } |
There was a problem hiding this comment.
2. Uncaught localstorage errors 🐞 Bug ⛯ Reliability
Locale initialization/persistence uses localStorage without any exception handling; if storage access is blocked or throws (SecurityError/QuotaExceededError), the renderer can fail during i18n init or when changing languages.
Agent Prompt
### Issue description
`getInitialLocale()` and `persistLocale()` call `window.localStorage.getItem/setItem` without guarding against runtime exceptions (e.g., blocked storage, quota exceeded). Because `getInitialLocale()` is executed during i18n initialization, an exception can prevent the renderer from booting.
### Issue Context
This code runs in the renderer and is invoked at import/init time.
### Fix Focus Areas
- src/renderer/i18n/settings.ts[25-36]
- src/renderer/i18n/settings.ts[53-59]
- src/renderer/i18n/index.ts[17-22]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1 participant
Summary
LanguageSelectorand locale settings utilitiesAGENTS.mdclarification to keepdocs/plan/*local-only and out of commitsValidation
npm run lintnpm test -- --runInBandnpm run qa:screenshotnpm run sonarNotes
docs/plan/*is not tracked in this branch (git ls-files | rg '^docs/plan/'=> no results)Summary by Sourcery
Add internationalization support to the renderer with a persistent language selector and localized UI text, and validate locale parity and persistence via new tests.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
Release Notes