From c52569cda6840e9df2bec47b76c23c04823eff00 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Thu, 14 May 2026 07:54:25 -0300 Subject: [PATCH] feat(direction): populate TableAttrs.tableDirectionContext in pm-adapter (SD-3138 Phase 1B) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1A (PR #3279) landed the getTableVisualDirection helper and migrated three consumers. The helper preferred a resolved TableDirectionContext but pm-adapter never wrote one - consumers fell through to the legacy fallback path on every read. Phase 1B closes the loop: - Add tableDirectionContext?: TableDirectionContext to TableAttrs in @superdoc/contracts (mirrors directionContext on ParagraphAttrs from Wave 1a / SD-2776). - In pm-adapter table.ts, resolve the effective table direction from cascade-resolved table properties (style cascade + inline override) and write the result via resolveTableDirection. The cascade resolution uses style-engine's resolveTableProperties for the style chain and `??` to layer inline on top. Because `??` treats null/undefined as missing but preserves explicit false, inline w:bidiVisual w:val="0" correctly overrides a style cascade true and produces visualDirection: 'ltr' (relies on SD-3141 resolver symmetry). After this lands, the helper's fast path is exercised at runtime: - Inline true → 'rtl' - Style cascade true → 'rtl' - Inline false overriding style true → 'ltr' - Absent signal → undefined Tests: - 4 new converter tests for the cascade scenarios above - pm-adapter: 1836 tests pass - painter table: 174 tests pass --- packages/layout-engine/contracts/src/index.ts | 19 ++- .../pm-adapter/src/converters/table.test.ts | 157 ++++++++++++++++++ .../pm-adapter/src/converters/table.ts | 30 ++++ 3 files changed, 205 insertions(+), 1 deletion(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index b1ade37cf9..f4af770e78 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -17,7 +17,12 @@ export type { RunScriptContext, } from './direction-context.js'; export { getParagraphInlineDirection, getTableVisualDirection } from './direction-context.js'; -import type { ParagraphDirectionContext, RunBidiContext, RunScriptContext } from './direction-context.js'; +import type { + ParagraphDirectionContext, + RunBidiContext, + RunScriptContext, + TableDirectionContext, +} from './direction-context.js'; // Export table contracts export { @@ -630,6 +635,18 @@ export type TableAttrs = { borderCollapse?: 'collapse' | 'separate'; cellSpacing?: CellSpacing; tableProperties?: TablePropertiesAttrs; + /** + * Resolved table direction context (SD-3138). Populated by pm-adapter from + * cascade-resolved table properties via `resolveTableDirection`. Consumers + * should call `getTableVisualDirection(attrs)` instead of reading + * `tableProperties.rightToLeft` directly — the helper prefers this field + * and falls back to the legacy raw read for compatibility. + * + * Per ECMA-376 §17.4.1, `w:bidiVisual` affects cell ordering and + * table-visual properties only; it does NOT propagate to cell paragraphs + * as inline direction. + */ + tableDirectionContext?: TableDirectionContext; sdt?: SdtMetadata; containerSdt?: SdtMetadata; [key: string]: unknown; diff --git a/packages/layout-engine/pm-adapter/src/converters/table.test.ts b/packages/layout-engine/pm-adapter/src/converters/table.test.ts index cabb09990a..86f7e24496 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -2526,4 +2526,161 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () => expect(cellBlocks[0].kind).toBe('paragraph'); expect((cellBlocks[0] as ParagraphBlock).runs[0].text).toBe('Inner DPO'); }); + + describe('tableDirectionContext (SD-3138 Phase 1B)', () => { + const mockBlockIdGenerator: BlockIdGenerator = vi.fn((kind) => `test-${kind}`); + const mockPositionMap: PositionMap = new Map(); + const mockParagraphConverter = vi.fn(() => [ + { kind: 'paragraph', id: 'p1', runs: [{ text: 'cell', fontFamily: 'Arial', fontSize: 12 }] } as ParagraphBlock, + ]); + + const buildTableNode = (tableProperties?: Record, tableStyleId?: string): PMNode => ({ + type: 'table', + attrs: { ...(tableStyleId ? { tableStyleId } : {}), ...(tableProperties ? { tableProperties } : {}) }, + content: [ + { + type: 'tableRow', + content: [{ type: 'tableCell', content: [{ type: 'paragraph', content: [{ type: 'text', text: 'cell' }] }] }], + }, + ], + }); + + const contextWithStyle = (styleId: string, styleTableProps: Record): ConverterContext => + ({ + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: {}, + latentStyles: {}, + styles: { + [styleId]: { + type: 'table', + tableProperties: styleTableProps, + }, + }, + }, + }) as ConverterContext; + + it('inline rightToLeft=true produces visualDirection=rtl', () => { + const result = tableNodeToBlock( + buildTableNode({ rightToLeft: true }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('rtl'); + }); + + it('style cascade rightToLeft=true produces visualDirection=rtl', () => { + const result = tableNodeToBlock( + buildTableNode(undefined, 'RtlStyle'), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + contextWithStyle('RtlStyle', { rightToLeft: true }), + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('rtl'); + }); + + it('inline rightToLeft=false overrides style cascade rightToLeft=true (visualDirection=ltr)', () => { + const result = tableNodeToBlock( + buildTableNode({ rightToLeft: false }, 'RtlStyle'), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + contextWithStyle('RtlStyle', { rightToLeft: true }), + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('ltr'); + }); + + it('inline bidiVisual=false overrides style cascade rightToLeft=true (alias-mixed override)', () => { + // Importer normalizes w:bidiVisual to `rightToLeft` so this shape is rare + // in practice, but the resolver must treat the two aliases as one signal + // per layer or an inline-false override against a style-true silently + // resolves to RTL. + const result = tableNodeToBlock( + buildTableNode({ bidiVisual: false }, 'RtlStyle'), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + contextWithStyle('RtlStyle', { rightToLeft: true }), + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext?.visualDirection).toBe('ltr'); + }); + + it('no signal anywhere leaves visualDirection undefined', () => { + const result = tableNodeToBlock( + buildTableNode(), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext).toBeDefined(); + expect(result?.attrs?.tableDirectionContext?.visualDirection).toBeUndefined(); + }); + + it('tableDirectionContext.parentSection propagates from converterContext.sectionDirectionContext', () => { + // The full TableDirectionContext shape is { visualDirection, parentSection }. + // Existing tests pin visualDirection; this one pins the section pass-through + // so a future regression that drops the sectionContext arg is caught here + // instead of by a runtime consumer reading parentSection. + const customSectionContext = { + pageDirection: 'rtl' as const, + writingMode: 'horizontal-tb' as const, + rtlGutter: true, + }; + const contextWithSection: ConverterContext = { + translatedNumbering: {}, + translatedLinkedStyles: { + docDefaults: {}, + latentStyles: {}, + styles: {}, + }, + sectionDirectionContext: customSectionContext, + }; + const result = tableNodeToBlock( + buildTableNode({ rightToLeft: true }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + contextWithSection, + ) as TableBlock; + expect(result?.attrs?.tableDirectionContext?.parentSection).toBe(customSectionContext); + }); + }); }); diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 0c2d566a53..ca0aab0f0e 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -55,10 +55,12 @@ import { import { TableProperties, resolveTableCellProperties, + resolveTableProperties, resolveExistingTableEffectiveStyleId, type TableInfo, } from '@superdoc/style-engine/ooxml'; import { resolveThemeColorValue } from '../marks/theme-color.js'; +import { resolveTableDirection, resolveSectionDirection } from '../direction/index.js'; /** * Normalizes tableCellSpacing from PM node to CellSpacing object format. @@ -1021,6 +1023,34 @@ export function tableNodeToBlock( tableAttrs.tableProperties = tableProperties as Record; } + // SD-3138 Phase 1B: resolve the table direction context from cascade-resolved + // table properties so downstream consumers (painter, layout-engine, editor + // navigation) read a typed `TableDirectionContext` instead of inspecting raw + // tableProperties.rightToLeft. Inline `w:bidiVisual` wins over the style + // cascade; explicit `false` overrides a cascade `true` per §17.4.1 + §17.17.4 + // (the resolver handles the explicit-false case via SD-3141). + const styleResolvedTableProps = + effectiveStyleId && converterContext?.translatedLinkedStyles + ? resolveTableProperties(effectiveStyleId, converterContext.translatedLinkedStyles) + : undefined; + // Normalize the rightToLeft / bidiVisual aliases to a single signal PER LAYER + // before layering inline over style. Otherwise an inline `bidiVisual: false` + // paired with a style `rightToLeft: true` would resolve RTL because the two + // aliases get layered independently (inline-false on bidiVisual loses to + // style-true on rightToLeft). The importer normalizes w:bidiVisual to + // `rightToLeft` so this matters most when style-engine emits raw OOXML keys. + const inlineProps = rawTableProperties as { rightToLeft?: boolean; bidiVisual?: boolean } | undefined; + const styleProps = styleResolvedTableProps as { rightToLeft?: boolean; bidiVisual?: boolean } | undefined; + const inlineVisual = inlineProps?.rightToLeft ?? inlineProps?.bidiVisual; + const styleVisual = styleProps?.rightToLeft ?? styleProps?.bidiVisual; + // `??` treats null/undefined as missing but preserves an explicit `false`, + // so an inline `` correctly overrides a style true. + const effectiveForDirection = { + rightToLeft: inlineVisual ?? styleVisual, + }; + const sectionContext = converterContext?.sectionDirectionContext ?? resolveSectionDirection(undefined); + tableAttrs.tableDirectionContext = resolveTableDirection(effectiveForDirection, sectionContext); + let columnWidths: number[] | undefined = undefined; const twipsToPixels = (twips: number): number => {