Skip to content

Commit 357e593

Browse files
authored
fix(super-editor): correct cursor movement direction in RTL paragraphs (SD-2390) (#2590)
* fix(super-editor): correct cursor movement direction in RTL paragraphs Set dir="rtl" on the hidden ProseMirror editor's paragraph DOM elements when paragraphProperties.rightToLeft is true. Without this, the browser's native bidi cursor handling treated all text as LTR, causing ArrowLeft to move the cursor visually right and ArrowRight visually left in Arabic text. Also fix resolvePositionAtGoalX binary search for vertical navigation (up/down arrows) to invert search direction in RTL paragraphs where X coordinates decrease with increasing PM positions. SD-2390 * refactor(super-editor): pass isRtl to resolvePositionAtGoalX from DOM Read RTL direction from the visual DOM element (set by DomPainter using resolved style properties) instead of calling doc.resolve(pmStart) inside the binary search helper. This fixes two review findings: 1. Eliminates RangeError crash when pmStart is stale (layout-derived positions can lag the current document after edits) 2. Correctly detects style-inherited RTL direction (not just inline paragraphProperties.rightToLeft) Also consolidates the duplicated makeRtlEditor test helper into the existing makeEditor, since isRtl is now a simple boolean parameter. * test(selection): add behavior and visual tests for RTL arrow key movement Add Playwright behavior test that verifies ArrowLeft/ArrowRight move the cursor in the correct visual direction in RTL paragraphs (and regression test for LTR). Add visual regression test that screenshots cursor position after arrow navigation in an RTL document. Both use the existing rtl-mixed-bidi.docx fixture. SD-2390
1 parent 3ad0ffc commit 357e593

5 files changed

Lines changed: 219 additions & 7 deletions

File tree

packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,12 @@ export class ParagraphNodeView {
141141
if (paragraphProperties.styleId) {
142142
this.dom.setAttribute('styleid', paragraphProperties.styleId);
143143
}
144+
145+
if (paragraphProperties.rightToLeft) {
146+
this.dom.setAttribute('dir', 'rtl');
147+
} else {
148+
this.dom.removeAttribute('dir');
149+
}
144150
}
145151

146152
/**

packages/super-editor/src/editors/v1/extensions/paragraph/ParagraphNodeView.test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,4 +301,45 @@ describe('ParagraphNodeView', () => {
301301
expect(nodeView.dom.getAttribute('data-level')).toBeNull();
302302
expect(nodeView.dom.classList.contains('sd-editor-dropcap')).toBe(false);
303303
});
304+
305+
it('sets dir="rtl" on RTL paragraphs', () => {
306+
isList.mockReturnValue(false);
307+
resolveParagraphProperties.mockReturnValue({ rightToLeft: true });
308+
309+
const { nodeView } = mountNodeView({
310+
attrs: {
311+
paragraphProperties: { rightToLeft: true },
312+
listRendering: {},
313+
},
314+
});
315+
316+
expect(nodeView.dom.getAttribute('dir')).toBe('rtl');
317+
});
318+
319+
it('does not set dir on LTR paragraphs', () => {
320+
isList.mockReturnValue(false);
321+
resolveParagraphProperties.mockReturnValue({});
322+
323+
const { nodeView } = mountNodeView();
324+
325+
expect(nodeView.dom.getAttribute('dir')).toBeNull();
326+
});
327+
328+
it('removes dir="rtl" when paragraph changes from RTL to LTR', () => {
329+
isList.mockReturnValue(false);
330+
resolveParagraphProperties.mockReturnValueOnce({ rightToLeft: true }).mockReturnValueOnce({});
331+
332+
const { nodeView } = mountNodeView({
333+
attrs: {
334+
paragraphProperties: { rightToLeft: true },
335+
listRendering: {},
336+
},
337+
});
338+
expect(nodeView.dom.getAttribute('dir')).toBe('rtl');
339+
340+
const ltrNode = createNode({ attrs: { paragraphProperties: {}, listRendering: {} } });
341+
nodeView.update(ltrNode, []);
342+
343+
expect(nodeView.dom.getAttribute('dir')).toBeNull();
344+
});
304345
});

packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.js

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ export const VerticalNavigation = Extension.create({
145145
) {
146146
// Hit test produced a position outside the adjacent line's range.
147147
// Resolve position directly from layout data using binary search at goalX.
148-
hit = resolvePositionAtGoalX(editor, adjacent.pmStart, adjacent.pmEnd, goalX);
148+
hit = resolvePositionAtGoalX(editor, adjacent.pmStart, adjacent.pmEnd, goalX, adjacent.isRtl);
149149
}
150150
}
151151

@@ -265,11 +265,16 @@ function getAdjacentLineClientTarget(editor, coords, direction) {
265265
const pmStart = Number(adjacentLine.dataset?.pmStart);
266266
const pmEnd = Number(adjacentLine.dataset?.pmEnd);
267267

268+
// Read direction from the visual DOM — DomPainter sets dir="rtl" on RTL lines
269+
// using fully resolved properties (style cascade, not just inline attrs).
270+
const isRtl = adjacentLine.closest?.('[dir="rtl"]') != null;
271+
268272
return {
269273
clientY,
270274
pageIndex: Number.isFinite(pageIndex) ? pageIndex : undefined,
271275
pmStart: Number.isFinite(pmStart) ? pmStart : undefined,
272276
pmEnd: Number.isFinite(pmEnd) ? pmEnd : undefined,
277+
isRtl,
273278
};
274279
}
275280

@@ -372,16 +377,15 @@ function findAdjacentLineElement(currentLine, direction, caretX) {
372377
* @param {number} pmStart - Start PM position of the target line.
373378
* @param {number} pmEnd - End PM position of the target line.
374379
* @param {number} goalX - Target X coordinate in layout space.
380+
* @param {boolean} [isRtl=false] - Whether the target line is RTL. In RTL lines,
381+
* X decreases as PM position increases, so the binary search must be inverted.
375382
* @returns {{ pos: number } | null}
376383
*/
377-
export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX) {
384+
export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX, isRtl = false) {
378385
const presentationEditor = editor.presentationEditor;
379386
let bestPos = pmStart;
380387
let bestDist = Infinity;
381388

382-
// Binary search: characters within a single line have monotonically increasing X.
383-
// NOTE: assumes LTR text. For RTL, X decreases with position so the search
384-
// direction would be inverted. bestPos/bestDist tracking limits the impact.
385389
let lo = pmStart;
386390
let hi = pmEnd;
387391

@@ -403,9 +407,13 @@ export function resolvePositionAtGoalX(editor, pmStart, pmEnd, goalX) {
403407
}
404408

405409
if (rect.x < goalX) {
406-
lo = mid + 1;
410+
// In LTR, X < goalX means search higher positions (further right).
411+
// In RTL, X < goalX means search lower positions (further right in RTL).
412+
if (isRtl) hi = mid - 1;
413+
else lo = mid + 1;
407414
} else if (rect.x > goalX) {
408-
hi = mid - 1;
415+
if (isRtl) lo = mid + 1;
416+
else hi = mid - 1;
409417
} else {
410418
// Exact match
411419
break;

packages/super-editor/src/editors/v1/extensions/vertical-navigation/vertical-navigation.test.js

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,4 +415,43 @@ describe('resolvePositionAtGoalX', () => {
415415
const result = resolvePositionAtGoalX(editor, 10, 10, 50);
416416
expect(result).toEqual({ pos: 10 });
417417
});
418+
419+
describe('RTL support', () => {
420+
it('finds correct position in RTL line (X decreases with position)', () => {
421+
// RTL: position 10 → x=40, position 14 → x=0 (X decreases with PM position)
422+
const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 }));
423+
const result = resolvePositionAtGoalX(editor, 10, 14, 25, true);
424+
// goalX=25: pos 11 has x=30 (dist=5), pos 12 has x=20 (dist=5)
425+
// Binary search with inverted direction should find pos 11 or 12
426+
expect(result.pos).toBeGreaterThanOrEqual(11);
427+
expect(result.pos).toBeLessThanOrEqual(12);
428+
});
429+
430+
it('returns pmStart for RTL when goalX matches the rightmost position', () => {
431+
// RTL: pmStart has highest X
432+
const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 }));
433+
const result = resolvePositionAtGoalX(editor, 10, 14, 40, true);
434+
expect(result).toEqual({ pos: 10 });
435+
});
436+
437+
it('returns pmEnd for RTL when goalX matches the leftmost position', () => {
438+
// RTL: pmEnd has lowest X
439+
const editor = makeEditor((pos) => ({ x: (14 - pos) * 10 }));
440+
const result = resolvePositionAtGoalX(editor, 10, 14, 0, true);
441+
expect(result).toEqual({ pos: 14 });
442+
});
443+
444+
it('does not invert search when isRtl is false', () => {
445+
// LTR: X increases with position (same as existing tests)
446+
const editor = makeEditor((pos) => ({ x: (pos - 10) * 10 }));
447+
const result = resolvePositionAtGoalX(editor, 10, 14, 25, false);
448+
expect(result).toEqual({ pos: 12 });
449+
});
450+
451+
it('defaults to LTR when isRtl is not provided', () => {
452+
const editor = makeEditor((pos) => ({ x: (pos - 10) * 10 }));
453+
const result = resolvePositionAtGoalX(editor, 10, 14, 25);
454+
expect(result).toEqual({ pos: 12 });
455+
});
456+
});
418457
});
Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
import fs from 'node:fs';
2+
import path from 'node:path';
3+
import { fileURLToPath } from 'node:url';
4+
import { test, expect } from '../../fixtures/superdoc.js';
5+
6+
const __dirname = path.dirname(fileURLToPath(import.meta.url));
7+
const DOC_PATH = path.resolve(__dirname, 'fixtures/rtl-mixed-bidi.docx');
8+
9+
test.skip(!fs.existsSync(DOC_PATH), 'RTL fixture not available');
10+
11+
test.use({ config: { toolbar: 'none', showCaret: true, showSelection: true } });
12+
13+
test.describe('RTL arrow key cursor movement (SD-2390)', () => {
14+
test('ArrowLeft moves cursor visually left in RTL paragraph', async ({ superdoc }) => {
15+
await superdoc.loadDocument(DOC_PATH);
16+
await superdoc.waitForStable();
17+
18+
// First line is RTL Arabic: "هذه فقرة كاملة باللغة العربية"
19+
const rtlLine = superdoc.page.locator('.superdoc-line').first();
20+
const box = await rtlLine.boundingBox();
21+
if (!box) throw new Error('RTL line not visible');
22+
23+
// Click near the right edge (logical start of RTL text)
24+
await superdoc.page.mouse.click(box.x + box.width - 20, box.y + box.height / 2);
25+
await superdoc.waitForStable();
26+
27+
const before = await superdoc.getSelection();
28+
29+
// Get caret X before
30+
const xBefore = await superdoc.page.evaluate((pos) => {
31+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
32+
return pe?.computeCaretLayoutRect(pos)?.x;
33+
}, before.from);
34+
35+
// Press ArrowLeft
36+
await superdoc.page.keyboard.press('ArrowLeft');
37+
await superdoc.waitForStable();
38+
39+
const after = await superdoc.getSelection();
40+
const xAfter = await superdoc.page.evaluate((pos) => {
41+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
42+
return pe?.computeCaretLayoutRect(pos)?.x;
43+
}, after.from);
44+
45+
// In RTL, ArrowLeft should move visually left (decreasing X)
46+
expect(xAfter).toBeLessThan(xBefore);
47+
// PM position should increase (moving toward end of line in document order)
48+
expect(after.from).toBeGreaterThan(before.from);
49+
});
50+
51+
test('ArrowRight moves cursor visually right in RTL paragraph', async ({ superdoc }) => {
52+
await superdoc.loadDocument(DOC_PATH);
53+
await superdoc.waitForStable();
54+
55+
const rtlLine = superdoc.page.locator('.superdoc-line').first();
56+
const box = await rtlLine.boundingBox();
57+
if (!box) throw new Error('RTL line not visible');
58+
59+
// Click near the middle of the line
60+
await superdoc.page.mouse.click(box.x + box.width / 2, box.y + box.height / 2);
61+
await superdoc.waitForStable();
62+
63+
const before = await superdoc.getSelection();
64+
const xBefore = await superdoc.page.evaluate((pos) => {
65+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
66+
return pe?.computeCaretLayoutRect(pos)?.x;
67+
}, before.from);
68+
69+
// Press ArrowRight
70+
await superdoc.page.keyboard.press('ArrowRight');
71+
await superdoc.waitForStable();
72+
73+
const after = await superdoc.getSelection();
74+
const xAfter = await superdoc.page.evaluate((pos) => {
75+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
76+
return pe?.computeCaretLayoutRect(pos)?.x;
77+
}, after.from);
78+
79+
// In RTL, ArrowRight should move visually right (increasing X)
80+
expect(xAfter).toBeGreaterThan(xBefore);
81+
// PM position should decrease (moving toward start of line in document order)
82+
expect(after.from).toBeLessThan(before.from);
83+
});
84+
85+
test('ArrowLeft/Right in LTR paragraph still works correctly', async ({ superdoc }) => {
86+
await superdoc.loadDocument(DOC_PATH);
87+
await superdoc.waitForStable();
88+
89+
// Second line is LTR English: "This is a complete English paragraph"
90+
const ltrLine = superdoc.page.locator('.superdoc-line').nth(1);
91+
const box = await ltrLine.boundingBox();
92+
if (!box) throw new Error('LTR line not visible');
93+
94+
// Click near the left edge
95+
await superdoc.page.mouse.click(box.x + 30, box.y + box.height / 2);
96+
await superdoc.waitForStable();
97+
98+
const before = await superdoc.getSelection();
99+
const xBefore = await superdoc.page.evaluate((pos) => {
100+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
101+
return pe?.computeCaretLayoutRect(pos)?.x;
102+
}, before.from);
103+
104+
// Press ArrowRight in LTR
105+
await superdoc.page.keyboard.press('ArrowRight');
106+
await superdoc.waitForStable();
107+
108+
const after = await superdoc.getSelection();
109+
const xAfter = await superdoc.page.evaluate((pos) => {
110+
const pe = (window as any).superdoc?.activeEditor?.presentationEditor;
111+
return pe?.computeCaretLayoutRect(pos)?.x;
112+
}, after.from);
113+
114+
// In LTR, ArrowRight moves visually right (increasing X) and increases PM position
115+
expect(xAfter).toBeGreaterThan(xBefore);
116+
expect(after.from).toBeGreaterThan(before.from);
117+
});
118+
});

0 commit comments

Comments
 (0)