feat: add Arabic localization support for editor, launch, settings, s…#529
Conversation
…hortcuts, timeline, common, and dialogs modules
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds full Arabic localization (seven new locale JSON files), registers "ar" in frontend and Electron i18n, localizes several main-process menu/tray labels, and replaces hardcoded cursor-highlight UI strings in SettingsPanel with i18n lookups; English settings.json adds matching cursorHighlight keys. ChangesArabic localization bundle
Cursor highlight i18n wiring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 679e306d31
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/locales/ar/common.json`:
- Line 3: The Arabic UI strings need standard spelling updates: replace the
value for the "cancel" key with the standard form "إلغاء", and similarly update
the other non-standard entries referenced in the comment (e.g., change "اغلاق"
to "إغلاق", "ايقاف" to "إيقاف", and "عربي" to "العربية") so all UI copy in
common.json uses the correct diacritics/hamza forms; locate these keys
("cancel", "close", "stop", "arabic" or their equivalents) in the same JSON and
update their string values accordingly to the standard Arabic spellings.
In `@src/i18n/locales/ar/dialogs.json`:
- Around line 59-60: The Arabic labels for file-save dialogs use "المصدر" which
is misleading; update the translation values for the keys "saveGif" and
"saveVideo" in dialogs.json to a neutral/exported-file wording (e.g., change
"حفظ GIF المصدر" → "حفظ ملف GIF" and "حفظ الفيديو المصدر" → "حفظ ملف الفيديو")
and ensure these match the intent/terminology used by other locales for
post-export save dialogs.
In `@src/i18n/locales/ar/editor.json`:
- Line 20: The translation key failedToSaveExportedVideo currently uses wording
that translates to “source video”; update the Arabic string for
failedToSaveExportedVideo in editor.json so it correctly refers to the exported
video (i.e., change the value to the Arabic phrase meaning “failed to save
exported video”) while keeping the key name unchanged.
🪄 Autofix (Beta)
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
Run ID: ca567bdc-4840-46f8-bd52-f87476816c76
📒 Files selected for processing (7)
src/i18n/locales/ar/common.jsonsrc/i18n/locales/ar/dialogs.jsonsrc/i18n/locales/ar/editor.jsonsrc/i18n/locales/ar/launch.jsonsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/ar/shortcuts.jsonsrc/i18n/locales/ar/timeline.json
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 (1)
src/components/video-editor/SettingsPanel.tsx (1)
1078-1100:⚠️ Potential issue | 🟠 MajorDon't enable the toggle unless the permission request actually succeeds.
The code currently shows a denial toast but still falls through and calls
onCursorHighlightChangewithonlyOnClicks: true, leaving the UI claiming the feature works when it can't. Also, the directwindow.electronAPI.requestAccessibilityAccess()call should be guarded—other parts of the codebase show the pattern either doesn't exist in non-Electron environments or fails silently.🔧 Suggested fix
- const result = await window.electronAPI.requestAccessibilityAccess(); - if (!result.granted) { - toast.message( - t("effects.cursorHighlight.accessibilityPermissionTitle"), - { - description: t( - "effects.cursorHighlight.accessibilityPermissionDescription", - ), - }, - ); - } + const result = + await window.electronAPI?.requestAccessibilityAccess?.(); + if (!result?.granted) { + toast.message( + t("effects.cursorHighlight.accessibilityPermissionTitle"), + { + description: t( + "effects.cursorHighlight.accessibilityPermissionDescription", + ), + }, + ); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/video-editor/SettingsPanel.tsx` around lines 1078 - 1100, The toggle currently flips UI state regardless of whether the accessibility permission was granted; update the onClick handler so it first checks that the Electron API exists (e.g., typeof window !== "undefined" && window.electronAPI && typeof window.electronAPI.requestAccessibilityAccess === "function"), then when turningOn call requestAccessibilityAccess() inside try/catch and if result.granted is false (or the call throws) show the toast and DO NOT call onCursorHighlightChange; only call onCursorHighlightChange({...cursorHighlight, onlyOnClicks: turningOn}) when either turningOff (no permission needed) or turningOn and result.granted is true, keeping references to cursorHighlight and onCursorHighlightChange intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/i18n/locales/ar/settings.json`:
- Around line 173-176: The Arabic locale's Google Fonts helper text (keys
dialogTitle, urlLabel, urlPlaceholder, urlHelp) still contains the English
phrase "Get font"; update the urlHelp value to fully Arabic text (e.g., replace
'Get font' with 'الحصول على الخط' or 'احصل على الخط') so the entire helper
string is localized and reads naturally in Arabic.
---
Outside diff comments:
In `@src/components/video-editor/SettingsPanel.tsx`:
- Around line 1078-1100: The toggle currently flips UI state regardless of
whether the accessibility permission was granted; update the onClick handler so
it first checks that the Electron API exists (e.g., typeof window !==
"undefined" && window.electronAPI && typeof
window.electronAPI.requestAccessibilityAccess === "function"), then when
turningOn call requestAccessibilityAccess() inside try/catch and if
result.granted is false (or the call throws) show the toast and DO NOT call
onCursorHighlightChange; only call onCursorHighlightChange({...cursorHighlight,
onlyOnClicks: turningOn}) when either turningOff (no permission needed) or
turningOn and result.granted is true, keeping references to cursorHighlight and
onCursorHighlightChange intact.
🪄 Autofix (Beta)
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
Run ID: acc4dcc7-1617-4617-8f73-fd12093180e5
📒 Files selected for processing (4)
src/components/video-editor/SettingsPanel.tsxsrc/i18n/locales/ar/editor.jsonsrc/i18n/locales/ar/settings.jsonsrc/i18n/locales/en/settings.json
✅ Files skipped from review due to trivial changes (1)
- src/i18n/locales/ar/editor.json
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
electron/i18n.ts (1)
23-52:⚠️ Potential issue | 🟠 Majorlocale registry mismatch will cause inconsistent Arabic support across the app.
Arabic translations are fully implemented and wired in
electron/i18n.ts, butsrc/i18n/config.tsstill doesn't list"ar"inSUPPORTED_LOCALES. this creates a pretty kinda cursed situation: the UI dropdowns will show Arabic via the dynamicavailableLocales, but the test coverage won't validate Arabic strings (only iteratesSUPPORTED_LOCALES), and anything keying off the config list might reject it.plus there's now three separate copies of the locale list floating around — the electron hardcoded allow-list, the config constant, and the dynamic loader — all prone to drift.
consolidate to one source of truth. at minimum, add
"ar"toSUPPORTED_LOCALESinsrc/i18n/config.tsand update the test'sdialogsByLocalemapping to include Arabic, so it actually gets validated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/i18n.ts` around lines 23 - 52, The codebase has an inconsistent locale registry: although electron/i18n.ts defines Locale including "ar" and wires Arabic messages (messages, Locale, setMainLocale), src/i18n/config.ts’s SUPPORTED_LOCALES and the test mapping (dialogsByLocale) are missing "ar"; update src/i18n/config.ts to include "ar" in SUPPORTED_LOCALES and add the Arabic entry to the test’s dialogsByLocale mapping so Arabic strings are validated, and preferably centralize the canonical locale list by exporting a single SUPPORTED_LOCALES (or Locale array) used by electron/i18n.ts, src/i18n/config.ts and tests to avoid drift.
🧹 Nitpick comments (1)
electron/i18n.ts (1)
41-52: ⚡ Quick winnit: derive the allow-list instead of repeating it.
setMainLocale()is already duplicating the locale list, which makes drift like this easy to ship again. Pulling this from a shared locale source would keep future additions from getting kinda cursed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/i18n.ts` around lines 41 - 52, The setMainLocale function duplicates the hard-coded allow-list; instead import or reference the shared locale list (e.g., the central locales constant/array used elsewhere) and derive the allow-list from that source rather than repeating values. Update setMainLocale to check membership against the shared locale array (or export a derived Set) so additions are centralized (refer to the setMainLocale function name and the shared locales constant used across the app) and remove the inline repeated strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@electron/i18n.ts`:
- Around line 23-52: The codebase has an inconsistent locale registry: although
electron/i18n.ts defines Locale including "ar" and wires Arabic messages
(messages, Locale, setMainLocale), src/i18n/config.ts’s SUPPORTED_LOCALES and
the test mapping (dialogsByLocale) are missing "ar"; update src/i18n/config.ts
to include "ar" in SUPPORTED_LOCALES and add the Arabic entry to the test’s
dialogsByLocale mapping so Arabic strings are validated, and preferably
centralize the canonical locale list by exporting a single SUPPORTED_LOCALES (or
Locale array) used by electron/i18n.ts, src/i18n/config.ts and tests to avoid
drift.
---
Nitpick comments:
In `@electron/i18n.ts`:
- Around line 41-52: The setMainLocale function duplicates the hard-coded
allow-list; instead import or reference the shared locale list (e.g., the
central locales constant/array used elsewhere) and derive the allow-list from
that source rather than repeating values. Update setMainLocale to check
membership against the shared locale array (or export a derived Set) so
additions are centralized (refer to the setMainLocale function name and the
shared locales constant used across the app) and remove the inline repeated
strings.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
electron/main.ts (2)
127-150: 💤 Low value
||fallbacks are dead for missing keys — only fire on empty-string translations
mainT(fromelectron/i18n.ts) always returns a non-empty string: it falls back to English, and if that's also absent it returns"namespace.key"— never falsy. SomainT("common", "actions.about") || "About OpenScreen"only actually uses the right-hand side when a locale file contains an empty string for the key, not when the key is outright missing.This isn't a breakage, but the pattern reads like a "missing-key guard" when it isn't — could mislead future maintainers into thinking the hardcoded string is the real safety net.
Consider dropping the
||fallbacks entirely (sincemainT's own"namespace.key"sentinel is the actual fallback), or at least add a comment making the intent clear. The latter also makes it easier to grep for un-translated keys.♻️ Example cleanup for one entry
-{ role: "about", label: mainT("common", "actions.about") || "About OpenScreen" }, +{ role: "about", label: mainT("common", "actions.about") },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/main.ts` around lines 127 - 150, The menu label fallbacks using "||" are misleading because mainT always returns a non-empty string (it falls back to English or the "namespace.key" sentinel); update the menu entries that use constructs like label: mainT("common", "actions.about") || "About OpenScreen" to remove the dead "||" fallback and instead rely on mainT directly (e.g., label: mainT("common", "actions.about")), or if you prefer to keep the literal for discoverability add a concise comment next to each use explaining that mainT guarantees a non-empty fallback (reference mainT in electron/i18n.ts and the menu label occurrences such as the "about", "services", "hide", "hideOthers", "unhide", and "quit" entries).
238-256: 💤 Low valuemacOS
zoomandfrontroles missinglabel— Arabic users see English Electron defaultsEvery other item in this Window submenu was given a
mainTlabel, but{ role: "zoom" }(line 243) and{ role: "front" }(line 245) were skipped. Electron falls back to its hardcoded English strings for these, so Arabic users will see"Zoom"and"Bring All to Front"instead of localized text.♻️ Proposed fix
-{ role: "zoom" }, +{ role: "zoom", label: mainT("common", "actions.zoom") || "Zoom" }, { type: "separator" }, -{ role: "front" }, +{ role: "front", label: mainT("common", "actions.front") || "Bring All to Front" },(You'll also need to add
actions.zoomandactions.frontkeys to bothen/common.jsonandar/common.json.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@electron/main.ts` around lines 238 - 256, The Window submenu entries for role "zoom" and role "front" are missing translated labels; update the array entries that currently use { role: "zoom" } and { role: "front" } to include label: mainT("common", "actions.zoom") || "Zoom" and label: mainT("common", "actions.front") || "Bring All to Front" respectively so they follow the same pattern as other items (use the existing mainT helper); also add corresponding keys "actions.zoom" and "actions.front" to both en/common.json and ar/common.json with appropriate translations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@electron/main.ts`:
- Around line 287-289: trayToolTip uses mainT(...) without the usual fallback,
so if the translation key is missing users may see an empty string or the key;
update the trayToolTip assignment (the trayToolTip constant where recording ?
mainT("common", "actions.recordingStatus", { source: selectedSourceName }) :
"OpenScreen") to append a fallback after mainT (e.g., || "Recording: {source}"
or a suitable literal like the other mainT fallbacks), ensuring the
placeholder/source is preserved when recording is true.
---
Nitpick comments:
In `@electron/main.ts`:
- Around line 127-150: The menu label fallbacks using "||" are misleading
because mainT always returns a non-empty string (it falls back to English or the
"namespace.key" sentinel); update the menu entries that use constructs like
label: mainT("common", "actions.about") || "About OpenScreen" to remove the dead
"||" fallback and instead rely on mainT directly (e.g., label: mainT("common",
"actions.about")), or if you prefer to keep the literal for discoverability add
a concise comment next to each use explaining that mainT guarantees a non-empty
fallback (reference mainT in electron/i18n.ts and the menu label occurrences
such as the "about", "services", "hide", "hideOthers", "unhide", and "quit"
entries).
- Around line 238-256: The Window submenu entries for role "zoom" and role
"front" are missing translated labels; update the array entries that currently
use { role: "zoom" } and { role: "front" } to include label: mainT("common",
"actions.zoom") || "Zoom" and label: mainT("common", "actions.front") || "Bring
All to Front" respectively so they follow the same pattern as other items (use
the existing mainT helper); also add corresponding keys "actions.zoom" and
"actions.front" to both en/common.json and ar/common.json with appropriate
translations.
🪄 Autofix (Beta)
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
Run ID: 17df72cf-4bc4-4065-aa09-74da71595023
📒 Files selected for processing (3)
electron/main.tssrc/i18n/locales/ar/common.jsonsrc/i18n/locales/en/common.json
✅ Files skipped from review due to trivial changes (2)
- src/i18n/locales/en/common.json
- src/i18n/locales/ar/common.json
|
hello @siddharthvaddem I've added AR to the app and I have to help, few other things translated like the main app nav they didn't have any translation previously: I loved the app the first time I used it so as a friend in web development and my little experience I had to do these changes so that the app can grow more in the Arabic area + there are few to none apps that actually support AR. Happy to help and please if there is anything let me now. I'll be contributing more to the app too. |
2 participants
Description
Adds Arabic (ar) localization support across the application, including translations for shortcuts, timeline, common, and dialogs modules.
Motivation
The application previously lacked support for Arabic-speaking users. This change introduces full Arabic localization to improve accessibility and usability for a broader audience, particularly right-to-left (RTL) language users.
Type of Change
Testing
To verify the changes:
Start the application:
Switch language to Arabic:
arValidate:
Checklist
Thank you for contributing!
Summary by CodeRabbit