Multi-language support and persistence#744
Multi-language support and persistence#744rolex-67 wants to merge 2 commits intoCircuitVerse:mainfrom
Conversation
✅ Deploy Preview for circuitverse ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThis PR adds Spanish, French, German, Japanese, Chinese, and Arabic locale files and expands i18n messages. i18n initialization now reads a persisted locale (via getInitialLocale) and exposes availableLocale entries. UserMenu.vue was refactored: avatar rendering moved to computed properties, locale changes are watched to persist to localStorage and set document.lang/dir, snackbar color mapping was added, authentication flows and error handling were consolidated to use showSnackbar, unread notifications badge was introduced, and a Bengali string was updated. Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive internationalization (i18n) support for the simulator, adding 7 new language locales and state persistence using localStorage. The PR also includes RTL layout support preparation, theme color normalization, and refactoring of the UserMenu component.
Key Changes:
- Added translations for Spanish, French, German, Japanese, and Chinese (5 new locales)
- Implemented locale persistence via localStorage with SSR-safe initialization
- Normalized color values to lowercase hex format across all themes
- Improved Custom Theme handling with better error handling and SSR safety
- Refactored jQuery dependencies to vanilla JavaScript in minimap.js
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| v0/src/locales/i18n.ts | Added getInitialLocale() function for locale persistence and registered 5 new locales |
| v0/src/locales/es.json | New Spanish translation file with complete translations |
| v0/src/locales/fr.json | New French translation file with complete translations |
| v0/src/locales/de.json | New German translation file with complete translations |
| v0/src/locales/jp.json | New Japanese translation file with complete translations |
| v0/src/locales/zh.json | New Chinese translation file with complete translations |
| v0/src/locales/bn.json | Updated Bengali locale with empty string issue |
| v0/src/components/Navbar/User/UserMenu.vue | Added locale watcher, RTL support logic, and refactored Vue syntax |
| v0/src/simulator/src/themer/themes.ts | Normalized hex colors and improved Custom Theme localStorage handling |
| v0/src/simulator/src/minimap.js | Migrated jQuery calls to vanilla JavaScript |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (newLocale === 'ar') { | ||
| document.documentElement.dir = 'rtl' | ||
| } else { | ||
| document.documentElement.dir = 'ltr' | ||
| } |
There was a problem hiding this comment.
RTL support checks for locale 'ar' (Arabic) on lines 263 and 280, but there is no Arabic locale file in the codebase and 'ar' is not included in the supported locales list in i18n.ts. This will cause the RTL logic to never execute. Either add the Arabic locale file or remove this RTL logic.
| } catch (error: any) { | ||
| showSnackbar(`Authentication failed: ${error.message}`, 'error') |
There was a problem hiding this comment.
The catch block now explicitly types the error parameter as 'any'. While this makes the type explicit, using 'any' defeats the purpose of TypeScript's type safety. Consider using 'unknown' and type-checking within the catch block, or defining a proper error type.
| } catch (error: any) { | |
| showSnackbar(`Authentication failed: ${error.message}`, 'error') | |
| } catch (error: unknown) { | |
| let errorMessage = 'Authentication failed' | |
| if (error instanceof Error && error.message) { | |
| errorMessage = `Authentication failed: ${error.message}` | |
| } else if (typeof error === 'string') { | |
| errorMessage = `Authentication failed: ${error}` | |
| } | |
| showSnackbar(errorMessage, 'error') |
v0/src/simulator/src/minimap.js
Outdated
| if (miniMapArea) { | ||
| miniMapArea.style.zIndex = '-1' | ||
| } | ||
| this.ctx.clearRect(0, 0, this.canvas.width, this.canvas.height) |
There was a problem hiding this comment.
The property name has been changed from 'context' to 'ctx' on line 174, but the rest of the code in the file uses 'this.ctx' (as seen in line 94, 95, etc.). Please verify that the original property name was actually 'context' and not 'ctx'. If it was already 'ctx', this change introduces a bug.
v0/src/locales/bn.json
Outdated
| { | ||
| "simulator": { | ||
| "save_online": "অনলাইন সংরক্ষণ করুন", | ||
| "save_online": "", |
There was a problem hiding this comment.
The translation for 'save_online' has been changed to an empty string, which will result in missing text in the UI. This should either be removed entirely to use the fallback locale, or properly translated. Note that line 31 has the same key with a translation, suggesting this may be a duplicate or incorrect location.
| "save_online": "", |
| */ | ||
| watch(locale, function(newLocale) { | ||
| localStorage.setItem('locale', newLocale) | ||
| document.documentElement.lang = newLocale | ||
|
|
||
| if (newLocale === 'ar') { | ||
| document.documentElement.dir = 'rtl' | ||
| } else { | ||
| document.documentElement.dir = 'ltr' | ||
| } | ||
| }) | ||
|
|
||
| /** | ||
| * Lifecycle hook to restore user's language preference on load. | ||
| */ | ||
| onMounted(function() { | ||
| const savedLocale = localStorage.getItem('locale') | ||
| if (savedLocale) { | ||
| locale.value = savedLocale | ||
| } | ||
|
|
||
| document.documentElement.lang = locale.value | ||
| if (locale.value === 'ar') { | ||
| document.documentElement.dir = 'rtl' | ||
| } else { | ||
| document.documentElement.dir = 'ltr' | ||
| } | ||
| }) |
There was a problem hiding this comment.
The locale is being set twice during initialization - once in onMounted (lines 274-276) and once already in the i18n factory via getInitialLocale(). This creates redundancy and potential inconsistency. The onMounted hook should check if the locale is already correctly set before overwriting it, or this initialization should be removed entirely since it's handled in i18n.ts.
| */ | |
| watch(locale, function(newLocale) { | |
| localStorage.setItem('locale', newLocale) | |
| document.documentElement.lang = newLocale | |
| if (newLocale === 'ar') { | |
| document.documentElement.dir = 'rtl' | |
| } else { | |
| document.documentElement.dir = 'ltr' | |
| } | |
| }) | |
| /** | |
| * Lifecycle hook to restore user's language preference on load. | |
| */ | |
| onMounted(function() { | |
| const savedLocale = localStorage.getItem('locale') | |
| if (savedLocale) { | |
| locale.value = savedLocale | |
| } | |
| document.documentElement.lang = locale.value | |
| if (locale.value === 'ar') { | |
| document.documentElement.dir = 'rtl' | |
| } else { | |
| document.documentElement.dir = 'ltr' | |
| } | |
| }) | |
| * Runs immediately to sync the DOM with the initial locale from i18n. | |
| */ | |
| watch( | |
| locale, | |
| function(newLocale) { | |
| localStorage.setItem('locale', newLocale) | |
| document.documentElement.lang = newLocale | |
| if (newLocale === 'ar') { | |
| document.documentElement.dir = 'rtl' | |
| } else { | |
| document.documentElement.dir = 'ltr' | |
| } | |
| }, | |
| { immediate: true } | |
| ) |
| visible: true, | ||
| message, | ||
| color: type | ||
| color: type === 'error' ? '#dc5656' : '#43b984' |
There was a problem hiding this comment.
The color mapping logic only handles 'error' type, but the showSnackbar function accepts multiple types including 'success', 'warning', and 'info'. All other types will incorrectly use the success color '#43b984'. Consider using a switch statement or object mapping to handle all snackbar types correctly.
| const requiredRule = function(v: string) { return !!v || 'This field is required' } | ||
| const emailRule = function(v: string) { return /.+@.+\..+/.test(v) || 'E-mail must be valid' } | ||
| const passwordRule = function(v: string) { return v.length >= 6 || 'Password must be at least 6 characters' } |
There was a problem hiding this comment.
Arrow functions have been replaced with function expressions (e.g., 'function(v: string) { return ... }'). While this is valid JavaScript, it's inconsistent with modern Vue 3 and TypeScript conventions where arrow functions are preferred for maintaining lexical 'this' context and better readability. Consider reverting to arrow function syntax.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @v0/src/locales/bn.json:
- Line 3: Add the missing Bengali translation for the simulator key by setting
"simulator.save_online" in bn.json to "অনলাইন সংরক্ষণ" (same as
"nav.project.save_online"), ensuring the JSON value is a proper string and
preserves surrounding commas/formatting.
In @v0/src/locales/es.json:
- Line 84: The translation value for the key "Gates" is misspelled as
"Comuertas"; update the string value for "Gates" in the locales file to the
correct Spanish word "Compuertas" so the JSON entry reads "Gates": "Compuertas".
In @v0/src/locales/i18n.ts:
- Around line 12-21: The supported locales list excludes Arabic so RTL code in
UserMenu.vue is never reachable; add the Arabic locale asset and wiring: import
the 'ar.json' locale file, include 'ar' in the supported array inside
getInitialLocale, and add 'ar' to the availableLocale export so Arabic can be
selected and the document.documentElement.dir = 'rtl' branch in UserMenu.vue
will run; alternatively remove the RTL branch if Arabic support is not intended.
In @v0/src/simulator/src/minimap.js:
- Around line 168-175: The clear() method incorrectly referenced this.context
previously and now correctly uses this.ctx; ensure the function checks the
global lightMode flag and returns early, selects the DOM element with id
'miniMapArea' and sets its style.zIndex to '-1' only if the element exists, and
finally calls this.ctx.clearRect(0, 0, this.canvas.width, this.canvas.height) to
clear the canvas; verify the symbol names clear(), lightMode, 'miniMapArea',
this.ctx, and this.canvas are used exactly as shown and maintain the defensive
null checks around miniMapArea before accessing style.
🧹 Nitpick comments (3)
v0/src/simulator/src/minimap.js (1)
196-199: Note: Fade animation removed.The refactoring changes the UX by replacing the smooth
fadeOut('fast')animation with an immediatedisplay = 'none'. This results in an abrupt disappearance of the minimap rather than a smooth transition. Consider whether this UX change is acceptable or if a CSS transition should be added to maintain the smooth hide effect.💡 Optional: Restore fade effect with CSS transition
If you want to maintain the smooth fade effect without jQuery, you can use CSS transitions:
const miniMap = document.getElementById('miniMap') if (miniMap) { + miniMap.style.opacity = '0' + miniMap.style.transition = 'opacity 0.2s' + setTimeout(() => { miniMap.style.display = 'none' + miniMap.style.opacity = '1' + miniMap.style.transition = '' + }, 200) }Alternatively, add a CSS class for the transition effect.
v0/src/locales/i18n.ts (1)
8-8: Consider using 'ja' instead of 'jp' for Japanese locale.The ISO 639-1 standard language code for Japanese is
'ja', not'jp'(which is the country code). While this works functionally, using the standard code improves interoperability with browser locale detection and i18n tooling.Also applies to: 40-40
v0/src/components/Navbar/User/UserMenu.vue (1)
273-285: Redundant locale restoration with missing validation.This
onMountedlogic is redundant becausei18n.tsalready initializes the locale from localStorage viagetInitialLocale(). More importantly, this code setslocale.valuedirectly from localStorage without validating against supported locales, unlikegetInitialLocale()which validates.If localStorage contains an invalid locale value, this could cause issues.
Suggested approach
Since
getInitialLocale()already handles restoration with validation, you can simplifyonMountedto just set the document attributes:onMounted(function() { - const savedLocale = localStorage.getItem('locale') - if (savedLocale) { - locale.value = savedLocale - } - document.documentElement.lang = locale.value if (locale.value === 'ar') { document.documentElement.dir = 'rtl' } else { document.documentElement.dir = 'ltr' } })
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
v0/src/components/Navbar/User/UserMenu.vuev0/src/locales/bn.jsonv0/src/locales/de.jsonv0/src/locales/es.jsonv0/src/locales/fr.jsonv0/src/locales/i18n.tsv0/src/locales/jp.jsonv0/src/locales/zh.jsonv0/src/simulator/src/minimap.jsv0/src/simulator/src/themer/themes.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (9)
v0/src/locales/fr.json (1)
1-167: LGTM!The French locale file is well-structured and comprehensive. The same key consistency verification requested for the Japanese locale file applies here as well.
v0/src/locales/zh.json (1)
1-167: LGTM!The Chinese locale file is well-structured and comprehensive. The same key consistency verification requested for the Japanese locale file applies here as well.
v0/src/locales/jp.json (1)
1-167: Update bn.json to include missing locale keys.The Bengali locale file (bn.json) is missing 16 keys that are present in all other locale files (de, en, es, fr, hi, jp, zh). These missing keys are:
simulator.nav.project.export_as_filesimulator.nav.project.import_projectsimulator.panel_header.keybinding_preferencesimulator.panel_body.verilog_module.apply_themessimulator.panel_body.verilog_module.select_themesimulator.panel_body.timing_diagram.one_cyclesimulator.panel_body.timing_diagram.unitssimulator.panel_body.custom_shortcutsection (5 keys: command, esc_cancel, keymapping, reset_to_default, save)simulator.panel_body.report_issueextended keys (4 keys: cancel_btn, close_btn, email, optional)Add these translations to bn.json to maintain consistency across all locales and prevent missing translations for Bengali users.
Likely an incorrect or invalid review comment.
v0/src/simulator/src/themer/themes.ts (1)
329-351: LGTM! Robust localStorage handling with SSR safety.The implementation correctly guards against SSR environments, handles JSON parse errors, and validates the parsed value is an object before merging. The spread order ensures user customizations properly override defaults.
v0/src/locales/de.json (1)
1-167: LGTM! German localization file is well-structured.The translation file provides comprehensive coverage of UI strings with proper German translations and maintains consistent structure with other locale files.
v0/src/locales/es.json (1)
1-83: Spanish translations are comprehensive and well-structured.The file provides complete coverage of UI strings with appropriate Spanish translations.
Also applies to: 85-167
v0/src/locales/i18n.ts (1)
23-42: LGTM! Clean i18n configuration with proper fallback.The initialization correctly uses the stored locale with validation and falls back to English. The
availableLocaleexport keeps UI dropdowns in sync with available translations.v0/src/components/Navbar/User/UserMenu.vue (2)
254-268: Locale persistence logic is well-implemented.The watcher correctly persists the locale preference and updates document attributes. Note that the RTL check for
'ar'won't trigger until Arabic is added to the available locales (see comment oni18n.ts).
287-290: LGTM! Form validation rules are correct.The validation rules properly check for required fields, valid email format, and minimum password length.
rolex-67
force-pushed
the
feat/multi-language-support
branch
from
24b6ffa to
bf7941d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v0/src/components/Navbar/User/UserMenu.vue (1)
37-46: Harden/centralize locale hydration + RTL handling (validate + guard browser APIs).
savedLocaleshould be whitelisted againstavailableLocalebefore assigning tolocale.value.- Accessing
localStorage/documentshould be guarded (SSR/tests/private mode can throw).- Since PR objectives mention moving locale initialization into the i18n factory, consider removing the component-level “restore locale” and only keep “apply dir/lang” (or use
watch(..., { immediate: true })).Proposed refactor (single helper + validation + guards)
+function applyDocumentLocale(nextLocale: string) { + // Guard for SSR/tests + if (typeof document === 'undefined') return + document.documentElement.lang = nextLocale + document.documentElement.dir = nextLocale === 'ar' ? 'rtl' : 'ltr' +} + +function isSupportedLocale(maybeLocale: string | null): maybeLocale is string { + return !!maybeLocale && (availableLocale as readonly string[]).includes(maybeLocale) +} + watch(locale, function(newLocale) { - localStorage.setItem('locale', newLocale) - document.documentElement.lang = newLocale - - if (newLocale === 'ar') { - document.documentElement.dir = 'rtl' - } else { - document.documentElement.dir = 'ltr' - } + try { + if (typeof localStorage !== 'undefined') localStorage.setItem('locale', newLocale) + } catch { + // ignore persistence failures + } + applyDocumentLocale(newLocale) }) onMounted(function() { - const savedLocale = localStorage.getItem('locale') - if (savedLocale) { - locale.value = savedLocale - } - - document.documentElement.lang = locale.value - if (locale.value === 'ar') { - document.documentElement.dir = 'rtl' - } else { - document.documentElement.dir = 'ltr' - } + let savedLocale: string | null = null + try { + if (typeof localStorage !== 'undefined') savedLocale = localStorage.getItem('locale') + } catch { + savedLocale = null + } + if (isSupportedLocale(savedLocale)) locale.value = savedLocale + applyDocumentLocale(locale.value) })Also applies to: 254-285
🤖 Fix all issues with AI agents
In @v0/src/components/Navbar/User/UserMenu.vue:
- Around line 288-290: The catch blocks currently type errors as any and assume
a .message property; update all catch handlers in UserMenu.vue to use unknown,
then normalize the message safely (e.g., const msg = error instanceof Error ?
error.message : String(error)); pass msg to showSnackbar. Also fix the snackbar
type→color mapping so 'warning' and 'info' do not map to the success/green
color: update the showSnackbar implementation or its caller(s) to map
'success'→green, 'warning'→amber/orange, 'info'→blue, and 'error'→red (adjust
callers that pass 'warning'|'info' accordingly), specifically change references
to showSnackbar(...) and any internal mapping logic so the visual color matches
the snackbar type.
- Around line 24-33: The v-list-item currently always binds prepend-avatar from
authStore.getUserAvatar which causes Vuetify to try loading "default" as an
image; update the binding in UserMenu.vue so prepend-avatar is only set when
authStore.getUserAvatar !== 'default' (return undefined/null otherwise) and keep
the existing prepend-icon fallback when avatar is 'default'—modify the
v-list-item's prepend-avatar binding expression (or use a computed property) to
conditionally supply the avatar URL only for non-'default' values.
- Around line 214-229: The v-btn is incorrectly using the :icon prop with the
mdiClose path object — replace that by removing the :icon="mdiClose" prop on the
v-btn and instead render a v-icon inside the button that receives the mdiClose
path (e.g. <v-icon :icon="mdiClose" />), keeping the existing v-btn attributes
(variant, click handler, color) and binding to snackbar.visible as before so the
close icon displays correctly.
In @v0/src/simulator/src/minimap.js:
- Around line 196-199: The current change instantly hides the element retrieved
into miniMap by setting miniMap.style.display = 'none', removing the previous
fadeOut animation; restore a fade by keeping the null check for miniMap, then
animate via CSS transition on #miniMap (e.g., opacity transition) and in the JS
set miniMap.style.opacity = '0' and after the transition completes (or via a
short setTimeout matching the transition duration) set miniMap.style.display =
'none' to fully hide it; ensure you reference the same miniMap element id and
clean up any inline styles or event listeners as needed.
🧹 Nitpick comments (1)
v0/src/simulator/src/themer/themes.ts (1)
3-327: Add a guardrail to prevent theme-token drift / visual regressions after the large hex updates.
Given the scale of token value changes, I’d strongly consider adding a lightweight “token parity” check (e.g., ensure each theme provides the same required CSS variable keys) and/or a small visual regression pass. This helps catch missing keys like--context-text-hov-style tokens across themes before release.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
v0/src/components/Navbar/User/UserMenu.vuev0/src/locales/bn.jsonv0/src/locales/de.jsonv0/src/locales/es.jsonv0/src/locales/fr.jsonv0/src/locales/i18n.tsv0/src/locales/jp.jsonv0/src/locales/zh.jsonv0/src/simulator/src/minimap.jsv0/src/simulator/src/themer/themes.ts
✅ Files skipped from review due to trivial changes (1)
- v0/src/locales/de.json
🚧 Files skipped from review as they are similar to previous changes (5)
- v0/src/locales/es.json
- v0/src/locales/zh.json
- v0/src/locales/jp.json
- v0/src/locales/i18n.ts
- v0/src/locales/bn.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.
Applied to files:
v0/src/components/Navbar/User/UserMenu.vue
🔇 Additional comments (2)
v0/src/simulator/src/minimap.js (1)
170-174: LGTM! Defensive null check and jQuery removal.The addition of a null check before manipulating the DOM element is a good defensive practice, and replacing jQuery with vanilla JavaScript is appropriate for reducing dependencies.
v0/src/locales/fr.json (1)
1-167: JSON structure is technically sound.The file is well-organized with a clear hierarchical structure mapping UI sections to French translations. The JSON syntax is valid and properly formatted. All key-value pairs match the English reference locale exactly (123 keys in each file).
However, please ensure the translations are reviewed by a native French speaker or professional translator to verify accuracy, cultural appropriateness, and proper technical terminology usage. Translation quality directly impacts user experience for French-speaking users.
v0/src/simulator/src/minimap.js
Outdated
| const miniMap = document.getElementById('miniMap') | ||
| if (miniMap) { | ||
| miniMap.style.display = 'none' | ||
| } |
There was a problem hiding this comment.
Animation lost: fadeOut replaced with instant hide.
The defensive null check is good, but replacing fadeOut('fast') with display = 'none' removes the smooth fade animation, resulting in an instant hide. This is a minor UX regression.
If the animation is desired, consider adding a CSS transition to #miniMap or using a brief setTimeout with opacity changes to replicate the fade effect.
🎨 Optional: Restore fade effect with CSS transition
Add a CSS transition to the miniMap element (in the relevant stylesheet):
#miniMap {
transition: opacity 0.2s ease-out;
}Then update the JavaScript to fade via opacity:
const miniMap = document.getElementById('miniMap')
if (miniMap) {
- miniMap.style.display = 'none'
+ miniMap.style.opacity = '0'
+ setTimeout(() => {
+ miniMap.style.display = 'none'
+ }, 200)
}📝 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.
| const miniMap = document.getElementById('miniMap') | |
| if (miniMap) { | |
| miniMap.style.display = 'none' | |
| } | |
| const miniMap = document.getElementById('miniMap') | |
| if (miniMap) { | |
| miniMap.style.opacity = '0' | |
| setTimeout(() => { | |
| miniMap.style.display = 'none' | |
| }, 200) | |
| } |
🤖 Prompt for AI Agents
In @v0/src/simulator/src/minimap.js around lines 196 - 199, The current change
instantly hides the element retrieved into miniMap by setting
miniMap.style.display = 'none', removing the previous fadeOut animation; restore
a fade by keeping the null check for miniMap, then animate via CSS transition on
#miniMap (e.g., opacity transition) and in the JS set miniMap.style.opacity =
'0' and after the transition completes (or via a short setTimeout matching the
transition duration) set miniMap.style.display = 'none' to fully hide it; ensure
you reference the same miniMap element id and clean up any inline styles or
event listeners as needed.
| const storedCustomTheme: Record<string, string> = (() => { | ||
| try { | ||
| if (typeof window === 'undefined' || !window.localStorage) { | ||
| return {}; | ||
| } | ||
| const raw = window.localStorage.getItem('Custom Theme'); | ||
| if (!raw) { | ||
| return {}; | ||
| } | ||
| const parsed = JSON.parse(raw); | ||
| if (parsed && typeof parsed === 'object') { | ||
| return parsed as Record<string, string>; | ||
| } | ||
| return {}; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| })(); | ||
|
|
||
| themes['Custom Theme'] = { | ||
| ...themes['Default Theme'], | ||
| ...storedCustomTheme, | ||
| }; |
There was a problem hiding this comment.
Sanitize storedCustomTheme before spreading (keys/values), and centralize the storage key.
Right now, Line 339-341 accepts any object shape and asserts Record<string, string>, but values can be non-strings and keys can be unexpected, which can break downstream CSS var assignment. Also, the 'Custom Theme' storage key is a fragile string literal.
Proposed fix (sanitize + constant key)
const themes: Themes = {
'Default Theme': {
'--text-navbar--alt': '#000000',
@@
},
'Color Blind': {
@@
},
}
-const storedCustomTheme: Record<string, string> = (() => {
- try {
- if (typeof window === 'undefined' || !window.localStorage) {
- return {};
- }
- const raw = window.localStorage.getItem('Custom Theme');
- if (!raw) {
- return {};
- }
- const parsed = JSON.parse(raw);
- if (parsed && typeof parsed === 'object') {
- return parsed as Record<string, string>;
- }
- return {};
- } catch {
- return {};
- }
-})();
+const CUSTOM_THEME_STORAGE_KEY = 'Custom Theme';
+
+const storedCustomTheme: Record<string, string> = (() => {
+ try {
+ if (typeof window === 'undefined' || !window.localStorage) return {};
+
+ const raw = window.localStorage.getItem(CUSTOM_THEME_STORAGE_KEY);
+ if (!raw) return {};
+
+ const parsed: unknown = JSON.parse(raw);
+ if (!parsed || typeof parsed !== 'object') return {};
+
+ // Only accept CSS variable overrides with string values.
+ return Object.fromEntries(
+ Object.entries(parsed as Record<string, unknown>)
+ .filter(([k, v]) => k.startsWith('--') && typeof v === 'string')
+ ) as Record<string, string>;
+ } catch {
+ return {};
+ }
+})();
themes['Custom Theme'] = {
...themes['Default Theme'],
...storedCustomTheme,
};📝 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.
| const storedCustomTheme: Record<string, string> = (() => { | |
| try { | |
| if (typeof window === 'undefined' || !window.localStorage) { | |
| return {}; | |
| } | |
| const raw = window.localStorage.getItem('Custom Theme'); | |
| if (!raw) { | |
| return {}; | |
| } | |
| const parsed = JSON.parse(raw); | |
| if (parsed && typeof parsed === 'object') { | |
| return parsed as Record<string, string>; | |
| } | |
| return {}; | |
| } catch { | |
| return {}; | |
| } | |
| })(); | |
| themes['Custom Theme'] = { | |
| ...themes['Default Theme'], | |
| ...storedCustomTheme, | |
| }; | |
| const CUSTOM_THEME_STORAGE_KEY = 'Custom Theme'; | |
| const storedCustomTheme: Record<string, string> = (() => { | |
| try { | |
| if (typeof window === 'undefined' || !window.localStorage) return {}; | |
| const raw = window.localStorage.getItem(CUSTOM_THEME_STORAGE_KEY); | |
| if (!raw) return {}; | |
| const parsed: unknown = JSON.parse(raw); | |
| if (!parsed || typeof parsed !== 'object') return {}; | |
| // Only accept CSS variable overrides with string values. | |
| return Object.fromEntries( | |
| Object.entries(parsed as Record<string, unknown>) | |
| .filter(([k, v]) => k.startsWith('--') && typeof v === 'string') | |
| ) as Record<string, string>; | |
| } catch { | |
| return {}; | |
| } | |
| })(); | |
| themes['Custom Theme'] = { | |
| ...themes['Default Theme'], | |
| ...storedCustomTheme, | |
| }; |
rolex-67
force-pushed
the
feat/multi-language-support
branch
from
bf7941d to
558cb44
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
v0/src/components/Navbar/User/UserMenu.vue (1)
233-269: RTL/lang must be applied on initial load (watch needsimmediate, or move to a global init).As written,
document.documentElement.dir/langonly updates after a locale change. On refresh with saved'ar', the page can start in LTR until the user changes locale.Proposed fix (apply immediately + remove console logs + optional BCP47 mapping)
import { ref, watch } from 'vue' ... watch(locale, (newLocale) => { try { localStorage.setItem('locale', newLocale) - document.documentElement.lang = newLocale + // Optional: map app locale codes to valid BCP47 language tags + const htmlLang = newLocale === 'jp' ? 'ja' : newLocale + document.documentElement.lang = htmlLang // RTL support: Set text direction to right-to-left for Arabic if (newLocale === 'ar') { document.documentElement.dir = 'rtl' - console.log('RTL mode activated for Arabic') } else { document.documentElement.dir = 'ltr' - console.log('LTR mode activated for', newLocale) } } catch (e) { console.error('Error saving locale:', e) } -}) +}, { immediate: true })
🤖 Fix all issues with AI agents
In @v0/src/components/Navbar/User/UserMenu.vue:
- Around line 24-28: The prepend-avatar/prepend-icon logic can leave both unset
if authStore.getUserAvatar is null/undefined; normalize the avatar value in the
UserMenu component (create a computed like userAvatarNormalized =
authStore.getUserAvatar ?? 'default' or coerce falsy values to 'default') and
use that normalized symbol in the v-list-item props (replace direct uses of
authStore.getUserAvatar with userAvatarNormalized in the :prepend-avatar and
:prepend-icon ternaries) so you never pass undefined and the icon/avatar
selection always behaves correctly.
In @v0/src/locales/bn.json:
- Around line 2-4: The "simulator.save_online" key currently has an empty string
which disables fallback and can render a blank label; either restore the correct
Bengali translation for the "save_online" key inside the "simulator" object
(replace the empty string with the prior Bengali text) or remove the
"save_online" key entirely so fallbackLocale: 'en' will provide the English
label instead. Ensure you edit the "save_online" entry (or delete it) in the
"simulator" block and run a quick UI check to confirm the label displays
correctly.
🧹 Nitpick comments (1)
v0/src/locales/i18n.ts (1)
12-26: Make supported locale codes a single source of truth (avoid drift betweensupported,messages, andavailableLocale).Right now,
getInitialLocale()hardcodessupportedseparately frommessages/availableLocale, which is easy to forget when adding/removing locales. Consider extracting a sharedSUPPORTED_LOCALESconstant and reusing it in both places.Proposed refactor
import { createI18n } from 'vue-i18n' import en from './en.json' import hi from './hi.json' import bn from './bn.json' import es from './es.json' import fr from './fr.json' import de from './de.json' import jp from './jp.json' import zh from './zh.json' import ar from './ar.json' +export const SUPPORTED_LOCALES = ['en', 'hi', 'bn', 'es', 'fr', 'de', 'jp', 'zh', 'ar'] as const +export type SupportedLocale = (typeof SUPPORTED_LOCALES)[number] + const getInitialLocale = (): string => { try { const saved = localStorage.getItem('locale') - // List of supported locale codes to validate the stored value - const supported = ['en', 'hi', 'bn', 'es', 'fr', 'de', 'jp', 'zh','ar'] - return (saved && supported.includes(saved)) ? saved : 'en' + return (saved && (SUPPORTED_LOCALES as readonly string[]).includes(saved)) ? saved : 'en' } catch (e) { console.error('Error reading locale from localStorage:', e) return 'en' } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
v0/src/components/Navbar/User/UserMenu.vuev0/src/locales/ar.jsonv0/src/locales/bn.jsonv0/src/locales/de.jsonv0/src/locales/es.jsonv0/src/locales/fr.jsonv0/src/locales/i18n.tsv0/src/locales/jp.jsonv0/src/locales/zh.json
✅ Files skipped from review due to trivial changes (2)
- v0/src/locales/ar.json
- v0/src/locales/fr.json
🚧 Files skipped from review as they are similar to previous changes (3)
- v0/src/locales/jp.json
- v0/src/locales/es.json
- v0/src/locales/zh.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.
Applied to files:
v0/src/components/Navbar/User/UserMenu.vue
🔇 Additional comments (4)
v0/src/locales/i18n.ts (1)
28-43: LGTM: locale hydration + expanded messages/locales.The initialization via
getInitialLocale()and the expandedmessages/availableLocalewiring look consistent. One thing to double-check: usingjpas the locale code (vs standardja) can have downstream effects (e.g., HTMLlangattribute, integrations expecting BCP47).Also applies to: 47-57
v0/src/components/Navbar/User/UserMenu.vue (2)
52-67: LGTM: simplifying click handlers.Switching to direct
@clickhandlers here is clearer; just ensure any previously relied-on event propagation behavior (from.stop) isn’t needed for nested list-item interactions.Also applies to: 73-112, 119-119
283-289: Nice improvement: centralized snackbar color mapping.This makes snackbar styling predictable and avoids coupling UI color to the “type” string directly.
Also applies to: 378-384
v0/src/locales/de.json (1)
1-167: Locale key parity confirmed—de.json matches en.json.Both files contain 123 keys with no missing or extra keys. The German locale is in sync with the English canonical locale.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
v0/src/components/Navbar/User/UserMenu.vue (2)
33-112: Hard-coded English strings should be moved to i18n ($t/t) to meet the PR goal.
Right now menu labels like “Locale”, “Sign In”, “Register”, “Dashboard”, “My Groups”, “Notifications”, “Logout”, plus the auth modal texts are not localized.Also applies to: 140-206
320-373: Blocker: remove hard-codedhttp://localhost:4000auth endpoints (and avoid baking env-specific URLs into the UI).
This is hard-coded across all versions (v0, v1, src) and will fail in any deployed environment. No existing API configuration convention found in the repo. Either useVITE_API_BASE_URLenv variable, or adopt the pattern fromproject.tswhich falls back towindow.location.origin.Minimal direction (adapt to repo conventions)
async function handleAuthSubmit() { const { valid } = await authForm.value.validate() if (!valid) return @@ - const url = isLoginMode.value - ? 'http://localhost:4000/api/v1/auth/login' - : 'http://localhost:4000/api/v1/auth/signup' + const API_BASE_URL = import.meta.env.VITE_API_BASE_URL || window.location.origin + + const url = isLoginMode.value + ? `${API_BASE_URL}/api/v1/auth/login` + : `${API_BASE_URL}/api/v1/auth/signup`Also update the same issue in v0, v1, and src versions of UserMenu.vue.
🤖 Fix all issues with AI agents
In @v0/src/components/Navbar/User/UserMenu.vue:
- Around line 268-285: The watcher on locale (watch(locale, ...)) only runs on
changes so dir/lang/localStorage may not be applied on initial render and it
uses console.log/console.error; update it to apply immediately and silently:
extract the logic into a helper (e.g., setLocaleAttributes) and call it once on
component mount/initialization and from the watch callback, set
document.documentElement.lang and document.documentElement.dir, treat RTL for
any locale starting with "ar" (e.g., newLocale.startsWith('ar')), persist locale
to localStorage inside a try/catch but avoid console logging (optionally use a
component logger or swallow the error), and remove console.log/console.error
calls to prevent noisy production output.
🧹 Nitpick comments (2)
v0/src/components/Navbar/User/UserMenu.vue (2)
233-251: TypeauthForm(and null-guard) to avoid runtime errors on.validate().
ref()defaults toundefined; a missing/changed ref will throw at submit time.Also applies to: 320-323
402-412: Prefer router navigation overwindow.location.hrefto avoid full reloads.
If Vue Router is available here,router.push(...)will preserve SPA state and be faster.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
v0/src/components/Navbar/User/UserMenu.vuev0/src/locales/bn.jsonv0/src/locales/es.json
🚧 Files skipped from review as they are similar to previous changes (1)
- v0/src/locales/bn.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-27T17:29:33.929Z
Learnt from: ThatDeparted2061
Repo: CircuitVerse/cv-frontend-vue PR: 442
File: src/simulator/src/wire.ts:0-0
Timestamp: 2025-01-27T17:29:33.929Z
Learning: In the CircuitVerse frontend Vue project, globalScope should be declared on the window object using TypeScript declaration files (.d.ts) rather than importing it as a module.
Applied to files:
v0/src/components/Navbar/User/UserMenu.vue
🔇 Additional comments (2)
v0/src/locales/es.json (1)
1-167: Add an automated “key parity” check against the baseline locale (and lint JSON).Since this is a brand-new locale file, the main integration risk is missing/extra/misspelled keys vs the baseline (typically
en.json), which leads to fallback strings or broken lookups (e.g., keys likepanel_body.circuit_elements.expansion_panel_title.Decoders & Plexers, and values with\nsuch asnew_verilog_module_html).Verification script (JSON validity + recursive key parity)
+#!/bin/bash +set -euo pipefail + +# Find baseline + all locale jsons (paths may vary slightly) +EN="$(fd -a '^en\.json$' v0/src/locales | head -n1 || true)" +ES="$(fd -a '^es\.json$' v0/src/locales | head -n1 || true)" + +if [[ -z "${EN}" || -z "${ES}" ]]; then + echo "Could not find en.json and/or es.json under v0/src/locales" + exit 1 +fi + +python - <<'PY' +import json, sys + +def load(p): + with open(p, "r", encoding="utf-8") as f: + return json.load(f) + +def keys(obj, prefix=""): + out=set() + if isinstance(obj, dict): + for k,v in obj.items(): + p = f"{prefix}.{k}" if prefix else k + out.add(p) + out |= keys(v, p) + elif isinstance(obj, list): + # Lists: don't enforce element-shape here; just record list path. + out.add(prefix) + return out + +en_path = sys.argv[1] +es_path = sys.argv[2] +en = load(en_path) +es = load(es_path) + +en_keys = keys(en) +es_keys = keys(es) + +missing = sorted(en_keys - es_keys) +extra = sorted(es_keys - en_keys) + +print(f"Baseline: {en_path}") +print(f"Locale: {es_path}") +print(f"Missing keys in es: {len(missing)}") +for k in missing[:200]: + print(" -", k) +if len(missing) > 200: + print(" ... (truncated)") + +print(f"Extra keys in es: {len(extra)}") +for k in extra[:200]: + print(" +", k) +if len(extra) > 200: + print(" ... (truncated)") + +sys.exit(1 if missing else 0) +PY "${EN}" "${ES}" +echo "OK: es.json key parity matches en.json"v0/src/components/Navbar/User/UserMenu.vue (1)
24-31: Avatar computed bindings look good (and avoid passing “default” through).Also applies to: 252-267
| // Watch for locale changes to update localStorage and document attributes | ||
| watch(locale, (newLocale) => { | ||
| try { | ||
| localStorage.setItem('locale', newLocale) | ||
| document.documentElement.lang = newLocale | ||
|
|
||
| // RTL support: Set text direction to right-to-left for Arabic | ||
| if (newLocale === 'ar') { | ||
| document.documentElement.dir = 'rtl' | ||
| console.log('RTL mode activated for Arabic') | ||
| } else { | ||
| document.documentElement.dir = 'ltr' | ||
| console.log('LTR mode activated for', newLocale) | ||
| } | ||
| } catch (e) { | ||
| console.error('Error saving locale:', e) | ||
| } | ||
| }) |
There was a problem hiding this comment.
Locale watcher: apply on first render + avoid console logging + handle RTL more robustly.
As written, dir/lang may not be set on page load (e.g., refresh while already on Arabic) unless something else does it globally; and console.log/console.error will be noisy in production. Also consider ar-* locales.
Proposed fix
-watch(locale, (newLocale) => {
- try {
- localStorage.setItem('locale', newLocale)
- document.documentElement.lang = newLocale
-
- // RTL support: Set text direction to right-to-left for Arabic
- if (newLocale === 'ar') {
- document.documentElement.dir = 'rtl'
- console.log('RTL mode activated for Arabic')
- } else {
- document.documentElement.dir = 'ltr'
- console.log('LTR mode activated for', newLocale)
- }
- } catch (e) {
- console.error('Error saving locale:', e)
- }
-})
+watch(
+ locale,
+ (newLocale) => {
+ // SSR / non-browser guard
+ if (typeof window === 'undefined') return
+ try {
+ window.localStorage.setItem('locale', newLocale)
+ document.documentElement.lang = newLocale
+
+ // RTL support (handle ar-* too)
+ document.documentElement.dir = newLocale === 'ar' || newLocale.startsWith('ar-') ? 'rtl' : 'ltr'
+ } catch {
+ // Intentionally swallow (private mode / storage blocked, etc.)
+ }
+ },
+ { immediate: true }
+)📝 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.
| // Watch for locale changes to update localStorage and document attributes | |
| watch(locale, (newLocale) => { | |
| try { | |
| localStorage.setItem('locale', newLocale) | |
| document.documentElement.lang = newLocale | |
| // RTL support: Set text direction to right-to-left for Arabic | |
| if (newLocale === 'ar') { | |
| document.documentElement.dir = 'rtl' | |
| console.log('RTL mode activated for Arabic') | |
| } else { | |
| document.documentElement.dir = 'ltr' | |
| console.log('LTR mode activated for', newLocale) | |
| } | |
| } catch (e) { | |
| console.error('Error saving locale:', e) | |
| } | |
| }) | |
| // Watch for locale changes to update localStorage and document attributes | |
| watch( | |
| locale, | |
| (newLocale) => { | |
| // SSR / non-browser guard | |
| if (typeof window === 'undefined') return | |
| try { | |
| window.localStorage.setItem('locale', newLocale) | |
| document.documentElement.lang = newLocale | |
| // RTL support (handle ar-* too) | |
| document.documentElement.dir = newLocale === 'ar' || newLocale.startsWith('ar-') ? 'rtl' : 'ltr' | |
| } catch { | |
| // Intentionally swallow (private mode / storage blocked, etc.) | |
| } | |
| }, | |
| { immediate: true } | |
| ) |
🤖 Prompt for AI Agents
In @v0/src/components/Navbar/User/UserMenu.vue around lines 268 - 285, The
watcher on locale (watch(locale, ...)) only runs on changes so
dir/lang/localStorage may not be applied on initial render and it uses
console.log/console.error; update it to apply immediately and silently: extract
the logic into a helper (e.g., setLocaleAttributes) and call it once on
component mount/initialization and from the watch callback, set
document.documentElement.lang and document.documentElement.dir, treat RTL for
any locale starting with "ar" (e.g., newLocale.startsWith('ar')), persist locale
to localStorage inside a try/catch but avoid console logging (optionally use a
component logger or swallow the error), and remove console.log/console.error
calls to prevent noisy production output.
|
Hey @rolex-67 could you please:
|
Fixes #742
Describe the changes you have made in this PR -
🚀 Feature Overview
Implemented comprehensive internationalization support to make the simulator accessible to a global audience.
🛠️ Changes Made:
7 New Locales: Added translation files for Spanish, French, Chinese, Arabic, German, Japanese, and Bengali.
State Persistence: Integrated localStorage to ensure language settings persist across page refreshes.
RTL Layout: Added dynamic support for Right-to-Left languages (Arabic).
Code Quality: Refactored UserMenu.vue for better compatibility and moved locale initialization to the i18n factory.
Screenshots of the changes-
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.