Skip to content

Commit 59de419

Browse files
authored
refactor(direction): add getTableVisualDirection helper, migrate consumers (SD-3138) (#3279)
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.
1 parent 8aa1509 commit 59de419

6 files changed

Lines changed: 95 additions & 8 deletions

File tree

packages/layout-engine/contracts/src/direction-context.test.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from 'vitest';
2-
import { getParagraphInlineDirection } from './direction-context.js';
2+
import { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js';
33

44
describe('getParagraphInlineDirection', () => {
55
it('returns undefined for null/undefined attrs', () => {
@@ -49,3 +49,50 @@ describe('getParagraphInlineDirection', () => {
4949
expect(getParagraphInlineDirection({ paragraphProperties: {} })).toBeUndefined();
5050
});
5151
});
52+
53+
describe('getTableVisualDirection', () => {
54+
it('returns undefined for null/undefined attrs', () => {
55+
expect(getTableVisualDirection(undefined)).toBeUndefined();
56+
expect(getTableVisualDirection(null)).toBeUndefined();
57+
});
58+
59+
it('prefers tableDirectionContext.visualDirection over legacy fields', () => {
60+
const attrs = {
61+
tableDirectionContext: { visualDirection: 'rtl' as const },
62+
tableProperties: { rightToLeft: false },
63+
};
64+
expect(getTableVisualDirection(attrs)).toBe('rtl');
65+
});
66+
67+
it('falls back past tableDirectionContext when visualDirection is null', () => {
68+
const attrs = {
69+
tableDirectionContext: { visualDirection: null },
70+
tableProperties: { rightToLeft: true },
71+
};
72+
expect(getTableVisualDirection(attrs)).toBe('rtl');
73+
});
74+
75+
it('falls back past tableDirectionContext when visualDirection is undefined', () => {
76+
const attrs = {
77+
tableDirectionContext: { visualDirection: undefined },
78+
tableProperties: { rightToLeft: true },
79+
};
80+
expect(getTableVisualDirection(attrs)).toBe('rtl');
81+
});
82+
83+
it('falls back to tableProperties.rightToLeft', () => {
84+
expect(getTableVisualDirection({ tableProperties: { rightToLeft: true } })).toBe('rtl');
85+
expect(getTableVisualDirection({ tableProperties: { rightToLeft: false } })).toBe('ltr');
86+
});
87+
88+
it('accepts bidiVisual as an alias for rightToLeft', () => {
89+
expect(getTableVisualDirection({ tableProperties: { bidiVisual: true } })).toBe('rtl');
90+
expect(getTableVisualDirection({ tableProperties: { bidiVisual: false } })).toBe('ltr');
91+
});
92+
93+
it('returns undefined when no signal is present', () => {
94+
expect(getTableVisualDirection({})).toBeUndefined();
95+
expect(getTableVisualDirection({ tableDirectionContext: {} })).toBeUndefined();
96+
expect(getTableVisualDirection({ tableProperties: {} })).toBeUndefined();
97+
});
98+
});

packages/layout-engine/contracts/src/direction-context.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,38 @@ export function getParagraphInlineDirection(
193193
}
194194
return undefined;
195195
}
196+
197+
/**
198+
* Read a table's visual direction (cell ordering axis) from its attributes.
199+
*
200+
* Prefers the resolved {@link TableDirectionContext} when present, falls
201+
* back to the legacy `tableProperties.rightToLeft` (or `bidiVisual` alias)
202+
* for compatibility. The AIDEV-NOTE on the fallback branch names the
203+
* retirement signal.
204+
*
205+
* Per ECMA-376 §17.4.1, `w:bidiVisual` affects only cell ordering and
206+
* table-visual properties. Cell paragraph inline direction is independent;
207+
* use {@link getParagraphInlineDirection} for that axis.
208+
*
209+
* Consumers should call this instead of reading `tableProperties.rightToLeft`
210+
* directly so the source check stays in one place and the resolver can take
211+
* over once pm-adapter populates `tableDirectionContext` everywhere.
212+
*/
213+
export function getTableVisualDirection(
214+
attrs:
215+
| {
216+
tableDirectionContext?: { visualDirection?: BaseDirection | null } | null;
217+
tableProperties?: { rightToLeft?: boolean | null; bidiVisual?: boolean | null } | null;
218+
}
219+
| null
220+
| undefined,
221+
): BaseDirection | undefined {
222+
const fromContext = attrs?.tableDirectionContext?.visualDirection;
223+
if (fromContext != null) return fromContext;
224+
// AIDEV-NOTE: compat-fallback - used when TableAttrs.tableDirectionContext is absent.
225+
// Retire once pm-adapter writes the resolved context onto every TableAttrs site.
226+
const tp = attrs?.tableProperties;
227+
if (tp?.rightToLeft === true || tp?.bidiVisual === true) return 'rtl';
228+
if (tp?.rightToLeft === false || tp?.bidiVisual === false) return 'ltr';
229+
return undefined;
230+
}

packages/layout-engine/contracts/src/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ export type {
1616
RunBidiContext,
1717
RunScriptContext,
1818
} from './direction-context.js';
19-
export { getParagraphInlineDirection } from './direction-context.js';
19+
export { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js';
2020
import type { ParagraphDirectionContext, RunBidiContext, RunScriptContext } from './direction-context.js';
2121

2222
// Export table contracts

packages/layout-engine/layout-engine/src/layout-table.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import type {
1111
ParagraphMeasure,
1212
ParagraphBlock,
1313
} from '@superdoc/contracts';
14-
import { OOXML_PCT_DIVISOR, rescaleColumnWidths, resolveTableWidthAttr } from '@superdoc/contracts';
14+
import {
15+
OOXML_PCT_DIVISOR,
16+
rescaleColumnWidths,
17+
resolveTableWidthAttr,
18+
getTableVisualDirection,
19+
} from '@superdoc/contracts';
1520
import type { PageState } from './paginator.js';
1621
import { computeFragmentPmRange, extractBlockPmRange } from './layout-utils.js';
1722
import { describeCellRenderBlocks, createCellSliceCursor, computeFullCellContentHeight } from './table-cell-slice.js';
@@ -182,7 +187,7 @@ export function resolveTableFrame(
182187
): { x: number; width: number } {
183188
const width = resolveRenderedTableWidth(columnWidth, tableWidth, attrs);
184189
const explicitJustification = typeof attrs?.justification === 'string' ? attrs.justification : undefined;
185-
const isRtlTable = attrs?.tableProperties?.rightToLeft === true;
190+
const isRtlTable = getTableVisualDirection(attrs) === 'rtl';
186191
const effectiveJustification = explicitJustification ?? (isRtlTable ? 'end' : undefined);
187192
const tableIndent = getTableIndentWidth(attrs);
188193

packages/layout-engine/painters/dom/src/table/renderTableFragment.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import type {
88
TableFragment,
99
TableMeasure,
1010
} from '@superdoc/contracts';
11+
import { getTableVisualDirection } from '@superdoc/contracts';
1112
import { CLASS_NAMES, fragmentStyles } from '../styles.js';
1213
import { DOM_CLASS_NAMES } from '../constants.js';
1314
import type { FragmentRenderContext } from '../renderer.js';
@@ -173,8 +174,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement
173174

174175
// RTL table: w:bidiVisual (ECMA-376 §17.4.1) — cells displayed right-to-left,
175176
// table-level properties (borders, margins, indent) are mirrored.
176-
const tableProperties = block.attrs?.tableProperties as Record<string, unknown> | undefined;
177-
const isRtl = tableProperties?.rightToLeft === true;
177+
const isRtl = getTableVisualDirection(block.attrs) === 'rtl';
178178
// Note: We don't use createTableBorderOverlay because we implement single-owner
179179
// border model where cells handle all borders (including outer table borders)
180180
// to prevent double borders when rendering with absolutely-positioned divs.

packages/super-editor/src/editors/v1/extensions/table/tableHelpers/tableBoundaryNavigation.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// @ts-check
22
import { Plugin, PluginKey, Selection, TextSelection } from 'prosemirror-state';
33
import { CellSelection, TableMap } from 'prosemirror-tables';
4+
import { getTableVisualDirection } from '@superdoc/contracts';
45

56
const TABLE_CELL_ROLES = new Set(['cell', 'header_cell']);
67

@@ -168,8 +169,7 @@ function getTableContext($head) {
168169
* @returns {boolean}
169170
*/
170171
function isRtlTable(table) {
171-
const tableProperties = table?.attrs?.tableProperties;
172-
return tableProperties?.rightToLeft === true;
172+
return getTableVisualDirection(table?.attrs) === 'rtl';
173173
}
174174

175175
/**

0 commit comments

Comments
 (0)