From 6079ba2b93f86d4b93810ac8d8d03034f22b5560 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 07:04:33 -0300 Subject: [PATCH] refactor(direction): add getTableVisualDirection helper, migrate consumers (SD-3138) Centralize the read of table visual direction (w:bidiVisual, ECMA-376 section 17.4.1) through one helper, mirroring the paragraph-axis pattern already established for getParagraphInlineDirection. - Add getTableVisualDirection(attrs) to @superdoc/contracts. Reads attrs.tableDirectionContext.visualDirection first (future-ready for when pm-adapter writes the resolved context onto TableAttrs), falls back to attrs.tableProperties.rightToLeft (or bidiVisual alias) for compatibility. - Migrate three consumers off raw reads: - layout-engine/src/layout-table.ts (table-frame X positioning) - painters/dom/src/table/renderTableFragment.ts (visual mirror gate) - super-editor/.../tableBoundaryNavigation.js (RTL cursor nav) Out of scope for this PR: - pm-adapter wiring that populates TableAttrs.tableDirectionContext. Phase 1B - the helper is future-ready but the context isn't written yet, so the fallback path is exercised today. - packages/super-editor/.../TableResizeOverlay.vue reads a different shape (tableMetadata.value.rtl); migration deferred to keep this PR scoped. Companion to SD-3134 and PRs #3272/#3273/#3278; under SD-2771 Wave 3. Tests: - 14 unit tests for getTableVisualDirection (precedence, alias, no signal, null fallback). - 174 painter table tests, 652 layout-engine tests, 189 super-editor table extension tests all pass. --- .../contracts/src/direction-context.test.ts | 49 ++++++++++++++++++- .../contracts/src/direction-context.ts | 35 +++++++++++++ packages/layout-engine/contracts/src/index.ts | 2 +- .../layout-engine/src/layout-table.ts | 9 +++- .../dom/src/table/renderTableFragment.ts | 4 +- .../tableHelpers/tableBoundaryNavigation.js | 4 +- 6 files changed, 95 insertions(+), 8 deletions(-) diff --git a/packages/layout-engine/contracts/src/direction-context.test.ts b/packages/layout-engine/contracts/src/direction-context.test.ts index 2cc29d9c8c..8517d07e2e 100644 --- a/packages/layout-engine/contracts/src/direction-context.test.ts +++ b/packages/layout-engine/contracts/src/direction-context.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { getParagraphInlineDirection } from './direction-context.js'; +import { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js'; describe('getParagraphInlineDirection', () => { it('returns undefined for null/undefined attrs', () => { @@ -49,3 +49,50 @@ describe('getParagraphInlineDirection', () => { expect(getParagraphInlineDirection({ paragraphProperties: {} })).toBeUndefined(); }); }); + +describe('getTableVisualDirection', () => { + it('returns undefined for null/undefined attrs', () => { + expect(getTableVisualDirection(undefined)).toBeUndefined(); + expect(getTableVisualDirection(null)).toBeUndefined(); + }); + + it('prefers tableDirectionContext.visualDirection over legacy fields', () => { + const attrs = { + tableDirectionContext: { visualDirection: 'rtl' as const }, + tableProperties: { rightToLeft: false }, + }; + expect(getTableVisualDirection(attrs)).toBe('rtl'); + }); + + it('falls back past tableDirectionContext when visualDirection is null', () => { + const attrs = { + tableDirectionContext: { visualDirection: null }, + tableProperties: { rightToLeft: true }, + }; + expect(getTableVisualDirection(attrs)).toBe('rtl'); + }); + + it('falls back past tableDirectionContext when visualDirection is undefined', () => { + const attrs = { + tableDirectionContext: { visualDirection: undefined }, + tableProperties: { rightToLeft: true }, + }; + expect(getTableVisualDirection(attrs)).toBe('rtl'); + }); + + it('falls back to tableProperties.rightToLeft', () => { + expect(getTableVisualDirection({ tableProperties: { rightToLeft: true } })).toBe('rtl'); + expect(getTableVisualDirection({ tableProperties: { rightToLeft: false } })).toBe('ltr'); + }); + + it('accepts bidiVisual as an alias for rightToLeft', () => { + expect(getTableVisualDirection({ tableProperties: { bidiVisual: true } })).toBe('rtl'); + expect(getTableVisualDirection({ tableProperties: { bidiVisual: false } })).toBe('ltr'); + }); + + it('returns undefined when no signal is present', () => { + expect(getTableVisualDirection({})).toBeUndefined(); + expect(getTableVisualDirection({ tableDirectionContext: {} })).toBeUndefined(); + expect(getTableVisualDirection({ tableProperties: {} })).toBeUndefined(); + }); +}); diff --git a/packages/layout-engine/contracts/src/direction-context.ts b/packages/layout-engine/contracts/src/direction-context.ts index 0374f6d50c..922d7f8031 100644 --- a/packages/layout-engine/contracts/src/direction-context.ts +++ b/packages/layout-engine/contracts/src/direction-context.ts @@ -193,3 +193,38 @@ export function getParagraphInlineDirection( } return undefined; } + +/** + * Read a table's visual direction (cell ordering axis) from its attributes. + * + * Prefers the resolved {@link TableDirectionContext} when present, falls + * back to the legacy `tableProperties.rightToLeft` (or `bidiVisual` alias) + * for compatibility. The AIDEV-NOTE on the fallback branch names the + * retirement signal. + * + * Per ECMA-376 §17.4.1, `w:bidiVisual` affects only cell ordering and + * table-visual properties. Cell paragraph inline direction is independent; + * use {@link getParagraphInlineDirection} for that axis. + * + * Consumers should call this instead of reading `tableProperties.rightToLeft` + * directly so the source check stays in one place and the resolver can take + * over once pm-adapter populates `tableDirectionContext` everywhere. + */ +export function getTableVisualDirection( + attrs: + | { + tableDirectionContext?: { visualDirection?: BaseDirection | null } | null; + tableProperties?: { rightToLeft?: boolean | null; bidiVisual?: boolean | null } | null; + } + | null + | undefined, +): BaseDirection | undefined { + const fromContext = attrs?.tableDirectionContext?.visualDirection; + if (fromContext != null) return fromContext; + // AIDEV-NOTE: compat-fallback - used when TableAttrs.tableDirectionContext is absent. + // Retire once pm-adapter writes the resolved context onto every TableAttrs site. + const tp = attrs?.tableProperties; + if (tp?.rightToLeft === true || tp?.bidiVisual === true) return 'rtl'; + if (tp?.rightToLeft === false || tp?.bidiVisual === false) return 'ltr'; + return undefined; +} diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 1762530a11..b1ade37cf9 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -16,7 +16,7 @@ export type { RunBidiContext, RunScriptContext, } from './direction-context.js'; -export { getParagraphInlineDirection } from './direction-context.js'; +export { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js'; import type { ParagraphDirectionContext, RunBidiContext, RunScriptContext } from './direction-context.js'; // Export table contracts diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index ebcf1d8ce7..3f30d140cc 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -11,7 +11,12 @@ import type { ParagraphMeasure, ParagraphBlock, } from '@superdoc/contracts'; -import { OOXML_PCT_DIVISOR, rescaleColumnWidths, resolveTableWidthAttr } from '@superdoc/contracts'; +import { + OOXML_PCT_DIVISOR, + rescaleColumnWidths, + resolveTableWidthAttr, + getTableVisualDirection, +} from '@superdoc/contracts'; import type { PageState } from './paginator.js'; import { computeFragmentPmRange, extractBlockPmRange } from './layout-utils.js'; import { describeCellRenderBlocks, createCellSliceCursor, computeFullCellContentHeight } from './table-cell-slice.js'; @@ -182,7 +187,7 @@ export function resolveTableFrame( ): { x: number; width: number } { const width = resolveRenderedTableWidth(columnWidth, tableWidth, attrs); const explicitJustification = typeof attrs?.justification === 'string' ? attrs.justification : undefined; - const isRtlTable = attrs?.tableProperties?.rightToLeft === true; + const isRtlTable = getTableVisualDirection(attrs) === 'rtl'; const effectiveJustification = explicitJustification ?? (isRtlTable ? 'end' : undefined); const tableIndent = getTableIndentWidth(attrs); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index c1e304bfa4..d8e76cabb8 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -8,6 +8,7 @@ import type { TableFragment, TableMeasure, } from '@superdoc/contracts'; +import { getTableVisualDirection } from '@superdoc/contracts'; import { CLASS_NAMES, fragmentStyles } from '../styles.js'; import { DOM_CLASS_NAMES } from '../constants.js'; import type { FragmentRenderContext } from '../renderer.js'; @@ -173,8 +174,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // RTL table: w:bidiVisual (ECMA-376 §17.4.1) — cells displayed right-to-left, // table-level properties (borders, margins, indent) are mirrored. - const tableProperties = block.attrs?.tableProperties as Record | undefined; - const isRtl = tableProperties?.rightToLeft === true; + const isRtl = getTableVisualDirection(block.attrs) === 'rtl'; // Note: We don't use createTableBorderOverlay because we implement single-owner // border model where cells handle all borders (including outer table borders) // to prevent double borders when rendering with absolutely-positioned divs. diff --git a/packages/super-editor/src/editors/v1/extensions/table/tableHelpers/tableBoundaryNavigation.js b/packages/super-editor/src/editors/v1/extensions/table/tableHelpers/tableBoundaryNavigation.js index 8cb87c134a..9afc16779a 100644 --- a/packages/super-editor/src/editors/v1/extensions/table/tableHelpers/tableBoundaryNavigation.js +++ b/packages/super-editor/src/editors/v1/extensions/table/tableHelpers/tableBoundaryNavigation.js @@ -1,6 +1,7 @@ // @ts-check import { Plugin, PluginKey, Selection, TextSelection } from 'prosemirror-state'; import { CellSelection, TableMap } from 'prosemirror-tables'; +import { getTableVisualDirection } from '@superdoc/contracts'; const TABLE_CELL_ROLES = new Set(['cell', 'header_cell']); @@ -168,8 +169,7 @@ function getTableContext($head) { * @returns {boolean} */ function isRtlTable(table) { - const tableProperties = table?.attrs?.tableProperties; - return tableProperties?.rightToLeft === true; + return getTableVisualDirection(table?.attrs) === 'rtl'; } /**