Repository navigation
Conversation
Improves i18n coverage so strings flow through to all locales via Crowdin: - Wrap ~95 hardcoded English strings with __() across Settings, Sidebar, Statistics, Programming Exercises, Batches and other components - Fix null-language fallback in get_translations(): logged-in users with no language set now fall back to System Settings, then "en" (previously passed null to get_all_translations); collapses the duplicate System Settings lookup into a single query - Add RTL font-family override (html[dir='rtl']) in index.css, preserving monospace fonts for code/pre blocks - Load IBM Plex Sans Arabic for proper RTL rendering Translations themselves are contributed via Crowdin, not included here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| frontend/src/components/Settings/Settings.vue | Wraps more settings text for translation while keeping stable lookup values unchanged. |
| lms/lms/api.py | Adds a fallback language path when the user or system language is empty. |
| frontend/src/index.css | Adds RTL font rules while preserving monospace rendering for code blocks. |
Reviews (2): Last reviewed commit: "fix: keep settings item labels in Englis..." | Re-trigger Greptile
Settings item labels double as selection keys: get_translations() open path matches `item.label === settingsStore.activeTab` and SidebarLink compares `link.label.includes(activeTab)` against stable English names. Wrapping the labels in __() broke section selection in non-English locales. SidebarLink and SettingDetails already translate labels at display time via __(link.label) / __(label), so the data-level labels should stay English. Unwraps the three affected item labels (Course Progress, Email Templates, Google Meet). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Thanks for sending this separately, could you rebase on top of latest develop (11 conflicts, and
|
|
Additional confirmation for the null-language case on a self-hosted Frappe v16 deployment with social-login provisioning: an authenticated user with an unset User.language can receive an empty translation dictionary even when the site language is Russian. The current LMS main at 4b73070 still reads User.language directly, while Frappe version-16 at 5cba016e returns {} for an empty language. This is separate from missing translation strings. The proposed use of frappe.local.lang in the maintainer review would address the fallback without requiring per-user backfills. A regression test covering a logged-in user with no explicit language and a non-English system language would be valuable. |
|
Closing this due to inactivity, please reopen with requested changes |
What
Follow-up to #2506, split out as requested by @raizasafeel ("Could you raise another PR with just the rest of the fixes?"). The Arabic translation CSV is excluded — translations are contributed via Crowdin. This PR contains only the supporting code fixes that benefit all locales.
Changes
__()across Settings, Sidebar (AppSidebar, UserDropdown), Statistics, Programming Exercises, Batches, Programs, and other components — so they become translatable in every Crowdin language, not just Arabic.get_translations()(lms/lms/api.py): a logged-in user with nolanguageset previously passednulltoget_all_translations(). It now falls back to System Settings, then"en". The duplicate System Settings lookup is collapsed into a single query.index.css:html[dir='rtl']font-family override, preserving monospace fonts forcode/preblocks — benefits any RTL locale (ar/he/fa/ur).Addresses prior review
The two P1 items the automated review raised on #2506 are already handled here:
Settings.vuedoctypeis a plain string (ref('LMS Settings')), not wrapped in__()— so the DocType lookup is unaffected.ar.csv, which is not part of this PR.Testing
Deployed and tested on a live instance: Desk sidebar, LMS frontend, onboarding, and settings render correctly, and RTL layout applies as expected.