From acfc17b5a287b8319d8944f0a2a921a9b9c5a589 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 29 Jan 2026 23:45:49 +0200 Subject: [PATCH 1/8] fix: support cell spacing --- packages/layout-engine/contracts/src/index.ts | 24 +++++- .../layout-engine/layout-bridge/src/index.ts | 19 +++-- .../layout-engine/src/layout-table.ts | 20 ++++- .../layout-engine/measuring/dom/src/index.ts | 74 ++++++++++++++++++- .../painters/dom/src/renderer.ts | 3 +- .../dom/src/table/renderTableFragment.ts | 59 ++++++++++++--- .../painters/dom/src/table/renderTableRow.ts | 25 ++++--- .../pm-adapter/src/converters/table.ts | 4 + .../v3/handlers/w/tbl/tbl-translator.js | 2 +- .../helpers/legacy-handle-table-cell-node.js | 11 ++- .../src/extensions/table/table.js | 18 ++++- 11 files changed, 221 insertions(+), 38 deletions(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 77bd061260..b5a55ba83e 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -453,7 +453,7 @@ export type TableCellAttrs = { export type TableAttrs = { borders?: TableBorders; borderCollapse?: 'collapse' | 'separate'; - cellSpacing?: number; + cellSpacing?: CellSpacing; sdt?: SdtMetadata; containerSdt?: SdtMetadata; [key: string]: unknown; @@ -1410,12 +1410,34 @@ export type TableRowMeasure = { height: number; }; +/** Outer table border widths in pixels (top, right, bottom, left). Used for total dimensions and content offset. */ +export type TableBorderWidths = { + top: number; + right: number; + bottom: number; + left: number; +}; + export type TableMeasure = { kind: 'table'; rows: TableRowMeasure[]; columnWidths: number[]; totalWidth: number; totalHeight: number; + /** + * Cell spacing in pixels (border-spacing between cells). + * Used for total table dimensions and cell x/y positioning when border-collapse is 'separate'. + */ + cellSpacingPx?: number; + /** + * Outer table border widths in pixels. Included in totalWidth/totalHeight; content is offset by (left, top). + */ + tableBorderWidths?: TableBorderWidths; +}; + +export type CellSpacing = { + type: string; + value: number; }; export type SectionBreakMeasure = { diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index f9c681381f..34f27a4105 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1601,11 +1601,16 @@ export function selectionToRects( return rowMeasure?.height ?? 0; }); + const cellSpacingPx = tableMeasure.cellSpacingPx ?? 0; + const tableBorderWidths = tableMeasure.tableBorderWidths; + const contentOffsetX = tableBorderWidths?.left ?? 0; + const contentOffsetY = tableBorderWidths?.top ?? 0; + const calculateCellX = (cellIdx: number, cellMeasure: TableCellMeasure) => { const gridStart = cellMeasure.gridColumnStart ?? cellIdx; - let x = 0; + let x = cellSpacingPx; // space before first column for (let i = 0; i < gridStart && i < tableMeasure.columnWidths.length; i += 1) { - x += tableMeasure.columnWidths[i]; + x += tableMeasure.columnWidths[i] + cellSpacingPx; } return x; }; @@ -1736,14 +1741,15 @@ export function selectionToRects( wordLayout: cellWordLayout, }); - const rectX = fragment.x + cellX + padding.left + textIndentAdjust + Math.min(startX, endX); + const rectX = + fragment.x + contentOffsetX + cellX + padding.left + textIndentAdjust + Math.min(startX, endX); const rectWidth = Math.max( 1, Math.min(Math.abs(endX - startX), line.width), // clamp to line width to prevent runaway widths ); const lineOffset = lineHeightBeforeIndex(info.measure, index) - lineHeightBeforeIndex(info.measure, info.startLine); - const rectY = fragment.y + rowOffset + blockTopCursor + lineOffset; + const rectY = fragment.y + contentOffsetY + rowOffset + blockTopCursor + lineOffset; rects.push({ x: rectX, @@ -1761,15 +1767,18 @@ export function selectionToRects( return rowOffset + rowHeight; }; - let rowCursor = 0; + // First row starts after space before table content (space between table border and first row) + let rowCursor = cellSpacingPx; const repeatHeaderCount = tableFragment.repeatHeaderCount ?? 0; for (let r = 0; r < repeatHeaderCount && r < tableMeasure.rows.length; r += 1) { rowCursor = processRow(r, rowCursor); + rowCursor += cellSpacingPx; // spacing after every row (including last) for outer spacing } for (let r = tableFragment.fromRow; r < tableFragment.toRow && r < tableMeasure.rows.length; r += 1) { rowCursor = processRow(r, rowCursor); + rowCursor += cellSpacingPx; // spacing after every row (including last) for outer spacing } return; diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index b02c5d9fbe..c6fe990ea0 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -211,7 +211,8 @@ function calculateColumnMinWidth(): number { */ function generateColumnBoundaries(measure: TableMeasure): TableColumnBoundary[] { const boundaries: TableColumnBoundary[] = []; - let xPosition = 0; + const cellSpacingPx = measure.cellSpacingPx ?? 0; + let xPosition = cellSpacingPx; // space before first column for (let i = 0; i < measure.columnWidths.length; i++) { const width = measure.columnWidths[i]; @@ -227,7 +228,8 @@ function generateColumnBoundaries(measure: TableMeasure): TableColumnBoundary[] boundaries.push(boundary); - xPosition += width; + // Next boundary is after this column plus spacing (border-spacing between columns) + xPosition += width + cellSpacingPx; } return boundaries; @@ -291,14 +293,28 @@ function calculateFragmentHeight( _headerCount: number, ): number { let height = 0; + let rowCount = 0; // Add header height if continuation with repeated headers if (fragment.repeatHeaderCount && fragment.repeatHeaderCount > 0) { height += sumRowHeights(measure.rows, 0, fragment.repeatHeaderCount); + rowCount += fragment.repeatHeaderCount; } // Add body row heights (fromRow to toRow, exclusive) + const bodyRowCount = fragment.toRow - fragment.fromRow; height += sumRowHeights(measure.rows, fragment.fromRow, fragment.toRow); + rowCount += bodyRowCount; + + // Add vertical gaps: space before first row, between rows, after last row (outer spacing) + const cellSpacingPx = measure.cellSpacingPx ?? 0; + if (rowCount > 0 && cellSpacingPx > 0) { + height += (rowCount + 1) * cellSpacingPx; + } + if (rowCount > 0 && measure.tableBorderWidths) { + const borderWidthV = measure.tableBorderWidths.top + measure.tableBorderWidths.bottom; + height += borderWidthV; + } return height; } diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 20f39c3daf..49b7fcca21 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -60,6 +60,9 @@ import { type DrawingGeometry, type DropCapDescriptor, type TableWidthAttr, + type CellSpacing, + type TableBorders, + type TableBorderValue, } from '@superdoc/contracts'; import type { WordParagraphLayoutOutput } from '@superdoc/word-layout'; import { @@ -68,7 +71,6 @@ import { DEFAULT_LIST_INDENT_BASE_PX as DEFAULT_LIST_INDENT_BASE, DEFAULT_LIST_INDENT_STEP_PX as DEFAULT_LIST_INDENT_STEP, DEFAULT_LIST_HANGING_PX as DEFAULT_LIST_HANGING, - SPACE_SUFFIX_GAP_PX, } from '@superdoc/common/layout-constants'; import { resolveListTextStartPx, type MinimalMarker } from '@superdoc/common/list-marker-utils'; import { calculateRotatedBounds, normalizeRotation } from '@superdoc/geometry-utils'; @@ -147,6 +149,52 @@ const _PX_PER_PT = 96 / 72; // Reserved for future pt↔px conversions const twipsToPx = (twips: number): number => twips / TWIPS_PER_PX; const pxToTwips = (px: number): number => Math.round(px * TWIPS_PER_PX); +/** + * Resolves table cell spacing to pixels (for border-spacing). + * Handles number (px) or { type, value }. The editor/DOCX decoder often stores value + * already in pixels (twipsToPixels), so we use value as px. If value is in twips (raw OOXML), + * type is 'dxa' and we convert; otherwise value is treated as px. + */ +function getCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { + if (cellSpacing == null) return 0; + if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); + const v = cellSpacing.value; + if (typeof v !== 'number' || !Number.isFinite(v)) return 0; + const t = (cellSpacing.type ?? '').toLowerCase(); + // Editor/store often has value already in px; raw OOXML has twips (dxa). Only convert when value looks like twips (large). + const asPx = t === 'dxa' && v >= 20 ? twipsToPx(v) : v; + return Math.max(0, asPx); +} + +/** + * Returns the border width in pixels for a table border value (matches painter border-utils logic). + * Used so total table dimensions include outer border sizes and there is enough space for last row/column spacing. + */ +function getTableBorderWidthPx(value: TableBorderValue | null | undefined): number { + if (value == null) return 0; + if (typeof value === 'object' && 'none' in value && value.none) return 0; + const raw = value as { style?: string; width?: number; size?: number }; + const w = typeof raw.width === 'number' ? raw.width : typeof raw.size === 'number' ? raw.size : 1; + const width = Math.max(0, w); + if (raw.style === 'none') return 0; + if (raw.style === 'thick') return Math.max(width * 2, 3); + return width; +} + +/** Computes outer table border widths in px from table attrs (for total dimensions and content offset). */ +function getTableBorderWidths(borders: TableBorders | null | undefined): { + top: number; + right: number; + bottom: number; + left: number; +} { + const top = getTableBorderWidthPx(borders?.top); + const right = getTableBorderWidthPx(borders?.right); + const bottom = getTableBorderWidthPx(borders?.bottom); + const left = getTableBorderWidthPx(borders?.left); + return { top, right, bottom, left }; +} + const DEFAULT_TAB_INTERVAL_PX = twipsToPx(DEFAULT_TAB_INTERVAL_TWIPS); const TAB_EPSILON = 0.1; const DEFAULT_DECIMAL_SEPARATOR = '.'; @@ -2781,14 +2829,34 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai rows[i].height = Math.max(0, rowHeights[i]); } - const totalHeight = rowHeights.reduce((sum, h) => sum + h, 0); - const totalWidth = columnWidths.reduce((a, b) => a + b, 0); + const contentHeight = rowHeights.reduce((sum, h) => sum + h, 0); + const contentWidth = columnWidths.reduce((a, b) => a + b, 0); + + // Cell margins (OOXML cellMargins) are applied as cell padding (attrs.padding) and are already + // included in row heights and content width: row height = content + paddingTop + paddingBottom, + // and content width per cell = cellWidth - paddingLeft - paddingRight. + + // Cell spacing (border-spacing): gaps between cells plus space before first and after last row/column + const cellSpacingPx = getCellSpacingPx(block.attrs?.cellSpacing); + const numRows = block.rows.length; + const horizontalGaps = gridColumnCount > 0 ? (gridColumnCount + 1) * cellSpacingPx : 0; + const verticalGaps = numRows > 0 ? (numRows + 1) * cellSpacingPx : 0; + + // Outer table border widths: include in total dimensions so there is enough space for last row/column spacing + const tableBorderWidths = getTableBorderWidths(block.attrs?.borders); + const borderWidthH = tableBorderWidths.left + tableBorderWidths.right; + const borderWidthV = tableBorderWidths.top + tableBorderWidths.bottom; + const totalWidth = contentWidth + horizontalGaps + borderWidthH; + const totalHeight = contentHeight + verticalGaps + borderWidthV; + return { kind: 'table', rows, columnWidths, totalWidth, totalHeight, + cellSpacingPx: cellSpacingPx > 0 ? cellSpacingPx : undefined, + tableBorderWidths: borderWidthH > 0 || borderWidthV > 0 ? tableBorderWidths : undefined, }; } diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index ae68f933f5..ba9977414b 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -5865,7 +5865,8 @@ const deriveBlockVersion = (block: FlowBlock): string => { hash = hashString(hash, tblAttrs.borderCollapse); } if (tblAttrs.cellSpacing !== undefined) { - hash = hashNumber(hash, tblAttrs.cellSpacing); + const cs = tblAttrs.cellSpacing; + hash = typeof cs === 'number' ? hashNumber(hash, cs) : hashString(hash, JSON.stringify(cs)); } } diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index 9f3a8383c9..224dc75076 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -1,4 +1,5 @@ import type { + CellSpacing, DrawingBlock, Fragment, Line, @@ -13,9 +14,27 @@ import { DOM_CLASS_NAMES } from '../constants.js'; import type { FragmentRenderContext, BlockLookup } from '../renderer.js'; import { renderTableRow } from './renderTableRow.js'; import { applySdtContainerStyling, type SdtBoundaryOptions } from '../utils/sdt-helpers.js'; +import { applyBorder, borderValueToSpec } from './border-utils'; type ApplyStylesFn = (el: HTMLElement, styles: Partial) => void; +/** 15 twips per pixel (96 dpi). Used when resolving raw dxa values in painter fallback. */ +const TWIPS_PER_PX = 15; + +/** + * Resolves table cell spacing to pixels from block attrs (painter fallback when measure has no cellSpacingPx). + * Editor/store often has value already in px; raw OOXML has twips (dxa). Only convert when value looks like twips. + */ +function resolveCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { + if (cellSpacing == null) return 0; + if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); + const v = cellSpacing.value; + if (typeof v !== 'number' || !Number.isFinite(v)) return 0; + const t = (cellSpacing.type ?? '').toLowerCase(); + const asPx = t === 'dxa' && v >= 20 ? v / TWIPS_PER_PX : v; + return Math.max(0, asPx); +} + /** * Dependencies required for rendering a table fragment. * @@ -175,12 +194,23 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement container.style.height = `${fragment.height}px`; applySdtDataset(container, block.attrs?.sdt); + // Outer table border widths: reserve space so border is inside fragment size; content is offset + const tableBorderWidths = measure.tableBorderWidths; + if (tableBorderWidths) { + container.style.boxSizing = 'border-box'; + } + const contentLeft = tableBorderWidths?.left ?? 0; + const contentTop = tableBorderWidths?.top ?? 0; + // Apply SDT container styling (document sections, structured content blocks) applySdtContainerStyling(doc, container, block.attrs?.sdt, block.attrs?.containerSdt, sdtBoundary); // Add table-specific class for resize overlay targeting and click mapping container.classList.add(DOM_CLASS_NAMES.TABLE_FRAGMENT); + // Cell spacing in px (border-spacing). Use measure when present, else resolve from block attrs (e.g. stale/cached measure). + const cellSpacingPx = measure.cellSpacingPx ?? resolveCellSpacingPx(block.attrs?.cellSpacing) ?? 0; + // Add metadata for interactive table resizing if (fragment.metadata?.columnBoundaries) { // Build row-aware boundary segments scoped to THIS fragment's rows. @@ -221,7 +251,8 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // For each rendered row, determine which grid columns have cell boundaries // A boundary exists at column X if there's a cell that ENDS at column X (gridColumnStart + colSpan = X) - let rowY = 0; + // rowY includes outer spacing (before first row, between rows, after last) so segment positions match rendered cells + let rowY = cellSpacingPx; for (let i = 0; i < renderedRows.length; i++) { const { rowIndex, height } = renderedRows[i]; const rowMeasure = measure.rows[rowIndex]; @@ -266,13 +297,13 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement } } - rowY += height; + rowY += height + cellSpacingPx; } const metadata = { columns: fragment.metadata.columnBoundaries.map((boundary) => ({ i: boundary.index, - x: boundary.x, + x: boundary.x + contentLeft, w: boundary.width, min: boundary.minWidth, r: boundary.resizable ? 1 : 0, @@ -281,7 +312,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement segments: boundarySegments.map((segs, colIndex) => segs.map((seg) => ({ c: colIndex, // column index - y: seg.y, // y position + y: seg.y + contentTop, // y position (relative to table container) h: seg.height, // height of segment })), ), @@ -295,9 +326,12 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement container.setAttribute('data-sd-block-id', block.id); } - const borderCollapse = block.attrs?.borderCollapse || 'collapse'; - if (borderCollapse === 'separate' && block.attrs?.cellSpacing) { - container.style.borderSpacing = `${block.attrs.cellSpacing}px`; + const borderCollapse = block.attrs?.borderCollapse ?? (block.attrs?.cellSpacing != null ? 'separate' : 'collapse'); + if (borderCollapse === 'separate' && block.attrs?.cellSpacing && tableBorders) { + applyBorder(container, 'Top', borderValueToSpec(tableBorders.top)); + applyBorder(container, 'Right', borderValueToSpec(tableBorders.right)); + applyBorder(container, 'Bottom', borderValueToSpec(tableBorders.bottom)); + applyBorder(container, 'Left', borderValueToSpec(tableBorders.left)); } // Pre-calculate all row heights for rowspan calculations @@ -312,7 +346,8 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement return r?.height ?? 0; }); - let y = 0; + // First row starts after space before table content (space between table border and first row) + let y = cellSpacingPx; // If this is a continuation fragment with repeated headers, render headers first. // NOTE: This header-then-body iteration must stay in sync with the metadata @@ -341,8 +376,10 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement // Headers are always rendered as-is (no border suppression) continuesFromPrev: false, continuesOnNext: false, + cellSpacingPx, }); - y += rowMeasure.height; + // Add row height + spacing after every row (including last) for outer spacing after last row + y += rowMeasure.height + cellSpacingPx; } } @@ -382,8 +419,10 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement continuesOnNext: isLastRenderedBodyRow && fragment.continuesOnNext === true, // Pass partial row data for mid-row splits partialRow: partialRowData, + cellSpacingPx, }); - y += actualRowHeight; + // Add row height + spacing after every row (including last) for outer spacing after last row + y += actualRowHeight + cellSpacingPx; } return container; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index cc8cf221fe..b41f17891a 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -78,6 +78,12 @@ type TableRowRenderDependencies = { * only a portion of the row's content. */ partialRow?: PartialRowInfo; + + /** + * Cell spacing in pixels (border-spacing between cells). + * Applied to cell x positions and row y advancement. + */ + cellSpacingPx?: number; }; /** @@ -135,14 +141,15 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { continuesFromPrev, continuesOnNext, partialRow, + cellSpacingPx = 0, } = deps; /** * Calculates the horizontal position (x-coordinate) for a cell based on its grid column index. * - * Sums the widths of all columns preceding the given column index to determine - * the left edge position of a cell. This handles both normal cells and cells - * offset by rowspans from previous rows. + * Sums the widths of all columns preceding the given column index plus spacing between + * columns (border-spacing). When cellSpacingPx > 0, each column after the first is + * offset by one spacing unit, so x = sum(columnWidths[0..gridColumnStart-1]) + gridColumnStart * cellSpacingPx. * * **Bounds Safety:** * Loop terminates at the minimum of `gridColumnStart` and `columnWidths.length` @@ -153,17 +160,15 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { * * @example * ```typescript - * // columnWidths = [100, 150, 200] - * calculateXPosition(0) // Returns: 0 (first column) - * calculateXPosition(1) // Returns: 100 (after first column) - * calculateXPosition(2) // Returns: 250 (after first two columns) - * calculateXPosition(10) // Returns: 450 (safe - stops at array length) + * // columnWidths = [100, 150, 200], cellSpacingPx = 4 + * calculateXPosition(0) // Returns: cellSpacingPx (space before first column) + * calculateXPosition(1) // Returns: cellSpacingPx + columnWidths[0] + cellSpacingPx * ``` */ const calculateXPosition = (gridColumnStart: number): number => { - let x = 0; + let x = cellSpacingPx; // space before first column for (let i = 0; i < gridColumnStart && i < columnWidths.length; i++) { - x += columnWidths[i]; + x += columnWidths[i] + cellSpacingPx; } return x; }; diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index 00e944e4fe..6c62b60615 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -762,6 +762,10 @@ export function tableNodeToBlock( tableAttrs.tableIndent = { ...node.attrs.tableIndent }; } + if (defaultCellPadding && typeof defaultCellPadding === 'object') { + tableAttrs.defaultCellPadding = { ...defaultCellPadding }; + } + // Pass tableLayout through (extracted by tblLayout-translator.js) const tableLayout = node.attrs?.tableLayout; if (tableLayout) { 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 baf6d52d50..51593514e7 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 @@ -87,7 +87,7 @@ const encode = (params, encodedAttrs) => { 'justification', 'tableLayout', ['tableIndent', ({ value, type }) => ({ width: twipsToPixels(value), type })], - ['tableCellSpacing', ({ value, type }) => ({ w: String(value), type })], + ['tableCellSpacing', ({ value, type }) => ({ value: twipsToPixels(value), type })], ].forEach((prop) => { /** @type {string} */ let key; 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 6ad889fbbc..a2860e71a3 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 @@ -55,6 +55,7 @@ export function handleTableCellNode({ isLastColumn, tableCellProperties, referencedStyles, + hasBorderSpacing: !!tableProperties?.tableCellSpacing, }); // Colspan if (colspan > 1) attributes['colspan'] = colspan; @@ -304,19 +305,20 @@ const processCellBorders = ({ isLastColumn, tableCellProperties, referencedStyles, + hasBorderSpacing, }) => { let cellBorders = {}; if (baseTableBorders) { - if (isFirstRow && baseTableBorders.top) { + if ((isFirstRow || hasBorderSpacing) && baseTableBorders.top) { cellBorders.top = baseTableBorders.top; } - if (isLastRow && baseTableBorders.bottom) { + if ((isLastRow || hasBorderSpacing) && baseTableBorders.bottom) { cellBorders.bottom = baseTableBorders.bottom; } - if (isFirstColumn && baseTableBorders.left) { + if ((isFirstColumn || hasBorderSpacing) && baseTableBorders.left) { cellBorders.left = baseTableBorders.left; } - if (isLastColumn && baseTableBorders.right) { + if ((isLastColumn || hasBorderSpacing) && baseTableBorders.right) { cellBorders.right = baseTableBorders.right; } } @@ -387,6 +389,7 @@ const processCellBorders = ({ // Process inline cell borders (cell-level overrides) const inlineBorders = processInlineCellBorders(tableCellProperties.borders, cellBorders); if (inlineBorders) cellBorders = Object.assign(cellBorders, inlineBorders); + return cellBorders; }; diff --git a/packages/super-editor/src/extensions/table/table.js b/packages/super-editor/src/extensions/table/table.js index cc47ea8f1f..c6d036d0d6 100644 --- a/packages/super-editor/src/extensions/table/table.js +++ b/packages/super-editor/src/extensions/table/table.js @@ -354,6 +354,17 @@ export const Table = Node.create({ */ borders: { default: {}, + renderDOM({ borders, borderCollapse, tableCellSpacing }) { + if (!borders && borderCollapse !== 'separate' && !tableCellSpacing) return {}; + + const style = Object.entries(borders).reduce((acc, [key, { size, color }]) => { + return `${acc}border-${key}: ${Math.ceil(size)}px solid ${color || 'black'};`; + }, ''); + + return { + style, + }; + }, }, /** @@ -413,7 +424,12 @@ export const Table = Node.create({ */ tableCellSpacing: { default: null, - rendered: false, + renderDOM({ tableCellSpacing }) { + if (!tableCellSpacing?.value) return {}; + return { + style: `border-spacing: ${tableCellSpacing.value}px`, + }; + }, }, /** From 45e96aa90b0789f4a64cac01f80e3ede11f34141 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 29 Jan 2026 23:54:04 +0200 Subject: [PATCH 2/8] fix: update test --- .../core/super-converter/v3/handlers/w/tbl/tbl-translator.js | 2 +- .../super-converter/v3/handlers/w/tbl/tbl-translator.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 51593514e7..49786ff214 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 @@ -87,7 +87,7 @@ const encode = (params, encodedAttrs) => { 'justification', 'tableLayout', ['tableIndent', ({ value, type }) => ({ width: twipsToPixels(value), type })], - ['tableCellSpacing', ({ value, type }) => ({ value: twipsToPixels(value), type })], + ['tableCellSpacing', ({ value }) => ({ value: twipsToPixels(value), type: 'px' })], ].forEach((prop) => { /** @type {string} */ let key; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js index 3e3406fac4..242b89e9dc 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js @@ -151,7 +151,7 @@ describe('w:tbl translator', () => { expect(result.attrs.justification).toBe('center'); expect(result.attrs.tableIndent).toEqual({ width: 7.2, type: 'dxa' }); expect(result.attrs.tableLayout).toBe('fixed'); - expect(result.attrs.tableCellSpacing).toEqual({ w: '10', type: 'dxa' }); + expect(result.attrs.tableCellSpacing).toEqual({ value: 0.5, type: 'px' }); expect(result.attrs.borderCollapse).toBe('separate'); // Check borders (merged from style and inline) From b99511b44d6d5fecc16f0e04e7b3b0e6a11e7bd6 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 30 Jan 2026 18:32:43 +0200 Subject: [PATCH 3/8] fix: pr comments --- packages/layout-engine/layout-bridge/src/index.ts | 4 ++-- .../core/super-converter/v3/handlers/w/tbl/tbl-translator.js | 2 +- .../super-converter/v3/handlers/w/tbl/tbl-translator.test.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 34f27a4105..396a1c52eb 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1603,8 +1603,8 @@ export function selectionToRects( const cellSpacingPx = tableMeasure.cellSpacingPx ?? 0; const tableBorderWidths = tableMeasure.tableBorderWidths; - const contentOffsetX = tableBorderWidths?.left ?? 0; - const contentOffsetY = tableBorderWidths?.top ?? 0; + const contentOffsetX = tableBlock.attrs?.borderCollapse === 'separate' ? (tableBorderWidths?.left ?? 0) : 0; + const contentOffsetY = tableBlock.attrs?.borderCollapse === 'separate' ? (tableBorderWidths?.top ?? 0) : 0; const calculateCellX = (cellIdx: number, cellMeasure: TableCellMeasure) => { const gridStart = cellMeasure.gridColumnStart ?? cellIdx; 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 49786ff214..51593514e7 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 @@ -87,7 +87,7 @@ const encode = (params, encodedAttrs) => { 'justification', 'tableLayout', ['tableIndent', ({ value, type }) => ({ width: twipsToPixels(value), type })], - ['tableCellSpacing', ({ value }) => ({ value: twipsToPixels(value), type: 'px' })], + ['tableCellSpacing', ({ value, type }) => ({ value: twipsToPixels(value), type })], ].forEach((prop) => { /** @type {string} */ let key; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js index 242b89e9dc..9222e0a874 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tbl/tbl-translator.test.js @@ -151,7 +151,7 @@ describe('w:tbl translator', () => { expect(result.attrs.justification).toBe('center'); expect(result.attrs.tableIndent).toEqual({ width: 7.2, type: 'dxa' }); expect(result.attrs.tableLayout).toBe('fixed'); - expect(result.attrs.tableCellSpacing).toEqual({ value: 0.5, type: 'px' }); + expect(result.attrs.tableCellSpacing).toEqual({ value: 0.5, type: 'dxa' }); expect(result.attrs.borderCollapse).toBe('separate'); // Check borders (merged from style and inline) From b0565b63f4f27646a332a061124294c4ea86f9dc Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 5 Feb 2026 13:33:35 +0200 Subject: [PATCH 4/8] fix: review comments and tests --- packages/layout-engine/contracts/src/index.ts | 2 +- .../layout-engine/src/layout-table.test.ts | 186 +++++++++++++++++- .../layout-engine/src/layout-table.ts | 24 ++- .../layout-engine/measuring/dom/src/index.ts | 2 +- .../layout-engine/painters/dom/package.json | 1 + .../dom/src/table/renderTableFragment.ts | 21 +- .../src/extensions/table/table.js | 2 +- 7 files changed, 206 insertions(+), 32 deletions(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index b5a55ba83e..0a450cd54b 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -1436,7 +1436,7 @@ export type TableMeasure = { }; export type CellSpacing = { - type: string; + type: 'dxa' | 'px'; value: number; }; diff --git a/packages/layout-engine/layout-engine/src/layout-table.test.ts b/packages/layout-engine/layout-engine/src/layout-table.test.ts index 9a286b75c2..3fc2e1f35c 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.test.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.test.ts @@ -82,14 +82,17 @@ function createMockTableBlock( * Format: lineHeightsPerRow[rowIndex] = [lineHeight1, lineHeight2, ...] * If omitted, cells will have no lines. This parameter enables testing of mid-row * splitting behavior where rows are split at line boundaries. + * @param cellSpacingPx - Optional cell spacing in pixels (border-spacing). When set, + * column boundary x positions and fragment height include spacing. * @returns A TableMeasure object with mocked cell, row, and line data */ function createMockTableMeasure( columnWidths: number[], rowHeights: number[], lineHeightsPerRow?: number[][], + cellSpacingPx?: number, ): TableMeasure { - return { + const base = { kind: 'table', rows: rowHeights.map((height, rowIdx) => ({ cells: columnWidths.map((width) => ({ @@ -116,6 +119,10 @@ function createMockTableMeasure( totalWidth: columnWidths.reduce((sum, w) => sum + w, 0), totalHeight: rowHeights.reduce((sum, h) => sum + h, 0), }; + if (cellSpacingPx !== undefined) { + return { ...base, cellSpacingPx }; + } + return base; } describe('layoutTableBlock', () => { @@ -335,6 +342,183 @@ describe('layoutTableBlock', () => { }); }); + describe('cellSpacing', () => { + it('should position column boundaries with cellSpacingPx (space before first column and between columns)', () => { + const block = createMockTableBlock(1); + const measure = createMockTableMeasure([100, 150, 200], [20], undefined, 4); + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 458, // 4 + 100 + 4 + 150 + 4 + 200 + 4 + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 0, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 0, + }); + + const boundaries = fragments[0].metadata?.columnBoundaries; + expect(boundaries).toBeDefined(); + expect(boundaries!.length).toBe(3); + // First column: x = cellSpacingPx + expect(boundaries![0].x).toBe(4); + expect(boundaries![0].width).toBe(100); + // Second column: x = cellSpacingPx + col0 + cellSpacingPx + expect(boundaries![1].x).toBe(108); // 4 + 100 + 4 + expect(boundaries![1].width).toBe(150); + // Third column: x = prev + col1 + cellSpacingPx + expect(boundaries![2].x).toBe(262); // 108 + 150 + 4 + expect(boundaries![2].width).toBe(200); + }); + + it('should use zero column boundary offset when cellSpacingPx is 0', () => { + const block = createMockTableBlock(1); + const measure = createMockTableMeasure([100, 150], [20], undefined, 0); + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 250, + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 0, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 0, + }); + + const boundaries = fragments[0].metadata?.columnBoundaries; + expect(boundaries).toBeDefined(); + expect(boundaries![0].x).toBe(0); + expect(boundaries![1].x).toBe(100); + }); + + it('should include vertical cell spacing in fragment height', () => { + const block = createMockTableBlock(2); + const measure = createMockTableMeasure([100, 150], [20, 25], undefined, 4); + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 250, + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 50, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 10, + }); + + expect(fragments).toHaveLength(1); + // Row heights 20 + 25 = 45; vertical gaps (rowCount+1)*cellSpacingPx = 3*4 = 12 + expect(fragments[0].height).toBe(57); // 45 + 12 + }); + + it('should not add vertical spacing when cellSpacingPx is 0', () => { + const block = createMockTableBlock(2); + const measure = createMockTableMeasure([100, 150], [20, 25], undefined, 0); + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 250, + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 50, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 10, + }); + + expect(fragments).toHaveLength(1); + expect(fragments[0].height).toBe(45); // 20 + 25 only + }); + + it('should not add vertical spacing when measure.cellSpacingPx is undefined', () => { + const block = createMockTableBlock(2); + const measure = createMockTableMeasure([100, 150], [20, 25]); + + const fragments: TableFragment[] = []; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 250, + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY: 50, + contentBottom: 1000, + }), + advanceColumn: (state) => state, + columnX: () => 10, + }); + + expect(fragments).toHaveLength(1); + expect(fragments[0].height).toBe(45); + }); + + it('should include cell spacing in fragment height when table splits across pages', () => { + const block = createMockTableBlock(4); + const measure = createMockTableMeasure([100], [20, 20, 20, 20], undefined, 2); + + const fragments: TableFragment[] = []; + let cursorY = 0; + const mockPage = { fragments }; + + layoutTableBlock({ + block, + measure, + columnWidth: 100, + ensurePage: () => ({ + page: mockPage, + columnIndex: 0, + cursorY, + contentBottom: 50, // Fits 2 rows + spacing (2+20+2+20+2 = 46), not 3 rows (68) + }), + advanceColumn: (state) => { + cursorY = 0; + return { + page: mockPage, + columnIndex: 0, + cursorY: 0, + contentBottom: 50, + }; + }, + columnX: () => 0, + }); + + expect(fragments.length).toBeGreaterThan(1); + // First fragment: 2 rows => height = 20+20 + (2+1)*2 = 46 + expect(fragments[0].height).toBe(46); + // Second fragment: 2 rows => height = 46 + expect(fragments[1].height).toBe(46); + }); + }); + describe('justification alignment', () => { it('positions the table based on justification', () => { const measure = createMockTableMeasure([100, 100], [20]); diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index c6fe990ea0..c264d2f2d5 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -174,18 +174,24 @@ function resolveTableFrame( return applyTableIndent(baseX, width, tableIndent); } +const COLUMN_MIN_WIDTH_PX = 25; +const COLUMN_MAX_WIDTH_PX = 200; + /** - * Calculate minimum width for a table column. + * Calculate minimum width for a table column from its measured width. * - * Uses a conservative minimum of 10px per column to match PM's - * columnResizing behavior. + * Clamps the measured width to [COLUMN_MIN_WIDTH_PX, COLUMN_MAX_WIDTH_PX] + * so that resize handles enforce a sensible range (min 25px, max 200px). + * Invalid/negative/zero measured widths are treated as the minimum. * - * @returns Minimum width in pixels (10px) + * @param measuredWidth - Measured width in pixels (may be invalid) + * @returns Clamped minimum width in pixels */ -function calculateColumnMinWidth(): number { - const DEFAULT_MIN_WIDTH = 10; // Minimum usable column width in pixels - - return DEFAULT_MIN_WIDTH; +function calculateColumnMinWidth(measuredWidth: number): number { + if (!Number.isFinite(measuredWidth) || measuredWidth <= 0) { + return COLUMN_MIN_WIDTH_PX; + } + return Math.max(COLUMN_MIN_WIDTH_PX, Math.min(COLUMN_MAX_WIDTH_PX, measuredWidth)); } /** @@ -216,7 +222,7 @@ function generateColumnBoundaries(measure: TableMeasure): TableColumnBoundary[] for (let i = 0; i < measure.columnWidths.length; i++) { const width = measure.columnWidths[i]; - const minWidth = calculateColumnMinWidth(); + const minWidth = calculateColumnMinWidth(width); const boundary = { index: i, diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 49b7fcca21..8fb47092ff 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -155,7 +155,7 @@ const pxToTwips = (px: number): number => Math.round(px * TWIPS_PER_PX); * already in pixels (twipsToPixels), so we use value as px. If value is in twips (raw OOXML), * type is 'dxa' and we convert; otherwise value is treated as px. */ -function getCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { +export function getCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { if (cellSpacing == null) return 0; if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); const v = cellSpacing.value; diff --git a/packages/layout-engine/painters/dom/package.json b/packages/layout-engine/painters/dom/package.json index c61ced9133..efa053218e 100644 --- a/packages/layout-engine/painters/dom/package.json +++ b/packages/layout-engine/painters/dom/package.json @@ -19,6 +19,7 @@ "dependencies": { "@superdoc/contracts": "workspace:*", "@superdoc/font-utils": "workspace:*", + "@superdoc/measuring-dom": "workspace:*", "@superdoc/pm-adapter": "workspace:*", "@superdoc/preset-geometry": "workspace:*", "@superdoc/url-validation": "workspace:*" diff --git a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts index 224dc75076..bbf7edc16c 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableFragment.ts @@ -9,6 +9,7 @@ import type { TableFragment, TableMeasure, } from '@superdoc/contracts'; +import { getCellSpacingPx } from '@superdoc/measuring-dom'; import { CLASS_NAMES, fragmentStyles } from '../styles.js'; import { DOM_CLASS_NAMES } from '../constants.js'; import type { FragmentRenderContext, BlockLookup } from '../renderer.js'; @@ -17,24 +18,6 @@ import { applySdtContainerStyling, type SdtBoundaryOptions } from '../utils/sdt- import { applyBorder, borderValueToSpec } from './border-utils'; type ApplyStylesFn = (el: HTMLElement, styles: Partial) => void; - -/** 15 twips per pixel (96 dpi). Used when resolving raw dxa values in painter fallback. */ -const TWIPS_PER_PX = 15; - -/** - * Resolves table cell spacing to pixels from block attrs (painter fallback when measure has no cellSpacingPx). - * Editor/store often has value already in px; raw OOXML has twips (dxa). Only convert when value looks like twips. - */ -function resolveCellSpacingPx(cellSpacing: CellSpacing | number | null | undefined): number { - if (cellSpacing == null) return 0; - if (typeof cellSpacing === 'number') return Math.max(0, cellSpacing); - const v = cellSpacing.value; - if (typeof v !== 'number' || !Number.isFinite(v)) return 0; - const t = (cellSpacing.type ?? '').toLowerCase(); - const asPx = t === 'dxa' && v >= 20 ? v / TWIPS_PER_PX : v; - return Math.max(0, asPx); -} - /** * Dependencies required for rendering a table fragment. * @@ -209,7 +192,7 @@ export const renderTableFragment = (deps: TableRenderDependencies): HTMLElement container.classList.add(DOM_CLASS_NAMES.TABLE_FRAGMENT); // Cell spacing in px (border-spacing). Use measure when present, else resolve from block attrs (e.g. stale/cached measure). - const cellSpacingPx = measure.cellSpacingPx ?? resolveCellSpacingPx(block.attrs?.cellSpacing) ?? 0; + const cellSpacingPx = measure.cellSpacingPx ?? getCellSpacingPx(block.attrs?.cellSpacing); // Add metadata for interactive table resizing if (fragment.metadata?.columnBoundaries) { diff --git a/packages/super-editor/src/extensions/table/table.js b/packages/super-editor/src/extensions/table/table.js index c6d036d0d6..57ae7b201f 100644 --- a/packages/super-editor/src/extensions/table/table.js +++ b/packages/super-editor/src/extensions/table/table.js @@ -355,7 +355,7 @@ export const Table = Node.create({ borders: { default: {}, renderDOM({ borders, borderCollapse, tableCellSpacing }) { - if (!borders && borderCollapse !== 'separate' && !tableCellSpacing) return {}; + if (!Object.keys(borders).length && borderCollapse !== 'separate' && !tableCellSpacing) return {}; const style = Object.entries(borders).reduce((acc, [key, { size, color }]) => { return `${acc}border-${key}: ${Math.ceil(size)}px solid ${color || 'black'};`; From 8d0b4c7b5116c189d979bb58b595da573b7267b2 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 6 Feb 2026 15:04:34 +0200 Subject: [PATCH 5/8] fix: update deps --- pnpm-lock.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index af2e89b99e..0eb29f91e2 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -815,6 +815,9 @@ importers: '@superdoc/font-utils': specifier: workspace:* version: link:../../../../shared/font-utils + '@superdoc/measuring-dom': + specifier: workspace:* + version: link:../../measuring/dom '@superdoc/pm-adapter': specifier: workspace:* version: link:../../pm-adapter From c4ee80cf98b62c173d55421e6e6b7a683f2fba76 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 13 Feb 2026 22:04:50 +0200 Subject: [PATCH 6/8] fix: add default value for cellPadding --- .../pm-adapter/src/attributes/borders.ts | 5 +- .../pm-adapter/src/converters/table-styles.ts | 6 +- .../pm-adapter/src/utilities.test.ts | 445 ++++++++++++------ .../layout-engine/pm-adapter/src/utilities.ts | 19 + 4 files changed, 325 insertions(+), 150 deletions(-) diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.ts index 3a0594e38b..dfa85ab905 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.ts @@ -16,7 +16,7 @@ import type { TableBorderValue, } from '@superdoc/contracts'; import type { OoxmlBorder } from '../types.js'; -import { normalizeColor, pickNumber, isFiniteNumber } from '../utilities.js'; +import { normalizeColor, pickNumber, isFiniteNumber, normalizeCellPaddingTopBottom } from '../utilities.js'; import { PX_PER_PT } from '../constants.js'; const EIGHTHS_PER_POINT = 8; @@ -299,7 +299,8 @@ export function extractCellPadding(cellAttrs: Record): BoxSpaci if (typeof margins.bottom === 'number') padding.bottom = margins.bottom; if (typeof margins.left === 'number') padding.left = margins.left; - return Object.keys(padding).length > 0 ? padding : undefined; + if (Object.keys(padding).length === 0) return undefined; + return normalizeCellPaddingTopBottom(padding); } /** 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 158b9bc4d7..7087658016 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table-styles.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table-styles.ts @@ -3,7 +3,7 @@ import { _getReferencedTableStyles } from '@superdoc/super-editor/converter/inte import type { PMNode } from '../types.js'; import type { ConverterContext, TableStyleParagraphProps } from '../converter-context.js'; import { hasTableStyleContext } from '../converter-context.js'; -import { twipsToPx } from '../utilities.js'; +import { twipsToPx, normalizeCellPaddingTopBottom } from '../utilities.js'; import { normalizeLineValue } from '../attributes/spacing-indent.js'; export type TableStyleHydration = { @@ -31,7 +31,7 @@ export const hydrateTableStyleAttrs = (tableNode: PMNode, context?: ConverterCon if (tableProps) { const padding = convertCellMarginsToPx(tableProps.cellMargins as Record); - if (padding) hydration.cellPadding = padding; + if (padding) hydration.cellPadding = normalizeCellPaddingTopBottom(padding); if (tableProps.borders && typeof tableProps.borders === 'object') { hydration.borders = clonePlainObject(tableProps.borders as Record); @@ -57,7 +57,7 @@ export const hydrateTableStyleAttrs = (tableNode: PMNode, context?: ConverterCon } if (!hydration.cellPadding && referenced.cellMargins) { const padding = convertCellMarginsToPx(referenced.cellMargins as Record); - if (padding) hydration.cellPadding = padding; + if (padding) hydration.cellPadding = normalizeCellPaddingTopBottom(padding); } if (!hydration.justification && referenced.justification) { hydration.justification = referenced.justification; diff --git a/packages/layout-engine/pm-adapter/src/utilities.test.ts b/packages/layout-engine/pm-adapter/src/utilities.test.ts index 7e9fad8390..a0d44a543c 100644 --- a/packages/layout-engine/pm-adapter/src/utilities.test.ts +++ b/packages/layout-engine/pm-adapter/src/utilities.test.ts @@ -4,21 +4,28 @@ */ import { describe, it, expect } from 'vitest'; -import type { FlowBlock } from '@superdoc/contracts'; +import type { FlowBlock, ParagraphIndent } from '@superdoc/contracts'; import { twipsToPx, ptToPx, + pxToPt, + convertIndentTwipsToPx, isFiniteNumber, isPlainObject, normalizePrefix, pickNumber, + pickDecimalSeparator, + pickLang, normalizeColor, normalizeString, coerceNumber, coercePositiveNumber, coerceBoolean, toBoolean, + isTruthy, + isExplicitFalse, toBoxSpacing, + normalizeCellPaddingTopBottom, normalizeMediaKey, inferExtensionFromPath, hydrateImageBlocks, @@ -31,11 +38,6 @@ import { normalizeShapeGroupChildren, normalizeLineEnds, normalizeEffectExtent, - coerceRelativeHeight, - normalizeZIndex, - getFragmentZIndex, - resolveFloatingZIndex, - OOXML_Z_INDEX_BASE, } from './utilities.js'; // ============================================================================ @@ -77,6 +79,72 @@ describe('Unit Conversion', () => { expect(ptToPx(-Infinity)).toBeUndefined(); }); }); + + describe('pxToPt', () => { + it('converts pixels to points', () => { + expect(pxToPt(16)).toBeCloseTo(12, 1); + expect(pxToPt(0)).toBe(0); + expect(pxToPt(96)).toBe(72); // 96px = 1 inch = 72pt + }); + + it('returns undefined for null/undefined/non-finite', () => { + expect(pxToPt(null)).toBeUndefined(); + expect(pxToPt(undefined)).toBeUndefined(); + expect(pxToPt(NaN)).toBeUndefined(); + expect(pxToPt(Infinity)).toBeUndefined(); + }); + }); + + describe('convertIndentTwipsToPx', () => { + it('converts all indent properties', () => { + const result = convertIndentTwipsToPx({ + left: 1440, + right: 720, + firstLine: 360, + hanging: 180, + }); + expect(result).toEqual({ + left: 96, + right: 48, + firstLine: 24, + hanging: 12, + }); + }); + + it('handles partial indent objects', () => { + const result = convertIndentTwipsToPx({ left: 1440 }); + expect(result).toEqual({ left: 96 }); + }); + + it('returns undefined for null/undefined', () => { + expect(convertIndentTwipsToPx(null)).toBeUndefined(); + expect(convertIndentTwipsToPx(undefined)).toBeUndefined(); + }); + + it('returns undefined for empty indent', () => { + expect(convertIndentTwipsToPx({})).toBeUndefined(); + }); + + it('ignores non-finite values', () => { + const result = convertIndentTwipsToPx({ + left: 1440, + right: NaN, + firstLine: Infinity, + } as ParagraphIndent); + expect(result).toEqual({ left: 96 }); + }); + + it('handles multiple valid properties', () => { + const result = convertIndentTwipsToPx({ + left: 720, + hanging: 360, + }); + expect(result).toEqual({ + left: 48, + hanging: 24, + }); + }); + }); }); // ============================================================================ @@ -177,6 +245,47 @@ describe('Normalization', () => { }); }); + describe('pickDecimalSeparator', () => { + it('accepts valid decimal separators', () => { + expect(pickDecimalSeparator('.')).toBe('.'); + expect(pickDecimalSeparator(',')).toBe(','); + }); + + it('trims whitespace', () => { + expect(pickDecimalSeparator(' . ')).toBe('.'); + expect(pickDecimalSeparator(' , ')).toBe(','); + }); + + it('returns undefined for invalid values', () => { + expect(pickDecimalSeparator(';')).toBeUndefined(); + expect(pickDecimalSeparator('.')).toBe('.'); + expect(pickDecimalSeparator(42 as never)).toBeUndefined(); + expect(pickDecimalSeparator(null as never)).toBeUndefined(); + }); + }); + + describe('pickLang', () => { + it('normalizes language codes', () => { + expect(pickLang('en-US')).toBe('en-us'); + expect(pickLang('FR')).toBe('fr'); + }); + + it('trims whitespace', () => { + expect(pickLang(' en ')).toBe('en'); + }); + + it('returns undefined for non-strings', () => { + expect(pickLang(null as never)).toBeUndefined(); + expect(pickLang(undefined as never)).toBeUndefined(); + expect(pickLang(42 as never)).toBeUndefined(); + }); + + it('returns undefined for empty strings', () => { + expect(pickLang('')).toBeUndefined(); + expect(pickLang(' ')).toBeUndefined(); + }); + }); + describe('normalizeColor', () => { it('adds # prefix when missing', () => { expect(normalizeColor('FF0000')).toBe('#FF0000'); @@ -378,6 +487,45 @@ describe('Coercion', () => { expect(toBoolean(undefined)).toBeUndefined(); }); }); + + describe('isTruthy', () => { + it('returns true for explicit truthy values', () => { + expect(isTruthy(true)).toBe(true); + expect(isTruthy(1)).toBe(true); + expect(isTruthy('true')).toBe(true); + expect(isTruthy('1')).toBe(true); + expect(isTruthy('on')).toBe(true); + }); + + it('returns false for falsy and unknown values', () => { + expect(isTruthy(false)).toBe(false); + expect(isTruthy(0)).toBe(false); + expect(isTruthy('false')).toBe(false); + expect(isTruthy('no')).toBe(false); + expect(isTruthy('yes')).toBe(false); // Only recognizes true/1/on + expect(isTruthy(null)).toBe(false); + expect(isTruthy(undefined)).toBe(false); + }); + }); + + describe('isExplicitFalse', () => { + it('returns true for explicit false values', () => { + expect(isExplicitFalse(false)).toBe(true); + expect(isExplicitFalse(0)).toBe(true); + expect(isExplicitFalse('false')).toBe(true); + expect(isExplicitFalse('FALSE')).toBe(true); + expect(isExplicitFalse('0')).toBe(true); + expect(isExplicitFalse('off')).toBe(true); + }); + + it('returns false for non-false values', () => { + expect(isExplicitFalse(true)).toBe(false); + expect(isExplicitFalse(1)).toBe(false); + expect(isExplicitFalse('true')).toBe(false); + expect(isExplicitFalse(null)).toBe(false); + expect(isExplicitFalse(undefined)).toBe(false); + }); + }); }); // ============================================================================ @@ -424,6 +572,51 @@ describe('toBoxSpacing', () => { }); }); +describe('normalizeCellPaddingTopBottom', () => { + it('raises top padding in (0, 2) to 2px', () => { + expect(normalizeCellPaddingTopBottom({ top: 0.5 })).toEqual({ top: 2 }); + expect(normalizeCellPaddingTopBottom({ top: 1 })).toEqual({ top: 2 }); + expect(normalizeCellPaddingTopBottom({ top: 1.99 })).toEqual({ top: 2 }); + }); + + it('raises bottom padding in (0, 2) to 2px', () => { + expect(normalizeCellPaddingTopBottom({ bottom: 0.5 })).toEqual({ bottom: 2 }); + expect(normalizeCellPaddingTopBottom({ bottom: 1.5 })).toEqual({ bottom: 2 }); + }); + + it('leaves zero top/bottom unchanged', () => { + expect(normalizeCellPaddingTopBottom({ top: 0 })).toEqual({ top: 0 }); + expect(normalizeCellPaddingTopBottom({ bottom: 0 })).toEqual({ bottom: 0 }); + expect(normalizeCellPaddingTopBottom({ top: 0, bottom: 0 })).toEqual({ top: 0, bottom: 0 }); + }); + + it('leaves top/bottom >= 2 unchanged', () => { + expect(normalizeCellPaddingTopBottom({ top: 2 })).toEqual({ top: 2 }); + expect(normalizeCellPaddingTopBottom({ top: 5, bottom: 10 })).toEqual({ top: 5, bottom: 10 }); + }); + + it('does not modify left/right', () => { + const padding = { top: 1, right: 3, bottom: 1, left: 4 }; + expect(normalizeCellPaddingTopBottom(padding)).toEqual({ + top: 2, + right: 3, + bottom: 2, + left: 4, + }); + }); + + it('returns a shallow copy and normalizes only top/bottom', () => { + const padding = { top: 1, left: 8 }; + const result = normalizeCellPaddingTopBottom(padding); + expect(result).toEqual({ top: 2, left: 8 }); + expect(result).not.toBe(padding); + }); + + it('handles padding with only left/right unchanged', () => { + expect(normalizeCellPaddingTopBottom({ left: 5, right: 5 })).toEqual({ left: 5, right: 5 }); + }); +}); + // ============================================================================ // Media Utilities Tests (Bug Fixes) // ============================================================================ @@ -1499,191 +1692,153 @@ describe('normalizeEffectExtent', () => { }); // ============================================================================ -// Z-Index Utilities (OOXML relativeHeight) +// OOXML Utilities Tests // ============================================================================ -describe('z-index utilities', () => { - describe('coerceRelativeHeight', () => { - it('returns number when given a finite number', () => { - expect(coerceRelativeHeight(251658240)).toBe(251658240); - expect(coerceRelativeHeight(0)).toBe(0); - }); +import { + asOoxmlElement, + findOoxmlChild, + getOoxmlAttribute, + parseOoxmlNumber, + hasOwnProperty, + type OoxmlElement, +} from './utilities.js'; - it('returns number when given a numeric string', () => { - expect(coerceRelativeHeight('251658240')).toBe(251658240); - expect(coerceRelativeHeight('251659318')).toBe(251659318); +describe('OOXML Utilities', () => { + describe('asOoxmlElement', () => { + it('returns undefined for null/undefined', () => { + expect(asOoxmlElement(null)).toBeUndefined(); + expect(asOoxmlElement(undefined)).toBeUndefined(); }); - it('returns undefined for non-finite number', () => { - expect(coerceRelativeHeight(NaN)).toBeUndefined(); - expect(coerceRelativeHeight(Infinity)).toBeUndefined(); + it('returns undefined for non-objects', () => { + expect(asOoxmlElement('string')).toBeUndefined(); + expect(asOoxmlElement(42)).toBeUndefined(); }); - it('returns undefined for empty or invalid string', () => { - expect(coerceRelativeHeight('')).toBeUndefined(); - expect(coerceRelativeHeight(' ')).toBeUndefined(); - expect(coerceRelativeHeight('abc')).toBeUndefined(); + it('returns undefined for empty objects', () => { + expect(asOoxmlElement({})).toBeUndefined(); }); - it('returns undefined for null, undefined, or non-number/string', () => { - expect(coerceRelativeHeight(null)).toBeUndefined(); - expect(coerceRelativeHeight(undefined)).toBeUndefined(); - expect(coerceRelativeHeight({})).toBeUndefined(); + it('returns element with name property', () => { + const element = { name: 'w:p' }; + expect(asOoxmlElement(element)).toBe(element); }); - }); - describe('normalizeZIndex', () => { - it('returns 0 for OOXML base relativeHeight', () => { - expect(normalizeZIndex({ relativeHeight: OOXML_Z_INDEX_BASE })).toBe(0); - expect(normalizeZIndex({ relativeHeight: '251658240' })).toBe(0); + it('returns element with attributes property', () => { + const element = { attributes: { 'w:val': '240' } }; + expect(asOoxmlElement(element)).toBe(element); }); - it('returns positive z-index for relativeHeight above base', () => { - expect(normalizeZIndex({ relativeHeight: OOXML_Z_INDEX_BASE + 2 })).toBe(2); - expect(normalizeZIndex({ relativeHeight: OOXML_Z_INDEX_BASE + 51 })).toBe(51); - expect(normalizeZIndex({ relativeHeight: '251658291' })).toBe(51); + it('returns element with elements property', () => { + const element = { elements: [] }; + expect(asOoxmlElement(element)).toBe(element); }); - it('returns undefined when relativeHeight is missing or invalid', () => { - expect(normalizeZIndex({})).toBeUndefined(); - expect(normalizeZIndex(null)).toBeUndefined(); - expect(normalizeZIndex(undefined)).toBeUndefined(); - expect(normalizeZIndex({ relativeHeight: '' })).toBeUndefined(); + it('returns full OOXML element', () => { + const element: OoxmlElement = { + name: 'w:pPr', + attributes: { 'w:rsidR': '00A77B3E' }, + elements: [{ name: 'w:spacing', attributes: { 'w:before': '240' } }], + }; + expect(asOoxmlElement(element)).toBe(element); }); }); - describe('resolveFloatingZIndex', () => { - it('returns 0 when behindDoc is true', () => { - expect(resolveFloatingZIndex(true, 42)).toBe(0); - expect(resolveFloatingZIndex(true, undefined)).toBe(0); - expect(resolveFloatingZIndex(true, 0)).toBe(0); + describe('findOoxmlChild', () => { + it('returns undefined for undefined parent', () => { + expect(findOoxmlChild(undefined, 'w:spacing')).toBeUndefined(); }); - it('returns raw value when non-behindDoc and raw >= 1', () => { - expect(resolveFloatingZIndex(false, 5)).toBe(5); - expect(resolveFloatingZIndex(false, 100)).toBe(100); + it('returns undefined for parent without elements', () => { + expect(findOoxmlChild({ name: 'w:pPr' }, 'w:spacing')).toBeUndefined(); }); - it('clamps raw 0 to 1 for non-behindDoc', () => { - expect(resolveFloatingZIndex(false, 0)).toBe(1); + it('returns undefined when child not found', () => { + const parent: OoxmlElement = { + name: 'w:pPr', + elements: [{ name: 'w:jc' }], + }; + expect(findOoxmlChild(parent, 'w:spacing')).toBeUndefined(); }); - it('returns fallback when raw is undefined', () => { - expect(resolveFloatingZIndex(false, undefined)).toBe(1); - expect(resolveFloatingZIndex(false, undefined, 5)).toBe(5); + it('finds child element by name', () => { + const spacingEl: OoxmlElement = { name: 'w:spacing', attributes: { 'w:before': '240' } }; + const parent: OoxmlElement = { + name: 'w:pPr', + elements: [{ name: 'w:jc' }, spacingEl, { name: 'w:ind' }], + }; + expect(findOoxmlChild(parent, 'w:spacing')).toBe(spacingEl); }); + }); - it('clamps fallback to at least 1', () => { - expect(resolveFloatingZIndex(false, undefined, 0)).toBe(1); - expect(resolveFloatingZIndex(false, undefined, -1)).toBe(1); + describe('getOoxmlAttribute', () => { + it('returns undefined for undefined element', () => { + expect(getOoxmlAttribute(undefined, 'w:before')).toBeUndefined(); }); - }); - describe('getFragmentZIndex', () => { - it('uses block.zIndex when set', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - zIndex: 42, - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE } }, - }; - expect(getFragmentZIndex(block)).toBe(42); + it('returns undefined for element without attributes', () => { + expect(getOoxmlAttribute({ name: 'w:spacing' }, 'w:before')).toBeUndefined(); }); - it('derives z-index from attrs.originalAttributes.relativeHeight (number)', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE + 10 } }, - }; - expect(getFragmentZIndex(block)).toBe(10); + it('gets attribute with w: prefix', () => { + const element: OoxmlElement = { name: 'w:spacing', attributes: { 'w:before': '240' } }; + expect(getOoxmlAttribute(element, 'w:before')).toBe('240'); }); - it('derives z-index from attrs.originalAttributes.relativeHeight (string)', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - attrs: { originalAttributes: { relativeHeight: '251658250' } }, - }; - expect(getFragmentZIndex(block)).toBe(10); + it('gets attribute without prefix when prefixed key requested', () => { + const element: OoxmlElement = { name: 'w:spacing', attributes: { before: '240' } }; + expect(getOoxmlAttribute(element, 'w:before')).toBe('240'); }); - it('preserves high z-index for wrapped anchored objects', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - anchor: { isAnchored: true, behindDoc: false }, - wrap: { type: 'Through' as const }, - zIndex: 7168, - }; - expect(getFragmentZIndex(block)).toBe(7168); + it('gets attribute with prefix when unprefixed key requested', () => { + const element: OoxmlElement = { name: 'w:spacing', attributes: { 'w:before': '240' } }; + expect(getOoxmlAttribute(element, 'before')).toBe('240'); }); + }); - it('preserves relativeHeight z-index for wrap None anchored objects', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - anchor: { isAnchored: true, behindDoc: false }, - wrap: { type: 'None' as const }, - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE + 10 } }, - }; - expect(getFragmentZIndex(block)).toBe(10); + describe('parseOoxmlNumber', () => { + it('returns undefined for null/undefined', () => { + expect(parseOoxmlNumber(null)).toBeUndefined(); + expect(parseOoxmlNumber(undefined)).toBeUndefined(); }); - it('returns 0 when anchor.behindDoc is true and no zIndex/originalAttributes', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - anchor: { isAnchored: true, behindDoc: true }, - }; - expect(getFragmentZIndex(block)).toBe(0); + it('returns number for number input', () => { + expect(parseOoxmlNumber(240)).toBe(240); + expect(parseOoxmlNumber(0)).toBe(0); + expect(parseOoxmlNumber(-100)).toBe(-100); }); - it('returns 1 when not behindDoc and no zIndex/originalAttributes', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - }; - expect(getFragmentZIndex(block)).toBe(1); + it('parses string to integer', () => { + expect(parseOoxmlNumber('240')).toBe(240); + expect(parseOoxmlNumber('0')).toBe(0); + expect(parseOoxmlNumber('-100')).toBe(-100); }); - it('does not treat base relativeHeight as behindDoc when behindDoc is false', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - anchor: { isAnchored: true, behindDoc: false }, - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE } }, - }; - expect(getFragmentZIndex(block)).toBeGreaterThan(0); + it('returns undefined for non-numeric strings', () => { + expect(parseOoxmlNumber('abc')).toBeUndefined(); + expect(parseOoxmlNumber('')).toBeUndefined(); }); - it('forces behindDoc fragments to zIndex 0 even with relativeHeight', () => { - const block = { - kind: 'image' as const, - id: 'img-1', - src: 'x.png', - anchor: { isAnchored: true, behindDoc: true }, - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE + 5 } }, - }; - expect(getFragmentZIndex(block)).toBe(0); + it('returns undefined for non-finite results', () => { + expect(parseOoxmlNumber(NaN)).toBeUndefined(); + expect(parseOoxmlNumber(Infinity)).toBeUndefined(); }); + }); - it('works for drawing blocks', () => { - const block = { - kind: 'drawing' as const, - id: 'd-1', - drawingKind: 'vectorShape' as const, - attrs: { originalAttributes: { relativeHeight: OOXML_Z_INDEX_BASE + 5 } }, - }; - expect(getFragmentZIndex(block)).toBe(5); + describe('hasOwnProperty', () => { + it('returns true for own properties', () => { + expect(hasOwnProperty({ a: 1 }, 'a')).toBe(true); + expect(hasOwnProperty({ a: undefined }, 'a')).toBe(true); + }); + + it('returns false for missing properties', () => { + expect(hasOwnProperty({ a: 1 }, 'b')).toBe(false); + }); + + it('returns false for inherited properties', () => { + expect(hasOwnProperty({ a: 1 }, 'toString')).toBe(false); + expect(hasOwnProperty({ a: 1 }, 'hasOwnProperty')).toBe(false); }); }); }); diff --git a/packages/layout-engine/pm-adapter/src/utilities.ts b/packages/layout-engine/pm-adapter/src/utilities.ts index 73ea7b3c6c..978a1621b7 100644 --- a/packages/layout-engine/pm-adapter/src/utilities.ts +++ b/packages/layout-engine/pm-adapter/src/utilities.ts @@ -416,6 +416,25 @@ export function toBoxSpacing(spacing?: Record): BoxSpacing | un return Object.keys(result).length > 0 ? result : undefined; } +/** Minimum top/bottom cell padding (px) when imported value is in (0, 2). */ +const MIN_TOP_BOTTOM_CELL_PADDING_PX = 2; + +/** + * Normalizes top/bottom cell padding: values greater than 0 but less than 2px + * are raised to 2px so small imported values remain usable. Zero and values >= 2 + * are unchanged. + */ +export function normalizeCellPaddingTopBottom(padding: BoxSpacing): BoxSpacing { + const out = { ...padding }; + if (typeof out.top === 'number' && out.top > 0 && out.top < MIN_TOP_BOTTOM_CELL_PADDING_PX) { + out.top = MIN_TOP_BOTTOM_CELL_PADDING_PX; + } + if (typeof out.bottom === 'number' && out.bottom > 0 && out.bottom < MIN_TOP_BOTTOM_CELL_PADDING_PX) { + out.bottom = MIN_TOP_BOTTOM_CELL_PADDING_PX; + } + return out; +} + // ============================================================================ // Position Map Building // ============================================================================ From ef0ec0354b30cf7a02208e1ba70a7b043c9d55c2 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 13 Feb 2026 22:54:31 +0200 Subject: [PATCH 7/8] fix: consider codex comments --- .../layout-engine/src/layout-table.ts | 34 +++++++++++++++---- .../layout-engine/measuring/dom/src/index.ts | 11 ++++-- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index c264d2f2d5..4a6ef50365 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -325,6 +325,27 @@ function calculateFragmentHeight( return height; } +/** + * Height of a body-only fragment (rows fromRow..toRow) including vertical spacing and borders. + * Must match the body portion of calculateFragmentHeight so findSplitPoint's fit check + * agrees with the actual rendered fragment height. + */ +function calculateBodyFragmentHeight(measure: TableMeasure, fromRow: number, toRow: number): number { + const rowCount = toRow - fromRow; + if (rowCount <= 0) { + return 0; + } + let height = sumRowHeights(measure.rows, fromRow, toRow); + const cellSpacingPx = measure.cellSpacingPx ?? 0; + if (cellSpacingPx > 0) { + height += (rowCount + 1) * cellSpacingPx; + } + if (measure.tableBorderWidths) { + height += measure.tableBorderWidths.top + measure.tableBorderWidths.bottom; + } + return height; +} + type SplitPointResult = { endRow: number; // Exclusive row index (next row after last included) partialRow: PartialRowInfo | null; // Null for row-boundary splits, PartialRowInfo for mid-row splits @@ -889,8 +910,7 @@ function findSplitPoint( fullPageHeight?: number, _pendingPartialRow?: PartialRowInfo | null, ): SplitPointResult { - let accumulatedHeight = 0; - let lastFitRow = startRow; // Last row that fit completely + let lastFitRow = startRow; // Last row that fit completely (exclusive end index) for (let i = startRow; i < block.rows.length; i++) { const row = block.rows[i]; @@ -901,14 +921,14 @@ function findSplitPoint( cantSplit = true; } - // Check if this row fits completely - if (accumulatedHeight + rowHeight <= availableHeight) { + // Check if this row fits: use full fragment height (rows + spacing + borders) so pagination matches render + const fragmentHeightWithRow = calculateBodyFragmentHeight(measure, startRow, i + 1); + if (fragmentHeightWithRow <= availableHeight) { // Row fits completely - accumulatedHeight += rowHeight; lastFitRow = i + 1; // Next row index (exclusive) } else { - // Row doesn't fit completely - const remainingHeight = availableHeight - accumulatedHeight; + // Row doesn't fit completely; remaining space after last full row set + const remainingHeight = availableHeight - calculateBodyFragmentHeight(measure, startRow, lastFitRow); // Check if this is an over-tall row (exceeds full page height) - force split regardless of cantSplit // This handles edge case where a row is taller than an entire page diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 8fb47092ff..69fa7c2e2a 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -2842,12 +2842,17 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai const horizontalGaps = gridColumnCount > 0 ? (gridColumnCount + 1) * cellSpacingPx : 0; const verticalGaps = numRows > 0 ? (numRows + 1) * cellSpacingPx : 0; - // Outer table border widths: include in total dimensions so there is enough space for last row/column spacing + // Outer table border widths: only add to total dimensions when borderCollapse === 'separate', + // since the DOM renderer only paints container-level outer borders in that path. For collapsed + // (default), borders are on cells and don't grow the table container, so including them would + // overstate size and cause premature wrapping/page breaks or alignment drift. const tableBorderWidths = getTableBorderWidths(block.attrs?.borders); const borderWidthH = tableBorderWidths.left + tableBorderWidths.right; const borderWidthV = tableBorderWidths.top + tableBorderWidths.bottom; - const totalWidth = contentWidth + horizontalGaps + borderWidthH; - const totalHeight = contentHeight + verticalGaps + borderWidthV; + const borderCollapse = block.attrs?.borderCollapse ?? (block.attrs?.cellSpacing != null ? 'separate' : 'collapse'); + const includeOuterBordersInTotal = borderCollapse === 'separate'; + const totalWidth = contentWidth + horizontalGaps + (includeOuterBordersInTotal ? borderWidthH : 0); + const totalHeight = contentHeight + verticalGaps + (includeOuterBordersInTotal ? borderWidthV : 0); return { kind: 'table', From 35b372110bc2dd13eed1e5663ae5fd27b4d9c496 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 13 Feb 2026 23:06:13 +0200 Subject: [PATCH 8/8] fix: add check for border-collapse --- .../layout-engine/src/layout-table.ts | 25 ++++++++++++++----- 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/packages/layout-engine/layout-engine/src/layout-table.ts b/packages/layout-engine/layout-engine/src/layout-table.ts index 4a6ef50365..d45c9dfbe0 100644 --- a/packages/layout-engine/layout-engine/src/layout-table.ts +++ b/packages/layout-engine/layout-engine/src/layout-table.ts @@ -297,6 +297,7 @@ function calculateFragmentHeight( fragment: Pick, measure: TableMeasure, _headerCount: number, + borderCollapse?: 'collapse' | 'separate', ): number { let height = 0; let rowCount = 0; @@ -317,7 +318,8 @@ function calculateFragmentHeight( if (rowCount > 0 && cellSpacingPx > 0) { height += (rowCount + 1) * cellSpacingPx; } - if (rowCount > 0 && measure.tableBorderWidths) { + // Only add outer border height when border-collapse is separate (DOM paints container-level borders only then) + if (rowCount > 0 && measure.tableBorderWidths && borderCollapse === 'separate') { const borderWidthV = measure.tableBorderWidths.top + measure.tableBorderWidths.bottom; height += borderWidthV; } @@ -328,9 +330,14 @@ function calculateFragmentHeight( /** * Height of a body-only fragment (rows fromRow..toRow) including vertical spacing and borders. * Must match the body portion of calculateFragmentHeight so findSplitPoint's fit check - * agrees with the actual rendered fragment height. + * agrees with the actual rendered fragment height. Borders only included when borderCollapse === 'separate'. */ -function calculateBodyFragmentHeight(measure: TableMeasure, fromRow: number, toRow: number): number { +function calculateBodyFragmentHeight( + measure: TableMeasure, + fromRow: number, + toRow: number, + borderCollapse?: 'collapse' | 'separate', +): number { const rowCount = toRow - fromRow; if (rowCount <= 0) { return 0; @@ -340,7 +347,7 @@ function calculateBodyFragmentHeight(measure: TableMeasure, fromRow: number, toR if (cellSpacingPx > 0) { height += (rowCount + 1) * cellSpacingPx; } - if (measure.tableBorderWidths) { + if (measure.tableBorderWidths && borderCollapse === 'separate') { height += measure.tableBorderWidths.top + measure.tableBorderWidths.bottom; } return height; @@ -911,6 +918,7 @@ function findSplitPoint( _pendingPartialRow?: PartialRowInfo | null, ): SplitPointResult { let lastFitRow = startRow; // Last row that fit completely (exclusive end index) + const borderCollapse = block.attrs?.borderCollapse ?? (block.attrs?.cellSpacing != null ? 'separate' : 'collapse'); for (let i = startRow; i < block.rows.length; i++) { const row = block.rows[i]; @@ -922,13 +930,14 @@ function findSplitPoint( } // Check if this row fits: use full fragment height (rows + spacing + borders) so pagination matches render - const fragmentHeightWithRow = calculateBodyFragmentHeight(measure, startRow, i + 1); + const fragmentHeightWithRow = calculateBodyFragmentHeight(measure, startRow, i + 1, borderCollapse); if (fragmentHeightWithRow <= availableHeight) { // Row fits completely lastFitRow = i + 1; // Next row index (exclusive) } else { // Row doesn't fit completely; remaining space after last full row set - const remainingHeight = availableHeight - calculateBodyFragmentHeight(measure, startRow, lastFitRow); + const remainingHeight = + availableHeight - calculateBodyFragmentHeight(measure, startRow, lastFitRow, borderCollapse); // Check if this is an over-tall row (exceeds full page height) - force split regardless of cantSplit // This handles edge case where a row is taller than an entire page @@ -1183,6 +1192,9 @@ export function layoutTableBlock({ return; } + // Resolve border-collapse for fragment height (match measuring/render: only add borders when separate) + const borderCollapse = block.attrs?.borderCollapse ?? (block.attrs?.cellSpacing != null ? 'separate' : 'collapse'); + // 4. Loop until all rows processed (including pending partial rows) while (currentRow < block.rows.length || pendingPartialRow !== null) { state = ensurePage(); @@ -1345,6 +1357,7 @@ export function layoutTableBlock({ { fromRow: bodyStartRow, toRow: endRow, repeatHeaderCount }, measure, headerCount, + borderCollapse, ); }