diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index f22d14296e..689e7f16d9 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -606,10 +606,16 @@ export type TableCellAttrs = { tableCellProperties?: Record; }; +export type TablePropertiesAttrs = { + rightToLeft?: boolean; + [key: string]: unknown; +}; + export type TableAttrs = { borders?: TableBorders; borderCollapse?: 'collapse' | 'separate'; cellSpacing?: CellSpacing; + tableProperties?: TablePropertiesAttrs; sdt?: SdtMetadata; containerSdt?: SdtMetadata; [key: string]: unknown; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index 5057b2677b..4b0d9cec2f 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -306,6 +306,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement min: boundary.minWidth, r: boundary.resizable ? 1 : 0, })), + rtl: isRtl, // Add segments for each column boundary (segments where resize handle should appear) segments: boundarySegments.map((segs, colIndex) => segs.map((seg) => ({ diff --git a/packages/super-editor/src/editors/v1/components/SuperEditor.vue b/packages/super-editor/src/editors/v1/components/SuperEditor.vue index 08fcc9bd0b..f274818449 100644 --- a/packages/super-editor/src/editors/v1/components/SuperEditor.vue +++ b/packages/super-editor/src/editors/v1/components/SuperEditor.vue @@ -457,6 +457,14 @@ const isNearColumnBoundary = (event, tableElement) => { try { const metadata = JSON.parse(boundariesAttr); if (!metadata.columns || !Array.isArray(metadata.columns)) return false; + const isRtl = metadata.rtl === true; + const firstColumn = metadata.columns[0]; + const lastColumn = metadata.columns[metadata.columns.length - 1]; + const tableContentLeft = firstColumn && typeof firstColumn.x === 'number' ? firstColumn.x : 0; + const tableContentWidth = + lastColumn && typeof lastColumn.x === 'number' && typeof lastColumn.w === 'number' + ? lastColumn.x + lastColumn.w + : 0; // Get zoom factor to properly compare screen coordinates with layout coordinates const zoom = getEditorZoom(); @@ -486,7 +494,9 @@ const isNearColumnBoundary = (event, tableElement) => { // The boundary x position is at (col.x + col.w) - the right edge of the column // This is in layout coordinates, so multiply by zoom to convert to screen space - const boundaryXScreen = (col.x + col.w) * zoom; + const logicalBoundaryX = col.x + col.w; + const visualBoundaryX = isRtl ? tableContentLeft + tableContentWidth - logicalBoundaryX : logicalBoundaryX; + const boundaryXScreen = visualBoundaryX * zoom; // Check if mouse is horizontally near this boundary (both in screen space now) if (Math.abs(mouseXScreen - boundaryXScreen) <= TABLE_RESIZE_HOVER_THRESHOLD) { @@ -514,8 +524,12 @@ const isNearColumnBoundary = (event, tableElement) => { } } - // Also check left edge of table (x = 0) - if (Math.abs(mouseXScreen) <= TABLE_RESIZE_HOVER_THRESHOLD) { + // Also check table outer edges. + const tableWidthScreen = tableRect.width; + if ( + Math.abs(mouseXScreen) <= TABLE_RESIZE_HOVER_THRESHOLD || + Math.abs(mouseXScreen - tableWidthScreen) <= TABLE_RESIZE_HOVER_THRESHOLD + ) { return true; } diff --git a/packages/super-editor/src/editors/v1/components/TableResizeOverlay.vue b/packages/super-editor/src/editors/v1/components/TableResizeOverlay.vue index b757cf2906..e4c0a0e731 100644 --- a/packages/super-editor/src/editors/v1/components/TableResizeOverlay.vue +++ b/packages/super-editor/src/editors/v1/components/TableResizeOverlay.vue @@ -381,6 +381,21 @@ function updateOverlayRect() { * - Inner boundaries (between columns) * - Right edge (resize last column) */ +const isRtlTable = computed(() => tableMetadata.value?.rtl === true); + +const tableContentWidth = computed(() => { + const columns = tableMetadata.value?.columns; + if (!columns || columns.length === 0) return 0; + const last = columns[columns.length - 1]; + return last.x + last.w; +}); + +const tableContentLeft = computed(() => { + const columns = tableMetadata.value?.columns; + if (!columns || columns.length === 0) return 0; + return columns[0].x; +}); + const resizableBoundaries = computed(() => { if (!tableMetadata.value?.columns) { return []; @@ -394,20 +409,32 @@ const resizableBoundaries = computed(() => { const col = columns[i]; const nextCol = columns[i + 1]; + const logicalX = nextCol.x; + const visualX = isRtlTable.value ? tableContentLeft.value + tableContentWidth.value - logicalX : logicalX; + boundaries.push({ ...col, index: i, - x: nextCol.x, + x: visualX, type: 'inner', }); } - // Add handle for right edge of table (resize last column) + // Add handle for right edge of table (resize last column). + // SD-2810 RTL note: the right-edge handle ALWAYS sits on the visual right + // side of the table. In an LTR table that means the trailing edge of the + // last logical column (`columns.length - 1`); in a `bidiVisual` RTL table + // the visually-rightmost column is the FIRST logical column (`0`) because + // cells are stored logically and rendered right-to-left. Downstream + // consumers that need "is this the table's outer edge?" should key off + // `type === 'right-edge'`, NOT `columnIndex === columns.length - 1`, + // because that index equality is LTR-only. const lastCol = columns[columns.length - 1]; + const rtlRightEdgeX = tableContentWidth.value; boundaries.push({ ...lastCol, - index: columns.length - 1, - x: lastCol.x + lastCol.w, + index: isRtlTable.value ? 0 : columns.length - 1, + x: isRtlTable.value ? rtlRightEdgeX : lastCol.x + lastCol.w, type: 'right-edge', }); @@ -727,7 +754,7 @@ function parseTableMetadata() { ) : undefined; - tableMetadata.value = { columns: validatedColumns, segments, rows }; + tableMetadata.value = { columns: validatedColumns, segments, rows, rtl: parsed.rtl === true }; } catch (error) { tableMetadata.value = null; emit('resize-error', { @@ -853,7 +880,8 @@ const mouseMoveThrottle = throttle((event) => { // Calculate raw delta in screen pixels, then convert to layout space // This ensures constraints (which are in layout space) can be compared correctly const screenDelta = event.clientX - dragState.value.initialX; - const delta = screenDelta / zoom; + const visualDelta = screenDelta / zoom; + const delta = isRtlTable.value && !dragState.value.isRightEdge ? -visualDelta : visualDelta; // Calculate constraints based on layout-computed minWidth (already in layout space) const minDelta = -(dragState.value.leftColumn.width - dragState.value.leftColumn.minWidth); @@ -886,9 +914,16 @@ const mouseMoveThrottle = throttle((event) => { // Constrain delta const constrainedDelta = Math.max(minDelta, Math.min(maxDelta, delta)); + const constrainedVisualDelta = + isRtlTable.value && !dragState.value.isRightEdge ? -constrainedDelta : constrainedDelta; - // Update visual guideline only (no PM transaction yet) - dragState.value.constrainedDelta = constrainedDelta; + // Update visual guideline only (no PM transaction yet). + // `constrainedDelta` on dragState stays in VISUAL coordinates so the preview + // guideline at line 591 (`initialBoundary.x + dragState.value.constrainedDelta`) + // tracks the cursor. The emitted `delta` below is LOGICAL: it matches the + // value applied to `newWidths` and preserves the pre-PR contract for + // external listeners (logging, analytics, undo metadata). + dragState.value.constrainedDelta = constrainedVisualDelta; emit('resize-move', { columnIndex: dragState.value.columnIndex, @@ -906,7 +941,8 @@ const onDocumentMouseMove = mouseMoveThrottle.throttled; function onDocumentMouseUp(event) { if (!dragState.value) return; - const finalDelta = dragState.value.constrainedDelta; + const visualFinalDelta = dragState.value.constrainedDelta; + const finalDelta = isRtlTable.value && !dragState.value.isRightEdge ? -visualFinalDelta : visualFinalDelta; const columnIndex = dragState.value.columnIndex; const initialWidths = dragState.value.initialWidths; const isRightEdge = dragState.value.isRightEdge; @@ -935,7 +971,10 @@ function onDocumentMouseUp(event) { dispatchResizeTransaction(columnIndex, newWidths); } - // Always emit resize-end so the parent can clear its dragging flag + // Always emit resize-end so the parent can clear its dragging flag. + // `delta` is the LOGICAL change applied to `newWidths`, matching the + // pre-PR contract. In LTR this equals `visualFinalDelta`; in RTL inner + // boundaries it is the sign-flipped value. emit('resize-end', { columnIndex, finalWidths: newWidths, diff --git a/packages/super-editor/src/editors/v1/tests/import-export/table-bidivisual-roundtrip.test.js b/packages/super-editor/src/editors/v1/tests/import-export/table-bidivisual-roundtrip.test.js new file mode 100644 index 0000000000..ac156afb4e --- /dev/null +++ b/packages/super-editor/src/editors/v1/tests/import-export/table-bidivisual-roundtrip.test.js @@ -0,0 +1,51 @@ +import { describe, it, expect } from 'vitest'; +import JSZip from 'jszip'; +import { Editor } from '@core/Editor.js'; +import DocxZipper from '@core/DocxZipper.js'; +import { initTestEditor, getTestDataAsFileBuffer } from '../helpers/helpers.js'; + +const TEST_DOC = 'table-width-issue.docx'; + +async function buildDocxWithBidiVisualTable() { + const baseBuffer = await getTestDataAsFileBuffer(TEST_DOC); + const zip = await JSZip.loadAsync(baseBuffer); + const documentEntry = zip.file('word/document.xml'); + if (!documentEntry) throw new Error('word/document.xml not found in fixture.'); + + const documentXml = await documentEntry.async('string'); + const patchedDocumentXml = documentXml.replace(//, ''); + + if (patchedDocumentXml === documentXml) { + throw new Error('Could not inject into first table.'); + } + + zip.file('word/document.xml', patchedDocumentXml); + return zip.generateAsync({ type: 'nodebuffer' }); +} + +describe('table bidiVisual import/export roundtrip', () => { + it('preserves w:bidiVisual in word/document.xml on export', async () => { + const patchedBuffer = await buildDocxWithBidiVisualTable(); + const inputFiles = await new DocxZipper().getDocxData(patchedBuffer, true); + const inputDocument = inputFiles.find((entry) => entry.name === 'word/document.xml')?.content; + expect(inputDocument).toContain(' entry.name === 'word/document.xml')?.content; + + expect(exportedDocument).toBeTruthy(); + expect(exportedDocument).toContain(' { const frag = document.querySelector('.superdoc-table-fragment[data-table-boundaries]'); if (!frag) throw new Error('No table fragment with boundaries found'); - const { columns } = JSON.parse(frag.getAttribute('data-table-boundaries')!); + const meta = JSON.parse(frag.getAttribute('data-table-boundaries')!); + const columns = meta.columns as Array<{ x: number; w: number }>; + const isRtl = meta.rtl === true; const col = t === 'right-edge' ? columns[columns.length - 1] : columns[t]; if (!col) throw new Error(`Column ${t} not found`); + const tableContentWidth = columns[columns.length - 1].x + columns[columns.length - 1].w; + const logicalBoundaryX = col.x + col.w; + const visualBoundaryX = isRtl ? tableContentWidth - logicalBoundaryX : logicalBoundaryX; const rect = frag.getBoundingClientRect(); // Hover 2px inside the right edge so the cursor stays within the table element - const offset = t === 'right-edge' ? -2 : 0; - return { x: rect.left + col.x + col.w + offset, y: rect.top + rect.height / 2 }; + const offset = t === 'right-edge' ? (isRtl ? 2 : -2) : 0; + return { x: rect.left + visualBoundaryX + offset, y: rect.top + rect.height / 2 }; }, target); await page.mouse.move(pos.x, pos.y); @@ -158,3 +168,41 @@ test('row handles are hidden during column resize drag (SD-2094)', async ({ supe await superdoc.page.mouse.up(); await superdoc.waitForStable(); }); + +test('rtl table shows resize indicator again after drag on same boundary', async ({ superdoc }) => { + await superdoc.loadDocument(RTL_DOC); + await superdoc.waitForStable(); + + await hoverColumnBoundary(superdoc.page, 0); + await superdoc.waitForStable(); + const handle = superdoc.page.locator('.resize-handle[data-boundary-type="inner"]').first(); + await expect(handle).toBeAttached({ timeout: 5000 }); + + await dragHandle(superdoc.page, handle, 40); + await superdoc.waitForStable(); + + await hoverColumnBoundary(superdoc.page, 0); + await superdoc.waitForStable(); + await expect(superdoc.page.locator('.resize-handle[data-boundary-type="inner"]').first()).toBeAttached({ + timeout: 5000, + }); +}); + +test('ltr table still shows resize indicator again after drag (guard)', async ({ superdoc }) => { + await superdoc.loadDocument(LTR_DOC); + await superdoc.waitForStable(); + + await hoverColumnBoundary(superdoc.page, 0); + await superdoc.waitForStable(); + const handle = superdoc.page.locator('.resize-handle[data-boundary-type="inner"]').first(); + await expect(handle).toBeAttached({ timeout: 5000 }); + + await dragHandle(superdoc.page, handle, 40); + await superdoc.waitForStable(); + + await hoverColumnBoundary(superdoc.page, 0); + await superdoc.waitForStable(); + await expect(superdoc.page.locator('.resize-handle[data-boundary-type="inner"]').first()).toBeAttached({ + timeout: 5000, + }); +}); diff --git a/tests/behavior/tests/tables/rtl-table-click-fallback.spec.ts b/tests/behavior/tests/tables/rtl-table-click-fallback.spec.ts new file mode 100644 index 0000000000..9c904897b8 --- /dev/null +++ b/tests/behavior/tests/tables/rtl-table-click-fallback.spec.ts @@ -0,0 +1,80 @@ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', showCaret: true, showSelection: true } }); + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOCS = [ + path.resolve(__dirname, 'fixtures/rtl-table-1.docx'), + path.resolve(__dirname, 'fixtures/rtl-table-2.docx'), +] as const; + +for (const docPath of DOCS) { + test(`rtl table click mapping stays in cell for text and empty-area clicks (${path.basename(docPath)})`, async ({ + superdoc, + }) => { + await superdoc.loadDocument(docPath); + await superdoc.waitForStable(); + + const data = await superdoc.page.evaluate(() => { + const line = document.querySelector('.superdoc-table-fragment .superdoc-line') as HTMLElement | null; + if (!line) { + return null; + } + + const lineRect = line.getBoundingClientRect(); + const linePmStart = Number(line.dataset.pmStart ?? 'NaN'); + const linePmEnd = Number(line.dataset.pmEnd ?? 'NaN'); + + // Find cell container by style signature used by DomPainter table cells. + let cell: HTMLElement | null = line.parentElement as HTMLElement | null; + while (cell) { + const style = getComputedStyle(cell); + if (style.position === 'absolute' && style.overflow === 'hidden') { + break; + } + cell = cell.parentElement as HTMLElement | null; + } + + if (!cell) { + return null; + } + + const cellRect = cell.getBoundingClientRect(); + + const textPoint = { + x: lineRect.x + Math.min(Math.max(8, lineRect.width * 0.5), Math.max(8, lineRect.width - 8)), + y: lineRect.y + lineRect.height / 2, + }; + + // Empty area in same cell: lower part of the cell, away from the text line. + const emptyPoint = { + x: Math.min(Math.max(cellRect.x + 8, textPoint.x), cellRect.right - 8), + y: Math.max(lineRect.bottom + 6, cellRect.y + cellRect.height * 0.8), + }; + + return { + linePmStart, + linePmEnd, + textPoint, + emptyPoint, + }; + }); + + expect(data).not.toBeNull(); + if (!data) return; + + await superdoc.page.mouse.click(data.textPoint.x, data.textPoint.y); + await superdoc.waitForStable(); + const afterTextClick = await superdoc.getSelection(); + expect(afterTextClick.from).toBeGreaterThanOrEqual(data.linePmStart); + expect(afterTextClick.from).toBeLessThanOrEqual(data.linePmEnd); + + await superdoc.page.mouse.click(data.emptyPoint.x, data.emptyPoint.y); + await superdoc.waitForStable(); + const afterEmptyClick = await superdoc.getSelection(); + expect(afterEmptyClick.from).toBeGreaterThanOrEqual(data.linePmStart); + expect(afterEmptyClick.from).toBeLessThanOrEqual(data.linePmEnd); + }); +} diff --git a/tests/behavior/tests/tables/rtl-table-tab-navigation.spec.ts b/tests/behavior/tests/tables/rtl-table-tab-navigation.spec.ts new file mode 100644 index 0000000000..2b96f9aad5 --- /dev/null +++ b/tests/behavior/tests/tables/rtl-table-tab-navigation.spec.ts @@ -0,0 +1,110 @@ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full', showCaret: true, showSelection: true } }); + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const DOCS = [ + path.resolve(__dirname, 'fixtures/rtl-table-1.docx'), + path.resolve(__dirname, 'fixtures/rtl-table-2.docx'), +] as const; + +async function getSelectionLineY(page: import('@playwright/test').Page): Promise { + return page.evaluate(() => { + const editor = (window as any).editor; + const pos = editor?.state?.selection?.from; + if (typeof pos !== 'number') return null; + + const lines = Array.from(document.querySelectorAll('.superdoc-line')); + let nearest: { y: number; distance: number } | null = null; + for (const line of lines) { + const start = Number(line.dataset.pmStart ?? 'NaN'); + const end = Number(line.dataset.pmEnd ?? 'NaN'); + if (!Number.isFinite(start) || !Number.isFinite(end)) continue; + if (pos >= start && pos <= end) { + return line.getBoundingClientRect().y; + } + + const distance = pos < start ? start - pos : pos > end ? pos - end : 0; + const y = line.getBoundingClientRect().y; + if (!nearest || distance < nearest.distance) { + nearest = { y, distance }; + } + } + + return nearest?.y ?? null; + }); +} + +for (const docPath of DOCS) { + test(`rtl table Tab from visual top-left moves out of the first visual row (${path.basename(docPath)})`, async ({ + superdoc, + }) => { + await superdoc.loadDocument(docPath); + await superdoc.waitForStable(); + + const clickPoint = await superdoc.page.evaluate(() => { + const frag = document.querySelector('.superdoc-table-fragment') as HTMLElement | null; + if (!frag) return null; + const r = frag.getBoundingClientRect(); + return { x: r.x + 8, y: r.y + 8 }; + }); + + expect(clickPoint).not.toBeNull(); + if (!clickPoint) return; + + await superdoc.page.mouse.click(clickPoint.x, clickPoint.y); + await superdoc.waitForStable(); + + const before = await getSelectionLineY(superdoc.page); + expect(before).not.toBeNull(); + if (!before) return; + + await superdoc.press('Tab'); + await superdoc.waitForStable(); + + const after = await getSelectionLineY(superdoc.page); + expect(after).not.toBeNull(); + if (!after) return; + + // Word-like RTL-table behavior reported by customers: from visual top-left, + // first Tab leaves the current visual row (typically to next row's visual right cell). + // We assert this by requiring a visible downward move of the painted caret. + expect(after).toBeGreaterThan(before + 1); + }); + + test(`rtl table Shift+Tab from second visual row returns to first visual row (${path.basename(docPath)})`, async ({ + superdoc, + }) => { + await superdoc.loadDocument(docPath); + await superdoc.waitForStable(); + + const clickPoint = await superdoc.page.evaluate(() => { + const frag = document.querySelector('.superdoc-table-fragment') as HTMLElement | null; + if (!frag) return null; + const r = frag.getBoundingClientRect(); + return { x: r.x + 8, y: r.y + 8 }; + }); + + expect(clickPoint).not.toBeNull(); + if (!clickPoint) return; + + await superdoc.page.mouse.click(clickPoint.x, clickPoint.y); + await superdoc.waitForStable(); + + await superdoc.press('Tab'); + await superdoc.waitForStable(); + const afterTab = await getSelectionLineY(superdoc.page); + expect(afterTab).not.toBeNull(); + if (!afterTab) return; + + await superdoc.page.keyboard.press('Shift+Tab'); + await superdoc.waitForStable(); + const afterShiftTab = await getSelectionLineY(superdoc.page); + expect(afterShiftTab).not.toBeNull(); + if (!afterShiftTab) return; + + expect(afterShiftTab).toBeLessThan(afterTab - 1); + }); +}