From eaf307e71e1e56fd23955ff614c9bfc5c3ed406b Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Mon, 29 Jun 2026 00:58:56 +0800 Subject: [PATCH] fix(translate): align RTL translated text to the start (#4844) Inline translation wrappers set lang but never dir, so RTL target languages (Arabic, Hebrew, Persian, etc.) inherited the source document's LTR base direction. Justified text then pushed its last line to the LTR start (left) instead of the RTL start (right). Derive the wrapper's dir from the target language via getDirFromLanguage so justified RTL translations align to the start. Extract the node construction into createTranslationTargetNode to make the behavior unit-testable. Co-authored-by: Claude Opus 4.8 (1M context) --- .../reader/hooks/useTextTranslation.test.ts | 82 +++++++++++++++++++ .../app/reader/hooks/useTextTranslation.ts | 62 ++++++++++---- 2 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 apps/readest-app/src/__tests__/app/reader/hooks/useTextTranslation.test.ts diff --git a/apps/readest-app/src/__tests__/app/reader/hooks/useTextTranslation.test.ts b/apps/readest-app/src/__tests__/app/reader/hooks/useTextTranslation.test.ts new file mode 100644 index 00000000..97eda1b8 --- /dev/null +++ b/apps/readest-app/src/__tests__/app/reader/hooks/useTextTranslation.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from 'vitest'; +import { createTranslationTargetNode } from '@/app/reader/hooks/useTextTranslation'; + +describe('createTranslationTargetNode', () => { + it('sets dir="rtl" on the wrapper for an RTL target language', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'مرحبا بالعالم', + lang: 'ar', + targetBlockClassName: 'translation-target-block', + hidden: false, + widthLineBreak: false, + }); + + expect(wrapper.getAttribute('lang')).toBe('ar'); + expect(wrapper.getAttribute('dir')).toBe('rtl'); + }); + + it('sets dir="rtl" for a region-qualified RTL language (e.g. ar-EG)', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'مرحبا', + lang: 'ar-EG', + targetBlockClassName: 'translation-target-block', + hidden: false, + widthLineBreak: false, + }); + + expect(wrapper.getAttribute('dir')).toBe('rtl'); + }); + + it('sets dir="auto" on the wrapper for a non-RTL target language', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'Hello world', + lang: 'en', + targetBlockClassName: 'translation-target-block', + hidden: false, + widthLineBreak: false, + }); + + expect(wrapper.getAttribute('dir')).toBe('auto'); + }); + + it('builds the expected nested structure with the translated text', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'مرحبا بالعالم', + lang: 'ar', + targetBlockClassName: 'translation-target-toc', + hidden: false, + widthLineBreak: false, + }); + + expect(wrapper.classList.contains('translation-target')).toBe(true); + expect(wrapper.getAttribute('translation-element-mark')).toBe('1'); + const block = wrapper.querySelector('.translation-target-toc'); + expect(block).not.toBeNull(); + const inner = wrapper.querySelector('.target-inner'); + expect(inner?.textContent).toBe('مرحبا بالعالم'); + }); + + it('marks the wrapper hidden when hidden is true', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'مرحبا', + lang: 'ar', + targetBlockClassName: 'translation-target-block', + hidden: true, + widthLineBreak: false, + }); + + expect(wrapper.classList.contains('hidden')).toBe(true); + }); + + it('prepends a
when widthLineBreak is true', () => { + const wrapper = createTranslationTargetNode({ + translatedText: 'مرحبا', + lang: 'ar', + targetBlockClassName: 'translation-target-block', + hidden: false, + widthLineBreak: true, + }); + + expect(wrapper.firstChild?.nodeName).toBe('BR'); + }); +}); diff --git a/apps/readest-app/src/app/reader/hooks/useTextTranslation.ts b/apps/readest-app/src/app/reader/hooks/useTextTranslation.ts index b9b243c5..17a7ec13 100644 --- a/apps/readest-app/src/app/reader/hooks/useTextTranslation.ts +++ b/apps/readest-app/src/app/reader/hooks/useTextTranslation.ts @@ -9,6 +9,44 @@ import { eventDispatcher } from '@/utils/event'; import { walkTextNodes } from '@/utils/walk'; import { debounce } from '@/utils/debounce'; import { getLocale } from '@/utils/misc'; +import { getDirFromLanguage } from '@/utils/rtl'; + +export const createTranslationTargetNode = ({ + translatedText, + lang, + targetBlockClassName, + hidden, + widthLineBreak, +}: { + translatedText: string; + lang: string; + targetBlockClassName: string; + hidden: boolean; + widthLineBreak: boolean; +}) => { + const wrapper = document.createElement('font'); + wrapper.className = `translation-target ${hidden ? 'hidden' : ''}`; + wrapper.setAttribute('translation-element-mark', '1'); + wrapper.setAttribute('lang', lang); + // Set the base direction from the target language so justified RTL text + // (e.g. Arabic) aligns to the start (right) instead of inheriting the + // source document's LTR direction. + wrapper.setAttribute('dir', getDirFromLanguage(lang)); + if (widthLineBreak) { + wrapper.appendChild(document.createElement('br')); + } + + const blockWrapper = document.createElement('font'); + blockWrapper.className = `translation-target ${targetBlockClassName}`; + + const inner = document.createElement('font'); + inner.className = 'translation-target target-inner target-inner-theme-none'; + inner.textContent = translatedText; + + blockWrapper.appendChild(inner); + wrapper.appendChild(blockWrapper); + return wrapper; +}; export function useTextTranslation( bookKey: string, @@ -260,23 +298,13 @@ export function useTextTranslation( const translatedText = translated[0]; if (!translatedText || text === translatedText) return; - const wrapper = document.createElement('font'); - wrapper.className = `translation-target ${!enabled.current ? 'hidden' : ''}`; - wrapper.setAttribute('translation-element-mark', '1'); - wrapper.setAttribute('lang', targetLang || getLocale()); - if (widthLineBreak) { - wrapper.appendChild(document.createElement('br')); - } - - const blockWrapper = document.createElement('font'); - blockWrapper.className = `translation-target ${targetBlockClassName}`; - - const inner = document.createElement('font'); - inner.className = 'translation-target target-inner target-inner-theme-none'; - inner.textContent = translatedText; - - blockWrapper.appendChild(inner); - wrapper.appendChild(blockWrapper); + const wrapper = createTranslationTargetNode({ + translatedText, + lang: targetLang || getLocale(), + targetBlockClassName, + hidden: !enabled.current, + widthLineBreak, + }); if (el.querySelector('.translation-target')) { return;