From 052719e72c59b8d78108d8026d5877d8407231ba Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Mon, 2 Mar 2026 20:11:16 -0800 Subject: [PATCH 1/2] fix(tables): defaultTableStyle support, cell fixes --- .../__mocks__/@converter/tbl-translator.d.ts | 9 - .../__mocks__/@converter/tbl-translator.js | 21 - .../pm-adapter/src/converter-context.ts | 5 + .../src/converters/table-styles.test.ts | 501 +++++++----------- .../pm-adapter/src/converters/table-styles.ts | 198 +++---- .../pm-adapter/src/converters/table.test.ts | 11 +- .../pm-adapter/src/converters/table.ts | 147 +++-- .../layout-engine/style-engine/src/cascade.ts | 4 +- .../style-engine/src/ooxml/index.test.ts | 185 +++++++ .../style-engine/src/ooxml/index.ts | 42 +- .../src/ooxml/table-style-selection.test.ts | 193 +++++++ .../src/ooxml/table-style-selection.ts | 143 +++++ .../src/core/commands/insertContent.test.js | 15 +- .../src/core/helpers/importHtml.js | 63 +-- .../helpers/markdown/mdastToProseMirror.ts | 6 +- .../presentation-editor/PresentationEditor.ts | 10 + .../v3/handlers/w/tbl/tbl-translator.js | 3 - .../helpers/legacy-handle-table-cell-node.js | 158 +----- .../legacy-handle-table-cell-node.test.js | 302 +---------- .../w/tc/helpers/translate-table-cell.js | 52 +- .../v3/handlers/w/tc/tc-translator.js | 14 - .../v3/handlers/w/tr/tr-translator.js | 103 +--- .../v3/handlers/w/tr/tr-translator.test.js | 48 -- .../document-settings.test.ts | 155 ++++++ .../document-settings.ts | 49 ++ .../document-api-adapters/tables-adapter.ts | 25 +- .../helpers/legacyBorderMigration.js | 54 ++ .../src/extensions/table-cell/table-cell.js | 8 +- .../extensions/table-header/table-header.js | 8 +- .../table/table.import-width.test.js | 4 +- .../src/extensions/table/table.js | 171 +++++- .../src/extensions/table/table.test.js | 48 +- .../table/tableHelpers/createTable.js | 18 +- .../tableHelpers/normalizeNewTableAttrs.js | 82 +++ .../table/tableHelpers/tableHelpers.test.js | 16 +- .../src/tests/import/tableImporter.test.js | 17 +- .../regression/contract-acc-import.test.js | 17 +- 37 files changed, 1614 insertions(+), 1291 deletions(-) delete mode 100644 packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.d.ts delete mode 100644 packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.js create mode 100644 packages/layout-engine/style-engine/src/ooxml/table-style-selection.test.ts create mode 100644 packages/layout-engine/style-engine/src/ooxml/table-style-selection.ts create mode 100644 packages/super-editor/src/document-api-adapters/document-settings.test.ts create mode 100644 packages/super-editor/src/extensions/table-cell/helpers/legacyBorderMigration.js create mode 100644 packages/super-editor/src/extensions/table/tableHelpers/normalizeNewTableAttrs.js diff --git a/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.d.ts b/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.d.ts deleted file mode 100644 index 762f6c5905..0000000000 --- a/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.d.ts +++ /dev/null @@ -1,9 +0,0 @@ -export function _getReferencedTableStyles(styleId: string, context: unknown): TableStyleProps; -/** - * Mock for @superdoc/super-editor/converter/internal/v3/handlers/w/tbl/tbl-translator.js _getReferencedTableStyles - */ -export type TableStyleProps = { - borders: unknown; - cellMargins: unknown; - justification: unknown; -}; diff --git a/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.js b/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.js deleted file mode 100644 index 6b4a0dcf4a..0000000000 --- a/packages/layout-engine/pm-adapter/__mocks__/@converter/tbl-translator.js +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Mock for @superdoc/converter/internal/v3/handlers/w/tbl/tbl-translator.js - * This module is part of super-editor and not available in pm-adapter tests - * - * @typedef {Object} TableStyleProps - * @property {unknown} borders - * @property {unknown} cellMargins - * @property {unknown} justification - */ - -/** - * Returns a tuple of [tableStyleProps, baseStyleProps] - * @param {string} _styleId - * @param {unknown} _context - * @returns {TableStyleProps} - */ -export const _getReferencedTableStyles = (_styleId, _context) => ({ - borders: null, - cellMargins: null, - justification: null, -}); diff --git a/packages/layout-engine/pm-adapter/src/converter-context.ts b/packages/layout-engine/pm-adapter/src/converter-context.ts index c68f9da87e..a245d54323 100644 --- a/packages/layout-engine/pm-adapter/src/converter-context.ts +++ b/packages/layout-engine/pm-adapter/src/converter-context.ts @@ -44,6 +44,11 @@ export type ConverterContext = { * contrast with the cell background per WCAG guidelines. */ backgroundColor?: string; + /** + * Default table style ID from `w:defaultTableStyle` in document settings. + * Used by table creation paths to determine which style to apply to new tables. + */ + defaultTableStyleId?: string; }; /** diff --git a/packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts b/packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts index 2659c61fb0..e12ae833ce 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts @@ -1,17 +1,18 @@ -import { describe, expect, it, beforeEach, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { hydrateTableStyleAttrs } from './table-styles.js'; import type { PMNode } from '../types.js'; -import * as tblTranslator from '@superdoc/super-editor/converter/internal/v3/handlers/w/tbl/tbl-translator.js'; +import type { ConverterContext } from '../converter-context.js'; +import type { StylesDocumentProperties } from '@superdoc/style-engine/ooxml'; -// Mock the external super-converter module that's imported by table-styles.ts -// This module is part of super-editor package and not available in pm-adapter tests -vi.mock('@superdoc/super-editor/converter/internal/v3/handlers/w/tbl/tbl-translator.js'); +const emptyStyles: StylesDocumentProperties = { docDefaults: {}, latentStyles: {}, styles: {} }; -describe('hydrateTableStyleAttrs', () => { - beforeEach(() => { - vi.clearAllMocks(); - }); +const buildContext = (styles?: StylesDocumentProperties): ConverterContext => + ({ + translatedLinkedStyles: styles ?? emptyStyles, + translatedNumbering: { abstracts: {}, definitions: {} }, + }) as ConverterContext; +describe('hydrateTableStyleAttrs', () => { it('hydrates from tableProperties even without converter context', () => { const table = { attrs: { @@ -31,13 +32,21 @@ describe('hydrateTableStyleAttrs', () => { expect(result?.tableWidth).toEqual({ width: 96, type: 'px' }); }); - it('merges referenced styles when context available', () => { - vi.mocked(tblTranslator._getReferencedTableStyles).mockReturnValue({ - borders: { top: { val: 'single', size: 8 } }, - cellMargins: { left: { value: 72, type: 'dxa' } }, - justification: 'center', - tableCellSpacing: { value: 24, type: 'dxa' }, - }); + it('merges style-resolved properties when context available', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + borders: { top: { val: 'single', size: 8 } } as Record, + cellMargins: { marginLeft: { value: 72, type: 'dxa' } }, + justification: 'center', + tableCellSpacing: { value: 24, type: 'dxa' }, + }, + }, + }, + }; const table = { attrs: { @@ -48,7 +57,7 @@ describe('hydrateTableStyleAttrs', () => { }, } as unknown as PMNode; - const result = hydrateTableStyleAttrs(table, { docx: {} }); + const result = hydrateTableStyleAttrs(table, buildContext(styles)); expect(result?.borders).toEqual({ top: { val: 'single', size: 8 } }); expect(result?.justification).toBe('center'); expect(result?.cellPadding?.left).toBeCloseTo((72 / 1440) * 96); @@ -56,316 +65,218 @@ describe('hydrateTableStyleAttrs', () => { expect(result?.tableWidth).toEqual({ width: 500, type: 'px' }); }); - describe('extractTableStyleParagraphProps (via hydrateTableStyleAttrs)', () => { - it('extracts spacing with all attributes (before, after, line, lineRule)', () => { - const mockDocx = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableNormal' }, - elements: [ - { - name: 'w:pPr', - elements: [ - { - name: 'w:spacing', - attributes: { - 'w:before': '120', - 'w:after': '240', - 'w:line': '276', - 'w:lineRule': 'auto', - }, - }, - ], - }, - ], - }, - ], - }, - ], - }, - }; - - const table = { - attrs: { - tableStyleId: 'TableNormal', - }, - } as unknown as PMNode; - - const result = hydrateTableStyleAttrs(table, { docx: mockDocx }); - expect(result?.paragraphProps).toBeDefined(); - expect(result?.paragraphProps?.spacing?.before).toBeCloseTo((120 / 1440) * 96); - expect(result?.paragraphProps?.spacing?.after).toBeCloseTo((240 / 1440) * 96); - // For 'auto' lineRule, line is in 240ths: 276/240 = 1.15 - expect(result?.paragraphProps?.spacing?.line).toBeCloseTo(1.3225); - expect(result?.paragraphProps?.spacing?.lineRule).toBe('auto'); - }); - - it('extracts spacing with partial attributes (only before/after)', () => { - const mockDocx = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableGrid' }, - elements: [ - { - name: 'w:pPr', - elements: [ - { - name: 'w:spacing', - attributes: { - 'w:before': '100', - 'w:after': '100', - }, - }, - ], - }, - ], - }, - ], - }, - ], + it('inline properties take precedence over style-resolved properties', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + borders: { top: { val: 'single', size: 4 } } as Record, + justification: 'center', + }, }, - }; + }, + }; - const table = { - attrs: { - tableStyleId: 'TableGrid', + const table = { + attrs: { + tableStyleId: 'TableGrid', + tableProperties: { + borders: { top: { val: 'single', size: 12 } }, + justification: 'left', }, - } as unknown as PMNode; + }, + } as unknown as PMNode; - const result = hydrateTableStyleAttrs(table, { docx: mockDocx }); - expect(result?.paragraphProps).toBeDefined(); - expect(result?.paragraphProps?.spacing?.before).toBeCloseTo((100 / 1440) * 96); - expect(result?.paragraphProps?.spacing?.after).toBeCloseTo((100 / 1440) * 96); - expect(result?.paragraphProps?.spacing?.line).toBeUndefined(); - expect(result?.paragraphProps?.spacing?.lineRule).toBeUndefined(); - }); + const result = hydrateTableStyleAttrs(table, buildContext(styles)); + // Inline borders win over style + expect(result?.borders).toEqual({ top: { val: 'single', size: 12 } }); + // Inline justification wins over style + expect(result?.justification).toBe('left'); + }); - it('handles different lineRule values (exact and atLeast use twipsToPx)', () => { - const mockDocxExact = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableExact' }, - elements: [ - { - name: 'w:pPr', - elements: [ - { - name: 'w:spacing', - attributes: { - 'w:line': '240', - 'w:lineRule': 'exact', - }, - }, - ], - }, - ], - }, - ], - }, - ], + it('per-side merge: partial inline borders preserve style borders on other sides', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + borders: { + top: { val: 'single', size: 4 }, + bottom: { val: 'single', size: 4 }, + left: { val: 'single', size: 4 }, + right: { val: 'single', size: 4 }, + } as Record, + }, }, - }; + }, + }; - const tableExact = { - attrs: { - tableStyleId: 'TableExact', + const table = { + attrs: { + tableStyleId: 'TableGrid', + tableProperties: { + borders: { top: { val: 'double', size: 8 } }, }, - } as unknown as PMNode; + }, + } as unknown as PMNode; - const resultExact = hydrateTableStyleAttrs(tableExact, { docx: mockDocxExact }); - // For 'exact' lineRule, use twipsToPx: (240/1440)*96 = 16 - expect(resultExact?.paragraphProps?.spacing?.line).toBeCloseTo(16); - expect(resultExact?.paragraphProps?.spacing?.lineUnit).toBe('px'); - expect(resultExact?.paragraphProps?.spacing?.lineRule).toBe('exact'); + const result = hydrateTableStyleAttrs(table, buildContext(styles)); + // Inline top wins + expect(result?.borders?.top).toEqual({ val: 'double', size: 8 }); + // Style fills other sides + expect(result?.borders?.bottom).toEqual({ val: 'single', size: 4 }); + expect(result?.borders?.left).toEqual({ val: 'single', size: 4 }); + expect(result?.borders?.right).toEqual({ val: 'single', size: 4 }); + }); - const mockDocxAtLeast = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableAtLeast' }, - elements: [ - { - name: 'w:pPr', - elements: [ - { - name: 'w:spacing', - attributes: { - 'w:line': '360', - 'w:lineRule': 'atLeast', - }, - }, - ], - }, - ], - }, - ], + it('per-side merge: partial inline cellPadding preserves style padding on other sides', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + cellMargins: { + marginTop: { value: 72, type: 'dxa' }, + marginBottom: { value: 72, type: 'dxa' }, + marginLeft: { value: 108, type: 'dxa' }, + marginRight: { value: 108, type: 'dxa' }, }, - ], - }, - }; - - const tableAtLeast = { - attrs: { - tableStyleId: 'TableAtLeast', + }, }, - } as unknown as PMNode; - - const resultAtLeast = hydrateTableStyleAttrs(tableAtLeast, { docx: mockDocxAtLeast }); - // For 'atLeast' lineRule, use twipsToPx: (360/1440)*96 = 24 - expect(resultAtLeast?.paragraphProps?.spacing?.line).toBeCloseTo(24); - expect(resultAtLeast?.paragraphProps?.spacing?.lineRule).toBe('atLeast'); - }); + }, + }; - it('returns undefined when style not found', () => { - const mockDocx = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'OtherStyle' }, - elements: [], - }, - ], - }, - ], + const table = { + attrs: { + tableStyleId: 'TableGrid', + tableProperties: { + cellMargins: { + marginLeft: { value: 50, type: 'px' }, + }, }, - }; + }, + } as unknown as PMNode; - const table = { - attrs: { - tableStyleId: 'NonExistentStyle', - }, - } as unknown as PMNode; + const result = hydrateTableStyleAttrs(table, buildContext(styles)); + // Inline left wins + expect(result?.cellPadding?.left).toBe(50); + // Style fills other sides + expect(result?.cellPadding?.top).toBeCloseTo((72 / 1440) * 96); + expect(result?.cellPadding?.bottom).toBeCloseTo((72 / 1440) * 96); + expect(result?.cellPadding?.right).toBeCloseTo((108 / 1440) * 96); + }); - const result = hydrateTableStyleAttrs(table, { docx: mockDocx }); - expect(result?.paragraphProps).toBeUndefined(); - }); + it('returns null when no properties found', () => { + const table = { attrs: {} } as unknown as PMNode; + const result = hydrateTableStyleAttrs(table, undefined); + expect(result).toBeNull(); + }); - it('returns undefined when style has no w:pPr', () => { - const mockDocx = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableNoPPr' }, - elements: [ - { - name: 'w:tblPr', - elements: [], - }, - ], - }, - ], - }, - ], + it('resolves style via effectiveStyleId parameter', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + MyCustomStyle: { + type: 'table', + tableProperties: { + justification: 'right', + }, }, - }; + }, + }; - const table = { - attrs: { - tableStyleId: 'TableNoPPr', - }, - } as unknown as PMNode; + const table = { + attrs: { tableStyleId: 'NonexistentStyle' }, + } as unknown as PMNode; - const result = hydrateTableStyleAttrs(table, { docx: mockDocx }); - expect(result?.paragraphProps).toBeUndefined(); - }); + // Pass effectiveStyleId directly — overrides the node's tableStyleId + const result = hydrateTableStyleAttrs(table, buildContext(styles), 'MyCustomStyle'); + expect(result?.justification).toBe('right'); + }); - it('returns undefined when w:pPr has no w:spacing', () => { - const mockDocx = { - 'word/styles.xml': { - elements: [ - { - elements: [ - { - name: 'w:style', - attributes: { 'w:styleId': 'TableNoSpacing' }, - elements: [ - { - name: 'w:pPr', - elements: [ - { - name: 'w:ind', - attributes: { 'w:left': '120' }, - }, - ], - }, - ], - }, - ], - }, - ], + it('follows basedOn chain for table properties', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableNormal: { + type: 'table', + tableProperties: { + cellMargins: { marginLeft: { value: 108, type: 'dxa' } }, + justification: 'left', + }, }, - }; - - const table = { - attrs: { - tableStyleId: 'TableNoSpacing', + TableGrid: { + type: 'table', + basedOn: 'TableNormal', + tableProperties: { + borders: { top: { val: 'single', size: 4 } } as Record, + }, }, - } as unknown as PMNode; + }, + }; - const result = hydrateTableStyleAttrs(table, { docx: mockDocx }); - expect(result?.paragraphProps).toBeUndefined(); - }); + const table = { + attrs: { tableStyleId: 'TableGrid' }, + } as unknown as PMNode; - it('gracefully handles malformed/missing XML structure', () => { - const mockDocxMalformed1 = { - 'word/styles.xml': { - elements: [], - }, - }; + const result = hydrateTableStyleAttrs(table, buildContext(styles)); + // From TableGrid + expect(result?.borders).toEqual({ top: { val: 'single', size: 4 } }); + // Inherited from TableNormal via basedOn + expect(result?.cellPadding?.left).toBeCloseTo((108 / 1440) * 96); + expect(result?.justification).toBe('left'); + }); - const table1 = { - attrs: { - tableStyleId: 'TableGrid', + it('does not fall back to raw tableStyleId when effectiveStyleId is null', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + SomeStyle: { + type: 'table', + tableProperties: { + justification: 'center', + }, }, - } as unknown as PMNode; + }, + }; - const result1 = hydrateTableStyleAttrs(table1, { docx: mockDocxMalformed1 }); - expect(result1?.paragraphProps).toBeUndefined(); + const table = { + attrs: { tableStyleId: 'SomeStyle' }, + } as unknown as PMNode; - const mockDocxMalformed2 = { - 'word/styles.xml': undefined, - }; + // effectiveStyleId = null means "resolver found no valid style" + const result = hydrateTableStyleAttrs(table, buildContext(styles), null); + // Should NOT resolve SomeStyle even though it's on the raw node + expect(result).toBeNull(); + }); - const table2 = { - attrs: { - tableStyleId: 'TableGrid', + it('handles marginStart/marginEnd for RTL table direction support', () => { + const styles: StylesDocumentProperties = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + cellMargins: { + marginStart: { value: 100, type: 'dxa' }, + marginEnd: { value: 200, type: 'dxa' }, + }, + }, }, - } as unknown as PMNode; - - const result2 = hydrateTableStyleAttrs(table2, { docx: mockDocxMalformed2 }); - expect(result2?.paragraphProps).toBeUndefined(); - - const mockDocxMalformed3 = {}; + }, + }; - const table3 = { - attrs: { - tableStyleId: 'TableGrid', - }, - } as unknown as PMNode; + const table = { + attrs: { tableStyleId: 'TableGrid' }, + } as unknown as PMNode; - const result3 = hydrateTableStyleAttrs(table3, { docx: mockDocxMalformed3 }); - expect(result3?.paragraphProps).toBeUndefined(); - }); + const result = hydrateTableStyleAttrs(table, buildContext(styles)); + // marginStart maps to left, marginEnd maps to right + expect(result?.cellPadding?.left).toBeCloseTo((100 / 1440) * 96); + expect(result?.cellPadding?.right).toBeCloseTo((200 / 1440) * 96); }); }); diff --git a/packages/layout-engine/pm-adapter/src/converters/table-styles.ts b/packages/layout-engine/pm-adapter/src/converters/table-styles.ts index 161bf4395e..1886837381 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table-styles.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table-styles.ts @@ -1,10 +1,9 @@ import type { BoxSpacing } from '@superdoc/contracts'; -import { _getReferencedTableStyles } from '@superdoc/super-editor/converter/internal/v3/handlers/w/tbl/tbl-translator.js'; +import { resolveTableProperties } from '@superdoc/style-engine/ooxml'; +import type { TableProperties, TableCellMargins } from '@superdoc/style-engine/ooxml'; import type { PMNode } from '../types.js'; -import type { ConverterContext, TableStyleParagraphProps } from '../converter-context.js'; -import { hasTableStyleContext } from '../converter-context.js'; +import type { ConverterContext } from '../converter-context.js'; import { twipsToPx, normalizeCellPaddingTopBottom } from '../utilities.js'; -import { normalizeLineValue } from '../attributes/spacing-indent.js'; export type TableStyleHydration = { borders?: Record; @@ -12,33 +11,39 @@ export type TableStyleHydration = { justification?: string; tableWidth?: { width?: number; type?: string }; tableCellSpacing?: { value?: number; type?: string }; - /** - * Paragraph properties from the table style's w:pPr element. - * Per OOXML spec, these should apply to all paragraphs inside the table - * as part of the style cascade: docDefaults → table style pPr → paragraph style → direct formatting. - */ - paragraphProps?: TableStyleParagraphProps; }; /** - * Hydrates table-level attributes from a table style definition. + * Hydrates table-level attributes from inline properties and the style-engine. + * + * Cascade: style-resolved properties fill gaps that inline properties don't cover. + * Inline properties (from PM node attrs) always win. * * The hydrator never mutates the PM node and only returns new objects, * so callers must merge the result with the node's attrs explicitly. */ -export const hydrateTableStyleAttrs = (tableNode: PMNode, context?: ConverterContext): TableStyleHydration | null => { +export const hydrateTableStyleAttrs = ( + tableNode: PMNode, + context?: ConverterContext, + effectiveStyleId?: string | null, +): TableStyleHydration | null => { const hydration: TableStyleHydration = {}; const tableProps = (tableNode.attrs?.tableProperties ?? null) as Record | null; + // Collect inline values first, then merge with style-resolved values below. + let inlineBorders: Record | undefined; + let inlinePadding: BoxSpacing | undefined; + + // 1. Inline properties (highest priority) if (tableProps) { const padding = convertCellMarginsToPx(tableProps.cellMargins as Record); - if (padding) hydration.cellPadding = normalizeCellPaddingTopBottom(padding); + if (padding) inlinePadding = normalizeCellPaddingTopBottom(padding); if (tableProps.borders && typeof tableProps.borders === 'object') { - hydration.borders = clonePlainObject(tableProps.borders as Record); + inlineBorders = clonePlainObject(tableProps.borders as Record); } - if (!hydration.justification && typeof tableProps.justification === 'string') { + if (typeof tableProps.justification === 'string') { hydration.justification = tableProps.justification; } @@ -48,39 +53,58 @@ export const hydrateTableStyleAttrs = (tableNode: PMNode, context?: ConverterCon } } - const styleId = typeof tableNode.attrs?.tableStyleId === 'string' ? tableNode.attrs.tableStyleId : undefined; - if (styleId && hasTableStyleContext(context)) { - // Cast to bypass JSDoc type mismatch - the JS function actually accepts { docx } - const referenced = _getReferencedTableStyles(styleId, { docx: context!.docx } as never); - if (referenced) { - if (!hydration.borders && referenced.borders) { - hydration.borders = clonePlainObject(referenced.borders); - } - if (!hydration.cellPadding && referenced.cellMargins) { - const padding = convertCellMarginsToPx(referenced.cellMargins as Record); - if (padding) hydration.cellPadding = normalizeCellPaddingTopBottom(padding); - } - if (!hydration.justification && referenced.justification) { - hydration.justification = referenced.justification; - } - if (referenced.tableCellSpacing) { - hydration.tableCellSpacing = referenced.tableCellSpacing as { value?: number; type?: string }; - } + // 2. Style-resolved properties (fill gaps not covered by inline, per-side) + // Three-state contract for effectiveStyleId: + // undefined = "not provided" → fall back to raw node attr + // null = "resolver found no valid style" → skip style resolution + // string = "use this style" + const styleId = + effectiveStyleId === null + ? undefined + : (effectiveStyleId ?? + (typeof tableNode.attrs?.tableStyleId === 'string' ? tableNode.attrs.tableStyleId : undefined)); + if (styleId && context?.translatedLinkedStyles) { + const resolved = resolveTableProperties(styleId, context.translatedLinkedStyles); + + // Per-side merge: inline sides win, style fills missing sides. + if (resolved.borders) { + const styleBorders = clonePlainObject(resolved.borders as unknown as Record); + hydration.borders = inlineBorders ? { ...styleBorders, ...inlineBorders } : styleBorders; + } else if (inlineBorders) { + hydration.borders = inlineBorders; } - // Extract paragraph properties (w:pPr) from the table style definition - // This is needed for the style cascade: docDefaults → table style pPr → paragraph style → direct - const paragraphProps = extractTableStyleParagraphProps(styleId, context.docx); - if (paragraphProps) { - hydration.paragraphProps = paragraphProps; + if (resolved.cellMargins) { + const stylePadding = convertCellMarginsToPx(resolved.cellMargins as unknown as Record); + if (stylePadding) { + const normalizedStylePadding = normalizeCellPaddingTopBottom(stylePadding); + hydration.cellPadding = inlinePadding + ? { ...normalizedStylePadding, ...inlinePadding } + : normalizedStylePadding; + } else if (inlinePadding) { + hydration.cellPadding = inlinePadding; + } + } else if (inlinePadding) { + hydration.cellPadding = inlinePadding; } - } - if (Object.keys(hydration).length > 0) { - return hydration; + if (!hydration.justification && resolved.justification) { + hydration.justification = resolved.justification; + } + if (resolved.tableCellSpacing) { + hydration.tableCellSpacing = resolved.tableCellSpacing as { value?: number; type?: string }; + } + if (!hydration.tableWidth && resolved.tableWidth) { + const tableWidth = normalizeTableWidth(resolved.tableWidth); + if (tableWidth) hydration.tableWidth = tableWidth; + } + } else { + // No style resolved — use inline values as-is. + if (inlineBorders) hydration.borders = inlineBorders; + if (inlinePadding) hydration.cellPadding = inlinePadding; } - return null; + return Object.keys(hydration).length > 0 ? hydration : null; }; const clonePlainObject = (value: Record): Record => ({ ...value }); @@ -97,6 +121,8 @@ const convertCellMarginsToPx = (margins: Record): BoxSpacing | marginBottom: 'bottom', marginLeft: 'left', marginRight: 'right', + marginStart: 'left', + marginEnd: 'right', }; Object.entries(margins).forEach(([key, value]) => { @@ -132,91 +158,3 @@ const normalizeTableWidth = (value: unknown): { width?: number; type?: string } } return { width: raw, type: measurement.type }; }; - -/** - * XML element type for OOXML parsing. - */ -type OoxmlElement = { - name?: string; - attributes?: Record; - elements?: OoxmlElement[]; -}; - -/** - * Extracts paragraph properties (w:pPr) from a table style definition. - * - * Per OOXML spec, table styles can define paragraph properties that apply - * to all paragraphs within the table. This includes spacing, indentation, etc. - * - * @param styleId - The table style ID (e.g., "TableGrid") - * @param docx - The docx object containing styles.xml - * @returns Paragraph properties from the table style, or undefined if not found - */ -const extractTableStyleParagraphProps = ( - styleId: string, - docx: Record, -): TableStyleParagraphProps | undefined => { - try { - // Navigate to styles.xml - const stylesXml = docx['word/styles.xml'] as OoxmlElement | undefined; - if (!stylesXml?.elements?.[0]?.elements) return undefined; - - const styleElements = stylesXml.elements[0].elements.filter((el: OoxmlElement) => el.name === 'w:style'); - - // Find the table style by styleId - const styleTag = styleElements.find((el: OoxmlElement) => el.attributes?.['w:styleId'] === styleId); - if (!styleTag?.elements) { - return undefined; - } - - // Find w:pPr (paragraph properties) in the style - const pPr = styleTag.elements.find((el: OoxmlElement) => el.name === 'w:pPr'); - if (!pPr?.elements) { - return undefined; - } - - // Extract w:spacing - const spacingEl = pPr.elements.find((el: OoxmlElement) => el.name === 'w:spacing'); - if (!spacingEl?.attributes) { - return undefined; - } - - // Cast attributes to Record for runtime validation - const attrs = spacingEl.attributes as Record; - const spacing: TableStyleParagraphProps['spacing'] = {}; - - // Convert spacing values from twips to pixels using parseIntSafe for type coercion - const before = parseIntSafe(attrs['w:before']); - const after = parseIntSafe(attrs['w:after']); - const line = parseIntSafe(attrs['w:line']); - - // Validate lineRule is one of the expected values - const rawLineRule = attrs['w:lineRule']; - const lineRule: 'auto' | 'exact' | 'atLeast' | undefined = - rawLineRule === 'auto' || rawLineRule === 'exact' || rawLineRule === 'atLeast' ? rawLineRule : undefined; - - if (before != null) spacing.before = twipsToPx(before); - if (after != null) spacing.after = twipsToPx(after); - if (line != null) { - const { value: normalizedLine, unit: lineUnit } = normalizeLineValue(line, lineRule); - spacing.line = normalizedLine; - spacing.lineUnit = lineUnit; - } - if (lineRule) spacing.lineRule = lineRule; - - const result = Object.keys(spacing).length > 0 ? { spacing } : undefined; - return result; - } catch { - // Gracefully handle any parsing errors - return undefined; - } -}; - -/** - * Safely parse an integer from an unknown value. - */ -const parseIntSafe = (value: unknown): number | undefined => { - if (value == null) return undefined; - const num = typeof value === 'number' ? value : parseInt(String(value), 10); - return Number.isFinite(num) ? num : undefined; -}; 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 ac7ca93fb9..6b8739d3c7 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -825,7 +825,9 @@ describe('table converter', () => { expect(result.rows[0].cells[0].attrs?.borders).toBeUndefined(); }); - it('keeps schema-default cell borders when no table-level borders exist', () => { + it('ignores legacy schema-default cell borders (style-engine resolves borders)', () => { + // Old schema defaults have { size, color } without `val` — these are no longer + // read from attrs.borders. Cell borders now come from style-engine resolution. const schemaDefaultBorders = { top: { size: 8, color: '000000' }, right: { size: 8, color: '000000' }, @@ -862,10 +864,9 @@ describe('table converter', () => { mockParagraphConverter, ) as TableBlock; - expect(result.rows[0].cells[0].attrs?.borders).toBeDefined(); - expect(result.rows[0].cells[0].attrs?.borders?.top).toEqual( - expect.objectContaining({ style: 'single', color: '#000000' }), - ); + // attrs.borders are ignored — style-engine-resolved borders (from resolveTableCellProperties) + // would provide borders, but this test has no style catalog so borders are undefined. + expect(result.rows[0].cells[0].attrs?.borders).toBeUndefined(); }); it('extracts cell padding when present', () => { diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 4bd173611d..9cd62da5a7 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -5,7 +5,9 @@ */ import type { + BorderStyle, BoxSpacing, + CellBorders, CellSpacing, FlowBlock, ParagraphBlock, @@ -32,7 +34,7 @@ import type { NestedConverters, TableNodeToBlockParams, } from '../types.js'; -import { extractTableBorders, extractCellBorders, extractCellPadding } from '../attributes/index.js'; +import { extractTableBorders, extractCellPadding, convertBorderSpec } from '../attributes/index.js'; import { pickNumber, twipsToPx } from '../utilities.js'; import { hydrateTableStyleAttrs } from './table-styles.js'; import { collectTrackedChangeFromMarks } from '../marks/index.js'; @@ -42,7 +44,11 @@ import { applySdtMetadataToParagraphBlocks, applySdtMetadataToTableBlock, } from '../sdt/index.js'; -import { TableProperties, resolveTableCellProperties } from '@superdoc/style-engine/ooxml'; +import { + TableProperties, + resolveTableCellProperties, + resolveExistingTableEffectiveStyleId, +} from '@superdoc/style-engine/ooxml'; /** * Normalizes tableCellSpacing from PM node to CellSpacing object format. @@ -62,6 +68,36 @@ function normalizeCellSpacing(raw: number | { value?: number; type?: string } | return { value, type }; } +function normalizeLegacyBorderStyle(value: string | undefined): BorderStyle { + switch ((value ?? '').trim().toLowerCase()) { + case 'none': + case 'nil': + return 'none'; + case 'double': + return 'double'; + case 'dashed': + return 'dashed'; + case 'dotted': + case 'dot': + return 'dotted'; + case 'thick': + return 'thick'; + case 'triple': + return 'triple'; + case 'dotdash': + return 'dotDash'; + case 'dotdotdash': + return 'dotDotDash'; + case 'wave': + return 'wave'; + case 'doublewave': + return 'doubleWave'; + case 'single': + default: + return 'single'; + } +} + type TableParserDependencies = { nextBlockId: BlockIdGenerator; positions: PositionMap; @@ -83,8 +119,6 @@ type ParseTableCellArgs = { context: TableParserDependencies; defaultCellPadding?: BoxSpacing; tableProperties?: TableProperties; - /** When true, table-level borders exist (from hydrated style or inline) so schema-default cell borders can be skipped. */ - hasTableLevelBorders?: boolean; }; type ParseTableRowArgs = { @@ -95,8 +129,6 @@ type ParseTableRowArgs = { defaultCellPadding?: BoxSpacing; /** Table style to pass to paragraph converter for style cascade */ tableProperties?: TableProperties; - /** When true, table-level borders exist so schema-default cell borders can be skipped. */ - hasTableLevelBorders?: boolean; }; const isTableRowNode = (node: PMNode): boolean => node.type === 'tableRow' || node.type === 'table_row'; @@ -199,17 +231,7 @@ const normalizeRowHeight = (rowProps?: Record): NormalizedRowHe * // Returns: null */ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { - const { - cellNode, - rowIndex, - cellIndex, - numCells, - numRows, - context, - defaultCellPadding, - tableProperties, - hasTableLevelBorders, - } = args; + const { cellNode, rowIndex, cellIndex, numCells, numRows, context, defaultCellPadding, tableProperties } = args; if (!isTableCellNode(cellNode) || !Array.isArray(cellNode.content)) { return null; } @@ -444,24 +466,40 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { const cellAttrs: TableCellAttrs = {}; - // Schema-default cell borders (from createCellBorders) have { size, color } without a `val` - // property, while OOXML-imported borders always include `val` (e.g., 'single', 'none'). - // When table-level borders exist (from a hydrated style like TableGrid), skip schema defaults - // so the painter's resolveTableCellBorders can distribute table borders to cells properly. - // When NO table-level borders exist, keep schema defaults as the fallback (matching Word's - // default behavior of showing single black borders on new tables). - const rawBorders = cellNode.attrs?.borders as Record> | undefined; - const isSchemaDefaultBorders = - rawBorders && - typeof rawBorders === 'object' && - ['top', 'right', 'bottom', 'left'].every((side) => { - const b = rawBorders[side]; - return b && typeof b === 'object' && !('val' in b); - }); + // Cell borders come from the style-engine cascade (resolvedTcProps.borders). + // Inline tableCellProperties.borders are already folded into resolvedTcProps + // by resolveTableCellProperties (inline wins over style cascade). + if (resolvedTcProps?.borders && typeof resolvedTcProps.borders === 'object') { + const resolvedBorders: CellBorders = {}; + for (const side of ['top', 'right', 'bottom', 'left'] as const) { + const spec = convertBorderSpec((resolvedTcProps.borders as Record)[side]); + if (spec) resolvedBorders[side] = spec; + } + if (Object.keys(resolvedBorders).length > 0) { + cellAttrs.borders = resolvedBorders; + } + } - if (!(isSchemaDefaultBorders && hasTableLevelBorders)) { - const borders = extractCellBorders(cellNode.attrs ?? {}); - if (borders) cellAttrs.borders = borders; + // Fallback: older persisted docs may store cell borders in attrs.borders + // (pre-migration pixel format: { size: px, color: hex, val: string }). + // The transaction-based migration only runs when an edit touches the table + // range, so untouched legacy cells need this fallback for rendering. + // Only borders with a `val` property qualify — old schema defaults from + // createCellBorders() lack `val` and should be ignored (the style-engine + // resolves those from the table style cascade). + if (!cellAttrs.borders && cellNode.attrs?.borders && typeof cellNode.attrs.borders === 'object') { + const legacy = cellNode.attrs.borders as Record; + const fallback: CellBorders = {}; + for (const side of ['top', 'right', 'bottom', 'left'] as const) { + const b = legacy[side]; + if (b && b.val && typeof b.size === 'number' && b.size > 0) { + const color = b.color ? (b.color.startsWith('#') ? b.color : `#${b.color}`) : '#000000'; + fallback[side] = { style: normalizeLegacyBorderStyle(b.val), width: b.size, color }; + } + } + if (Object.keys(fallback).length > 0) { + cellAttrs.borders = fallback; + } } const padding = @@ -541,7 +579,7 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => { * // Returns: null */ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { - const { rowNode, rowIndex, context, defaultCellPadding, tableProperties, numRows, hasTableLevelBorders } = args; + const { rowNode, rowIndex, context, defaultCellPadding, tableProperties, numRows } = args; if (!isTableRowNode(rowNode) || !Array.isArray(rowNode.content)) { return null; } @@ -557,7 +595,6 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { tableProperties, numCells: rowNode?.content?.length || 1, numRows, - hasTableLevelBorders, }); if (parsedCell) { cells.push(parsedCell); @@ -751,24 +788,27 @@ export function tableNodeToBlock( enableComments, }; - const hydratedTableStyle = hydrateTableStyleAttrs(node, converterContext); + // Compute the effective table style ID once per table. This single canonical + // style ID is used for both table-level hydration and cell/paragraph cascades. + const explicitStyleId = typeof node.attrs?.tableStyleId === 'string' ? node.attrs.tableStyleId : null; + const resolvedStyle = resolveExistingTableEffectiveStyleId(explicitStyleId, converterContext?.translatedLinkedStyles); + const effectiveStyleId = resolvedStyle.styleId; + + const hydratedTableStyle = hydrateTableStyleAttrs(node, converterContext, effectiveStyleId); const defaultCellPadding = hydratedTableStyle?.cellPadding; - // Pre-compute whether table-level borders exist (from inline or hydrated style). - // When they do, schema-default cell borders can be safely skipped so the painter's - // resolveTableCellBorders distributes table borders to cells. - // When they don't, cell schema defaults are kept as fallback. - const inlineBorders = node.attrs?.borders; - const hasInlineBorders = - inlineBorders && - typeof inlineBorders === 'object' && - inlineBorders !== null && - Object.keys(inlineBorders as Record).length > 0; - const hasHydratedBorders = - hydratedTableStyle?.borders && - typeof hydratedTableStyle.borders === 'object' && - Object.keys(hydratedTableStyle.borders).length > 0; - const hasTableLevelBorders = !!(hasInlineBorders || hasHydratedBorders); + // Build tableProperties with the effective style ID for consistent cascade resolution. + // PM node attrs are never mutated — the effective ID lives only in this transient object. + // When effectiveStyleId is null (resolver found no style), strip any raw tableStyleId + // from the cascade object to prevent invalid IDs from influencing resolution. + const rawTableProperties = node.attrs?.tableProperties as TableProperties | undefined; + const tablePropertiesForCascade: TableProperties | undefined = + effectiveStyleId || rawTableProperties + ? { + ...rawTableProperties, + tableStyleId: effectiveStyleId ?? undefined, + } + : undefined; const rows: TableRow[] = []; node.content.forEach((rowNode, rowIndex) => { @@ -778,8 +818,7 @@ export function tableNodeToBlock( numRows: node?.content?.length ?? 1, context: parserDeps, defaultCellPadding, - tableProperties: node.attrs?.tableProperties as TableProperties | undefined, - hasTableLevelBorders, + tableProperties: tablePropertiesForCascade, }); if (parsedRow) { rows.push(parsedRow); diff --git a/packages/layout-engine/style-engine/src/cascade.ts b/packages/layout-engine/style-engine/src/cascade.ts index 293dde176c..9b62ba772c 100644 --- a/packages/layout-engine/style-engine/src/cascade.ts +++ b/packages/layout-engine/style-engine/src/cascade.ts @@ -11,9 +11,9 @@ */ import { ParagraphProperties, RunFontFamilyProperties, RunProperties } from './ooxml/types'; -import type { TableCellProperties } from './ooxml/styles-types'; +import type { TableCellProperties, TableProperties } from './ooxml/styles-types'; -export type PropertyObject = ParagraphProperties | RunProperties | TableCellProperties; +export type PropertyObject = ParagraphProperties | RunProperties | TableCellProperties | TableProperties; /** * Performs a deep merge on an ordered list of property objects. diff --git a/packages/layout-engine/style-engine/src/ooxml/index.test.ts b/packages/layout-engine/style-engine/src/ooxml/index.test.ts index eb1242c0ae..63174a5d2d 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.test.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.test.ts @@ -7,6 +7,7 @@ import { resolveParagraphProperties, resolveCellStyles, resolveTableCellProperties, + resolveTableProperties, type OoxmlResolverParams, } from './index.js'; @@ -543,3 +544,187 @@ describe('ooxml - resolveTableCellProperties', () => { expect(result.borders?.bottom).toEqual({ val: 'double', color: '000000', size: 8 }); }); }); + +// ────────────────────────────────────────────────────────────────────────────── +// resolveStyleChain – cycle detection +// ────────────────────────────────────────────────────────────────────────────── + +describe('ooxml - resolveStyleChain cycle detection', () => { + it('handles direct cycle: A → B → A', () => { + const params = buildParams({ + translatedLinkedStyles: { + ...emptyStyles, + styles: { + A: { basedOn: 'B', runProperties: { bold: true } }, + B: { basedOn: 'A', runProperties: { italic: true } }, + }, + }, + }); + // Should not infinite loop — returns combined properties from the partial chain + const result = resolveStyleChain('runProperties', params, 'A'); + expect(result).toEqual({ bold: true, italic: true }); + }); + + it('handles indirect cycle: A → B → C → B', () => { + const params = buildParams({ + translatedLinkedStyles: { + ...emptyStyles, + styles: { + A: { basedOn: 'B', runProperties: { bold: true } }, + B: { basedOn: 'C', runProperties: { italic: true } }, + C: { basedOn: 'B', runProperties: { fontSize: 24 } }, + }, + }, + }); + const result = resolveStyleChain('runProperties', params, 'A'); + expect(result.bold).toBe(true); + expect(result.italic).toBe(true); + expect(result.fontSize).toBe(24); + }); + + it('handles self-referencing cycle: A → A', () => { + const params = buildParams({ + translatedLinkedStyles: { + ...emptyStyles, + styles: { + A: { basedOn: 'A', runProperties: { bold: true } }, + }, + }, + }); + const result = resolveStyleChain('runProperties', params, 'A'); + expect(result).toEqual({ bold: true }); + }); +}); + +// ────────────────────────────────────────────────────────────────────────────── +// resolveTableProperties +// ────────────────────────────────────────────────────────────────────────────── + +describe('ooxml - resolveTableProperties', () => { + it('returns empty object for null/undefined style ID', () => { + expect(resolveTableProperties(null, emptyStyles)).toEqual({}); + expect(resolveTableProperties(undefined, emptyStyles)).toEqual({}); + }); + + it('returns empty object when style does not exist', () => { + expect(resolveTableProperties('MissingStyle', emptyStyles)).toEqual({}); + }); + + it('resolves table properties from a single style', () => { + const styles = { + ...emptyStyles, + styles: { + TableGrid: { + type: 'table', + tableProperties: { + borders: { top: { val: 'single', size: 4, color: '000000' } }, + justification: 'center', + }, + }, + }, + }; + const result = resolveTableProperties('TableGrid', styles); + expect(result.borders).toEqual({ top: { val: 'single', size: 4, color: '000000' } }); + expect(result.justification).toBe('center'); + }); + + it('follows basedOn chain for table properties (single level)', () => { + const styles = { + ...emptyStyles, + styles: { + TableNormal: { + type: 'table', + tableProperties: { + cellMargins: { marginLeft: { value: 108, type: 'dxa' } }, + justification: 'left', + }, + }, + TableGrid: { + type: 'table', + basedOn: 'TableNormal', + tableProperties: { + borders: { top: { val: 'single', size: 4 } }, + }, + }, + }, + }; + const result = resolveTableProperties('TableGrid', styles); + // From TableGrid + expect(result.borders).toEqual({ top: { val: 'single', size: 4 } }); + // Inherited from TableNormal + expect(result.cellMargins).toEqual({ marginLeft: { value: 108, type: 'dxa' } }); + expect(result.justification).toBe('left'); + }); + + it('follows multi-level basedOn chain', () => { + const styles = { + ...emptyStyles, + styles: { + Base: { + type: 'table', + tableProperties: { justification: 'left' }, + }, + Mid: { + type: 'table', + basedOn: 'Base', + tableProperties: { cellMargins: { marginTop: { value: 50, type: 'dxa' } } }, + }, + Derived: { + type: 'table', + basedOn: 'Mid', + tableProperties: { borders: { top: { val: 'single' } } }, + }, + }, + }; + const result = resolveTableProperties('Derived', styles); + expect(result.borders).toEqual({ top: { val: 'single' } }); + expect(result.cellMargins).toEqual({ marginTop: { value: 50, type: 'dxa' } }); + expect(result.justification).toBe('left'); + }); + + it('derived properties override base properties', () => { + const styles = { + ...emptyStyles, + styles: { + Base: { + type: 'table', + tableProperties: { justification: 'left', tableCellSpacing: { value: 10, type: 'dxa' } }, + }, + Derived: { + type: 'table', + basedOn: 'Base', + tableProperties: { justification: 'center' }, + }, + }, + }; + const result = resolveTableProperties('Derived', styles); + // Overridden + expect(result.justification).toBe('center'); + // Inherited + expect(result.tableCellSpacing).toEqual({ value: 10, type: 'dxa' }); + }); + + it('returns empty object when translatedLinkedStyles is null', () => { + expect(resolveTableProperties('TableGrid', null)).toEqual({}); + }); + + it('handles marginStart/marginEnd in cellMargins', () => { + const styles = { + ...emptyStyles, + styles: { + RTLTable: { + type: 'table', + tableProperties: { + cellMargins: { + marginStart: { value: 100, type: 'dxa' }, + marginEnd: { value: 200, type: 'dxa' }, + }, + }, + }, + }, + }; + const result = resolveTableProperties('RTLTable', styles); + expect(result.cellMargins?.marginStart).toEqual({ value: 100, type: 'dxa' }); + expect(result.cellMargins?.marginEnd).toEqual({ value: 200, type: 'dxa' }); + }); +}); diff --git a/packages/layout-engine/style-engine/src/ooxml/index.ts b/packages/layout-engine/style-engine/src/ooxml/index.ts index fd83dbc23b..53b1a46b41 100644 --- a/packages/layout-engine/style-engine/src/ooxml/index.ts +++ b/packages/layout-engine/style-engine/src/ooxml/index.ts @@ -22,6 +22,18 @@ export type { PropertyObject }; export type * from './types.ts'; export type * from './numbering-types.ts'; export type * from './styles-types.ts'; +export { + TABLE_STYLE_ID_TABLE_GRID, + TABLE_STYLE_ID_TABLE_NORMAL, + TABLE_FALLBACK_BORDER, + TABLE_FALLBACK_BORDERS, + TABLE_FALLBACK_CELL_PADDING, + isKnownTableStyleId, + findTypeDefaultTableStyleId, + resolveExistingTableEffectiveStyleId, + resolvePreferredNewTableStyleId, +} from './table-style-selection.js'; +export type { ResolvedStyle, ResolvedStyleSource } from './table-style-selection.js'; export interface OoxmlResolverParams { translatedNumbering: NumberingProperties | null | undefined; @@ -249,7 +261,7 @@ export function resolveParagraphProperties( } export function resolveStyleChain( - propertyType: 'paragraphProperties' | 'runProperties', + propertyType: 'paragraphProperties' | 'runProperties' | 'tableProperties', params: OoxmlResolverParams, styleId: string | undefined, followBasedOnChain = true, @@ -263,13 +275,13 @@ export function resolveStyleChain( const basedOn = styleDef.basedOn; let styleChain: T[] = [styleProps]; - const seenStyles = new Set(); + const seenStyles = new Set([styleId]); let nextBasedOn = basedOn; while (followBasedOnChain && nextBasedOn) { if (seenStyles.has(nextBasedOn as string)) { break; } - seenStyles.add(basedOn as string); + seenStyles.add(nextBasedOn as string); const basedOnStyleDef = params.translatedLinkedStyles?.styles?.[nextBasedOn]; const basedOnProps = basedOnStyleDef?.[propertyType as keyof typeof basedOnStyleDef] as T; @@ -338,6 +350,30 @@ export function getNumberingProperties('tableProperties', params, tableStyleId); +} + export function resolveDocxFontFamily( attributes: Record | null | undefined, docx: Record | null | undefined, diff --git a/packages/layout-engine/style-engine/src/ooxml/table-style-selection.test.ts b/packages/layout-engine/style-engine/src/ooxml/table-style-selection.test.ts new file mode 100644 index 0000000000..e625bf2baa --- /dev/null +++ b/packages/layout-engine/style-engine/src/ooxml/table-style-selection.test.ts @@ -0,0 +1,193 @@ +import { describe, expect, it } from 'vitest'; +import { + TABLE_STYLE_ID_TABLE_GRID, + TABLE_STYLE_ID_TABLE_NORMAL, + isKnownTableStyleId, + findTypeDefaultTableStyleId, + resolveExistingTableEffectiveStyleId, + resolvePreferredNewTableStyleId, +} from './table-style-selection.ts'; +import type { StylesDocumentProperties } from './styles-types.ts'; + +const emptyStyles: StylesDocumentProperties = { docDefaults: {}, latentStyles: {}, styles: {} }; + +const withStyles = (styles: Record): StylesDocumentProperties => ({ + ...emptyStyles, + styles: Object.fromEntries(Object.entries(styles).map(([id, def]) => [id, { styleId: id, ...def }])), +}); + +// ────────────────────────────────────────────────────────────────────────────── +// isKnownTableStyleId +// ────────────────────────────────────────────────────────────────────────────── + +describe('isKnownTableStyleId', () => { + it('returns true for a valid table style', () => { + const styles = withStyles({ TableGrid: { type: 'table' } }); + expect(isKnownTableStyleId('TableGrid', styles)).toBe(true); + }); + + it('returns false for a non-table style type', () => { + const styles = withStyles({ Normal: { type: 'paragraph' } }); + expect(isKnownTableStyleId('Normal', styles)).toBe(false); + }); + + it('returns false for a missing style', () => { + expect(isKnownTableStyleId('DoesNotExist', emptyStyles)).toBe(false); + }); + + it('returns false for null/undefined inputs', () => { + expect(isKnownTableStyleId(null, emptyStyles)).toBe(false); + expect(isKnownTableStyleId(undefined, emptyStyles)).toBe(false); + expect(isKnownTableStyleId('TableGrid', null)).toBe(false); + expect(isKnownTableStyleId('TableGrid', undefined)).toBe(false); + }); +}); + +// ────────────────────────────────────────────────────────────────────────────── +// findTypeDefaultTableStyleId +// ────────────────────────────────────────────────────────────────────────────── + +describe('findTypeDefaultTableStyleId', () => { + it('finds the style with type=table and default=true', () => { + const styles = withStyles({ + TableNormal: { type: 'table', default: true }, + TableGrid: { type: 'table' }, + }); + expect(findTypeDefaultTableStyleId(styles)).toBe('TableNormal'); + }); + + it('returns null when no type-default table style exists', () => { + const styles = withStyles({ + TableGrid: { type: 'table' }, + Normal: { type: 'paragraph', default: true }, + }); + expect(findTypeDefaultTableStyleId(styles)).toBeNull(); + }); + + it('returns null for empty/null styles', () => { + expect(findTypeDefaultTableStyleId(emptyStyles)).toBeNull(); + expect(findTypeDefaultTableStyleId(null)).toBeNull(); + }); +}); + +// ────────────────────────────────────────────────────────────────────────────── +// resolveExistingTableEffectiveStyleId +// ────────────────────────────────────────────────────────────────────────────── + +describe('resolveExistingTableEffectiveStyleId', () => { + it('returns explicit source when style exists and is type=table', () => { + const styles = withStyles({ TableGrid: { type: 'table' } }); + const result = resolveExistingTableEffectiveStyleId('TableGrid', styles); + expect(result).toEqual({ styleId: 'TableGrid', source: 'explicit' }); + }); + + it('falls through to type-default when explicit style is invalid', () => { + const styles = withStyles({ + Normal: { type: 'paragraph' }, + TableNormal: { type: 'table', default: true }, + }); + const result = resolveExistingTableEffectiveStyleId('Normal', styles); + expect(result).toEqual({ styleId: 'TableNormal', source: 'type-default' }); + }); + + it('falls through to type-default when explicit style does not exist', () => { + const styles = withStyles({ + TableNormal: { type: 'table', default: true }, + }); + const result = resolveExistingTableEffectiveStyleId('MissingStyle', styles); + expect(result).toEqual({ styleId: 'TableNormal', source: 'type-default' }); + }); + + it('returns none when no style can be resolved', () => { + const result = resolveExistingTableEffectiveStyleId('MissingStyle', emptyStyles); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); + + it('returns none when explicit is null', () => { + const result = resolveExistingTableEffectiveStyleId(null, emptyStyles); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); +}); + +// ────────────────────────────────────────────────────────────────────────────── +// resolvePreferredNewTableStyleId +// ────────────────────────────────────────────────────────────────────────────── + +describe('resolvePreferredNewTableStyleId', () => { + it('uses settings default when valid', () => { + const styles = withStyles({ MyTableStyle: { type: 'table' } }); + const result = resolvePreferredNewTableStyleId('MyTableStyle', styles); + expect(result).toEqual({ styleId: 'MyTableStyle', source: 'settings-default' }); + }); + + it('ignores settings default when it references a non-existent style', () => { + const styles = withStyles({ TableGrid: { type: 'table' } }); + const result = resolvePreferredNewTableStyleId('DoesNotExist', styles); + expect(result).toEqual({ styleId: 'TableGrid', source: 'builtin-fallback' }); + }); + + it('ignores settings default when it references a non-table style', () => { + const styles = withStyles({ + Normal: { type: 'paragraph' }, + TableGrid: { type: 'table' }, + }); + const result = resolvePreferredNewTableStyleId('Normal', styles); + expect(result).toEqual({ styleId: 'TableGrid', source: 'builtin-fallback' }); + }); + + it('skips TableNormal as type-default (it is the base/reset style)', () => { + const styles = withStyles({ + TableNormal: { type: 'table', default: true }, + }); + const result = resolvePreferredNewTableStyleId(null, styles); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); + + it('uses a non-TableNormal type-default', () => { + const styles = withStyles({ + CustomTableStyle: { type: 'table', default: true }, + }); + const result = resolvePreferredNewTableStyleId(null, styles); + expect(result).toEqual({ styleId: 'CustomTableStyle', source: 'type-default' }); + }); + + it('falls through to TableGrid builtin fallback', () => { + const styles = withStyles({ TableGrid: { type: 'table' } }); + const result = resolvePreferredNewTableStyleId(null, styles); + expect(result).toEqual({ styleId: TABLE_STYLE_ID_TABLE_GRID, source: 'builtin-fallback' }); + }); + + it('skips TableNormal even as builtin fallback', () => { + const styles = withStyles({ TableNormal: { type: 'table' } }); + const result = resolvePreferredNewTableStyleId(null, styles); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); + + it('returns none when no styles exist at all', () => { + const result = resolvePreferredNewTableStyleId(null, emptyStyles); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); + + it('returns none when styles is null', () => { + const result = resolvePreferredNewTableStyleId(null, null); + expect(result).toEqual({ styleId: null, source: 'none' }); + }); + + it('settings default takes precedence over type-default', () => { + const styles = withStyles({ + CustomDefault: { type: 'table' }, + TableNormal: { type: 'table', default: true }, + }); + const result = resolvePreferredNewTableStyleId('CustomDefault', styles); + expect(result).toEqual({ styleId: 'CustomDefault', source: 'settings-default' }); + }); + + it('type-default takes precedence over builtin fallback', () => { + const styles = withStyles({ + TableGrid: { type: 'table' }, + SomeDefault: { type: 'table', default: true }, + }); + const result = resolvePreferredNewTableStyleId(null, styles); + expect(result).toEqual({ styleId: 'SomeDefault', source: 'type-default' }); + }); +}); diff --git a/packages/layout-engine/style-engine/src/ooxml/table-style-selection.ts b/packages/layout-engine/style-engine/src/ooxml/table-style-selection.ts new file mode 100644 index 0000000000..fc7c097dbe --- /dev/null +++ b/packages/layout-engine/style-engine/src/ooxml/table-style-selection.ts @@ -0,0 +1,143 @@ +/** + * Table Style ID Selection + * + * Central module for determining which table style applies in a given context. + * Owns all style-ID-selection precedence logic. No other package re-implements + * this precedence. + */ + +import type { StylesDocumentProperties, StyleDefinition } from './styles-types.ts'; + +// ────────────────────────────────────────────────────────────────────────────── +// Constants +// ────────────────────────────────────────────────────────────────────────────── + +export const TABLE_STYLE_ID_TABLE_GRID = 'TableGrid'; +export const TABLE_STYLE_ID_TABLE_NORMAL = 'TableNormal'; + +/** Fallback border applied when no table style exists at all (source: 'none'). */ +export const TABLE_FALLBACK_BORDER = { val: 'single', size: 4, color: '#000000' } as const; + +export const TABLE_FALLBACK_BORDERS = { + top: { ...TABLE_FALLBACK_BORDER }, + left: { ...TABLE_FALLBACK_BORDER }, + bottom: { ...TABLE_FALLBACK_BORDER }, + right: { ...TABLE_FALLBACK_BORDER }, + insideH: { ...TABLE_FALLBACK_BORDER }, + insideV: { ...TABLE_FALLBACK_BORDER }, +} as const; + +/** Default cell padding in dxa (twips). left/right = 108 dxa ≈ 0.075in, matching Word defaults. */ +export const TABLE_FALLBACK_CELL_PADDING = { top: 0, bottom: 0, left: 108, right: 108 } as const; + +// ────────────────────────────────────────────────────────────────────────────── +// Types +// ────────────────────────────────────────────────────────────────────────────── + +export type ResolvedStyleSource = 'explicit' | 'settings-default' | 'type-default' | 'builtin-fallback' | 'none'; + +export interface ResolvedStyle { + styleId: string | null; + source: ResolvedStyleSource; +} + +// ────────────────────────────────────────────────────────────────────────────── +// Helpers +// ────────────────────────────────────────────────────────────────────────────── + +/** + * Checks whether a style ID exists in the catalog and is of type `table`. + */ +export function isKnownTableStyleId( + styleId: string | null | undefined, + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): boolean { + if (!styleId || !translatedLinkedStyles?.styles) return false; + const def = translatedLinkedStyles.styles[styleId]; + return def != null && def.type === 'table'; +} + +/** + * Finds the type-default table style: `w:style w:type="table" w:default="1"`. + * Typically `TableNormal`. + */ +export function findTypeDefaultTableStyleId( + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): string | null { + if (!translatedLinkedStyles?.styles) return null; + for (const [styleId, def] of Object.entries(translatedLinkedStyles.styles)) { + if (def.type === 'table' && def.default === true) { + return styleId; + } + } + return null; +} + +// ────────────────────────────────────────────────────────────────────────────── +// Existing Table Resolution +// ────────────────────────────────────────────────────────────────────────────── + +/** + * Determines the effective style ID for a table already in the document. + * + * Precedence: + * 1. Explicit `tableStyleId` on the node, if valid (exists + type=table). + * 2. Type-default table style from catalog (`w:default="1"`). + * 3. No style (`source: 'none'`). + */ +export function resolveExistingTableEffectiveStyleId( + explicitTableStyleId: string | null | undefined, + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): ResolvedStyle { + // 1. Explicit + if (explicitTableStyleId && isKnownTableStyleId(explicitTableStyleId, translatedLinkedStyles)) { + return { styleId: explicitTableStyleId, source: 'explicit' }; + } + + // 2. Type-default + const typeDefault = findTypeDefaultTableStyleId(translatedLinkedStyles); + if (typeDefault) { + return { styleId: typeDefault, source: 'type-default' }; + } + + // 3. No style + return { styleId: null, source: 'none' }; +} + +// ────────────────────────────────────────────────────────────────────────────── +// New Table Resolution +// ────────────────────────────────────────────────────────────────────────────── + +/** + * Determines the preferred style ID for a newly created table. + * + * Precedence: + * 1. `w:defaultTableStyle` from settings, if present and resolvable. + * 2. Type-default table style (`w:default="1"`). + * 3. `TableGrid` if it exists in catalog (builtin-fallback). + * 4. `TableNormal` if it exists in catalog (builtin-fallback). + * 5. No style (`source: 'none'`). + */ +export function resolvePreferredNewTableStyleId( + settingsDefaultTableStyleId: string | null | undefined, + translatedLinkedStyles: StylesDocumentProperties | null | undefined, +): ResolvedStyle { + // 1. Settings default + if (settingsDefaultTableStyleId && isKnownTableStyleId(settingsDefaultTableStyleId, translatedLinkedStyles)) { + return { styleId: settingsDefaultTableStyleId, source: 'settings-default' }; + } + + // 2. Type-default (skip TableNormal — it's the OOXML base/reset style, not a visual style) + const typeDefault = findTypeDefaultTableStyleId(translatedLinkedStyles); + if (typeDefault && typeDefault !== TABLE_STYLE_ID_TABLE_NORMAL) { + return { styleId: typeDefault, source: 'type-default' }; + } + + // 3. TableGrid builtin + if (isKnownTableStyleId(TABLE_STYLE_ID_TABLE_GRID, translatedLinkedStyles)) { + return { styleId: TABLE_STYLE_ID_TABLE_GRID, source: 'builtin-fallback' }; + } + + // 4. No style — use inline fallback borders + return { styleId: null, source: 'none' }; +} diff --git a/packages/super-editor/src/core/commands/insertContent.test.js b/packages/super-editor/src/core/commands/insertContent.test.js index f0a6023ad9..2557b3bc8a 100644 --- a/packages/super-editor/src/core/commands/insertContent.test.js +++ b/packages/super-editor/src/core/commands/insertContent.test.js @@ -322,7 +322,7 @@ describe('insertContent (integration) list export', () => { }); }); - it('normalizes imported HTML table header borders for render and export parity', async () => { + it('does not inject inline cell borders on imported HTML table headers', async () => { const editor = await setupEditor(); editor.commands.insertContent( '
Search QueryFindings / Assessment
AB
', @@ -334,11 +334,8 @@ describe('insertContent (integration) list export', () => { expect(tableNode).toBeTruthy(); const headerCell = tableNode?.content?.[0]?.content?.[0]; expect(headerCell?.type).toBe('tableHeader'); - const borders = headerCell?.attrs?.borders; - expect(borders?.top).toBeDefined(); - expect(borders?.right).toBeDefined(); - expect(borders?.bottom).toBeDefined(); - expect(borders?.left).toBeDefined(); + // Headers should NOT have inline borders — style cascade owns them + expect(headerCell?.attrs?.borders).toBeNull(); const result = await exportFromEditorContent(editor); const body = result.elements?.find((el) => el.name === 'w:body'); @@ -347,11 +344,9 @@ describe('insertContent (integration) list export', () => { const firstCell = firstRow?.elements?.find((el) => el.name === 'w:tc'); const firstCellProperties = firstCell?.elements?.find((el) => el.name === 'w:tcPr'); const firstCellBorders = firstCellProperties?.elements?.find((el) => el.name === 'w:tcBorders'); - const topBorder = firstCellBorders?.elements?.find((el) => el.name === 'w:top'); - expect(firstCellBorders).toBeDefined(); - expect(topBorder?.attributes?.['w:val']).toBe('single'); - expect(Number(topBorder?.attributes?.['w:sz'])).toBeGreaterThan(0); + // No inline cell borders should be emitted — table-level fallback borders handle this + expect(firstCellBorders).toBeUndefined(); }); }); diff --git a/packages/super-editor/src/core/helpers/importHtml.js b/packages/super-editor/src/core/helpers/importHtml.js index f42a1368b7..4e598320a3 100644 --- a/packages/super-editor/src/core/helpers/importHtml.js +++ b/packages/super-editor/src/core/helpers/importHtml.js @@ -1,68 +1,10 @@ //@ts-check -import { DOMParser, Fragment } from 'prosemirror-model'; +import { DOMParser } from 'prosemirror-model'; import { stripHtmlStyles } from './htmlSanitizer.js'; import { htmlHandler } from '../InputRule.js'; import { wrapTextsInRuns } from '../inputRules/docx-paste/docx-paste.js'; -import { createCellBorders } from '../../extensions/table-cell/helpers/createCellBorders.js'; import { detectUnsupportedContent } from './catchAllSchema.js'; -const TABLE_HEADER_NODE_NAME = 'tableHeader'; - -/** - * @param {unknown} borderValue - * @returns {boolean} - */ -const hasMeaningfulCellBorders = (borderValue) => { - if (!borderValue || typeof borderValue !== 'object') return false; - - return Object.values(borderValue).some((side) => side && typeof side === 'object' && Object.keys(side).length > 0); -}; - -/** - * Fill missing border metadata for imported HTML header cells (). - * This keeps editor rendering and DOCX export aligned without overriding explicit borders. - * - * @param {import('prosemirror-model').Node} doc - * @returns {import('prosemirror-model').Node} - */ -const normalizeImportedHtmlTableHeaders = (doc) => { - const normalizeNode = (node) => { - let nextNode = node; - - if (node.childCount > 0) { - const nextChildren = []; - let childrenChanged = false; - - node.forEach((child) => { - const normalizedChild = normalizeNode(child); - if (normalizedChild !== child) childrenChanged = true; - nextChildren.push(normalizedChild); - }); - - if (childrenChanged) { - nextNode = node.copy(Fragment.fromArray(nextChildren)); - } - } - - if (nextNode.type.name !== TABLE_HEADER_NODE_NAME) { - return nextNode; - } - - if (hasMeaningfulCellBorders(nextNode.attrs?.borders)) { - return nextNode; - } - - const nextAttrs = { - ...nextNode.attrs, - borders: createCellBorders(), - }; - - return nextNode.type.create(nextAttrs, nextNode.content, nextNode.marks); - }; - - return normalizeNode(doc); -}; - /** * @typedef {import('./catchAllSchema.js').UnsupportedContentItem} UnsupportedContentItem */ @@ -119,9 +61,6 @@ export function createDocFromHTML(content, editor, options = {}) { } let doc = DOMParser.fromSchema(editor.schema).parse(parsedContent); - if (isImport) { - doc = normalizeImportedHtmlTableHeaders(doc); - } doc = wrapTextsInRuns(doc); return doc; } diff --git a/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts b/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts index d597860336..7ea8aa3a70 100644 --- a/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts +++ b/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts @@ -337,10 +337,14 @@ function convertTable(node: MdastTable, ctx: MdastConversionContext): JsonNode { }); } + // Standalone markdown conversion has no editor context. Mark the table + // for deferred style normalization so the table extension can resolve the + // correct style after insertion into an editor instance. return { type: 'table', attrs: { - tableStyleId: 'TableGrid', + tableStyleId: null, + needsTableStyleNormalization: true, tableProperties: { tableWidth: { value: FULL_WIDTH_TABLE_PCT, diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index f5f32b2647..b29159c993 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -70,6 +70,7 @@ import { HeaderFooterSessionManager } from './header-footer/HeaderFooterSessionM import { decodeRPrFromMarks } from '../super-converter/styles.js'; import { halfPointToPoints } from '../super-converter/helpers.js'; import { toFlowBlocks, ConverterContext, FlowBlockCache } from '@superdoc/pm-adapter'; +import { readSettingsRoot, readDefaultTableStyle } from '../../document-api-adapters/document-settings.js'; import { incrementalLayout, selectionToRects, @@ -3246,12 +3247,21 @@ export class PresentationEditor extends EventEmitter { } } catch {} + let defaultTableStyleId: string | undefined; + if (converter) { + const settingsRoot = readSettingsRoot(converter); + if (settingsRoot) { + defaultTableStyleId = readDefaultTableStyle(settingsRoot) ?? undefined; + } + } + converterContext = converter ? { docx: converter.convertedXml, ...(Object.keys(footnoteNumberById).length ? { footnoteNumberById } : {}), translatedLinkedStyles: converter.translatedLinkedStyles, translatedNumbering: converter.translatedNumbering, + ...(defaultTableStyleId ? { defaultTableStyleId } : {}), } : undefined; const atomNodeTypes = getAtomNodeTypesFromSchema(this.#editor?.schema ?? null); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js index 940592b965..d8e6611a6a 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.js @@ -146,7 +146,6 @@ const encode = (params, encodedAttrs) => { } } - const tableLook = encodedAttrs.tableProperties.tblLook; // Table borders can be specified in tblPr or inside a referenced style tag const borderProps = _processTableBorders(encodedAttrs.tableProperties.borders || {}); const referencedStyles = _getReferencedTableStyles(encodedAttrs.tableStyleId, params) || {}; @@ -211,8 +210,6 @@ const encode = (params, encodedAttrs) => { row, table: node, tableProperties: encodedAttrs.tableProperties, - tableBorders: encodedAttrs.borders, - tableLook, columnWidths, activeRowSpans: activeRowSpans.slice(), rowIndex, diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js index aa58d8823c..a81322d9dc 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js @@ -1,4 +1,4 @@ -import { eighthPointsToPixels, twipsToPixels, resolveShadingFillColor } from '@converter/helpers'; +import { twipsToPixels, resolveShadingFillColor } from '@converter/helpers'; import { translator as tcPrTranslator } from '../../tcPr'; /** @@ -11,16 +11,9 @@ export function handleTableCellNode({ table, row, tableProperties, - rowBorders, - baseTableBorders, - tableLook, - rowCnfStyle, columnIndex, columnWidth = null, allColumnWidths = [], - rowIndex = 0, - totalRows = 1, - totalColumns, preferTableGridWidths = false, _referencedStyles, }) { @@ -33,31 +26,8 @@ export function handleTableCellNode({ const tableCellProperties = tcPr ? (tcPrTranslator.encode({ ...params, nodes: [tcPr] }) ?? {}) : {}; attributes['tableCellProperties'] = tableCellProperties; - // Determine cell position for border application - // Fall back to allColumnWidths.length if totalColumns not provided - // Fall back to counting table rows if totalRows not provided - const effectiveTotalColumns = totalColumns ?? (allColumnWidths.length || 1); - const effectiveTotalRows = totalRows ?? (table?.elements?.filter((el) => el.name === 'w:tr').length || 1); - const colspan = parseInt(tableCellProperties.gridSpan || 1, 10); - const isFirstRow = rowIndex === 0; - const isLastRow = rowIndex === effectiveTotalRows - 1; - const isFirstColumn = columnIndex === 0; - const isLastColumn = columnIndex + colspan >= effectiveTotalColumns; - - attributes['borders'] = processCellBorders({ - baseTableBorders, - rowBorders, - tableLook, - rowCnfStyle, - isFirstRow, - isLastRow, - isFirstColumn, - isLastColumn, - tableCellProperties, - referencedStyles, - hasBorderSpacing: !!tableProperties?.tableCellSpacing, - }); // Colspan + const colspan = parseInt(tableCellProperties.gridSpan || 1, 10); if (colspan > 1) attributes['colspan'] = colspan; // Width @@ -278,130 +248,6 @@ function normalizeTableCellContent(content, editor) { return normalized; } -const processInlineCellBorders = (borders, rowBorders) => { - if (!borders) return null; - - return ['bottom', 'top', 'left', 'right'].reduce((acc, direction) => { - const borderAttrs = borders[direction]; - const rowBorderAttrs = rowBorders[direction]; - - if (borderAttrs && borderAttrs['val'] !== 'none') { - const color = borderAttrs['color']; - let size = borderAttrs['size']; - if (size) size = eighthPointsToPixels(size); - acc[direction] = { color, size, val: borderAttrs['val'] }; - return acc; - } - if (borderAttrs && borderAttrs['val'] === 'none') { - // When inline border explicitly says 'none', always create an entry to disable the border - // Copy base border attrs if available, but always set val='none' - const border = Object.assign({}, rowBorderAttrs || {}); - border['val'] = 'none'; - acc[direction] = border; - return acc; - } - return acc; - }, {}); -}; - -const processCellBorders = ({ - baseTableBorders, - rowBorders, - tableLook, - rowCnfStyle, - isFirstRow, - isLastRow, - isFirstColumn, - isLastColumn, - tableCellProperties, - referencedStyles, - hasBorderSpacing, -}) => { - let cellBorders = {}; - if (baseTableBorders) { - if ((isFirstRow || hasBorderSpacing) && baseTableBorders.top) { - cellBorders.top = baseTableBorders.top; - } - if ((isLastRow || hasBorderSpacing) && baseTableBorders.bottom) { - cellBorders.bottom = baseTableBorders.bottom; - } - if ((isFirstColumn || hasBorderSpacing) && baseTableBorders.left) { - cellBorders.left = baseTableBorders.left; - } - if ((isLastColumn || hasBorderSpacing) && baseTableBorders.right) { - cellBorders.right = baseTableBorders.right; - } - } - - if (rowBorders) { - if (rowBorders.top?.val) { - cellBorders.top = rowBorders.top; - } - - if (rowBorders.bottom?.val) { - cellBorders.bottom = rowBorders.bottom; - } - - if (rowBorders.left?.val) { - const applyLeftToAll = rowBorders.left.val === 'none'; - if (applyLeftToAll || isFirstColumn) { - cellBorders.left = rowBorders.left; - } - } - - if (rowBorders.right?.val) { - const applyRightToAll = rowBorders.right.val === 'none'; - if (applyRightToAll || isLastColumn) { - cellBorders.right = rowBorders.right; - } - } - - // INNER BORDERS: Position-based (including for 'none' values) - // insideH creates horizontal lines between rows (applied as bottom border to non-last rows) - if (!isLastRow && rowBorders.insideH) { - cellBorders.bottom = rowBorders.insideH; - } - // insideV creates vertical lines between columns (applied as right border to non-last columns) - if (!isLastColumn && rowBorders.insideV) { - cellBorders.right = rowBorders.insideV; - } - } - - const getStyleTableCellBorders = (styleVariant) => styleVariant?.tableCellProperties?.borders ?? null; - - // Check if a conditional style flag is enabled using cascading priority: cell > row > tableLook - const cellCnfStyle = tableCellProperties?.cnfStyle; - const getFlag = (source, flag) => - source && Object.prototype.hasOwnProperty.call(source, flag) ? source[flag] : undefined; - const isStyleEnabled = (flag) => - getFlag(cellCnfStyle, flag) ?? getFlag(rowCnfStyle, flag) ?? getFlag(tableLook, flag) ?? true; - - // Apply table style conditional formatting borders. - // Only apply the relevant edge per conditional style: - // - firstRow/lastRow => top/bottom - // - firstCol/lastCol => left/right - const applyStyleBorders = (styleVariant, allowedDirections) => { - const styleBorders = getStyleTableCellBorders(styleVariant); - if (!styleBorders) return; - const filteredBorders = allowedDirections.reduce((acc, direction) => { - if (styleBorders[direction]) acc[direction] = styleBorders[direction]; - return acc; - }, {}); - const styleOverrides = processInlineCellBorders(filteredBorders, cellBorders); - if (styleOverrides) cellBorders = Object.assign(cellBorders, styleOverrides); - }; - - if (isFirstRow && isStyleEnabled('firstRow')) applyStyleBorders(referencedStyles?.firstRow, ['top', 'bottom']); - if (isLastRow && isStyleEnabled('lastRow')) applyStyleBorders(referencedStyles?.lastRow, ['top', 'bottom']); - if (isFirstColumn && isStyleEnabled('firstColumn')) applyStyleBorders(referencedStyles?.firstCol, ['left', 'right']); - if (isLastColumn && isStyleEnabled('lastColumn')) applyStyleBorders(referencedStyles?.lastCol, ['left', 'right']); - - // Process inline cell borders (cell-level overrides) - const inlineBorders = processInlineCellBorders(tableCellProperties.borders, cellBorders); - if (inlineBorders) cellBorders = Object.assign(cellBorders, inlineBorders); - - return cellBorders; -}; const getTableCellVMerge = (node) => { const tcPr = node.elements.find((el) => el.name === 'w:tcPr'); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js index c1a48cd91a..0fdaca67a9 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js @@ -35,7 +35,7 @@ describe('legacy-handle-table-cell-node', () => { vi.clearAllMocks(); }); - it('builds SuperDoc tableCell with attrs merged from tcPr, styles, borders, and vertical merge', () => { + it('builds SuperDoc tableCell with attrs merged from tcPr, styles, and vertical merge', () => { // tc with properties const cellNode = { name: 'w:tc', @@ -92,18 +92,6 @@ describe('legacy-handle-table-cell-node', () => { const table = { name: 'w:tbl', elements: [row1, row2, row3] }; - const rowBorders = { - left: { color: '#00FF00', size: 1, space: 0 }, - right: { color: '#111111', size: 1, space: 1 }, - }; - - const tableBorders = { - right: { color: '#111111', size: 1, space: 1 }, - left: { color: '#111111', size: 1, space: 1 }, - top: { color: '#111111', size: 1, space: 1 }, - bottom: { color: '#111111', size: 1, space: 1 }, - }; - const params = { docx: {}, nodeListHandler: { handler: vi.fn(() => 'CONTENT') }, @@ -116,8 +104,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row: row1, - rowBorders, - baseTableBorders: tableBorders, columnIndex: 1, columnWidth: null, allColumnWidths: [90, 100, 110], @@ -143,15 +129,8 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.fontSize).toBe('12pt'); expect(out.attrs.fontFamily).toBe('Arial'); - // borders merged: inline bottom overrides - // With position-based logic: cell at columnIndex=1 is not first column, so no left border from table-level - // Inline left with val="nil" explicitly disables left border (val='none' entry created) - // Cell spans columns 1-2 (last columns), so right border applies - expect(out.attrs.borders.bottom.color).toBe('#FF0000'); - expect(out.attrs.borders.bottom.size).toBeCloseTo(4, 3); - expect(out.attrs.borders.left).toEqual({ val: 'none' }); // inline nil creates explicit 'none' entry - // right comes from rowBorders (cell is in last column position) - expect(out.attrs.borders.right).toEqual(tableBorders.right); + // Border cascade logic was removed from the importer; borders are now resolved by the style-engine + expect(out.attrs.borders).toBeUndefined(); // rowspan derived from vertical merge (restart + 2 continuations) expect(out.attrs.rowspan).toBe(3); @@ -177,8 +156,6 @@ describe('legacy-handle-table-cell-node', () => { tableProperties: { shading: { val: 'pct50', color: '000000', fill: 'FFFFFF' }, }, - rowBorders: {}, - baseTableBorders: null, columnIndex: 0, columnWidth: null, allColumnWidths: [90], @@ -191,58 +168,6 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.background).toEqual({ color: '808080' }); }); - it('applies firstRow/firstCol conditional borders from referenced styles', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row1 = { name: 'w:tr', elements: [cellNode] }; - const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }] }; - const table = { name: 'w:tbl', elements: [row1, row2] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row: row1, - rowBorders: {}, - baseTableBorders: null, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 0, - totalRows: 2, - totalColumns: 2, - _referencedStyles: { - firstRow: { - tableCellProperties: { - borders: { - top: { val: 'single', color: '#00FF00', size: 8 }, - left: { val: 'single', color: '#FF00FF', size: 8 }, // should be ignored (firstRow only controls top) - }, - }, - }, - firstCol: { - tableCellProperties: { - borders: { - left: { val: 'single', color: '#0000FF', size: 16 }, - top: { val: 'single', color: '#FFFF00', size: 16 }, // should be ignored (firstCol only controls left) - }, - }, - }, - }, - }); - - expect(out.attrs.borders.top).toEqual({ val: 'single', color: '#00FF00', size: expect.any(Number) }); - expect(out.attrs.borders.top.size).toBeCloseTo(1.3333, 3); - expect(out.attrs.borders.left).toEqual({ val: 'single', color: '#0000FF', size: expect.any(Number) }); - expect(out.attrs.borders.left.size).toBeCloseTo(2.6666, 3); - }); - it('prefers table grid widths when requested', () => { const cellNode = { name: 'w:tc', @@ -269,8 +194,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - baseTableBorders: null, columnIndex: 0, columnWidth: 50, allColumnWidths: [50, 60], @@ -306,8 +229,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - baseTableBorders: null, columnIndex: 0, columnWidth: 200, allColumnWidths: [200], @@ -347,8 +268,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - baseTableBorders: null, columnIndex: 0, columnWidth: 50, allColumnWidths: [50], @@ -388,8 +307,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - baseTableBorders: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -403,207 +320,6 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.widthUnit).toBeUndefined(); }); - it('skips firstRow conditional borders when tblLook disables it', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row1 = { name: 'w:tr', elements: [cellNode] }; - const table = { name: 'w:tbl', elements: [row1] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row: row1, - rowBorders: {}, - baseTableBorders: null, - tableLook: { firstRow: false }, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90], - rowIndex: 0, - totalRows: 1, - totalColumns: 1, - _referencedStyles: { - firstRow: { - tableCellProperties: { - borders: { - top: { val: 'single', color: '#00FF00', size: 8 }, - bottom: { val: 'single', color: '#00FF00', size: 8 }, - }, - }, - }, - }, - }); - - expect(out.attrs.borders).toEqual({}); - }); - - it('applies row-level left/right borders only to outer cells', () => { - const makeCell = () => ({ name: 'w:tc', elements: [{ name: 'w:p' }] }); - const firstCell = makeCell(); - const secondCell = makeCell(); - const row = { name: 'w:tr', elements: [firstCell, secondCell] }; - const table = { name: 'w:tbl', elements: [row] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const rowBorders = { - top: { val: 'single', color: '#111111', size: 1 }, - bottom: { val: 'single', color: '#111111', size: 1 }, - left: { val: 'single', color: '#111111', size: 1 }, - right: { val: 'single', color: '#111111', size: 1 }, - }; - - const firstOut = handleTableCellNode({ - params, - node: firstCell, - table, - row, - rowBorders, - baseTableBorders: null, - columnIndex: 0, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 0, - totalRows: 1, - totalColumns: 2, - _referencedStyles: null, - }); - - const secondOut = handleTableCellNode({ - params, - node: secondCell, - table, - row, - rowBorders, - baseTableBorders: null, - columnIndex: 1, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 0, - totalRows: 1, - totalColumns: 2, - _referencedStyles: null, - }); - - expect(firstOut.attrs.borders.left).toEqual(rowBorders.left); - expect(firstOut.attrs.borders.right).toBeUndefined(); - expect(secondOut.attrs.borders.right).toEqual(rowBorders.right); - expect(secondOut.attrs.borders.left).toBeUndefined(); - expect(firstOut.attrs.borders.top).toEqual(rowBorders.top); - expect(secondOut.attrs.borders.top).toEqual(rowBorders.top); - expect(firstOut.attrs.borders.bottom).toEqual(rowBorders.bottom); - expect(secondOut.attrs.borders.bottom).toEqual(rowBorders.bottom); - }); - - it('applies row-level left/right none overrides to all cells', () => { - const makeCell = () => ({ name: 'w:tc', elements: [{ name: 'w:p' }] }); - const firstCell = makeCell(); - const middleCell = makeCell(); - const lastCell = makeCell(); - const row = { name: 'w:tr', elements: [firstCell, middleCell, lastCell] }; - const table = { name: 'w:tbl', elements: [row] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const rowBorders = { - left: { val: 'none' }, - right: { val: 'none' }, - }; - - const middleOut = handleTableCellNode({ - params, - node: middleCell, - table, - row, - rowBorders, - baseTableBorders: null, - columnIndex: 1, - columnWidth: null, - allColumnWidths: [90, 100, 110], - rowIndex: 0, - totalRows: 1, - totalColumns: 3, - _referencedStyles: null, - }); - - expect(middleOut.attrs.borders.left).toEqual(rowBorders.left); - expect(middleOut.attrs.borders.right).toEqual(rowBorders.right); - }); - - it('applies lastRow/lastCol conditional borders', () => { - const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; - const row1 = { - name: 'w:tr', - elements: [ - { name: 'w:tc', elements: [{ name: 'w:p' }] }, - { name: 'w:tc', elements: [{ name: 'w:p' }] }, - ], - }; - const row2 = { name: 'w:tr', elements: [{ name: 'w:tc', elements: [{ name: 'w:p' }] }, cellNode] }; - const table = { name: 'w:tbl', elements: [row1, row2] }; - - const params = { - docx: {}, - nodeListHandler: { handler: vi.fn(() => []) }, - path: [], - editor: createEditorStub(), - }; - - const out = handleTableCellNode({ - params, - node: cellNode, - table, - row: row2, - rowBorders: {}, - baseTableBorders: null, - columnIndex: 1, - columnWidth: null, - allColumnWidths: [90, 100], - rowIndex: 1, - totalRows: 2, - totalColumns: 2, - _referencedStyles: { - lastRow: { - tableCellProperties: { - borders: { - bottom: { val: 'single', color: '#00AAAA', size: 8 }, - top: { val: 'single', color: '#AA0000', size: 8 }, - }, - }, - }, - lastCol: { - tableCellProperties: { - borders: { - right: { val: 'single', color: '#AAAA00', size: 16 }, - bottom: { val: 'single', color: '#0000AA', size: 16 }, // should be ignored (lastCol only controls right) - }, - }, - }, - }, - }); - - expect(out.attrs.borders.bottom).toEqual({ val: 'single', color: '#00AAAA', size: expect.any(Number) }); - expect(out.attrs.borders.right).toEqual({ val: 'single', color: '#AAAA00', size: expect.any(Number) }); - expect(out.attrs.borders.top).toEqual({ val: 'single', color: '#AA0000', size: expect.any(Number) }); - }); - it('moves leading bookmark markers into the first block within the cell', () => { const bookmarkStart = { type: 'bookmarkStart', attrs: { id: '0', name: 'title' } }; const bookmarkEnd = { type: 'bookmarkEnd', attrs: { id: '0' } }; @@ -629,8 +345,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -671,8 +385,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -709,8 +421,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -748,8 +458,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -786,8 +494,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], @@ -823,8 +529,6 @@ describe('legacy-handle-table-cell-node', () => { node: cellNode, table, row, - rowBorders: {}, - styleTag: null, columnIndex: 0, columnWidth: null, allColumnWidths: [], diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js index 6f6fe51e68..4c94d765a2 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/translate-table-cell.js @@ -1,12 +1,10 @@ -import { - pixelsToTwips, - inchesToTwips, - pixelsToEightPoints, - twipsToPixels, - eighthPointsToPixels, -} from '@converter/helpers'; +import { pixelsToTwips, inchesToTwips, twipsToPixels } from '@converter/helpers'; import { translateChildNodes } from '@converter/v2/exporter/helpers/index'; import { translator as tcPrTranslator } from '../../tcPr'; +import { + isLegacySchemaDefaultBorders, + convertBordersToOoxmlFormat, +} from '../../../../../../../extensions/table-cell/helpers/legacyBorderMigration.js'; /** * Main translation function for a table cell. @@ -34,7 +32,7 @@ export function translateTableCell(params) { * @returns {import('@converter/exporter').XmlReadyNode} */ export function generateTableCellProperties(node) { - const tableCellProperties = { ...(node.attrs?.tableCellProperties || {}) }; + let tableCellProperties = { ...(node.attrs?.tableCellProperties || {}) }; const { attrs } = node; @@ -112,35 +110,15 @@ export function generateTableCellProperties(node) { delete tableCellProperties.vMerge; } - const { borders = {} } = attrs; - if (!!borders && Object.keys(borders).length) { - ['top', 'bottom', 'left', 'right'].forEach((side) => { - if (borders[side]) { - let currentPropertyValue = tableCellProperties.borders?.[side]; - let currentPropertySizePixels = eighthPointsToPixels(currentPropertyValue?.size); - let color = borders[side].color; - if (borders[side].color && color === '#000000') { - color = 'auto'; - } - if ( - currentPropertySizePixels !== borders[side].size || - currentPropertyValue?.color !== color || - borders[side].val !== currentPropertyValue?.val - ) { - if (!tableCellProperties.borders) tableCellProperties['borders'] = {}; - tableCellProperties.borders[side] = { - size: pixelsToEightPoints(borders[side].size || 0), - color: color, - space: borders[side].space || 0, - val: borders[side].val || 'single', - }; - } - } else if (tableCellProperties.borders?.[side]) { - delete tableCellProperties.borders[side]; - } - }); - } else if (tableCellProperties?.borders) { - delete tableCellProperties.borders; + // Legacy fallback: if tableCellProperties.borders is absent but attrs.borders + // has non-default values, migrate them on the fly for export (read-only, no node mutation). + if (!tableCellProperties?.borders && attrs.borders != null) { + if (!isLegacySchemaDefaultBorders(attrs.borders)) { + tableCellProperties = { + ...(tableCellProperties ?? {}), + borders: convertBordersToOoxmlFormat(attrs.borders), + }; + } } const result = tcPrTranslator.decode({ node: { ...node, attrs: { ...node.attrs, tableCellProperties } } }); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/tc-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/tc-translator.js index 8550eeda1f..99881b6966 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/tc-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/tc-translator.js @@ -26,16 +26,9 @@ function encode(params, encodedAttrs) { table, row, tableProperties, - rowBorders, - baseTableBorders, - tableLook, - rowCnfStyle, columnIndex, columnWidth, columnWidths: allColumnWidths, - rowIndex, - totalRows, - totalColumns, preferTableGridWidths, _referencedStyles, } = params.extraParams; @@ -46,16 +39,9 @@ function encode(params, encodedAttrs) { table, row, tableProperties, - rowBorders, - baseTableBorders, - tableLook, - rowCnfStyle, columnIndex, columnWidth, allColumnWidths, - rowIndex, - totalRows, - totalColumns, preferTableGridWidths, _referencedStyles, }); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js index d2675685b4..de0d9b7096 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js @@ -1,6 +1,6 @@ // @ts-check import { NodeTranslator } from '@translator'; -import { twipsToPixels, pixelsToTwips, eighthPointsToPixels } from '@core/super-converter/helpers.js'; +import { twipsToPixels, pixelsToTwips } from '@core/super-converter/helpers.js'; import { createAttributeHandler } from '@converter/v3/handlers/utils.js'; import { translateChildNodes } from '@core/super-converter/v2/exporter/helpers/index.js'; @@ -38,7 +38,7 @@ const getColspan = (cell) => { * @returns {import('@translator').SCEncoderResult} */ const encode = (params, encodedAttrs) => { - const { row, tableLook } = params.extraParams; + const { row } = params.extraParams; let tableRowProperties = {}; const tPr = row.elements.find((el) => el.name === 'w:trPr'); @@ -52,20 +52,23 @@ const encode = (params, encodedAttrs) => { const safeGridBefore = typeof gridBeforeRaw === 'number' && Number.isFinite(gridBeforeRaw) && gridBeforeRaw > 0 ? gridBeforeRaw : 0; + // Preserve row-level table property exceptions (w:tblPrEx/w:tblBorders). + // These are per-row border overrides stored in raw OOXML format for the + // style-engine to factor into the cascade at resolution time. + const tblPrEx = row.elements?.find((el) => el.name === 'w:tblPrEx'); + const tblPrExBorders = tblPrEx?.elements?.find((el) => el.name === 'w:tblBorders'); + if (tblPrExBorders) { + const parsed = tblBordersTranslator.encode({ ...params, nodes: [tblPrExBorders] }); + if (parsed && Object.keys(parsed).length > 0) { + tableRowProperties = { ...tableRowProperties, tblPrExBorders: parsed }; + } + } + encodedAttrs['tableRowProperties'] = Object.freeze(tableRowProperties); // Move some properties up a level for easier access encodedAttrs['rowHeight'] = twipsToPixels(tableRowProperties['rowHeight']?.value); encodedAttrs['cantSplit'] = tableRowProperties['cantSplit']; - const rowCnfStyle = tableRowProperties?.cnfStyle; - - // Handle borders - const baseBorders = params.extraParams?.tableBorders; - const rowBorders = getRowBorders({ - params, - row, - baseBorders, - }); // Handling cells const { columnWidths: gridColumnWidths, activeRowSpans = [] } = params.extraParams; @@ -107,10 +110,6 @@ const encode = (params, encodedAttrs) => { path: [...(params.path || []), node], extraParams: { ...params.extraParams, - rowBorders, - baseTableBorders: baseBorders, - tableLook, - rowCnfStyle, node, columnIndex: startColumn, columnWidth, @@ -146,65 +145,6 @@ const encode = (params, encodedAttrs) => { return newNode; }; -/** - * Row-level table property exceptions (w:tblPrEx) can override table borders for a specific row. - * @param {Object} args - * @param {import('@translator').SCEncoderConfig} args.params - * @param {Object} args.row - OOXML element - * @param {Record | undefined} args.baseBorders - Processed base table borders for the table - * @returns {Record | undefined} - */ -function getRowBorders({ params, row, baseBorders }) { - const tblPrEx = row?.elements?.find?.((el) => el.name === 'w:tblPrEx'); - const tblBorders = tblPrEx?.elements?.find?.((el) => el.name === 'w:tblBorders'); - /** @type {Record} */ - const rowBaseBorders = {}; - if (baseBorders?.insideV) { - rowBaseBorders.insideV = baseBorders?.insideV; - } - - if (baseBorders?.insideH) { - rowBaseBorders.insideH = baseBorders?.insideH; - } - - if (!tblBorders) { - return rowBaseBorders; - } - - const rawOverrides = tblBordersTranslator.encode({ ...params, nodes: [tblBorders] }) || {}; - const overrides = processRawTableBorders(rawOverrides); - - if (!Object.keys(overrides).length) { - return rowBaseBorders; - } - - const rowBorders = { ...rowBaseBorders, ...overrides }; - return rowBorders; -} - -/** - * Normalize raw w:tblBorders output to match table border processing. - * @param {Object} [rawBorders] - * @returns {Object} - */ -function processRawTableBorders(rawBorders) { - const borders = {}; - Object.entries(rawBorders || {}).forEach(([name, attributes]) => { - const attrs = {}; - const color = attributes?.color; - const size = attributes?.size; - const val = attributes?.val; - - if (color && color !== 'auto') attrs.color = color.startsWith('#') ? color : `#${color}`; - if (size != null && size !== 'auto') attrs.size = eighthPointsToPixels(size); - if (val) attrs.val = val; - - borders[name] = attrs; - }); - - return borders; -} - /** * Decode the tableRow node back into OOXML . * @param {import('@translator').SCDecoderConfig} params @@ -285,9 +225,22 @@ const decode = (params, decodedAttrs) => { } } tableRowProperties['cantSplit'] = node.attrs['cantSplit']; + // Export tblPrEx borders back as w:tblPrEx before passing to trPr + // (tblPrEx is a sibling of trPr inside w:tr, not a child of trPr) + const { tblPrExBorders, ...trPrProperties } = tableRowProperties; + if (tblPrExBorders && typeof tblPrExBorders === 'object' && Object.keys(tblPrExBorders).length > 0) { + const bordersXml = tblBordersTranslator.decode({ + ...params, + node: { type: 'tableRow', attrs: { borders: tblPrExBorders } }, + }); + if (bordersXml) { + elements.unshift({ name: 'w:tblPrEx', elements: [bordersXml] }); + } + } + const trPr = trPrTranslator.decode({ ...params, - node: { ...node, attrs: { ...node.attrs, tableRowProperties } }, + node: { ...node, attrs: { ...node.attrs, tableRowProperties: trPrProperties } }, }); if (trPr) elements.unshift(trPr); } diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js index 36d445a0a8..ca548ffab4 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js @@ -32,12 +32,6 @@ vi.mock('../tc', () => ({ }, })); -vi.mock('../tblBorders', () => ({ - translator: { - encode: vi.fn(() => ({})), - }, -})); - vi.mock('../trPr', () => ({ translator: { encode: vi.fn(() => ({ encoded: 'trPr' })), @@ -48,7 +42,6 @@ vi.mock('../trPr', () => ({ import { translator } from './tr-translator.js'; import { NodeTranslator } from '@translator'; import { translator as tcTranslator } from '../tc'; -import { translator as tblBordersTranslator } from '../tblBorders'; import { translator as trPrTranslator } from '../trPr'; import { translateChildNodes } from '@core/super-converter/v2/exporter/helpers/index.js'; @@ -193,47 +186,6 @@ describe('w:tr translator', () => { expect(trPrTranslator.encode).not.toHaveBeenCalled(); expect(tcTranslator.encode).not.toHaveBeenCalled(); }); - - it('merges w:trPr/w:tblPrEx/w:tblBorders into tableBorders passed to cells', () => { - const baseTableBorders = { - top: { size: 0.5, val: 'single' }, - }; - - const tblBorders = { - name: 'w:tblBorders', - elements: [{ name: 'w:top', attributes: { 'w:val': 'nil' } }], - }; - - const mockRowWithOverride = { - name: 'w:tr', - elements: [ - { - name: 'w:tblPrEx', - elements: [tblBorders], - }, - { name: 'w:tc', elements: [] }, - ], - }; - - tblBordersTranslator.encode.mockReturnValue({ - top: { val: 'none' }, - }); - - const params = { - nodes: [mockRowWithOverride], - extraParams: { - row: mockRowWithOverride, - rowBorders: baseTableBorders, - columnWidths: [100], - activeRowSpans: [], - }, - }; - - translator.encode(params, {}); - - const firstCellCall = tcTranslator.encode.mock.calls[0][0]; - expect(firstCellCall.extraParams.rowBorders.top).toEqual({ val: 'none' }); - }); }); describe('decode', () => { diff --git a/packages/super-editor/src/document-api-adapters/document-settings.test.ts b/packages/super-editor/src/document-api-adapters/document-settings.test.ts new file mode 100644 index 0000000000..a8c28a9a40 --- /dev/null +++ b/packages/super-editor/src/document-api-adapters/document-settings.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, it } from 'vitest'; +import { + readDefaultTableStyle, + setDefaultTableStyle, + removeDefaultTableStyle, + ensureSettingsRoot, + readSettingsRoot, + hasOddEvenHeadersFooters, + type ConverterWithDocumentSettings, +} from './document-settings.ts'; + +function makeConverter(settingsElements: unknown[] = []): ConverterWithDocumentSettings { + return { + convertedXml: { + 'word/settings.xml': { + type: 'element', + name: 'document', + elements: [ + { + type: 'element', + name: 'w:settings', + elements: settingsElements, + }, + ], + }, + }, + }; +} + +describe('readDefaultTableStyle', () => { + it('returns the style ID when w:defaultTableStyle is present', () => { + const converter = makeConverter([ + { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': 'GridTable4-Accent1' }, + elements: [], + }, + ]); + const root = readSettingsRoot(converter)!; + expect(readDefaultTableStyle(root)).toBe('GridTable4-Accent1'); + }); + + it('returns null when w:defaultTableStyle is absent', () => { + const converter = makeConverter([{ type: 'element', name: 'w:evenAndOddHeaders', elements: [] }]); + const root = readSettingsRoot(converter)!; + expect(readDefaultTableStyle(root)).toBeNull(); + }); + + it('returns null for empty w:val', () => { + const converter = makeConverter([ + { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': '' }, + elements: [], + }, + ]); + const root = readSettingsRoot(converter)!; + expect(readDefaultTableStyle(root)).toBeNull(); + }); + + it('returns null when settings.xml is absent', () => { + const converter: ConverterWithDocumentSettings = { convertedXml: {} }; + const root = readSettingsRoot(converter); + expect(root).toBeNull(); + }); +}); + +describe('setDefaultTableStyle', () => { + it('adds w:defaultTableStyle when not present', () => { + const converter = makeConverter([]); + const root = ensureSettingsRoot(converter); + setDefaultTableStyle(root, 'TableGrid'); + expect(readDefaultTableStyle(root)).toBe('TableGrid'); + }); + + it('replaces existing w:defaultTableStyle', () => { + const converter = makeConverter([ + { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': 'OldStyle' }, + elements: [], + }, + ]); + const root = readSettingsRoot(converter)!; + setDefaultTableStyle(root, 'NewStyle'); + expect(readDefaultTableStyle(root)).toBe('NewStyle'); + }); + + it('preserves sibling settings elements', () => { + const converter = makeConverter([{ type: 'element', name: 'w:evenAndOddHeaders', elements: [] }]); + const root = readSettingsRoot(converter)!; + setDefaultTableStyle(root, 'TableGrid'); + expect(hasOddEvenHeadersFooters(root)).toBe(true); + expect(readDefaultTableStyle(root)).toBe('TableGrid'); + }); +}); + +describe('removeDefaultTableStyle', () => { + it('removes w:defaultTableStyle when present', () => { + const converter = makeConverter([ + { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': 'TableGrid' }, + elements: [], + }, + ]); + const root = readSettingsRoot(converter)!; + removeDefaultTableStyle(root); + expect(readDefaultTableStyle(root)).toBeNull(); + }); + + it('is a no-op when w:defaultTableStyle is absent', () => { + const converter = makeConverter([{ type: 'element', name: 'w:evenAndOddHeaders', elements: [] }]); + const root = readSettingsRoot(converter)!; + removeDefaultTableStyle(root); + expect(hasOddEvenHeadersFooters(root)).toBe(true); + }); + + it('preserves sibling elements', () => { + const converter = makeConverter([ + { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': 'TableGrid' }, + elements: [], + }, + { type: 'element', name: 'w:evenAndOddHeaders', elements: [] }, + ]); + const root = readSettingsRoot(converter)!; + removeDefaultTableStyle(root); + expect(readDefaultTableStyle(root)).toBeNull(); + expect(hasOddEvenHeadersFooters(root)).toBe(true); + }); +}); + +describe('defaultTableStyle roundtrip', () => { + it('set then read returns the same value', () => { + const converter = makeConverter([]); + const root = ensureSettingsRoot(converter); + setDefaultTableStyle(root, 'GridTable5-Dark-Accent2'); + expect(readDefaultTableStyle(root)).toBe('GridTable5-Dark-Accent2'); + }); + + it('set then remove then read returns null', () => { + const converter = makeConverter([]); + const root = ensureSettingsRoot(converter); + setDefaultTableStyle(root, 'TableGrid'); + removeDefaultTableStyle(root); + expect(readDefaultTableStyle(root)).toBeNull(); + }); +}); diff --git a/packages/super-editor/src/document-api-adapters/document-settings.ts b/packages/super-editor/src/document-api-adapters/document-settings.ts index 7c091ad7df..42ff1439e7 100644 --- a/packages/super-editor/src/document-api-adapters/document-settings.ts +++ b/packages/super-editor/src/document-api-adapters/document-settings.ts @@ -66,6 +66,55 @@ export function ensureSettingsRoot(converter: ConverterWithDocumentSettings): Xm return fallbackRoot; } +// ────────────────────────────────────────────────────────────────────────────── +// w:defaultTableStyle +// ────────────────────────────────────────────────────────────────────────────── + +/** + * Reads `w:defaultTableStyle` from settings.xml. + * Returns the style ID (`w:val`) or null if not present. + */ +export function readDefaultTableStyle(settingsRoot: XmlElement): string | null { + const el = settingsRoot.elements?.find((entry) => entry.name === 'w:defaultTableStyle'); + if (!el) return null; + const val = (el.attributes as Record | undefined)?.['w:val']; + return typeof val === 'string' && val.length > 0 ? val : null; +} + +/** + * Sets `w:defaultTableStyle` in settings.xml to the given style ID. + * Creates the element if absent, replaces it if already present. + */ +export function setDefaultTableStyle(settingsRoot: XmlElement, styleId: string): void { + const elements = ensureSettingsRootElements(settingsRoot); + const idx = elements.findIndex((entry) => entry.name === 'w:defaultTableStyle'); + + const newEl: XmlElement = { + type: 'element', + name: 'w:defaultTableStyle', + attributes: { 'w:val': styleId }, + elements: [], + }; + + if (idx !== -1) { + elements[idx] = newEl; + } else { + elements.push(newEl); + } +} + +/** + * Removes `w:defaultTableStyle` from settings.xml. + */ +export function removeDefaultTableStyle(settingsRoot: XmlElement): void { + const elements = ensureSettingsRootElements(settingsRoot); + settingsRoot.elements = elements.filter((entry) => entry.name !== 'w:defaultTableStyle'); +} + +// ────────────────────────────────────────────────────────────────────────────── +// w:evenAndOddHeaders +// ────────────────────────────────────────────────────────────────────────────── + export function hasOddEvenHeadersFooters(settingsRoot: XmlElement): boolean { return settingsRoot.elements?.some((entry) => entry.name === 'w:evenAndOddHeaders') === true; } diff --git a/packages/super-editor/src/document-api-adapters/tables-adapter.ts b/packages/super-editor/src/document-api-adapters/tables-adapter.ts index b93a595812..27f9e1ea59 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -384,13 +384,12 @@ function cellSidesForEdge( function tableBorderToCellBorder(border: Record): Record { const val = typeof border.val === 'string' ? border.val : 'single'; const color = typeof border.color === 'string' ? border.color : 'auto'; - const sizeEighthPoints = typeof border.size === 'number' ? border.size : 0; - const sizePx = val === 'none' || val === 'nil' ? 0 : (sizeEighthPoints / 8) * POINTS_TO_PIXELS; + const size = typeof border.size === 'number' ? border.size : 0; return { val, color, - size: sizePx, + size: val === 'none' || val === 'nil' ? 0 : size, space: 0, }; } @@ -423,14 +422,17 @@ function applyTableEdgeToCellBorders( if (!cellNode) continue; const cellAttrs = cellNode.attrs as Record; - const borders = { ...((cellAttrs.borders ?? {}) as Record) }; + const tcp = { ...((cellAttrs.tableCellProperties ?? {}) as Record) }; + const borders = { ...((tcp.borders ?? {}) as Record) }; for (const side of targetSides) { borders[side] = { ...cellBorder }; } + tcp.borders = borders; tr.setNodeMarkup(tr.mapping.slice(mapFrom).map(tableStart + relPos), null, { ...cellAttrs, - borders, + borders: null, + tableCellProperties: tcp, }); } } @@ -464,7 +466,8 @@ function applyTableBorderPresetToCellBorders( if (!cellNode) continue; const cellAttrs = cellNode.attrs as Record; - const borders = { ...((cellAttrs.borders ?? {}) as Record) }; + const tcp = { ...((cellAttrs.tableCellProperties ?? {}) as Record) }; + const borders = { ...((tcp.borders ?? {}) as Record) }; if (preset === 'none') { borders.top = { ...noneBorder }; @@ -483,10 +486,12 @@ function applyTableBorderPresetToCellBorders( borders.left = { ...singleBorder }; borders.right = { ...singleBorder }; } + tcp.borders = borders; tr.setNodeMarkup(tr.mapping.slice(mapFrom).map(tableStart + relPos), null, { ...cellAttrs, - borders, + borders: null, + tableCellProperties: tcp, }); } } @@ -2871,7 +2876,8 @@ export function tablesSetBorderAdapter( currentProps.borders = currentBorders; const syncAttrs = resolved.scope === 'table' ? syncExtractedTableAttrs(currentProps) : {}; - tr.setNodeMarkup(resolved.pos, null, { ...currentAttrs, [propsKey]: currentProps, ...syncAttrs }); + const cellClear = resolved.scope === 'cell' ? { borders: null } : {}; + tr.setNodeMarkup(resolved.pos, null, { ...currentAttrs, [propsKey]: currentProps, ...syncAttrs, ...cellClear }); if (resolved.scope === 'table' && isBoundaryEdge(input.edge)) { applyTableEdgeToCellBorders( @@ -2923,7 +2929,8 @@ export function tablesClearBorderAdapter( currentProps.borders = currentBorders; const syncAttrs = resolved.scope === 'table' ? syncExtractedTableAttrs(currentProps) : {}; - tr.setNodeMarkup(resolved.pos, null, { ...currentAttrs, [propsKey]: currentProps, ...syncAttrs }); + const cellClear = resolved.scope === 'cell' ? { borders: null } : {}; + tr.setNodeMarkup(resolved.pos, null, { ...currentAttrs, [propsKey]: currentProps, ...syncAttrs, ...cellClear }); if (resolved.scope === 'table' && isBoundaryEdge(input.edge)) { applyTableEdgeToCellBorders( diff --git a/packages/super-editor/src/extensions/table-cell/helpers/legacyBorderMigration.js b/packages/super-editor/src/extensions/table-cell/helpers/legacyBorderMigration.js new file mode 100644 index 0000000000..483f0b9e9b --- /dev/null +++ b/packages/super-editor/src/extensions/table-cell/helpers/legacyBorderMigration.js @@ -0,0 +1,54 @@ +// @ts-check + +/** + * Helpers for migrating legacy `attrs.borders` (px-based schema defaults) + * to `tableCellProperties.borders` (OOXML format, canonical source). + * + * Used by: + * - table.js appendTransaction plugin (Path A: editing-mode normalization) + * - translate-table-cell.js (Path B: exporter fallback for immediate export) + */ + +const PX_PER_PT = 96 / 72; + +/** Pixels to eighths-of-a-point (OOXML ST_EighthPointMeasure) */ +const pxToEighthPoints = (px) => Math.round((px / PX_PER_PT) * 8); + +const SIDES = ['top', 'right', 'bottom', 'left']; + +/** + * Detects the old `createCellBorders()` schema-default shape. + * These borders have `{ size, color }` without a `val` property on every side. + * + * @param {Record | null | undefined} borders + * @returns {boolean} + */ +export function isLegacySchemaDefaultBorders(borders) { + if (!borders || typeof borders !== 'object') return false; + return SIDES.every((side) => { + const b = borders[side]; + if (!b || typeof b !== 'object') return false; + return !('val' in b) && b.size === 0.66665 && b.color === '#000000'; + }); +} + +/** + * Converts old px-based `attrs.borders` to OOXML format for `tableCellProperties.borders`. + * + * @param {Record} borders - Old-format borders `{ top: { size, color, val? }, ... }` + * @returns {Record} OOXML-format borders `{ top: { val, size, space, color }, ... }` + */ +export function convertBordersToOoxmlFormat(borders) { + const result = {}; + for (const side of SIDES) { + const b = borders[side]; + if (!b || typeof b !== 'object') continue; + result[side] = { + val: b.val || 'single', + size: typeof b.size === 'number' ? pxToEighthPoints(b.size) : 4, + space: b.space || 0, + color: b.color === '#000000' ? 'auto' : b.color || 'auto', + }; + } + return result; +} diff --git a/packages/super-editor/src/extensions/table-cell/table-cell.js b/packages/super-editor/src/extensions/table-cell/table-cell.js index ad6432c38f..97b4b15f01 100644 --- a/packages/super-editor/src/extensions/table-cell/table-cell.js +++ b/packages/super-editor/src/extensions/table-cell/table-cell.js @@ -38,7 +38,6 @@ */ import { Node, Attribute } from '@core/index.js'; -import { createCellBorders } from './helpers/createCellBorders.js'; import { renderCellBorderStyle } from './helpers/renderCellBorderStyle.js'; /** @@ -190,8 +189,11 @@ export const TableCell = Node.create({ }, borders: { - default: () => createCellBorders(), - renderDOM: ({ borders }) => renderCellBorderStyle(borders), + default: null, + renderDOM: ({ borders }) => { + if (!borders) return {}; + return renderCellBorderStyle(borders); + }, }, widthType: { diff --git a/packages/super-editor/src/extensions/table-header/table-header.js b/packages/super-editor/src/extensions/table-header/table-header.js index ab03af2371..7d91569b30 100644 --- a/packages/super-editor/src/extensions/table-header/table-header.js +++ b/packages/super-editor/src/extensions/table-header/table-header.js @@ -1,7 +1,6 @@ // @ts-nocheck import { Node, Attribute } from '@core/index.js'; -import { createCellBorders } from '../table-cell/helpers/createCellBorders.js'; import { renderCellBorderStyle } from '../table-cell/helpers/renderCellBorderStyle.js'; /** @@ -122,8 +121,11 @@ export const TableHeader = Node.create({ }, borders: { - default: () => createCellBorders(), - renderDOM: ({ borders }) => renderCellBorderStyle(borders), + default: null, + renderDOM: ({ borders }) => { + if (!borders) return {}; + return renderCellBorderStyle(borders); + }, }, widthType: { diff --git a/packages/super-editor/src/extensions/table/table.import-width.test.js b/packages/super-editor/src/extensions/table/table.import-width.test.js index 1c9d52ed14..260e05831c 100644 --- a/packages/super-editor/src/extensions/table/table.import-width.test.js +++ b/packages/super-editor/src/extensions/table/table.import-width.test.js @@ -20,7 +20,8 @@ describe('Table import width defaults', () => { type: 'pct', }, }); - expect(attributes.tableStyleId.parseDOM(tableElement)).toBe('TableGrid'); + expect(attributes.tableStyleId.parseDOM(tableElement)).toBeNull(); + expect(attributes.needsTableStyleNormalization.parseDOM(tableElement)).toBe(true); }); it('leaves non-imported tables unchanged', () => { @@ -30,5 +31,6 @@ describe('Table import width defaults', () => { expect(attributes.tableProperties.parseDOM(tableElement)).toBeUndefined(); expect(attributes.tableStyleId.parseDOM(tableElement)).toBeUndefined(); + expect(attributes.needsTableStyleNormalization.parseDOM(tableElement)).toBeUndefined(); }); }); diff --git a/packages/super-editor/src/extensions/table/table.js b/packages/super-editor/src/extensions/table/table.js index 6fc9741e01..2d57154019 100644 --- a/packages/super-editor/src/extensions/table/table.js +++ b/packages/super-editor/src/extensions/table/table.js @@ -176,11 +176,15 @@ import { /* TableView */ createTableView } from './TableView.js'; import { createTable } from './tableHelpers/createTable.js'; import { createColGroup } from './tableHelpers/createColGroup.js'; import { deleteTableWhenSelected } from './tableHelpers/deleteTableWhenSelected.js'; -import { isInTable } from '@helpers/isInTable.js'; -import { createCellBorders } from '../table-cell/helpers/createCellBorders.js'; +import { normalizeNewTableAttrs } from './tableHelpers/normalizeNewTableAttrs.js'; import { createTableBorders } from './tableHelpers/createTableBorders.js'; +import { + isLegacySchemaDefaultBorders, + convertBordersToOoxmlFormat, +} from '../table-cell/helpers/legacyBorderMigration.js'; +import { isInTable } from '@helpers/isInTable.js'; import { findParentNode } from '@helpers/findParentNode.js'; -import { TextSelection } from 'prosemirror-state'; +import { TextSelection, Plugin, PluginKey } from 'prosemirror-state'; import { isCellSelection } from './tableHelpers/isCellSelection.js'; import { addColumnBefore as originalAddColumnBefore, @@ -489,13 +493,27 @@ export const Table = Node.create({ }, /** - * Ensure HTML-parsed tables get the default TableGrid style, matching - * Word's behavior of stamping every new table with this style reference. + * Table style reference. HTML-parsed tables get null here; the actual + * style is resolved via normalizeNewTableAttrs only when + * needsTableStyleNormalization=true. */ tableStyleId: { parseDOM: (element) => { if (!isImportedTableElement(element)) return undefined; - return 'TableGrid'; + return null; + }, + rendered: false, + }, + + /** + * Marker for imported/non-DOCX tables that still need style normalization. + * Existing DOCX tables may legitimately have tableStyleId=null. + */ + needsTableStyleNormalization: { + default: false, + parseDOM: (element) => { + if (!isImportedTableElement(element)) return undefined; + return true; }, rendered: false, }, @@ -620,7 +638,14 @@ export const Table = Node.create({ } } - const node = createTable(editor.schema, rows, cols, withHeaderRow, null, widths); + const resolved = normalizeNewTableAttrs(editor); + const tableAttrs = { + ...(resolved.tableStyleId ? { tableStyleId: resolved.tableStyleId } : {}), + ...(resolved.borders ? { borders: resolved.borders } : {}), + ...(resolved.tableProperties ? { tableProperties: resolved.tableProperties } : {}), + }; + + const node = createTable(editor.schema, rows, cols, withHeaderRow, null, widths, tableAttrs); if (dispatch) { let offset = tr.selection.$from.end() + 1; @@ -654,7 +679,7 @@ export const Table = Node.create({ */ insertTableAt: ({ pos, rows, columns, sdBlockId, paraId, tracked } = {}) => - ({ tr, state, dispatch }) => { + ({ tr, state, dispatch, editor }) => { const tableType = state.schema.nodes.table; const tableRowType = state.schema.nodes.tableRow; const tableCellType = state.schema.nodes.tableCell; @@ -679,8 +704,11 @@ export const Table = Node.create({ const row = tableRowType.createChecked(null, cellNodes); rowNodes.push(row); } + const resolved = normalizeNewTableAttrs(editor); const tableAttrs = { - tableStyleId: 'TableGrid', + ...(resolved.tableStyleId ? { tableStyleId: resolved.tableStyleId } : {}), + ...(resolved.borders ? { borders: resolved.borders } : {}), + ...(resolved.tableProperties ? { tableProperties: resolved.tableProperties } : {}), ...(sdBlockId ? { sdBlockId } : {}), ...(paraId ? { paraId } : {}), }; @@ -1220,12 +1248,22 @@ export const Table = Node.create({ const from = table.pos; const to = table.pos + table.node.nodeSize; - // remove from cells + // remove from cells — write nil borders to tableCellProperties.borders (canonical source) + const nilBorder = { val: 'nil', size: 0, space: 0, color: 'auto' }; state.doc.nodesBetween(from, to, (node, pos) => { if (['tableCell', 'tableHeader'].includes(node.type.name)) { tr.setNodeMarkup(pos, undefined, { ...node.attrs, - borders: createCellBorders({ size: 0, space: 0, val: 'none', color: 'auto' }), + borders: null, + tableCellProperties: { + ...(node.attrs.tableCellProperties ?? {}), + borders: { + top: { ...nilBorder }, + bottom: { ...nilBorder }, + left: { ...nilBorder }, + right: { ...nilBorder }, + }, + }, }); } }); @@ -1269,6 +1307,7 @@ export const Table = Node.create({ addPmPlugins() { const resizable = this.options.resizable && this.editor.isEditable; + const editor = this.editor; return [ ...(resizable @@ -1296,6 +1335,116 @@ export const Table = Node.create({ // @ts-expect-error - Options types will be fixed in TS migration allowTableNodeSelection: this.options.allowTableNodeSelection, }), + + // Normalize table style on paste / setContent / insertContent. + // Only tables explicitly marked with needsTableStyleNormalization + // are normalized, so DOCX-imported tables with tableStyleId=null keep + // their original semantics. + // + // The plugin applies the resolved style via + // normalizeNewTableAttrs, so every creation path is covered. + // + // On the first call, scans the entire document to catch tables + // present in the initial content (which don't come through a + // docChanged transaction). Subsequent calls only scan changed ranges. + (() => { + let initialScanDone = false; + return new Plugin({ + key: new PluginKey('tableStyleNormalization'), + appendTransaction(transactions, _oldState, newState) { + const needsInitialScan = !initialScanDone; + initialScanDone = true; + + const hasDocChange = transactions.some((t) => t.docChanged); + if (!hasDocChange && !needsInitialScan) return null; + + // Build scan ranges: full document on first call, changed ranges thereafter. + const ranges = []; + if (needsInitialScan) { + ranges.push(0, newState.doc.content.size); + } else { + const allMaps = []; + for (const t of transactions) { + for (let i = 0; i < t.mapping.maps.length; i++) { + allMaps.push(t.mapping.maps[i]); + } + } + for (let i = 0; i < allMaps.length; i++) { + allMaps[i].forEach((_oldFrom, _oldTo, newFrom, newTo) => { + for (let j = i + 1; j < allMaps.length; j++) { + newFrom = allMaps[j].map(newFrom, 1); + newTo = allMaps[j].map(newTo, -1); + } + ranges.push(newFrom, Math.max(newFrom, newTo)); + }); + } + } + if (ranges.length === 0) return null; + + let tr = null; + for (let r = 0; r < ranges.length; r += 2) { + const from = ranges[r]; + const to = Math.min(ranges[r + 1], newState.doc.content.size); + if (from >= to) continue; + newState.doc.nodesBetween(from, to, (node, pos) => { + if (node.type.name === 'table' && node.attrs.needsTableStyleNormalization === true) { + const attrs = { + ...node.attrs, + needsTableStyleNormalization: false, + }; + + // Respect explicit table borders from imported content. + if (!Object.keys(node.attrs.borders ?? {}).length) { + const resolved = normalizeNewTableAttrs(editor); + // Use undefined as sentinel for "normalized, no style found" + attrs.tableStyleId = resolved.tableStyleId ?? undefined; + if (resolved.borders) { + attrs.borders = resolved.borders; + } + if (resolved.tableProperties) { + attrs.tableProperties = { ...(node.attrs.tableProperties ?? {}), ...resolved.tableProperties }; + } + } + + if (!tr) tr = newState.tr; + tr.setNodeMarkup(pos, undefined, attrs); + } + + // Migrate legacy attrs.borders on cells to tableCellProperties.borders + if ( + (node.type.name === 'tableCell' || node.type.name === 'tableHeader') && + node.attrs.borders != null + ) { + const tcpBorders = node.attrs.tableCellProperties?.borders; + const cellAttrs = { ...node.attrs }; + + if (tcpBorders && typeof tcpBorders === 'object' && Object.keys(tcpBorders).length > 0) { + // Cell already has canonical inline borders — clear legacy attrs.borders + cellAttrs.borders = null; + } else if (isLegacySchemaDefaultBorders(node.attrs.borders)) { + // Matches old schema-default shape — drop; style cascade owns borders + cellAttrs.borders = null; + } else { + // Non-default borders from a prior session — migrate to tableCellProperties.borders + cellAttrs.tableCellProperties = { + ...(node.attrs.tableCellProperties ?? {}), + borders: convertBordersToOoxmlFormat(node.attrs.borders), + }; + cellAttrs.borders = null; + } + + if (cellAttrs.borders !== node.attrs.borders) { + if (!tr) tr = newState.tr; + tr.setNodeMarkup(pos, undefined, cellAttrs); + } + } + }); + } + + return tr; + }, + }); + })(), ]; }, diff --git a/packages/super-editor/src/extensions/table/table.test.js b/packages/super-editor/src/extensions/table/table.test.js index 5d32765bf9..0a3d87123b 100644 --- a/packages/super-editor/src/extensions/table/table.test.js +++ b/packages/super-editor/src/extensions/table/table.test.js @@ -8,6 +8,7 @@ import { promises as fs } from 'fs'; // Cache DOCX data to avoid repeated file loading let cachedBlankDoc = null; let cachedBordersDoc = null; +let cachedNoTableStyleDoc = null; /** * Find the first table position within the provided document. @@ -38,6 +39,7 @@ describe('Table commands', async () => { beforeAll(async () => { cachedBlankDoc = await loadTestDataForEditorTests('blank-doc.docx'); cachedBordersDoc = await loadTestDataForEditorTests('SD-978-remove-table-borders.docx'); + cachedNoTableStyleDoc = await loadTestDataForEditorTests('ooxml-bold-rstyle-linked-combos-demo.docx'); }); const setupTestTable = async () => { @@ -846,22 +848,23 @@ describe('Table commands', async () => { const sharedTests = async () => { it('removes all borders on the table', async () => { - // Expect table cell borders to be removed + const nilBorders = Object.assign( + {}, + ...['top', 'left', 'bottom', 'right'].map((side) => ({ + [side]: { + color: 'auto', + size: 0, + space: 0, + val: 'nil', + }, + })), + ); + + // Expect cell borders cleared from attrs.borders, written to tableCellProperties.borders table.children.forEach((tableRow) => { tableRow.children.forEach((tableCell) => { - expect(tableCell.attrs.borders).toEqual( - Object.assign( - {}, - ...['top', 'left', 'bottom', 'right'].map((side) => ({ - [side]: { - color: 'auto', - size: 0, - space: 0, - val: 'none', - }, - })), - ), - ); + expect(tableCell.attrs.borders).toBeNull(); + expect(tableCell.attrs.tableCellProperties?.borders).toEqual(nilBorders); }); }); @@ -959,4 +962,21 @@ describe('Table commands', async () => { sharedTests(); }); }); + + describe('table style normalization', async () => { + it('does not force TableGrid on imported DOCX tables that have OOXML table metadata', async () => { + const { docx, media, mediaFiles, fonts } = cachedNoTableStyleDoc; + ({ editor } = initTestEditor({ content: docx, media, mediaFiles, fonts })); + + const tablePos = findTablePos(editor.state.doc); + expect(tablePos).not.toBeNull(); + + const tableNode = editor.state.doc.nodeAt(tablePos); + expect(tableNode?.type.name).toBe('table'); + expect(tableNode?.attrs.tableStyleId).toBeNull(); + expect(tableNode?.attrs.tableProperties?.tableStyleId).toBeUndefined(); + expect(Array.isArray(tableNode?.attrs.grid) && tableNode.attrs.grid.length > 0).toBe(true); + expect(tableNode?.attrs.tableProperties?.tblLook).toBeDefined(); + }); + }); }); diff --git a/packages/super-editor/src/extensions/table/tableHelpers/createTable.js b/packages/super-editor/src/extensions/table/tableHelpers/createTable.js index 97616ae5ca..2f86537489 100644 --- a/packages/super-editor/src/extensions/table/tableHelpers/createTable.js +++ b/packages/super-editor/src/extensions/table/tableHelpers/createTable.js @@ -1,7 +1,6 @@ // @ts-check import { getNodeType } from '@core/helpers/getNodeType.js'; import { createCell } from './createCell.js'; -import { createTableBorders } from './createTableBorders.js'; /** * Create a new table with specified dimensions @@ -13,7 +12,8 @@ import { createTableBorders } from './createTableBorders.js'; * @param {boolean} withHeaderRow - Create first row as header * @param {Object} [cellContent=null] - Initial cell content * @param {number[]} [columnWidths=null] - Array of pixel widths per column - * @returns {Object} Complete table node with borders + * @param {Object} [tableAttrsOverride=null] - Table attributes (tableStyleId, borders, tableProperties, etc.) + * @returns {Object} Complete table node * @example * const table = createTable(schema, 3, 3, true) * @example @@ -21,7 +21,15 @@ import { createTableBorders } from './createTableBorders.js'; * @example * const table = createTable(schema, 3, 3, false, null, [200, 100, 200]) */ -export const createTable = (schema, rowsCount, colsCount, withHeaderRow, cellContent = null, columnWidths = null) => { +export const createTable = ( + schema, + rowsCount, + colsCount, + withHeaderRow, + cellContent = null, + columnWidths = null, + tableAttrsOverride = null, +) => { const types = { table: getNodeType('table', schema), tableRow: getNodeType('tableRow', schema), @@ -53,7 +61,5 @@ export const createTable = (schema, rowsCount, colsCount, withHeaderRow, cellCon rows.push(types.tableRow.createChecked(rowAttrs, cellsToInsert)); } - const tableBorders = createTableBorders(); - - return types.table.createChecked({ borders: tableBorders, tableStyleId: 'TableGrid' }, rows); + return types.table.createChecked(tableAttrsOverride, rows); }; diff --git a/packages/super-editor/src/extensions/table/tableHelpers/normalizeNewTableAttrs.js b/packages/super-editor/src/extensions/table/tableHelpers/normalizeNewTableAttrs.js new file mode 100644 index 0000000000..3b07e2571b --- /dev/null +++ b/packages/super-editor/src/extensions/table/tableHelpers/normalizeNewTableAttrs.js @@ -0,0 +1,82 @@ +// @ts-check +import { + resolvePreferredNewTableStyleId, + TABLE_FALLBACK_BORDERS, + TABLE_FALLBACK_CELL_PADDING, + TABLE_STYLE_ID_TABLE_GRID, +} from '@superdoc/style-engine/ooxml'; +import { readDefaultTableStyle, readSettingsRoot } from '../../../document-api-adapters/document-settings.js'; + +/** + * @typedef {Object} NormalizedTableAttrs + * @property {string | null} tableStyleId - Resolved style ID (null if source is 'none') + * @property {Object} [borders] - Fallback borders when no style exists + * @property {Object} [tableProperties] - Table properties including fallback cellMargins + */ + +/** + * Resolves the preferred table style ID from the editor's converter context. + * Encapsulates settings access and style catalog lookup. + * + * @param {Object} editor - The super-editor instance + * @returns {{ styleId: string | null, source: string }} + */ +export function resolvePreferredNewTableStyleIdFromEditor(editor) { + const converter = editor?.converter; + const translatedLinkedStyles = converter?.translatedLinkedStyles ?? null; + + let settingsDefaultStyleId = null; + if (converter) { + const settingsRoot = readSettingsRoot(converter); + if (settingsRoot) { + settingsDefaultStyleId = readDefaultTableStyle(settingsRoot); + } + } + + return resolvePreferredNewTableStyleId(settingsDefaultStyleId, translatedLinkedStyles); +} + +/** + * Computes the attributes for a newly created table. + * + * When a style is resolved, returns only `tableStyleId` — borders come from + * the style at render time via `resolveTableProperties`. + * When no style exists (`source: 'none'`), returns inline fallback borders + * and cell padding so the table renders with sensible defaults. + * + * @param {Object} editor - The super-editor instance + * @returns {NormalizedTableAttrs} + */ +export function normalizeNewTableAttrs(editor) { + const resolved = resolvePreferredNewTableStyleIdFromEditor(editor); + + if (resolved.source === 'none') { + return { + tableStyleId: null, + borders: { ...TABLE_FALLBACK_BORDERS }, + tableProperties: { + borders: { ...TABLE_FALLBACK_BORDERS }, + cellMargins: { + marginTop: { value: TABLE_FALLBACK_CELL_PADDING.top, type: 'dxa' }, + marginBottom: { value: TABLE_FALLBACK_CELL_PADDING.bottom, type: 'dxa' }, + marginLeft: { value: TABLE_FALLBACK_CELL_PADDING.left, type: 'dxa' }, + marginRight: { value: TABLE_FALLBACK_CELL_PADDING.right, type: 'dxa' }, + }, + }, + }; + } + + return { + tableStyleId: resolved.styleId, + // Also include tableStyleId inside tableProperties so the exporter's + // decodeProperties loop (which iterates Object.keys(tableProperties)) + // finds it and writes into . + tableProperties: { tableStyleId: resolved.styleId }, + }; +} + +/** + * Fallback style ID for standalone contexts (markdown import without editor). + * Matches Word behavior where `TableGrid` is always the default. + */ +export const STANDALONE_TABLE_STYLE_ID = TABLE_STYLE_ID_TABLE_GRID; diff --git a/packages/super-editor/src/extensions/table/tableHelpers/tableHelpers.test.js b/packages/super-editor/src/extensions/table/tableHelpers/tableHelpers.test.js index 93fca3a2cf..2f1b972e32 100644 --- a/packages/super-editor/src/extensions/table/tableHelpers/tableHelpers.test.js +++ b/packages/super-editor/src/extensions/table/tableHelpers/tableHelpers.test.js @@ -7,7 +7,6 @@ import { createCell } from './createCell.js'; import { createColGroup } from './createColGroup.js'; import { createTableBorders } from './createTableBorders.js'; import { getColStyleDeclaration } from './getColStyleDeclaration.js'; -import { createCellBorders } from '../../table-cell/helpers/createCellBorders.js'; import { deleteTableWhenSelected } from './deleteTableWhenSelected.js'; import { isCellSelection } from './isCellSelection.js'; import { cellAround } from './cellAround.js'; @@ -171,8 +170,9 @@ describe('tableHelpers', () => { expect(result.colgroupValues).toEqual([120, 150, 96]); }); - it('createTable builds tables with rows, optional header, and borders', () => { - const table = createTable(schema, 2, 3, true); + it('createTable builds tables with rows, optional header, and custom attrs', () => { + const tableAttrs = { tableStyleId: 'TableGrid', borders: { top: { size: 1 } } }; + const table = createTable(schema, 2, 3, true, null, null, tableAttrs); expect(table.type.name).toBe('table'); expect(table.firstChild.childCount).toBe(3); expect(table.attrs.borders.top).toBeDefined(); @@ -186,6 +186,12 @@ describe('tableHelpers', () => { expect(table.child(1).attrs.tableRowProperties?.repeatHeader).toBeFalsy(); }); + it('createTable builds tables without attrs when none provided', () => { + const table = createTable(schema, 2, 3, false); + expect(table.type.name).toBe('table'); + expect(table.childCount).toBe(2); + }); + it('createTable applies column widths when provided', () => { const columnWidths = [200, 100, 200]; const table = createTable(schema, 2, 3, false, null, columnWidths); @@ -465,7 +471,7 @@ describe('tableHelpers', () => { }); expect(newRow?.content.content[0].type.name).toBe('tableCell'); - expect(newRow?.content.content[0].attrs.borders).toEqual(createCellBorders()); + expect(newRow?.content.content[0].attrs.borders).toBeNull(); }); it('buildRowFromTemplateRow copies style when copyRowStyle is true', () => { @@ -550,7 +556,7 @@ describe('tableHelpers', () => { const updatedTable = tr.doc.nodeAt(tablePos); const insertedCell = updatedTable?.child(1)?.child(0); expect(insertedCell?.type.name).toBe('tableCell'); - expect(insertedCell?.attrs.borders).toEqual(createCellBorders()); + expect(insertedCell?.attrs.borders).toBeNull(); }); }); }); diff --git a/packages/super-editor/src/tests/import/tableImporter.test.js b/packages/super-editor/src/tests/import/tableImporter.test.js index 41a11920a1..379e720e87 100644 --- a/packages/super-editor/src/tests/import/tableImporter.test.js +++ b/packages/super-editor/src/tests/import/tableImporter.test.js @@ -508,20 +508,21 @@ describe('table live xml test', () => { expect(extractParagraphText(result.nodes[0].content[1].content[1].content[0])).toBe('COL 2 ROW 2'); }); - it('gets styles from base tab and parse internal borders', () => { + it('gets styles from base tab and parse internal borders via tableCellProperties', () => { const nodes = parseXmlToJson(nilBordersTableXml).elements; const styles = parseXmlToJson(simpleTableStyleXml); const docx = { 'word/styles.xml': styles, }; const result = tableNodeHandlerEntity.handler({ nodes, docx, nodeListHandler: defaultNodeListHandler() }); - expect(result.nodes[0].content[0].content[0].attrs.borders).toBeDefined(); - expect(result.nodes[0].content[0].content[1].attrs.borders).toBeDefined(); - expect(result.nodes[0].content[0].content[0].attrs.borders.right.val).toBe('none'); - expect(result.nodes[0].content[0].content[1].attrs.borders.bottom.val).toBe('none'); - expect(result.nodes[0].content[0].content[0].attrs.cellMargins).toBeDefined(); - expect(result.nodes[0].content[0].content[0].attrs.cellMargins.left).toBeCloseTo(7.2, 1); - expect(result.nodes[0].content[0].content[0].attrs.cellMargins.right).toBeCloseTo(7.2, 1); + // attrs.borders is no longer set during import — borders are in tableCellProperties.borders + const cell0 = result.nodes[0].content[0].content[0].attrs; + const cell1 = result.nodes[0].content[0].content[1].attrs; + expect(cell0.tableCellProperties?.borders?.right?.val).toBe('none'); + expect(cell1.tableCellProperties?.borders?.bottom?.val).toBe('none'); + expect(cell0.cellMargins).toBeDefined(); + expect(cell0.cellMargins.left).toBeCloseTo(7.2, 1); + expect(cell0.cellMargins.right).toBeCloseTo(7.2, 1); }); it('correctly gets colwidth for cells without inline width', () => { diff --git a/packages/super-editor/src/tests/regression/contract-acc-import.test.js b/packages/super-editor/src/tests/regression/contract-acc-import.test.js index 988bb31767..f018721421 100644 --- a/packages/super-editor/src/tests/regression/contract-acc-import.test.js +++ b/packages/super-editor/src/tests/regression/contract-acc-import.test.js @@ -9,6 +9,14 @@ describe('contract_acc import', () => { const json = editor.getJSON(); const table = json.content.find((node) => node.type === 'table'); expect(table).toBeDefined(); + // This DOCX intentionally has no tblStyle. Import should preserve that state + // and must not inject fallback table borders/cell margins. + expect(table.attrs.tableStyleId).toBeNull(); + expect(table.attrs.needsTableStyleNormalization).not.toBe(true); + expect(Object.keys(table.attrs.borders ?? {})).toHaveLength(0); + expect(table.attrs.tableProperties?.cellMargins?.marginLeft?.value).toBe(10); + expect(table.attrs.tableProperties?.cellMargins?.marginRight?.value).toBe(10); + // Collect the placeholder markers for each cell so we can assert that only metadata-only columns are tracked. const placeholderReasons = table.content.map((row) => row.content.map((cell) => cell.attrs?.__placeholder).filter(Boolean), @@ -25,13 +33,8 @@ describe('contract_acc import', () => { table.content.forEach((row) => { row.content.forEach((cell) => { if (!cell.attrs?.__placeholder) return; - const sides = ['top', 'right', 'bottom', 'left']; - // Placeholder cells should not render borders—double-check the translator stripped styling. - sides.forEach((side) => { - const border = cell.attrs.borders?.[side]; - expect(border?.val).toBe('none'); - expect(border?.size).toBe(0); - }); + // Placeholder cells should not have inline borders — borders are null (schema default). + expect(cell.attrs.borders).toBeNull(); }); }); editor.destroy(); From a8eb8ccb7e605118f30c7515df64af48770e5e9e Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Mon, 2 Mar 2026 20:52:25 -0800 Subject: [PATCH 2/2] chore: update tests --- .../layout-bridge/test/performance.test.ts | 12 +++-- .../src/multi-section-page-count.test.ts | 46 +++++++++++-------- .../tests/src/sd-1495-auto-page-break.test.ts | 21 ++++++++- 3 files changed, 56 insertions(+), 23 deletions(-) diff --git a/packages/layout-engine/layout-bridge/test/performance.test.ts b/packages/layout-engine/layout-bridge/test/performance.test.ts index 0ff96ecd4d..f732f4a044 100644 --- a/packages/layout-engine/layout-bridge/test/performance.test.ts +++ b/packages/layout-engine/layout-bridge/test/performance.test.ts @@ -24,6 +24,7 @@ beforeAll(() => { const describeIfRealCanvas = usingStub ? describe.skip : describe; const IS_CI = Boolean(process.env.CI); +const NON_CI_LATENCY_VARIANCE_FACTOR = 1.05; const LATENCY_TARGETS = IS_CI ? { // CI environments are slower and more variable; use generous buffers @@ -37,6 +38,11 @@ const LATENCY_TARGETS = IS_CI p99: 90, }; const MIN_HIT_RATE = 0.95; +const latencyBudget = (target: number): number => { + if (IS_CI) return target; + // Full-suite runs can introduce small scheduling variance; keep a tight but non-brittle budget. + return target * NON_CI_LATENCY_VARIANCE_FACTOR; +}; describeIfRealCanvas('incremental pipeline benchmarks', () => { it('meets latency and cache targets across document sizes', async () => { @@ -69,9 +75,9 @@ describeIfRealCanvas('incremental pipeline benchmarks', () => { ); } expect(result.actualPages).toBe(result.targetPages); - expect(result.latency.p50).toBeLessThanOrEqual(LATENCY_TARGETS.p50); - expect(result.latency.p90).toBeLessThanOrEqual(LATENCY_TARGETS.p90); - expect(result.latency.p99).toBeLessThanOrEqual(LATENCY_TARGETS.p99); + expect(result.latency.p50).toBeLessThanOrEqual(latencyBudget(LATENCY_TARGETS.p50)); + expect(result.latency.p90).toBeLessThanOrEqual(latencyBudget(LATENCY_TARGETS.p90)); + expect(result.latency.p99).toBeLessThanOrEqual(latencyBudget(LATENCY_TARGETS.p99)); if (result.targetPages >= 10) { expect(result.cache.hitRate).toBeGreaterThanOrEqual(MIN_HIT_RATE); } else { diff --git a/packages/layout-engine/tests/src/multi-section-page-count.test.ts b/packages/layout-engine/tests/src/multi-section-page-count.test.ts index 3cf287fa3f..983f1f9a3b 100644 --- a/packages/layout-engine/tests/src/multi-section-page-count.test.ts +++ b/packages/layout-engine/tests/src/multi-section-page-count.test.ts @@ -14,7 +14,7 @@ * @module multi-section-page-count.test */ -import { describe, it, expect } from 'vitest'; +import { describe, it, expect, beforeAll } from 'vitest'; import { toFlowBlocks } from '@superdoc/pm-adapter'; import { layoutDocument } from '@superdoc/layout-engine'; import { measureBlocks } from './test-helpers/section-test-utils.js'; @@ -24,6 +24,10 @@ import path from 'path'; import { fileURLToPath } from 'node:url'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const MULTI_SECTION_DOCX_PATH = path.join(__dirname, '../../../super-editor/src/tests/data/multi_section_doc.docx'); + +type LoadedMultiSectionFixture = Awaited>; +let loadedFixture: LoadedMultiSectionFixture | null = null; /** * Load DOCX file and convert to ProseMirror JSON @@ -139,22 +143,26 @@ function analyzeSectionBreaks(blocks: FlowBlock[]): { } describe('Multi-Section Document Page Count', () => { - it('should render multi_section_doc.docx as exactly 4 pages', async () => { - // Load the DOCX file - const docxPath = path.join(__dirname, '../../../super-editor/src/tests/data/multi_section_doc.docx'); + beforeAll(async () => { + if (!fs.existsSync(MULTI_SECTION_DOCX_PATH)) { + throw new Error(`Test document not found: ${MULTI_SECTION_DOCX_PATH}`); + } - if (!fs.existsSync(docxPath)) { - throw new Error(`Test document not found: ${docxPath}`); + // Load/convert once; this conversion is expensive under full-suite parallel runs. + loadedFixture = await docxToPMJson(MULTI_SECTION_DOCX_PATH); + }, 30000); + + it('should render multi_section_doc.docx as exactly 4 pages', async () => { + if (!loadedFixture) { + throw new Error('Expected test fixture to be loaded in beforeAll'); } - // Convert DOCX to PM JSON - console.log('Converting DOCX to ProseMirror JSON...'); - const { pmDoc, converterContext, themeColors } = await docxToPMJson(docxPath); + const { pmDoc, converterContext, themeColors } = loadedFixture; console.log(`PM Doc has ${pmDoc.content?.length ?? 0} top-level nodes`); // Convert PM JSON to flow blocks console.log('Converting to flow blocks...'); - const { blocks, bookmarks } = toFlowBlocks(pmDoc, { + const { blocks } = toFlowBlocks(pmDoc, { emitSectionBreaks: true, converterContext, themeColors, @@ -213,10 +221,11 @@ describe('Multi-Section Document Page Count', () => { } }); - it('should emit 3 section break blocks for a 4-section document', async () => { - const docxPath = path.join(__dirname, '../../../super-editor/src/tests/data/multi_section_doc.docx'); - - const { pmDoc, converterContext, themeColors } = await docxToPMJson(docxPath); + it('should emit 3 section break blocks for a 4-section document', () => { + if (!loadedFixture) { + throw new Error('Expected test fixture to be loaded in beforeAll'); + } + const { pmDoc, converterContext, themeColors } = loadedFixture; const { blocks } = toFlowBlocks(pmDoc, { emitSectionBreaks: true, converterContext, @@ -240,10 +249,11 @@ describe('Multi-Section Document Page Count', () => { expect(lastBreak.orientation).toBe('landscape'); }); - it('should have correct section break types', async () => { - const docxPath = path.join(__dirname, '../../../super-editor/src/tests/data/multi_section_doc.docx'); - - const { pmDoc, converterContext, themeColors } = await docxToPMJson(docxPath); + it('should have correct section break types', () => { + if (!loadedFixture) { + throw new Error('Expected test fixture to be loaded in beforeAll'); + } + const { pmDoc, converterContext, themeColors } = loadedFixture; const { blocks } = toFlowBlocks(pmDoc, { emitSectionBreaks: true, converterContext, diff --git a/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts b/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts index 4aef622f1f..2f3c02169b 100644 --- a/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts +++ b/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from 'vitest'; +import { beforeAll, describe, expect, it } from 'vitest'; import { layoutDocument } from '@superdoc/layout-engine'; import { toFlowBlocks, type ConverterContext } from '@superdoc/pm-adapter'; import { measureBlock } from '@superdoc/measuring-dom'; @@ -26,6 +26,7 @@ const FIXTURES: FixtureCase[] = [ const FIXTURE_DIR = path.join(__dirname, '../../../super-editor/src/tests/data'); const EXTENSIONS_TO_CONVERT = new Set(['.xml', '.rels']); +const fixtureCache = new Map(); async function loadDocxFixture(filename: string): Promise<{ pmDoc: PMNode; converterContext: ConverterContext }> { const { default: DocxZipper } = await import('../../../super-editor/src/core/DocxZipper.js'); @@ -90,8 +91,24 @@ function findPageIndex(layout: ReturnType, blockId?: stri } describe('SD-1495 auto page breaks', () => { + beforeAll(async () => { + const loadedFixtures = await Promise.all( + FIXTURES.map(async ({ filename }) => ({ + filename, + data: await loadDocxFixture(filename), + })), + ); + loadedFixtures.forEach(({ filename, data }) => { + fixtureCache.set(filename, data); + }); + }, 30000); + it.each(FIXTURES)('pushes heading to next page for %s', async ({ filename, headingText }) => { - const { pmDoc, converterContext } = await loadDocxFixture(filename); + const cachedFixture = fixtureCache.get(filename); + if (!cachedFixture) { + throw new Error(`Expected fixture "${filename}" to be loaded in beforeAll`); + } + const { pmDoc, converterContext } = cachedFixture; const { blocks } = toFlowBlocks(pmDoc, { emitSectionBreaks: true, converterContext }); const contentWidth = LETTER.pageSize.w - (LETTER.margins.left + LETTER.margins.right);