feat: setup translatable frontend for V2 #458
Conversation
WalkthroughAdds i18n support across the wiki app: new server API Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wiki/public/js/render_wiki.js (1)
384-397: String comparison with translated content will fail for non-English locales.Line 387 compares server-rendered HTML (
initial_content) with a client-side translated string (message). If the server rendered<h3>No Revisions</h3>in English but the user's locale translates it differently, the comparison will always fail, breaking the revision display logic.Consider one of these approaches:
- Use a data attribute or specific CSS class in the server-rendered HTML to identify the "no revisions" state instead of string comparison
- Ensure the server renders this content using the same translation system as the frontend
- Compare against the untranslated English string before translation
🔎 Example fix using data attribute approach
In the server template (Python/Jinja):
<div class="revision-content" data-empty="true"> <h3>{{ _("No Revisions") }}</h3> </div>In JavaScript:
- const initial_content = $(".revision-content").html().trim(); + const isEmpty = $(".revision-content").data("empty"); let revisions = []; let currentRevisionIndex = 1; let message = __("No Revisions"); // set initial revision - if (initial_content !== `<h3>${message}</h3>`) { + if (!isEmpty) { $(".revision-content")[0].innerHTML = HtmlDiff.execute(
🧹 Nitpick comments (6)
wiki/wiki/doctype/wiki_space/wiki_space.py (1)
20-110: Translation wrapping implemented correctly.All user-facing strings are properly wrapped with the
_()translation function, and format placeholders are correctly positioned outside the translation calls.Note: Several strings contain HTML tags (e.g.,
<b>{0}</b>) within the translatable content. While this is a common pattern, translators could accidentally modify or remove these tags, potentially breaking the UI. Consider whether this is acceptable for your project's translation workflow.wiki/wiki_search.py (2)
46-61: Consider removing the intermediate variable rename.The rename from
idtodoc_idon Line 47 creates an intermediate variable that's immediately passed to the parent class'sadd_document(id, ...)method. Since the parent method expects a parameter namedid, this rename doesn't add meaningful clarity and introduces an unnecessary variable assignment.🔎 Suggested simplification
def index_doc(self, doc): - doc_id = f"Wiki Page:{doc.name}" fields = { "title": doc.title, "content": strip_html_tags(doc.content), "route": doc.route, "meta_description": doc.meta_description or "", "meta_keywords": doc.meta_keywords or "", "modified": doc.modified, } payload = { "route": doc.route, "published": doc.published, "allow_guest": doc.allow_guest, } - self.add_document(doc_id, fields, payload=payload) + self.add_document(f"Wiki Page:{doc.name}", fields, payload=payload)
63-66: Consider removing the intermediate variable rename.Similar to
index_doc, the rename on Line 65 creates an unnecessary intermediate variable that's immediately passed to the parent'sremove_document(id)method.🔎 Suggested simplification
def remove_doc(self, doc): if doc.doctype == "Wiki Page": - doc_id = f"Wiki Page:{doc.name}" - self.remove_document(doc_id) + self.remove_document(f"Wiki Page:{doc.name}")wiki/www/contributions.py (1)
14-19: Module-level translations evaluated at import time.The
status_label_mapuses_()at module level, which means translations are evaluated once when the module is imported, not per-request. This may not reflect the current user's language preference if it differs from the language at module load time.Consider moving the status label translation into the function that uses it, or documenting this behavior if it's acceptable for your use case.
🔎 Alternative: Per-request translation
-status_label_map = { - "Changes Requested": _("Changes Requested"), - "Under Review": _("Under Review"), - "Rejected": _("Rejected"), - "Approved": _("Approved"), -} - def get_context(context) -> dict: + status_label_map = { + "Changes Requested": _("Changes Requested"), + "Under Review": _("Under Review"), + "Rejected": _("Rejected"), + "Approved": _("Approved"), + } + context.pilled_title = _("My Contributions")Or create a helper function:
def get_status_label(status: str) -> str: """Get translated status label""" return { "Changes Requested": _("Changes Requested"), "Under Review": _("Under Review"), "Rejected": _("Rejected"), "Approved": _("Approved"), }.get(status, status)wiki/public/js/editor.js (1)
335-335: Improve table template cell labels for clarity.The table template uses duplicate labels for cells in the same row (e.g., both cells in a row are labeled "Row 1"). This could be confusing for users. Consider using distinct labels like "Cell 1" and "Cell 2" or more descriptive placeholders.
🔎 Suggested improvement
case "table": - insertion = `${selection}\n| ${__("Header 1")} | ${__("Header 2")} |\n| -------- | -------- |\n| ${__("Row 1")} | ${__("Row 1")} |\n| ${__("Row 2")} | ${__("Row 2")} |`; + insertion = `${selection}\n| ${__("Header 1")} | ${__("Header 2")} |\n| -------- | -------- |\n| ${__("Cell 1")} | ${__("Cell 2")} |\n| ${__("Cell 3")} | ${__("Cell 4")} |`; break;wiki/public/js/translation.js (1)
11-32: Consider testing placeholder presence on translated message.Line 26 checks for placeholders in the original
message, but if the translated message has a different structure, this could cause issues. It's safer to test thetranslatedMessageinstead.🔎 Suggested refinement
if (!translatedMessage) { translatedMessage = translatedMessages[message] || message; } - const hasPlaceholders = /{\d+}/.test(message); + const hasPlaceholders = replace && /{\d+}/.test(translatedMessage); if (!hasPlaceholders) { return translatedMessage; } return format(translatedMessage, replace);Also added a check for
replaceto avoid unnecessary formatting when no replacements are provided.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
wiki/api.pywiki/install.pywiki/public/js/editor.jswiki/public/js/render_wiki.jswiki/public/js/translation.jswiki/public/js/wiki.bundle.jswiki/public/js/wiki.jswiki/search.pywiki/wiki/doctype/wiki_page/search.pywiki/wiki/doctype/wiki_page/templates/comment.htmlwiki/wiki/doctype/wiki_page/templates/editor.htmlwiki/wiki/doctype/wiki_page/templates/feedback.htmlwiki/wiki/doctype/wiki_page/templates/navbar_search.htmlwiki/wiki/doctype/wiki_page/templates/page_settings.htmlwiki/wiki/doctype/wiki_page/templates/revisions.htmlwiki/wiki/doctype/wiki_page/templates/show.htmlwiki/wiki/doctype/wiki_page/templates/web_sidebar.htmlwiki/wiki/doctype/wiki_page/templates/wiki_doc.htmlwiki/wiki/doctype/wiki_page/templates/wiki_navbar.htmlwiki/wiki/doctype/wiki_page/wiki_page.pywiki/wiki/doctype/wiki_settings/wiki_settings.jswiki/wiki/doctype/wiki_space/wiki_space.jswiki/wiki/doctype/wiki_space/wiki_space.pywiki/wiki_search.pywiki/www/__pycache__/__init__.pywiki/www/contributions.htmlwiki/www/contributions.pywiki/www/drafts.htmlwiki/www/drafts.py
🧰 Additional context used
🧬 Code graph analysis (9)
wiki/wiki/doctype/wiki_settings/wiki_settings.js (3)
wiki/public/js/editor.js (1)
__(5-5)wiki/public/js/render_wiki.js (1)
__(2-2)wiki/public/js/wiki.js (1)
__(1-1)
wiki/wiki/doctype/wiki_space/wiki_space.js (3)
wiki/public/js/editor.js (1)
__(5-5)wiki/public/js/render_wiki.js (1)
__(2-2)wiki/public/js/wiki.js (1)
__(1-1)
wiki/public/js/render_wiki.js (2)
wiki/public/js/editor.js (1)
__(5-5)wiki/public/js/wiki.js (1)
__(1-1)
wiki/public/js/translation.js (1)
wiki/public/js/render_wiki.js (1)
message(384-384)
wiki/public/js/wiki.js (2)
wiki/public/js/editor.js (1)
__(5-5)wiki/public/js/render_wiki.js (1)
__(2-2)
wiki/www/contributions.py (1)
wiki/www/drafts.py (1)
get_context(8-26)
wiki/wiki/doctype/wiki_page/wiki_page.py (1)
wiki/public/js/render_wiki.js (1)
message(384-384)
wiki/wiki_search.py (1)
wiki/search.py (2)
add_document(46-56)remove_document(58-61)
wiki/www/drafts.py (1)
wiki/wiki/doctype/wiki_page/wiki_page.py (1)
get_open_contributions(446-453)
🔇 Additional comments (31)
wiki/wiki/doctype/wiki_page/search.py (3)
6-6: LGTM! Translation import added correctly.The import follows Frappe's i18n conventions and is properly placed with other framework imports.
126-126: LGTM! Progress message properly internationalized.The translation wrapper follows Frappe's i18n pattern correctly, using positional placeholders for the translatable string.
192-192: LGTM! Background job message properly internationalized.The translation wrapper is correctly implemented using Frappe's i18n pattern with positional placeholders.
wiki/wiki/doctype/wiki_space/wiki_space.py (1)
7-7: LGTM! Translation function imported correctly.The import of the
_()translation function from frappe is the standard approach for internationalization in Frappe applications.wiki/install.py (1)
6-6: LGTM! Proper i18n implementation.The translation wrapper
_()is correctly applied to user-facing strings for the wiki homepage title and content.Also applies to: 12-14
wiki/search.py (2)
8-8: LGTM! Variable rename improves clarity.The import of the translation function and the rename from
idtodoc_idwhen extracting the document identifier on Line 91 is appropriate here, as it distinguishes the processed identifier from the rawdoc.idattribute.Also applies to: 91-93
103-103: LGTM! Log message translation.Properly wraps the log message with
_()for internationalization support.wiki/wiki/doctype/wiki_page/wiki_page.py (2)
44-44: LGTM! Proper translation of user-facing strings.User-facing strings including the wiki page creation message, error messages, and UI labels are correctly wrapped with
_()for internationalization.Also applies to: 192-194, 311-311
329-329: LGTM! Clean concatenation with dynamic content.The labels are properly translated and concatenated with the dynamic count badges returned by
get_open_contributions()andget_open_drafts().Also applies to: 333-333
wiki/www/contributions.html (4)
24-27: LGTM! Proper semantic HTML.Correctly uses
<th>elements for table headers instead of<td>, improving accessibility and semantic correctness.
34-34: LGTM! Uses translated status labels.The change from
j.statustoj.status_labelcorrectly displays the translated status values provided by the backend'sstatus_label_map.Also applies to: 83-83
37-37: Verify button class change from btn-secondary-dark to btn-default.The button class was changed from
btn-secondary-darktobtn-default. Ensure this styling change is intentional and doesn't negatively impact the UI appearance or theme compatibility.Also applies to: 86-86
55-55: LGTM! Translated button text.The "More" button text is properly wrapped with
_()for internationalization.wiki/www/contributions.py (1)
22-40: LGTM! Type hints and defensive status label handling.The added type hints improve code clarity, and the use of
.get()with a fallback on Line 62 ensures robustness when encountering unexpected status values.Also applies to: 44-46, 48-65
wiki/wiki/doctype/wiki_page/templates/navbar_search.html (1)
13-13: LGTM! Translated search placeholder.The "Search" label is properly wrapped with
_()for internationalization support.wiki/wiki/doctype/wiki_page/templates/page_settings.html (1)
6-36: LGTM! Translation wrappers correctly applied.All UI strings are properly wrapped with
_()for internationalization in the Jinja template context.wiki/wiki/doctype/wiki_page/templates/comment.html (1)
9-9: LGTM! Translation wrapper correctly applied.The "Activity" header is properly internationalized.
wiki/www/drafts.py (2)
9-9: LGTM! Translation wrapper correctly applied.The pilled title is properly internationalized.
19-19: LGTM! Translation wrapper correctly applied.The "My Contributions" label is properly internationalized. The concatenation with
get_open_contributions()(which returns HTML with a count badge) is placed outside the translation scope, which is appropriate since the count is dynamic.wiki/wiki/doctype/wiki_page/templates/wiki_navbar.html (1)
75-75: LGTM! Translation wrapper correctly applied.The search input placeholder is properly internationalized using the correct Jinja template syntax.
wiki/public/js/wiki.bundle.js (1)
1-1: LGTM! Translation module imported correctly.Importing the translation module first ensures translations are available before other modules execute.
wiki/wiki/doctype/wiki_page/templates/revisions.html (2)
6-6: LGTM! Semantic improvement.Adding
flex-grow-1improves the layout flexibility of the header container.
10-26: LGTM! Translation wrappers correctly applied.All UI strings ("edited", "Previous", "Next") are properly internationalized using correct Jinja template syntax.
wiki/wiki/doctype/wiki_space/wiki_space.js (1)
8-9: LGTM! Translation wrappers correctly applied.Both UI strings are properly internationalized using the correct
__()function for JavaScript context.wiki/wiki/doctype/wiki_settings/wiki_settings.js (1)
8-14: LGTM! Translation wrappers correctly applied.Both the button label and alert message are properly internationalized using the correct
__()function for JavaScript context.wiki/public/js/render_wiki.js (5)
157-157: LGTM!Good modernization:
letprovides proper block scoping compared tovar.
240-240: LGTM!Proper translation wrapping for dialog action labels.
Also applies to: 353-353
278-278: LGTM!Correctly translates the sidebar item label for new pages.
594-594: LGTM!Properly translates the empty search results message.
2-2: Remove or revise the race condition concern—it's not applicable to line 2.The alias
const __ = globalThis.__is created aftertranslation.jsexecutes, which synchronously setsglobalThis.__at line 3. No race condition exists at this point;globalThis.__is immediately available whenrender_wiki.jsloads. Thefetch()call intranslation.jsis fire-and-forget and runs in the background, but thetranslate()function handles missing translations gracefully with the fallbackglobalThis.translatedMessages || {}. Since__()calls (starting line 236) are primarily inside theDOMContentLoadedhandler (line 95), the fetch request completes well before these functions execute.Likely an incorrect or invalid review comment.
wiki/public/js/translation.js (1)
5-9: LGTM!The placeholder replacement logic correctly handles numbered placeholders and gracefully preserves them if no replacement value is provided.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wiki/public/js/translation.js (2)
1-16: Good approach to handling async translation loading.The implementation properly addresses the race condition concern from previous reviews by using promise tracking and graceful fallback. The
translatefunction will return the original English text if translations aren't loaded yet, which prevents errors while maintaining a functional UI.Optional: Consider adding a loading indicator for better UX
If you want to avoid the brief flash of English text before translations load, you could add a simple loading state:
// After fetchTranslations completes, dispatch an event async function fetchTranslations() { try { const response = await fetch("/api/method/wiki.api.get_translations"); const data = await response.json(); globalThis.translatedMessages = data.message || {}; // Dispatch event so UI can re-render with translations window.dispatchEvent(new CustomEvent('translationsLoaded')); } catch (error) { console.error("Failed to load translations", error); globalThis.translatedMessages = {}; } }Then consuming code can listen for this event to re-render if needed.
51-60: Add response validation before parsing JSON.The error handling is much improved from previous reviews. However, the code should validate the response status before attempting to parse JSON, as unsuccessful responses will throw when calling
.json().🔎 Recommended enhancement
async function fetchTranslations() { try { const response = await fetch("/api/method/wiki.api.get_translations"); + + if (!response.ok) { + throw new Error(`HTTP ${response.status}: ${response.statusText}`); + } + const data = await response.json(); globalThis.translatedMessages = data.message || {}; } catch (error) { console.error("Failed to load translations", error); globalThis.translatedMessages = {}; } }Optional enhancements for better resilience:
- Add localStorage caching to enable offline fallback
- Implement retry logic with exponential backoff for transient network failures
- Show a user-visible notification when translations fail to load
These are noted in previous review comments and could be addressed in a follow-up if offline support or retry resilience becomes important.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
wiki/api.pywiki/install.pywiki/public/js/render_wiki.jswiki/public/js/translation.jswiki/wiki/doctype/wiki_page/templates/editor.htmlwiki/wiki/doctype/wiki_page/templates/web_sidebar.htmlwiki/wiki/doctype/wiki_page/templates/wiki_navbar.htmlwiki/wiki/doctype/wiki_space/wiki_space.pywiki/wiki_search.pywiki/www/contributions.htmlwiki/www/drafts.htmlwiki/www/drafts.py
🚧 Files skipped from review as they are similar to previous changes (10)
- wiki/wiki/doctype/wiki_page/templates/web_sidebar.html
- wiki/wiki/doctype/wiki_page/templates/wiki_navbar.html
- wiki/wiki/doctype/wiki_space/wiki_space.py
- wiki/www/drafts.html
- wiki/www/contributions.html
- wiki/wiki_search.py
- wiki/public/js/render_wiki.js
- wiki/api.py
- wiki/www/drafts.py
- wiki/install.py
🧰 Additional context used
🧬 Code graph analysis (1)
wiki/public/js/translation.js (1)
wiki/public/js/render_wiki.js (1)
message(385-385)
🔇 Additional comments (5)
wiki/public/js/translation.js (2)
18-39: Well-structured translation logic with proper fallback handling.The function correctly implements:
- Context-aware translation lookup (
message:context→message→ original)- Placeholder detection and replacement
- Graceful fallback when translations aren't available
The implementation is clean and maintainable.
41-49: Clean helper functions for placeholder formatting.The
formatandhasPlaceholdersfunctions are well-implemented:
- Properly handles missing replacements by preserving the placeholder
- Uses optional chaining for defensive coding
- Regex pattern correctly matches numeric placeholders
wiki/wiki/doctype/wiki_page/templates/editor.html (3)
1-22: Proper i18n implementation for editor control buttons.All button labels and dropdown items are correctly wrapped with
{{ _("...") }}for translation. The implementation is consistent and complete.
25-34: Complete i18n coverage for labels and messages.All user-facing text including labels, warning messages, and input placeholders are properly wrapped for translation.
36-131: Toolbar button titles properly localized with consistent capitalization.All toolbar button titles are correctly wrapped for translation and use consistent title case formatting. The previous issue regarding lowercase 'video' has been addressed - line 105 now correctly uses
{{ _('Video') }}with proper capitalization.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
wiki/www/contributions.py (1)
13-24:context.update()returnsNone, breaking the function contract.Line 13 assigns the result of
context.update(...)back tocontext. Sincedict.update()returnsNone, this setscontexttoNone, causing line 26 to returnNoneinstead of adict, violating the new type hint on line 8.🔎 Proposed fix
- context = context.update( + context.update( { "post_login": [ {"label": _("My Account"), "url": "/me"}, {"label": _("Logout"), "url": "/?cmd=web_logout"}, { "label": _("My Drafts ") + get_open_drafts(), "url": "/drafts", }, ] } )
🧹 Nitpick comments (1)
wiki/wiki_search.py (1)
61-61: LGTM! Consider extracting the document ID format.The direct string usage is correct and maintains consistency. The format
f"Wiki Page:{doc.name}"appears in bothindex_docandremove_docmethods.Optional: Extract document ID format to reduce duplication
Consider extracting the document ID format to a helper method for easier maintenance:
+ def _get_doc_id(self, doc_name): + return f"Wiki Page:{doc_name}" + def index_doc(self, doc): fields = { "title": doc.title, "content": strip_html_tags(doc.content), "route": doc.route, "meta_description": doc.meta_description or "", "meta_keywords": doc.meta_keywords or "", "modified": doc.modified, } payload = { "route": doc.route, "published": doc.published, "allow_guest": doc.allow_guest, } - self.add_document(f"Wiki Page:{doc.name}", fields, payload=payload) + self.add_document(self._get_doc_id(doc.name), fields, payload=payload) def remove_doc(self, doc): if doc.doctype == "Wiki Page": - self.remove_document(f"Wiki Page:{doc.name}") + self.remove_document(self._get_doc_id(doc.name))Also applies to: 65-65
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wiki/wiki_search.pywiki/www/contributions.py
🧰 Additional context used
🧬 Code graph analysis (2)
wiki/wiki_search.py (1)
wiki/search.py (2)
add_document(46-56)remove_document(58-61)
wiki/www/contributions.py (2)
wiki/www/drafts.py (1)
get_context(8-26)wiki/wiki/doctype/wiki_page/wiki_page.py (1)
get_context(218-338)
🔇 Additional comments (4)
wiki/wiki_search.py (1)
7-7: LGTM! Translation support properly added.The import of
_and wrapping of the progress bar label align well with the PR objectives to enable i18n for the Wiki frontend.Also applies to: 43-43
wiki/www/contributions.py (3)
30-31: LGTM!The type hint accurately reflects the return type.
34-47: LGTM!The type hint is correct, and moving
color_mapto local scope improves encapsulation. The newstatus_label_mapproperly translates status values for the UI.
62-62: LGTM!The fallback to the original status value ensures graceful handling of any unmapped status values.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wiki/www/contributions.py (1)
13-24: Prefer format strings over concatenation for i18n.Line 19 concatenates a translated string with a function result. This pattern can cause issues in languages where word order or number placement differs. Use a format string instead:
{ - "label": _("My Drafts ") + get_open_drafts(), + "label": _("My Drafts ({0})").format(get_open_drafts()), "url": "/drafts", },This allows translators to reposition the placeholder as needed for their language.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wiki/www/contributions.py
🔇 Additional comments (4)
wiki/www/contributions.py (4)
8-9: LGTM! Good i18n implementation.The return type annotation and translation wrapping for the title follow good practices for making the UI translatable.
30-30: LGTM!The type annotation is a helpful addition for code clarity.
62-62: LGTM! Good defensive programming.The fallback pattern using
.get()ensures that unknown status values don't cause errors and still display something meaningful to the user.
35-47: Database correctly stores English status values.The hardcoded English keys ("Changes Requested", "Under Review", "Rejected", "Approved") are appropriate. The doctype definition confirms that Wiki Page Patch status field stores English values:
"options": "Under Review\nChanges Requested\nRejected\nApproved\nDraft". This design correctly separates database storage (English) from UI labels (translated viastatus_label_map).However, note that line 61 uses unsafe direct dict access
color_map[wiki_page_patch.status], unlike line 62's safer.get()method. If a new status is added to the doctype, line 61 would raise a KeyError while line 62 would gracefully handle it. Consider usingcolor_map.get(wiki_page_patch.status)for consistency and robustness.
1 participant
This PR sets up the necessary logic to make the Wiki frontend translatable. Since Version 3 is still in development, these changes ensure that UI strings are properly wrapped and prepared for localization.
I have integrated some work previously done by @NagariaHussain in this commit
Quick demo using pt-BR .po file:
Screen.Recording.2025-12-22.at.21.10.13.mov
Summary by CodeRabbit
New Features
Style
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.