Add compose-side translation to editor#3325
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds a compose-time "Translate post" feature: a new ChangesCompose Translate Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant EditorToolbar
participant MarkdownEditorView
participant ComposeTranslateModal
participant translateMarkdown
User->>EditorToolbar: tap translate icon
EditorToolbar->>MarkdownEditorView: handleShowTranslate()
MarkdownEditorView->>ComposeTranslateModal: SheetManager.show(COMPOSE_TRANSLATE, {body, title, onApply})
ComposeTranslateModal->>ComposeTranslateModal: detect source language
User->>ComposeTranslateModal: select languages, press Translate
ComposeTranslateModal->>translateMarkdown: translateMarkdown(body, source, target, onProgress)
translateMarkdown-->>ComposeTranslateModal: translated markdown + progress
User->>ComposeTranslateModal: press Apply
ComposeTranslateModal->>MarkdownEditorView: onApply(appendix, titleMarker)
MarkdownEditorView->>MarkdownEditorView: append appendix to body, update title
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![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
🧹 Nitpick comments (2)
src/providers/translation/translateMarkdown.ts (1)
140-168: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftHeading/quote/list lines never batch, even with each other.
Every translatable marker line becomes its own sequential request (see
keeps image and URL lines inside a list untouchedtest only exercising a single line). For a list-heavy post this means one full round trip per list item, run sequentially — noticeably slower than the plain-paragraph path, which batches up toMAX_BLOCK_BATCH_CHARS.Consider batching consecutive marker lines of the same block into one request (e.g., joined by a stable, translation-safe separator and split back on response) to cut down round trips for long lists, while keeping the existing per-line behavior as a fallback for edge cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/providers/translation/translateMarkdown.ts` around lines 140 - 168, The marker-line handling in translateMarkdown currently sends each heading/quote/list line as its own request, so consecutive translatable lines never batch together. Update the block processing in translateMarkdown to group consecutive marker lines from the same block into a single translation request using a stable separator, then split the translated response back per line when updating translatedLines and output. Keep the existing per-line request logic as a fallback for edge cases where batching is unsafe.src/components/composeTranslateModal/composeTranslateModal.tsx (1)
271-277: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTranslated preview bypasses the app's markdown-render pattern.
The translated markdown is shown as raw text instead of through
renderPostBody(), unlike the sibling preview elsewhere in the editor (_bodyHtmlForPreviewinmarkdownEditorView.tsxusesrenderPostBody(bodyTextRef.current || '...', true, false)). Showing the raw markdown means headings, lists, and quote markers won't visually match what will actually be published, which could confuse the author when validating the translation before applying it.As per coding guidelines: "Post rendering must use
renderPostBody()from@ecency/render-helperfor body content display."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/composeTranslateModal/composeTranslateModal.tsx` around lines 271 - 277, The translated preview currently renders raw markdown text instead of using the app’s standard post renderer. Update the preview block in composeTranslateModal.tsx so it uses renderPostBody() from `@ecency/render-helper` for the translated body, matching the existing preview flow in markdownEditorView.tsx (_bodyHtmlForPreview). Keep the existing translated/isRtlLang gating and swap the Text-based raw display for the rendered HTML output so headings, lists, and quotes match published formatting.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/composeTranslateModal/composeTranslateModal.tsx`:
- Around line 105-118: The translation flow in composeTranslateModal is still
committing stale results from an older language pair after the user changes
source or target. Update the _translate logic so it snapshots the requested
source/target pair and only calls setTranslated or setFailed if the current pair
still matches when the request resolves, or otherwise prevent pair changes while
a translation is in flight. Keep the fix aligned with the existing useEffect
reset behavior and the heading/_apply state in ComposeTranslateModal.
- Around line 63-72: The sample generation in composeTranslateModal’s useMemo is
passing 0 into postBodySummary, which causes the preview text to be empty and
breaks source-language detection fallback. Update the sample-building logic in
composeTranslateModal.tsx so postBodySummary receives a non-zero length based on
the sliced body content, and keep the existing SAMPLE_CHARS truncation
afterward.
In `@src/utils/iso639.test.ts`:
- Around line 117-121: The new localization test in languageDisplayName only
covers non-zt inputs, so add a case for the zt + uiLang path to lock down the
duplicate “(Traditional)” behavior. Extend the existing languageDisplayName test
block in iso639.test.ts with an assertion using languageDisplayName('zt', 'zt')
or languageDisplayName('zt', 'es'), and make sure it verifies the corrected
localized name from languageDisplayName in iso639.ts.
In `@src/utils/iso639.ts`:
- Around line 220-237: The language label logic in languageDisplayName is adding
a hardcoded “(Traditional)” suffix for the zt/zh-Hant case, which can duplicate
or mix languages with Intl.DisplayNames output. Update languageDisplayName to
return the Intl-generated name directly when available, and only keep a separate
localized qualifier if needed; reference the languageDisplayName function and
the zt handling branch to locate the change.
---
Nitpick comments:
In `@src/components/composeTranslateModal/composeTranslateModal.tsx`:
- Around line 271-277: The translated preview currently renders raw markdown
text instead of using the app’s standard post renderer. Update the preview block
in composeTranslateModal.tsx so it uses renderPostBody() from
`@ecency/render-helper` for the translated body, matching the existing preview
flow in markdownEditorView.tsx (_bodyHtmlForPreview). Keep the existing
translated/isRtlLang gating and swap the Text-based raw display for the rendered
HTML output so headings, lists, and quotes match published formatting.
In `@src/providers/translation/translateMarkdown.ts`:
- Around line 140-168: The marker-line handling in translateMarkdown currently
sends each heading/quote/list line as its own request, so consecutive
translatable lines never batch together. Update the block processing in
translateMarkdown to group consecutive marker lines from the same block into a
single translation request using a stable separator, then split the translated
response back per line when updating translatedLines and output. Keep the
existing per-line request logic as a fallback for edge cases where batching is
unsafe.
🪄 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: 41121548-6600-4264-84fe-cd06d9d814b8
📒 Files selected for processing (13)
src/components/composeTranslateModal/composeTranslateModal.styles.tssrc/components/composeTranslateModal/composeTranslateModal.tsxsrc/components/composeTranslateModal/index.tssrc/components/index.tsxsrc/components/markdownEditor/children/editorToolbar.tsxsrc/components/markdownEditor/view/markdownEditorView.tsxsrc/config/locales/en-US.jsonsrc/navigation/sheets.tsxsrc/providers/translation/translateMarkdown.test.tssrc/providers/translation/translateMarkdown.tssrc/providers/translation/translation.tssrc/utils/iso639.test.tssrc/utils/iso639.ts
| const sample = useMemo( | ||
| () => | ||
| body | ||
| ? postBodySummary(body.slice(0, RAW_SAMPLE_CHARS), 0, Platform.OS as any).slice( | ||
| 0, | ||
| SAMPLE_CHARS, | ||
| ) | ||
| : '', | ||
| [body], | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
@ecency/render-helper postBodySummary function accept string or Post object first argument
💡 Result:
The postBodySummary function from @ecency/render-helper accepts a string as its first argument, representing the body of the post [1][2]. While some documentation and usage patterns may refer to passing a post object or "entry" in broader contexts within Ecency applications, the specific implementation of postBodySummary(entryBody, length = 200, platform = "web") expects the first argument to be the post's body content (a string) [2]. When working with full post objects, developers typically pass the body property of that object to this function [3]. Summary of function signature: postBodySummary(entryBody: string, length?: number, platform?: string): string [2] - entryBody: The string content of the post to be summarized [2]. - length: (Optional) The maximum length of the returned summary; defaults to 200 characters [2]. - platform: (Optional) The target platform, which defaults to 'web' [1][2]. Using other values (e.g., 'react-native') adjusts the underlying implementation for mobile environments [1].
Citations:
- 1: https://registry.npmjs.org/%40ecency%2Frender-helper
- 2: ecency/vision-next@5d2a714
- 3: Fix post enhancement in Chats #3198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== target file excerpt ==\n'
sed -n '1,140p' src/components/composeTranslateModal/composeTranslateModal.tsx
printf '\n== postBodySummary call sites ==\n'
rg -n "postBodySummary\(" srcRepository: ecency/ecency-mobile
Length of output: 6803
🌐 Web query:
@ecency/render-helper postBodySummary length 0 behavior implementation
💡 Result:
In the @ecency/render-helper library, the postBodySummary function is designed to generate a text summary from post content [1][2]. Its implementation accepts three parameters: entryBody, length, and platform [1][3]. When the length parameter is set to 0, the function's internal logic—specifically where it processes text splitting using a joint function—effectively behaves as an empty or near-empty operation [1][3]. In the source code, the length parameter defaults to 200 [3]. If a value of 0 is passed, the internal text processing (which often involves splitting the body by spaces and truncating to the specified length) results in an empty string or no content being returned, as it enforces a length limit of zero characters [1][3]. Developers typically use this function by importing it from the package and specifying the target platform (e.g., 'web' or 'react-native') to ensure compatibility with different environments [1]. Because it relies on standard markdown-to-text processing, setting the length to 0 is a valid way to request no summary text from the provided post body [1][3].
Citations:
- 1: https://registry.npmjs.org/%40ecency%2Frender-helper
- 2: https://www.npmjs.com/package/@ecency/render-helper?activeTab=versions
- 3: ecency/vision-next@5d2a714
src/components/composeTranslateModal/composeTranslateModal.tsx:66 — pass a non-zero length here. postBodySummary(..., 0, ...) returns an empty string, so sample never contains text and source-language detection falls back to the default.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/composeTranslateModal/composeTranslateModal.tsx` around lines
63 - 72, The sample generation in composeTranslateModal’s useMemo is passing 0
into postBodySummary, which causes the preview text to be empty and breaks
source-language detection fallback. Update the sample-building logic in
composeTranslateModal.tsx so postBodySummary receives a non-zero length based on
the sliced body content, and keep the existing SAMPLE_CHARS truncation
afterward.
| useEffect(() => { | ||
| if (source !== 'en') { | ||
| setTarget('en'); | ||
| return; | ||
| } | ||
| const reader = normLang(appLang); | ||
| setTarget(reader && reader !== 'en' && LIBRETRANSLATE_TARGETS.has(reader) ? reader : 'es'); | ||
| }, [source, appLang]); | ||
|
|
||
| // A result translated into a previous language pair must never be applied. | ||
| useEffect(() => { | ||
| setTranslated(''); | ||
| setFailed(false); | ||
| }, [source, target]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/composeTranslateModal/composeTranslateModal.tsx"
echo "== line count =="
wc -l "$FILE"
echo
echo "== outline =="
ast-grep outline "$FILE" --view expanded || true
echo
echo "== relevant lines 1 =="
sed -n '1,260p' "$FILE" | nl -ba | sed -n '1,260p' | sed -n '90,230p'
echo
echo "== search translateMarkdown / DropdownButton / translating =="
rg -n "translateMarkdown|DropdownButton|translat|setTranslated|setFailed|source|target|closedRef|_apply" "$FILE"Repository: ecency/ecency-mobile
Length of output: 593
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/composeTranslateModal/composeTranslateModal.tsx"
echo "== translation handler and state effects =="
sed -n '120,190p' "$FILE" | cat -n
echo
echo "== dropdown JSX =="
sed -n '190,235p' "$FILE" | cat -n
echo
echo "== imports and state declarations =="
sed -n '1,120p' "$FILE" | cat -nRepository: ecency/ecency-mobile
Length of output: 9607
Stale translation results can overwrite the current language pair.
_translate still commits setTranslated(result) for the old source/target pair after the user changes either dropdown. The reset effect clears the preview, but the in-flight request can repopulate it with a translation that no longer matches the current heading/_apply state. Snapshot the requested pair and only commit if it still matches, or disable the dropdowns while translating.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/composeTranslateModal/composeTranslateModal.tsx` around lines
105 - 118, The translation flow in composeTranslateModal is still committing
stale results from an older language pair after the user changes source or
target. Update the _translate logic so it snapshots the requested source/target
pair and only calls setTranslated or setFailed if the current pair still matches
when the request resolves, or otherwise prevent pair changes while a translation
is in flight. Keep the fix aligned with the existing useEffect reset behavior
and the heading/_apply state in ComposeTranslateModal.
|
|
||
| it('localizes the name when a UI language is passed', () => { | ||
| expect(languageDisplayName('es', 'es').toLowerCase()).toBe('español'); | ||
| expect(languageDisplayName('de', 'de')).toBe('Deutsch'); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add coverage for the zt + uiLang path.
Existing new tests only exercise non-zt codes. Given the duplicate "(Traditional)" suffix bug flagged in iso639.ts, add a case like languageDisplayName('zt', 'zt') or languageDisplayName('zt', 'es') to lock in correct behavior once fixed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/iso639.test.ts` around lines 117 - 121, The new localization test
in languageDisplayName only covers non-zt inputs, so add a case for the zt +
uiLang path to lock down the duplicate “(Traditional)” behavior. Extend the
existing languageDisplayName test block in iso639.test.ts with an assertion
using languageDisplayName('zt', 'zt') or languageDisplayName('zt', 'es'), and
make sure it verifies the corrected localized name from languageDisplayName in
iso639.ts.
| export function languageDisplayName(code: string, uiLang?: string): string { | ||
| const norm = normLang(code); | ||
| if (DISPLAY_NAME_FALLBACK[norm]) { | ||
| if (!uiLang && DISPLAY_NAME_FALLBACK[norm]) { | ||
| return DISPLAY_NAME_FALLBACK[norm]; | ||
| } | ||
| try { | ||
| const AnyIntl = Intl as any; | ||
| if (AnyIntl && typeof AnyIntl.DisplayNames === 'function') { | ||
| const dn = new AnyIntl.DisplayNames(['en'], { type: 'language' }); | ||
| const dn = new AnyIntl.DisplayNames(uiLang ? [uiLang, 'en'] : ['en'], { type: 'language' }); | ||
| const name = dn.of(norm === 'zt' ? 'zh-Hant' : norm); | ||
| if (name && name.toLowerCase() !== norm) { | ||
| return name; | ||
| return norm === 'zt' && uiLang ? `${name} (Traditional)` : name; | ||
| } | ||
| } | ||
| } catch { | ||
| // ignore | ||
| } | ||
| return code; | ||
| return DISPLAY_NAME_FALLBACK[norm] ?? code; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file map =="
git ls-files src/utils/iso639.ts src/components/composeTranslateModal.tsx src | sed -n '1,120p'
echo
echo "== iso639 outline =="
ast-grep outline src/utils/iso639.ts --view expanded || true
echo
echo "== search call sites =="
rg -n "languageDisplayName\(" src -S
echo
echo "== relevant slices =="
sed -n '1,220p' src/utils/iso639.ts
echo
sed -n '220,280p' src/utils/iso639.ts
echo
if [ -f src/components/composeTranslateModal.tsx ]; then
sed -n '1,240p' src/components/composeTranslateModal.tsx
fiRepository: ecency/ecency-mobile
Length of output: 13316
🏁 Script executed:
python3 - <<'PY'
from pathlib import Path
p = Path('src/utils/iso639.ts')
print(p.exists(), p.stat().st_size if p.exists() else None)
PYRepository: ecency/ecency-mobile
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== call sites =="
rg -n "languageDisplayName\(" src -S
echo
echo "== composeTranslateModal slice if present =="
fd -a "composeTranslateModal.tsx" src || true
echo
echo "== DisplayNames behavior probe =="
node - <<'JS'
const samples = [
['en', 'zh-Hant'],
['es', 'zh-Hant'],
['zt', 'zh-Hant'],
['en', 'zh'],
['es', 'zh'],
];
for (const [uiLang, code] of samples) {
try {
const dn = new Intl.DisplayNames([uiLang, 'en'], { type: 'language' });
const name = dn.of(code);
console.log({ uiLang, code, name });
} catch (e) {
console.log({ uiLang, code, error: e.message });
}
}
JSRepository: ecency/ecency-mobile
Length of output: 2380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read only the relevant source slices.
if [ -f src/utils/iso639.ts ]; then
echo "== src/utils/iso639.ts (selected lines) =="
sed -n '200,280p' src/utils/iso639.ts
fi
echo
echo "== search for languageDisplayName =="
rg -n "languageDisplayName\(" src -S || true
echo
echo "== locate composeTranslateModal.tsx =="
fd -a "composeTranslateModal.tsx" src || trueRepository: ecency/ecency-mobile
Length of output: 3357
Avoid appending (Traditional) in languageDisplayName src/utils/iso639.ts:220-237 — Intl.DisplayNames already returns a localized zh-Hant name, so the extra English suffix can produce duplicated wording in some locales and mixes languages in others. Return name directly, or localize the qualifier separately.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/utils/iso639.ts` around lines 220 - 237, The language label logic in
languageDisplayName is adding a hardcoded “(Traditional)” suffix for the
zt/zh-Hant case, which can duplicate or mix languages with Intl.DisplayNames
output. Update languageDisplayName to return the Intl-generated name directly
when available, and only keep a separate localized qualifier if needed;
reference the languageDisplayName function and the zt handling branch to locate
the change.
1 participant
Mobile counterpart of ecency/vision-next#1080: a Translate action in the editor toolbar that machine-translates the author's own writing and appends it under a divider with a target-language heading, matching the common bilingual posting format.
What
[ES | EN]title marker (skipped when it would exceed the title limit).translateMarkdown(ported from web) translates block-aware while preserving structure: code fences, images, tables, raw HTML, bare URLs and horizontal rules pass through untouched; heading/quote/list markers are re-attached; indented continuation lines keep their whitespace; plain blocks batch per request. Any failure rejects the whole run and closing the sheet cancels the request chain.Notes
languageDisplayNamegained an optional locale argument (1-arg behavior unchanged, covered by an added test).Summary by CodeRabbit
New Features
Bug Fixes