From ddb52442847fcba8d952762ee364147c2d0246b6 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Mon, 26 Jan 2026 20:00:42 +0200 Subject: [PATCH 1/7] fix: before paragraph spacing inside table cells --- .../layout-engine/layout-bridge/src/index.ts | 5 + .../layout-engine/measuring/dom/src/index.ts | 6 +- .../dom/src/table/renderTableCell.test.ts | 141 ++++++++++++++++++ .../painters/dom/src/table/renderTableCell.ts | 9 +- 4 files changed, 159 insertions(+), 2 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 6a9f9d3622..1a3b77e5e7 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1695,6 +1695,11 @@ export function selectionToRects( if (typeof totalHeight === 'number' && totalHeight > height) { height = totalHeight; } + const spacingBefore = (paraBlock.attrs as { spacing?: { before?: number } } | undefined)?.spacing + ?.before; + if (typeof spacingBefore === 'number' && spacingBefore > 0) { + height += spacingBefore; + } const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { height += spacingAfter; diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index a7abd2b950..d77a3c81e4 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -2714,11 +2714,15 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai contentHeight += blockHeight; - // Add paragraph spacing.after to content height. + // Add paragraph spacing.after/spacing.before to content height. // For the last paragraph, Word absorbs spacing.after into cell bottom padding — // so only add the excess beyond what the padding already provides. const isLastBlock = blockIndex === cellBlocks.length - 1; if (block.kind === 'paragraph') { + const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; + if (typeof spacingBefore === 'number' && spacingBefore > 0) { + contentHeight += spacingBefore; + } const spacingAfter = (block as ParagraphBlock).attrs?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { if (isLastBlock) { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts index 41fcbd13d3..eff5886ab0 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts @@ -1111,6 +1111,147 @@ describe('renderTableCell', () => { }); }); + describe('spacing.before margin-top rendering', () => { + const baseMeasure: ParagraphMeasure = { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 10, + width: 100, + ascent: 12, + descent: 4, + lineHeight: 20, + }, + ], + totalHeight: 20, + }; + + it('applies margin-top only for positive spacing.before', () => { + const para1: ParagraphBlock = { + kind: 'paragraph', + id: 'para-before-zero', + runs: [{ text: 'Zero spacing', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 0 } }, + }; + + const para2: ParagraphBlock = { + kind: 'paragraph', + id: 'para-before-negative', + runs: [{ text: 'Negative spacing', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: -6 } }, + }; + + const para3: ParagraphBlock = { + kind: 'paragraph', + id: 'para-before-positive', + runs: [{ text: 'Positive spacing', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 9 } }, + }; + + const cellMeasure: TableCellMeasure = { + blocks: [baseMeasure, baseMeasure, baseMeasure], + width: 120, + height: 80, + gridColumnStart: 0, + colSpan: 1, + rowSpan: 1, + }; + + const cell: TableCell = { + id: 'cell-spacing-before-conditional', + blocks: [para1, para2, para3], + attrs: {}, + }; + + const { cellElement } = renderTableCell({ + ...createBaseDeps(), + cellMeasure, + cell, + }); + + const contentElement = cellElement.firstElementChild as HTMLElement; + const paraWrappers = contentElement.children; + + expect((paraWrappers[0] as HTMLElement).style.marginTop).toBe(''); + expect((paraWrappers[1] as HTMLElement).style.marginTop).toBe(''); + expect((paraWrappers[2] as HTMLElement).style.marginTop).toBe('9px'); + }); + + it('skips spacing.before for partial renders', () => { + const para: ParagraphBlock = { + kind: 'paragraph', + id: 'para-before-partial', + runs: [{ text: 'Partial render test', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 11 } }, + }; + + const measure: ParagraphMeasure = { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 10, + width: 100, + ascent: 12, + descent: 4, + lineHeight: 20, + }, + { + fromRun: 0, + fromChar: 10, + toRun: 0, + toChar: 19, + width: 100, + ascent: 12, + descent: 4, + lineHeight: 20, + }, + ], + totalHeight: 40, + }; + + const cellMeasure: TableCellMeasure = { + blocks: [measure], + width: 120, + height: 60, + gridColumnStart: 0, + colSpan: 1, + rowSpan: 1, + }; + + const cell: TableCell = { + id: 'cell-before-partial', + blocks: [para], + attrs: {}, + }; + + const { cellElement: partialCell } = renderTableCell({ + ...createBaseDeps(), + cellMeasure, + cell, + fromLine: 1, + toLine: 2, + }); + + const partialWrapper = (partialCell.firstElementChild as HTMLElement).firstElementChild as HTMLElement; + expect(partialWrapper.style.marginTop).toBe(''); + + const { cellElement: fullCell } = renderTableCell({ + ...createBaseDeps(), + cellMeasure, + cell, + }); + + const fullWrapper = (fullCell.firstElementChild as HTMLElement).firstElementChild as HTMLElement; + expect(fullWrapper.style.marginTop).toBe('11px'); + }); + }); + describe('list marker rendering', () => { const createParagraphWithMarker = (markerText: string, markerWidth = 20, gutterWidth = 8, indentLeft = 30) => { const para: ParagraphBlock = { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index e9d3af6e4b..e2065f9f08 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -1278,7 +1278,6 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen const lines = paragraphMeasure.lines; const blockLineCount = lines?.length || 0; - paragraphTopById.set(block.id, flowCursorY); /** * Extract Word layout information from paragraph attributes. * This contains computed marker positioning and indent details from the word-layout engine. @@ -1345,6 +1344,14 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen applyParagraphBorderStyles(paraWrapper, block.attrs?.borders); applyParagraphShadingStyles(paraWrapper, block.attrs?.shading); + // Apply paragraph spacing.before when rendering from the top of the paragraph. + const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; + if (localStartLine === 0 && typeof spacingBefore === 'number' && spacingBefore > 0) { + paraWrapper.style.marginTop = `${spacingBefore}px`; + flowCursorY += spacingBefore; + } + paragraphTopById.set(block.id, flowCursorY); + // Calculate height of rendered content for proper block accumulation let renderedHeight = 0; From a240764555ade30b11eea64d8c7e6a937300adcc Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Mon, 26 Jan 2026 20:54:25 +0200 Subject: [PATCH 2/7] fix: remove extra code in layout bridge --- packages/layout-engine/layout-bridge/src/index.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 1a3b77e5e7..6a9f9d3622 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1695,11 +1695,6 @@ export function selectionToRects( if (typeof totalHeight === 'number' && totalHeight > height) { height = totalHeight; } - const spacingBefore = (paraBlock.attrs as { spacing?: { before?: number } } | undefined)?.spacing - ?.before; - if (typeof spacingBefore === 'number' && spacingBefore > 0) { - height += spacingBefore; - } const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { height += spacingAfter; From d575c189fd49ca7035c6a80791f5b0c8a0e5f2b3 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 13 Feb 2026 17:21:35 +0200 Subject: [PATCH 3/7] fix: extend test cases and remove unused code --- .../dom/src/table/renderTableCell.test.ts | 53 ++++++++++++------- .../painters/dom/src/table/renderTableCell.ts | 2 - 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts index eff5886ab0..5e7b935024 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts @@ -1112,23 +1112,6 @@ describe('renderTableCell', () => { }); describe('spacing.before margin-top rendering', () => { - const baseMeasure: ParagraphMeasure = { - kind: 'paragraph', - lines: [ - { - fromRun: 0, - fromChar: 0, - toRun: 0, - toChar: 10, - width: 100, - ascent: 12, - descent: 4, - lineHeight: 20, - }, - ], - totalHeight: 20, - }; - it('applies margin-top only for positive spacing.before', () => { const para1: ParagraphBlock = { kind: 'paragraph', @@ -1152,7 +1135,7 @@ describe('renderTableCell', () => { }; const cellMeasure: TableCellMeasure = { - blocks: [baseMeasure, baseMeasure, baseMeasure], + blocks: [paragraphMeasure, paragraphMeasure, paragraphMeasure], width: 120, height: 80, gridColumnStart: 0, @@ -1250,6 +1233,40 @@ describe('renderTableCell', () => { const fullWrapper = (fullCell.firstElementChild as HTMLElement).firstElementChild as HTMLElement; expect(fullWrapper.style.marginTop).toBe('11px'); }); + + it('applies both margin-top and margin-bottom when paragraph has spacing.before and spacing.after', () => { + const para: ParagraphBlock = { + kind: 'paragraph', + id: 'para-before-and-after', + runs: [{ text: 'Both spacing', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 12, after: 18 } }, + }; + + const cellMeasure: TableCellMeasure = { + blocks: [paragraphMeasure], + width: 120, + height: 60, + gridColumnStart: 0, + colSpan: 1, + rowSpan: 1, + }; + + const cell: TableCell = { + id: 'cell-before-and-after', + blocks: [para], + attrs: {}, + }; + + const { cellElement } = renderTableCell({ + ...createBaseDeps(), + cellMeasure, + cell, + }); + + const paraWrapper = (cellElement.firstElementChild as HTMLElement).firstElementChild as HTMLElement; + expect(paraWrapper.style.marginTop).toBe('12px'); + expect(paraWrapper.style.marginBottom).toBe('18px'); + }); }); describe('list marker rendering', () => { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index e2065f9f08..d47b960f1c 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -1112,7 +1112,6 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen const effectiveCellWidth = cellWidth ?? cellMeasure.width; const contentWidthPx = Math.max(0, effectiveCellWidth - paddingLeft - paddingRight); const contentHeightPx = Math.max(0, rowHeight - paddingTop - paddingBottom); - const paragraphTopById = new Map(); let flowCursorY = 0; const anchoredBlocks: Array<{ block: ImageBlock | DrawingBlock; measure: ImageMeasure | DrawingMeasure }> = []; const renderedLines: RenderedLineInfo[] = []; @@ -1350,7 +1349,6 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen paraWrapper.style.marginTop = `${spacingBefore}px`; flowCursorY += spacingBefore; } - paragraphTopById.set(block.id, flowCursorY); // Calculate height of rendered content for proper block accumulation let renderedHeight = 0; From 7331cbb67d393cae8aa5e1d7cb5825d220933d79 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 26 Feb 2026 18:33:36 +0200 Subject: [PATCH 4/7] fix: failing test --- .../dom/src/table/renderTableCell.test.ts | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts index 5e7b935024..d0ca156bc6 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts @@ -1235,17 +1235,41 @@ describe('renderTableCell', () => { }); it('applies both margin-top and margin-bottom when paragraph has spacing.before and spacing.after', () => { - const para: ParagraphBlock = { + const paraWithBoth: ParagraphBlock = { kind: 'paragraph', id: 'para-before-and-after', runs: [{ text: 'Both spacing', fontFamily: 'Arial', fontSize: 16 }], attrs: { spacing: { before: 12, after: 18 } }, }; + const secondPara: ParagraphBlock = { + kind: 'paragraph', + id: 'para-second', + runs: [{ text: 'Second', fontFamily: 'Arial', fontSize: 16 }], + attrs: {}, + }; + + const secondMeasure: ParagraphMeasure = { + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 7, + width: 50, + ascent: 12, + descent: 4, + lineHeight: 20, + }, + ], + totalHeight: 20, + }; + const cellMeasure: TableCellMeasure = { - blocks: [paragraphMeasure], + blocks: [paragraphMeasure, secondMeasure], width: 120, - height: 60, + height: 100, gridColumnStart: 0, colSpan: 1, rowSpan: 1, @@ -1253,7 +1277,7 @@ describe('renderTableCell', () => { const cell: TableCell = { id: 'cell-before-and-after', - blocks: [para], + blocks: [paraWithBoth, secondPara], attrs: {}, }; @@ -1263,9 +1287,10 @@ describe('renderTableCell', () => { cell, }); - const paraWrapper = (cellElement.firstElementChild as HTMLElement).firstElementChild as HTMLElement; - expect(paraWrapper.style.marginTop).toBe('12px'); - expect(paraWrapper.style.marginBottom).toBe('18px'); + const contentElement = cellElement.firstElementChild as HTMLElement; + const firstParaWrapper = contentElement.children[0] as HTMLElement; + expect(firstParaWrapper.style.marginTop).toBe('12px'); + expect(firstParaWrapper.style.marginBottom).toBe('18px'); }); }); From 3ae1db6c1cf2ca4ab1ea43201a1d77b0cd7930ce Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Mon, 2 Mar 2026 15:47:09 +0200 Subject: [PATCH 5/7] fix: consider padding for the first block in line --- .../layout-engine/layout-bridge/src/index.ts | 27 ++- .../measuring/dom/src/index.test.ts | 162 ++++++++++++++++++ .../layout-engine/measuring/dom/src/index.ts | 11 +- .../painters/dom/src/table/renderTableCell.ts | 10 +- 4 files changed, 204 insertions(+), 6 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index 6a9f9d3622..d6626d4eaa 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1695,9 +1695,18 @@ export function selectionToRects( if (typeof totalHeight === 'number' && totalHeight > height) { height = totalHeight; } + const isFirstBlock = i === 0; + const isLastBlock = i === cellBlocks.length - 1; + const spacingBefore = (paraBlock.attrs as { spacing?: { before?: number } } | undefined)?.spacing + ?.before; + if (typeof spacingBefore === 'number' && spacingBefore > 0) { + // First paragraph: spacing.before absorbed by cell paddingTop (match measurer/renderer) + height += isFirstBlock ? Math.max(0, spacingBefore - padding.top) : spacingBefore; + } const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { - height += spacingAfter; + // Last paragraph: spacing.after absorbed by cell paddingBottom (match measurer/renderer) + height += isLastBlock ? Math.max(0, spacingAfter - padding.bottom) : spacingAfter; } } @@ -1718,7 +1727,7 @@ export function selectionToRects( let blockTopCursor = padding.top + verticalOffset; - renderedBlocks.forEach((info) => { + renderedBlocks.forEach((info, blockIndex) => { const paragraphMarkerWidth = info.measure.marker?.markerWidth ?? 0; // List items in table cells are also rendered with left alignment const cellIsListItem = isListItem(paragraphMarkerWidth, info.block); @@ -1731,6 +1740,17 @@ export function selectionToRects( const intersectingLines = findLinesIntersectingRange(info.block, info.measure, from, to); + // Match renderer: first paragraph's spacing.before is absorbed by cell paddingTop (renderTableCell.ts) + const rawSpacingBefore = (info.block.attrs as { spacing?: { before?: number } } | undefined)?.spacing + ?.before; + const isFirstBlock = blockIndex === 0; + const effectiveSpacingBeforePx = + typeof rawSpacingBefore === 'number' && rawSpacingBefore > 0 + ? isFirstBlock + ? Math.max(0, rawSpacingBefore - padding.top) + : rawSpacingBefore + : 0; + intersectingLines.forEach(({ line, index }) => { if (index < info.startLine || index >= info.endLine) { return; @@ -1768,7 +1788,8 @@ export function selectionToRects( ); const lineOffset = lineHeightBeforeIndex(info.measure, index) - lineHeightBeforeIndex(info.measure, info.startLine); - const rectY = fragment.y + contentOffsetY + rowOffset + blockTopCursor + lineOffset; + const rectY = + fragment.y + contentOffsetY + rowOffset + blockTopCursor + effectiveSpacingBeforePx + lineOffset; rects.push({ x: rectX, diff --git a/packages/layout-engine/measuring/dom/src/index.test.ts b/packages/layout-engine/measuring/dom/src/index.test.ts index 69abf2f365..57f0d90697 100644 --- a/packages/layout-engine/measuring/dom/src/index.test.ts +++ b/packages/layout-engine/measuring/dom/src/index.test.ts @@ -5033,6 +5033,168 @@ describe('measureBlock', () => { }); }); + describe('table cell measurement with spacing.before', () => { + it('should add spacing.before to content height for each paragraph', async () => { + const table: FlowBlock = { + kind: 'table', + id: 'table-spacing-before', + attrs: {}, + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0-0', + attrs: {}, + blocks: [ + { + kind: 'paragraph', + id: 'para-0', + runs: [{ text: 'First paragraph', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 10 } }, + }, + { + kind: 'paragraph', + id: 'para-1', + runs: [{ text: 'Second paragraph', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 20 } }, + }, + ], + }, + ], + }, + ], + }; + + const measure = await measureBlock(table, 1000); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + const cellMeasure = measure.rows[0].cells[0]; + const block0Measure = cellMeasure.blocks[0]; + const block1Measure = cellMeasure.blocks[1]; + + expect(block0Measure.kind).toBe('paragraph'); + expect(block1Measure.kind).toBe('paragraph'); + + const para0Height = block0Measure.kind === 'paragraph' ? block0Measure.totalHeight : 0; + const para1Height = block1Measure.kind === 'paragraph' ? block1Measure.totalHeight : 0; + + // Cell height includes: 10 (spacing.before para-0) + para0Height + 20 (spacing.before para-1) + para1Height + const expectedCellHeight = 10 + para0Height + 20 + para1Height; + expect(cellMeasure.height).toBe(expectedCellHeight); + }); + + it('should only add positive spacing.before', async () => { + const table: FlowBlock = { + kind: 'table', + id: 'table-spacing-before-zero', + attrs: {}, + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0-0', + attrs: {}, + blocks: [ + { + kind: 'paragraph', + id: 'para-0', + runs: [{ text: 'With before', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 12 } }, + }, + { + kind: 'paragraph', + id: 'para-1', + runs: [{ text: 'Zero before', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 0 } }, + }, + { + kind: 'paragraph', + id: 'para-2', + runs: [{ text: 'Negative before', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: -5 } }, + }, + ], + }, + ], + }, + ], + }; + + const measure = await measureBlock(table, 1000); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + const cellMeasure = measure.rows[0].cells[0]; + const block0 = cellMeasure.blocks[0]; + const block1 = cellMeasure.blocks[1]; + const block2 = cellMeasure.blocks[2]; + + const para0Height = block0.kind === 'paragraph' ? block0.totalHeight : 0; + const para1Height = block1.kind === 'paragraph' ? block1.totalHeight : 0; + const para2Height = block2.kind === 'paragraph' ? block2.totalHeight : 0; + + // Only positive spacing.before (12) is added; 0 and negative are ignored + const expectedCellHeight = 12 + para0Height + para1Height + para2Height; + expect(cellMeasure.height).toBe(expectedCellHeight); + }); + + it('should absorb first paragraph spacing.before into cell paddingTop (Word semantics)', async () => { + // Word absorbs the first paragraph's spacing.before into the cell's top padding, + // same as last paragraph's spacing.after and paddingBottom. Only the excess is added. + const paddingTop = 10; + const paddingBottom = 0; + const table: FlowBlock = { + kind: 'table', + id: 'table-spacing-before-absorbed', + attrs: {}, + rows: [ + { + id: 'row-0', + cells: [ + { + id: 'cell-0-0', + attrs: { padding: { top: paddingTop, left: 4, right: 4, bottom: paddingBottom } }, + blocks: [ + { + kind: 'paragraph', + id: 'para-0', + runs: [{ text: 'First in cell', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 10 } }, // same as paddingTop → excess 0 + }, + { + kind: 'paragraph', + id: 'para-1', + runs: [{ text: 'Second in cell', fontFamily: 'Arial', fontSize: 16 }], + attrs: { spacing: { before: 20 } }, // not first → full 20 + }, + ], + }, + ], + }, + ], + }; + + const measure = await measureBlock(table, 1000); + expect(measure.kind).toBe('table'); + if (measure.kind !== 'table') throw new Error('expected table measure'); + + const cellMeasure = measure.rows[0].cells[0]; + const block0 = cellMeasure.blocks[0]; + const block1 = cellMeasure.blocks[1]; + + const para0Height = block0.kind === 'paragraph' ? block0.totalHeight : 0; + const para1Height = block1.kind === 'paragraph' ? block1.totalHeight : 0; + + // First para: spacing.before 10, paddingTop 10 → excess 0. Second para: full 20. + // Cell height = paddingTop + 0 + para0Height + 20 + para1Height + paddingBottom + const expectedCellHeight = paddingTop + 0 + para0Height + 20 + para1Height + paddingBottom; + expect(cellMeasure.height).toBe(expectedCellHeight); + }); + }); + describe('table column count with rowspan', () => { it('should preserve all grid columns when rows have fewer physical cells due to rowspan', async () => { // Simulates PCI table structure: 4 grid columns, but some rows have only 2-3 physical cells diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index d77a3c81e4..ef478ca2e5 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -2717,11 +2717,20 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai // Add paragraph spacing.after/spacing.before to content height. // For the last paragraph, Word absorbs spacing.after into cell bottom padding — // so only add the excess beyond what the padding already provides. + // For the first paragraph, we treat spacing.before symmetrically: only add the excess + // beyond paddingTop (Word absorbs the first paragraph's spacing.before into cell top + // padding). Verify against a reference .docx if behavior needs to be confirmed. + const isFirstBlock = blockIndex === 0; const isLastBlock = blockIndex === cellBlocks.length - 1; if (block.kind === 'paragraph') { const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; if (typeof spacingBefore === 'number' && spacingBefore > 0) { - contentHeight += spacingBefore; + if (isFirstBlock) { + const excess = Math.max(0, spacingBefore - paddingTop); + contentHeight += excess; + } else { + contentHeight += spacingBefore; + } } const spacingAfter = (block as ParagraphBlock).attrs?.spacing?.after; if (typeof spacingAfter === 'number' && spacingAfter > 0) { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index d47b960f1c..d733dc5a4e 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -1344,10 +1344,16 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen applyParagraphShadingStyles(paraWrapper, block.attrs?.shading); // Apply paragraph spacing.before when rendering from the top of the paragraph. + // For the first paragraph in the cell, Word absorbs spacing.before into cell top padding — + // only apply the excess beyond paddingTop (mirrors last paragraph's spacing.after / paddingBottom). const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; if (localStartLine === 0 && typeof spacingBefore === 'number' && spacingBefore > 0) { - paraWrapper.style.marginTop = `${spacingBefore}px`; - flowCursorY += spacingBefore; + const isFirstBlock = i === 0; + const effectiveBefore = isFirstBlock ? Math.max(0, spacingBefore - paddingTop) : spacingBefore; + if (effectiveBefore > 0) { + paraWrapper.style.marginTop = `${effectiveBefore}px`; + flowCursorY += effectiveBefore; + } } // Calculate height of rendered content for proper block accumulation From 20787c0400e1483f5a5e53bc80133ed66d12864c Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Mon, 2 Mar 2026 18:33:38 +0200 Subject: [PATCH 6/7] fix: add helper/ improve test coverage --- packages/layout-engine/contracts/src/index.ts | 2 + .../contracts/src/table-cell-spacing.test.ts | 26 ++ .../contracts/src/table-cell-spacing.ts | 17 + .../layout-engine/layout-bridge/src/index.ts | 21 +- .../layout-bridge/test/mock-data.ts | 295 ++++++++++++++++-- .../test/selectionToRects.test.ts | 52 +++ .../layout-engine/measuring/dom/src/index.ts | 26 +- .../painters/dom/src/table/renderTableCell.ts | 9 +- 8 files changed, 384 insertions(+), 64 deletions(-) create mode 100644 packages/layout-engine/contracts/src/table-cell-spacing.test.ts create mode 100644 packages/layout-engine/contracts/src/table-cell-spacing.ts diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index eb677d367f..d6ede92fa6 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -7,6 +7,8 @@ export type { TabStop }; // Export table contracts export { OOXML_PCT_DIVISOR, type TableWidthAttr, type TableColumnSpec } from './engines/tables.js'; +export { effectiveTableCellSpacing } from './table-cell-spacing.js'; + // Export justify utilities export { shouldApplyJustify, diff --git a/packages/layout-engine/contracts/src/table-cell-spacing.test.ts b/packages/layout-engine/contracts/src/table-cell-spacing.test.ts new file mode 100644 index 0000000000..c36f74fb2b --- /dev/null +++ b/packages/layout-engine/contracts/src/table-cell-spacing.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { effectiveTableCellSpacing } from './table-cell-spacing.js'; + +describe('effectiveTableCellSpacing', () => { + it('returns 0 when spacing is undefined', () => { + expect(effectiveTableCellSpacing(undefined, false, 0)).toBe(0); + expect(effectiveTableCellSpacing(undefined, true, 10)).toBe(0); + }); + + it('returns 0 when spacing is <= 0', () => { + expect(effectiveTableCellSpacing(0, false, 0)).toBe(0); + expect(effectiveTableCellSpacing(-5, true, 0)).toBe(0); + }); + + it('returns full spacing when not at boundary', () => { + expect(effectiveTableCellSpacing(20, false, 10)).toBe(20); + expect(effectiveTableCellSpacing(20, false, 0)).toBe(20); + }); + + it('returns excess over padding when at boundary', () => { + expect(effectiveTableCellSpacing(20, true, 10)).toBe(10); + expect(effectiveTableCellSpacing(20, true, 0)).toBe(20); + expect(effectiveTableCellSpacing(10, true, 10)).toBe(0); + expect(effectiveTableCellSpacing(5, true, 10)).toBe(0); + }); +}); diff --git a/packages/layout-engine/contracts/src/table-cell-spacing.ts b/packages/layout-engine/contracts/src/table-cell-spacing.ts new file mode 100644 index 0000000000..734beff07f --- /dev/null +++ b/packages/layout-engine/contracts/src/table-cell-spacing.ts @@ -0,0 +1,17 @@ +/** + * Effective paragraph spacing in table cells. + * + * Word absorbs the first paragraph's spacing.before into the cell's top padding, + * and the last paragraph's spacing.after into the cell's bottom padding. + * This helper returns the amount to add to height/position: at a boundary, + * only the excess of spacing over padding; otherwise the full spacing. + * + * Use for both spacing.before (isBoundary = first block, padding = paddingTop) + * and spacing.after (isBoundary = last block, padding = paddingBottom). + */ +export function effectiveTableCellSpacing(spacing: number | undefined, isBoundary: boolean, padding: number): number { + if (typeof spacing !== 'number' || spacing <= 0) { + return 0; + } + return isBoundary ? Math.max(0, spacing - padding) : spacing; +} diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index d6626d4eaa..dee92aad7a 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -13,7 +13,7 @@ import type { ParagraphBlock, ParagraphMeasure, } from '@superdoc/contracts'; -import { computeLinePmRange as computeLinePmRangeUnified } from '@superdoc/contracts'; +import { computeLinePmRange as computeLinePmRangeUnified, effectiveTableCellSpacing } from '@superdoc/contracts'; import { charOffsetToPm, findCharacterAtX, measureCharacterX } from './text-measurement.js'; import { clickToPositionDom, findPageElement } from './dom-mapping.js'; import { @@ -1699,15 +1699,9 @@ export function selectionToRects( const isLastBlock = i === cellBlocks.length - 1; const spacingBefore = (paraBlock.attrs as { spacing?: { before?: number } } | undefined)?.spacing ?.before; - if (typeof spacingBefore === 'number' && spacingBefore > 0) { - // First paragraph: spacing.before absorbed by cell paddingTop (match measurer/renderer) - height += isFirstBlock ? Math.max(0, spacingBefore - padding.top) : spacingBefore; - } + height += effectiveTableCellSpacing(spacingBefore, isFirstBlock, padding.top); const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after; - if (typeof spacingAfter === 'number' && spacingAfter > 0) { - // Last paragraph: spacing.after absorbed by cell paddingBottom (match measurer/renderer) - height += isLastBlock ? Math.max(0, spacingAfter - padding.bottom) : spacingAfter; - } + height += effectiveTableCellSpacing(spacingAfter, isLastBlock, padding.bottom); } renderedBlocks.push({ block: paraBlock, measure: paraMeasure, startLine, endLine, height }); @@ -1740,16 +1734,11 @@ export function selectionToRects( const intersectingLines = findLinesIntersectingRange(info.block, info.measure, from, to); - // Match renderer: first paragraph's spacing.before is absorbed by cell paddingTop (renderTableCell.ts) + // Match renderer: spacing.before is only applied when rendering from the start of the block (startLine === 0). const rawSpacingBefore = (info.block.attrs as { spacing?: { before?: number } } | undefined)?.spacing ?.before; - const isFirstBlock = blockIndex === 0; const effectiveSpacingBeforePx = - typeof rawSpacingBefore === 'number' && rawSpacingBefore > 0 - ? isFirstBlock - ? Math.max(0, rawSpacingBefore - padding.top) - : rawSpacingBefore - : 0; + info.startLine === 0 ? effectiveTableCellSpacing(rawSpacingBefore, blockIndex === 0, padding.top) : 0; intersectingLines.forEach(({ line, index }) => { if (index < info.startLine || index >= info.endLine) { diff --git a/packages/layout-engine/layout-bridge/test/mock-data.ts b/packages/layout-engine/layout-bridge/test/mock-data.ts index ae71c8c5a2..9252d8da1b 100644 --- a/packages/layout-engine/layout-bridge/test/mock-data.ts +++ b/packages/layout-engine/layout-bridge/test/mock-data.ts @@ -192,12 +192,25 @@ export const drawingLayout: Layout = { ], }; +const TABLE_CELL_LINE_HEIGHT = 18; + const tableParagraph = { kind: 'paragraph', id: 'table-cell-para', runs: [{ text: 'Table text', fontFamily: 'Arial', fontSize: 14, pmStart: 1, pmEnd: 11 }], } as const; +const tableParagraphLine = { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 10, + width: 80, + ascent: 10, + descent: 4, + lineHeight: TABLE_CELL_LINE_HEIGHT, +} as const; + export const tableBlock: FlowBlock = { kind: 'table', id: 'table-0', @@ -228,19 +241,8 @@ export const tableMeasure: Measure = { blocks: [ { kind: 'paragraph', - lines: [ - { - fromRun: 0, - fromChar: 0, - toRun: 0, - toChar: 10, - width: 80, - ascent: 10, - descent: 4, - lineHeight: 18, - }, - ], - totalHeight: 18, + lines: [tableParagraphLine], + totalHeight: TABLE_CELL_LINE_HEIGHT, }, ], }, @@ -252,27 +254,278 @@ export const tableMeasure: Measure = { totalHeight: 24, }; +const tablePageFragment = { + kind: 'table' as const, + blockId: 'table-0', + fromRow: 0, + toRow: 1, + x: 30, + y: 60, + width: 120, + height: 24, +}; + export const tableLayout: Layout = { pageSize: { w: 400, h: 500 }, pages: [ { number: 1, + fragments: [tablePageFragment], + }, + ], +}; + +// Table cell spacing.before — selectionToRects tests (effective spacing, absorption, partial row) +const TABLE_SPACING_BEFORE = 12; +const TABLE_SPACING_FRAGMENT_Y = 50; + +export const tableSpacingBeforeBlock: FlowBlock = { + ...tableBlock, + id: 'table-spacing-before', + rows: [ + { + ...tableBlock.rows[0], + cells: [ + { + ...tableBlock.rows[0].cells[0], + attrs: { padding: { top: 0, bottom: 0, left: 4, right: 4 } }, + blocks: [ + { + ...tableParagraph, + id: 'p1', + runs: [{ ...tableParagraph.runs[0], text: 'Cell text', pmEnd: 9 }], + attrs: { spacing: { before: TABLE_SPACING_BEFORE } }, + }, + ], + }, + ], + }, + ], +}; + +export const tableSpacingBeforeMeasure: Measure = { + kind: 'table', + rows: [ + { + height: TABLE_SPACING_BEFORE + TABLE_CELL_LINE_HEIGHT, + cells: [ + { + width: 100, + height: TABLE_SPACING_BEFORE + TABLE_CELL_LINE_HEIGHT, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [{ ...tableParagraphLine, toChar: 8, width: 60, ascent: 12 }], + totalHeight: TABLE_CELL_LINE_HEIGHT, + }, + ], + }, + ], + }, + ], + columnWidths: [100], + totalWidth: 100, + totalHeight: TABLE_SPACING_BEFORE + TABLE_CELL_LINE_HEIGHT, +}; + +export const tableSpacingBeforeLayout: Layout = { + ...tableLayout, + pages: [ + { + ...tableLayout.pages[0], fragments: [ { - kind: 'table', - blockId: 'table-0', - fromRow: 0, - toRow: 1, - x: 30, - y: 60, - width: 120, - height: 24, + ...tablePageFragment, + blockId: 'table-spacing-before', + x: 20, + y: TABLE_SPACING_FRAGMENT_Y, + width: 100, + height: TABLE_SPACING_BEFORE + TABLE_CELL_LINE_HEIGHT, + }, + ], + }, + ], +}; + +export const TABLE_SPACING_BEFORE_FRAGMENT_Y = TABLE_SPACING_FRAGMENT_Y; +export const TABLE_SPACING_BEFORE_SPACING = TABLE_SPACING_BEFORE; + +// First paragraph absorption: paddingTop === spacing.before => effective 0 +const TABLE_ABSORBED_PADDING_TOP = 10; +const TABLE_ABSORBED_SPACING = 10; +const TABLE_ABSORBED_FRAGMENT_Y = 50; + +export const tableSpacingAbsorbedBlock: FlowBlock = { + ...tableBlock, + id: 'table-spacing-absorbed', + rows: [ + { + ...tableBlock.rows[0], + cells: [ + { + ...tableBlock.rows[0].cells[0], + attrs: { padding: { top: TABLE_ABSORBED_PADDING_TOP, bottom: 0, left: 4, right: 4 } }, + blocks: [ + { + ...tableParagraph, + id: 'p1', + runs: [{ ...tableParagraph.runs[0], text: 'Cell', pmEnd: 5 }], + attrs: { spacing: { before: TABLE_ABSORBED_SPACING } }, + }, + ], + }, + ], + }, + ], +}; + +export const tableSpacingAbsorbedMeasure: Measure = { + kind: 'table', + rows: [ + { + height: TABLE_ABSORBED_PADDING_TOP + TABLE_CELL_LINE_HEIGHT, + cells: [ + { + width: 100, + height: TABLE_ABSORBED_PADDING_TOP + TABLE_CELL_LINE_HEIGHT, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [{ ...tableParagraphLine, toChar: 4, width: 40, ascent: 12 }], + totalHeight: TABLE_CELL_LINE_HEIGHT, + }, + ], + }, + ], + }, + ], + columnWidths: [100], + totalWidth: 100, + totalHeight: TABLE_ABSORBED_PADDING_TOP + TABLE_CELL_LINE_HEIGHT, +}; + +export const tableSpacingAbsorbedLayout: Layout = { + ...tableLayout, + pages: [ + { + ...tableLayout.pages[0], + fragments: [ + { + ...tablePageFragment, + blockId: 'table-spacing-absorbed', + x: 20, + y: TABLE_ABSORBED_FRAGMENT_Y, + width: 100, + height: TABLE_ABSORBED_PADDING_TOP + TABLE_CELL_LINE_HEIGHT, }, ], }, ], }; +export const TABLE_ABSORBED_FRAGMENT_Y_EXPORT = TABLE_ABSORBED_FRAGMENT_Y; +export const TABLE_ABSORBED_PADDING_TOP_EXPORT = TABLE_ABSORBED_PADDING_TOP; + +// Partial row: startLine > 0 so spacing.before not applied +const TABLE_PARTIAL_SPACING = 12; +const TABLE_PARTIAL_FRAGMENT_Y = 40; + +export const tableSpacingPartialBlock: FlowBlock = { + ...tableBlock, + id: 'table-partial', + rows: [ + { + ...tableBlock.rows[0], + cells: [ + { + ...tableBlock.rows[0].cells[0], + attrs: { padding: { top: 0, bottom: 0, left: 4, right: 4 } }, + blocks: [ + { + kind: 'paragraph', + id: 'p1', + runs: [ + { ...tableParagraph.runs[0], text: 'First ', pmEnd: 7 }, + { text: 'second line', fontFamily: 'Arial', fontSize: 14, pmStart: 7, pmEnd: 19 }, + ], + attrs: { spacing: { before: TABLE_PARTIAL_SPACING } }, + }, + ], + }, + ], + }, + ], +}; + +export const tableSpacingPartialMeasure: Measure = { + kind: 'table', + rows: [ + { + height: TABLE_PARTIAL_SPACING + TABLE_CELL_LINE_HEIGHT * 2, + cells: [ + { + width: 100, + height: TABLE_PARTIAL_SPACING + TABLE_CELL_LINE_HEIGHT * 2, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [ + { ...tableParagraphLine, toChar: 6, width: 50, ascent: 12 }, + { + fromRun: 1, + fromChar: 0, + toRun: 1, + toChar: 11, + width: 70, + ascent: 12, + descent: 4, + lineHeight: TABLE_CELL_LINE_HEIGHT, + }, + ], + totalHeight: TABLE_CELL_LINE_HEIGHT * 2, + }, + ], + }, + ], + }, + ], + columnWidths: [100], + totalWidth: 100, + totalHeight: TABLE_PARTIAL_SPACING + TABLE_CELL_LINE_HEIGHT * 2, +}; + +export const tableSpacingPartialLayout: Layout = { + ...tableLayout, + pages: [ + { + ...tableLayout.pages[0], + fragments: [ + { + ...tablePageFragment, + blockId: 'table-partial', + x: 20, + y: TABLE_PARTIAL_FRAGMENT_Y, + width: 100, + height: TABLE_CELL_LINE_HEIGHT, + partialRow: { + rowIndex: 0, + fromLineByCell: [1], + toLineByCell: [2], + isFirstPart: false, + isLastPart: true, + partialHeight: TABLE_CELL_LINE_HEIGHT, + }, + }, + ], + }, + ], +}; + +export const TABLE_PARTIAL_FRAGMENT_Y_EXPORT = TABLE_PARTIAL_FRAGMENT_Y; + // Mock data for table with rowspan (SD-1626 / IT-22) // Table structure: // Row 0: [Cell A (rowspan=2)] [Cell B] [Cell C] diff --git a/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts b/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts index 7e57fde36e..c17615001f 100644 --- a/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts +++ b/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts @@ -17,6 +17,20 @@ import { tableLayout, tableBlock, tableMeasure, + tableSpacingBeforeBlock, + tableSpacingBeforeMeasure, + tableSpacingBeforeLayout, + TABLE_SPACING_BEFORE_FRAGMENT_Y, + TABLE_SPACING_BEFORE_SPACING, + tableSpacingAbsorbedBlock, + tableSpacingAbsorbedMeasure, + tableSpacingAbsorbedLayout, + TABLE_ABSORBED_FRAGMENT_Y_EXPORT, + TABLE_ABSORBED_PADDING_TOP_EXPORT, + tableSpacingPartialBlock, + tableSpacingPartialMeasure, + tableSpacingPartialLayout, + TABLE_PARTIAL_FRAGMENT_Y_EXPORT, } from './mock-data'; import { PageGeometryHelper } from '../src/page-geometry-helper'; @@ -49,6 +63,44 @@ describe('selectionToRects', () => { expect(rects[0].x).toBeGreaterThan(tableLayout.pages[0].fragments[0].x); }); + describe('table cell spacing.before', () => { + it('includes effective spacing.before in rect Y when paragraph has spacing.before', () => { + const rects = selectionToRects( + tableSpacingBeforeLayout, + [tableSpacingBeforeBlock], + [tableSpacingBeforeMeasure], + 1, + 9, + ); + expect(rects).toHaveLength(1); + expect(rects[0].y).toBe(TABLE_SPACING_BEFORE_FRAGMENT_Y + TABLE_SPACING_BEFORE_SPACING); + }); + + it('uses only excess over paddingTop for first paragraph (Word absorption)', () => { + const rects = selectionToRects( + tableSpacingAbsorbedLayout, + [tableSpacingAbsorbedBlock], + [tableSpacingAbsorbedMeasure], + 1, + 5, + ); + expect(rects).toHaveLength(1); + expect(rects[0].y).toBe(TABLE_ABSORBED_FRAGMENT_Y_EXPORT + TABLE_ABSORBED_PADDING_TOP_EXPORT); + }); + + it('does not add spacing.before to rect Y when block starts mid-paragraph (startLine > 0)', () => { + const rects = selectionToRects( + tableSpacingPartialLayout, + [tableSpacingPartialBlock], + [tableSpacingPartialMeasure], + 7, + 19, + ); + expect(rects).toHaveLength(1); + expect(rects[0].y).toBe(TABLE_PARTIAL_FRAGMENT_Y_EXPORT); + }); + }); + describe('firstLineIndentMode integration', () => { it('uses textStartPx for first line of list with firstLineIndentMode', () => { // Create a list item with firstLineIndentMode and textStartPx diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index ef478ca2e5..4b38545a0b 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -63,6 +63,7 @@ import { type CellSpacing, type TableBorders, type TableBorderValue, + effectiveTableCellSpacing, } from '@superdoc/contracts'; import type { WordParagraphLayoutOutput } from '@superdoc/word-layout'; import { @@ -2715,33 +2716,14 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai contentHeight += blockHeight; // Add paragraph spacing.after/spacing.before to content height. - // For the last paragraph, Word absorbs spacing.after into cell bottom padding — - // so only add the excess beyond what the padding already provides. - // For the first paragraph, we treat spacing.before symmetrically: only add the excess - // beyond paddingTop (Word absorbs the first paragraph's spacing.before into cell top - // padding). Verify against a reference .docx if behavior needs to be confirmed. + // Word absorbs first paragraph's spacing.before into paddingTop and last's spacing.after into paddingBottom. const isFirstBlock = blockIndex === 0; const isLastBlock = blockIndex === cellBlocks.length - 1; if (block.kind === 'paragraph') { const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; - if (typeof spacingBefore === 'number' && spacingBefore > 0) { - if (isFirstBlock) { - const excess = Math.max(0, spacingBefore - paddingTop); - contentHeight += excess; - } else { - contentHeight += spacingBefore; - } - } + contentHeight += effectiveTableCellSpacing(spacingBefore, isFirstBlock, paddingTop); const spacingAfter = (block as ParagraphBlock).attrs?.spacing?.after; - if (typeof spacingAfter === 'number' && spacingAfter > 0) { - if (isLastBlock) { - // Only add the portion not absorbed by cell bottom padding - const excess = Math.max(0, spacingAfter - paddingBottom); - contentHeight += excess; - } else { - contentHeight += spacingAfter; - } - } + contentHeight += effectiveTableCellSpacing(spacingAfter, isLastBlock, paddingBottom); } } diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index d733dc5a4e..5739336238 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -18,6 +18,7 @@ import type { WrapExclusion, WrapTextMode, } from '@superdoc/contracts'; +import { effectiveTableCellSpacing } from '@superdoc/contracts'; import { toCssFontFamily } from '@superdoc/font-utils'; import { rescaleColumnWidths } from '@superdoc/layout-engine'; import { normalizeZIndex } from '@superdoc/pm-adapter/utilities.js'; @@ -1344,12 +1345,10 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen applyParagraphShadingStyles(paraWrapper, block.attrs?.shading); // Apply paragraph spacing.before when rendering from the top of the paragraph. - // For the first paragraph in the cell, Word absorbs spacing.before into cell top padding — - // only apply the excess beyond paddingTop (mirrors last paragraph's spacing.after / paddingBottom). + // Word absorbs first paragraph's spacing.before into cell paddingTop (effectiveTableCellSpacing). const spacingBefore = (block as ParagraphBlock).attrs?.spacing?.before; - if (localStartLine === 0 && typeof spacingBefore === 'number' && spacingBefore > 0) { - const isFirstBlock = i === 0; - const effectiveBefore = isFirstBlock ? Math.max(0, spacingBefore - paddingTop) : spacingBefore; + if (localStartLine === 0) { + const effectiveBefore = effectiveTableCellSpacing(spacingBefore, i === 0, paddingTop); if (effectiveBefore > 0) { paraWrapper.style.marginTop = `${effectiveBefore}px`; flowCursorY += effectiveBefore; From cc8aa3d2324b3f4164fab723b6e9869f527b21fc Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 2 Mar 2026 14:04:53 -0300 Subject: [PATCH 7/7] refactor: clean up DX and add spacing.after test coverage - Export mock-data constants directly, removing unnecessary _EXPORT aliases - Use cleaner (block as ParagraphBlock) cast pattern in layout-bridge - Add spacing.after absorption test for selectionToRects (two-paragraph cell) --- .../layout-engine/layout-bridge/src/index.ts | 8 +- .../layout-bridge/test/mock-data.ts | 112 +++++++++++++++--- .../test/selectionToRects.test.ts | 41 +++++-- 3 files changed, 133 insertions(+), 28 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/index.ts b/packages/layout-engine/layout-bridge/src/index.ts index dee92aad7a..a99eadccf0 100644 --- a/packages/layout-engine/layout-bridge/src/index.ts +++ b/packages/layout-engine/layout-bridge/src/index.ts @@ -1697,10 +1697,9 @@ export function selectionToRects( } const isFirstBlock = i === 0; const isLastBlock = i === cellBlocks.length - 1; - const spacingBefore = (paraBlock.attrs as { spacing?: { before?: number } } | undefined)?.spacing - ?.before; + const spacingBefore = (paraBlock as ParagraphBlock).attrs?.spacing?.before; height += effectiveTableCellSpacing(spacingBefore, isFirstBlock, padding.top); - const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after; + const spacingAfter = (paraBlock as ParagraphBlock).attrs?.spacing?.after; height += effectiveTableCellSpacing(spacingAfter, isLastBlock, padding.bottom); } @@ -1735,8 +1734,7 @@ export function selectionToRects( const intersectingLines = findLinesIntersectingRange(info.block, info.measure, from, to); // Match renderer: spacing.before is only applied when rendering from the start of the block (startLine === 0). - const rawSpacingBefore = (info.block.attrs as { spacing?: { before?: number } } | undefined)?.spacing - ?.before; + const rawSpacingBefore = (info.block as ParagraphBlock).attrs?.spacing?.before; const effectiveSpacingBeforePx = info.startLine === 0 ? effectiveTableCellSpacing(rawSpacingBefore, blockIndex === 0, padding.top) : 0; diff --git a/packages/layout-engine/layout-bridge/test/mock-data.ts b/packages/layout-engine/layout-bridge/test/mock-data.ts index 9252d8da1b..5ad1c6f108 100644 --- a/packages/layout-engine/layout-bridge/test/mock-data.ts +++ b/packages/layout-engine/layout-bridge/test/mock-data.ts @@ -192,7 +192,7 @@ export const drawingLayout: Layout = { ], }; -const TABLE_CELL_LINE_HEIGHT = 18; +export const TABLE_CELL_LINE_HEIGHT = 18; const tableParagraph = { kind: 'paragraph', @@ -276,8 +276,8 @@ export const tableLayout: Layout = { }; // Table cell spacing.before — selectionToRects tests (effective spacing, absorption, partial row) -const TABLE_SPACING_BEFORE = 12; -const TABLE_SPACING_FRAGMENT_Y = 50; +export const TABLE_SPACING_BEFORE = 12; +export const TABLE_SPACING_FRAGMENT_Y = 50; export const tableSpacingBeforeBlock: FlowBlock = { ...tableBlock, @@ -348,13 +348,10 @@ export const tableSpacingBeforeLayout: Layout = { ], }; -export const TABLE_SPACING_BEFORE_FRAGMENT_Y = TABLE_SPACING_FRAGMENT_Y; -export const TABLE_SPACING_BEFORE_SPACING = TABLE_SPACING_BEFORE; - // First paragraph absorption: paddingTop === spacing.before => effective 0 -const TABLE_ABSORBED_PADDING_TOP = 10; -const TABLE_ABSORBED_SPACING = 10; -const TABLE_ABSORBED_FRAGMENT_Y = 50; +export const TABLE_ABSORBED_PADDING_TOP = 10; +export const TABLE_ABSORBED_SPACING = 10; +export const TABLE_ABSORBED_FRAGMENT_Y = 50; export const tableSpacingAbsorbedBlock: FlowBlock = { ...tableBlock, @@ -425,12 +422,9 @@ export const tableSpacingAbsorbedLayout: Layout = { ], }; -export const TABLE_ABSORBED_FRAGMENT_Y_EXPORT = TABLE_ABSORBED_FRAGMENT_Y; -export const TABLE_ABSORBED_PADDING_TOP_EXPORT = TABLE_ABSORBED_PADDING_TOP; - // Partial row: startLine > 0 so spacing.before not applied -const TABLE_PARTIAL_SPACING = 12; -const TABLE_PARTIAL_FRAGMENT_Y = 40; +export const TABLE_PARTIAL_SPACING = 12; +export const TABLE_PARTIAL_FRAGMENT_Y = 40; export const tableSpacingPartialBlock: FlowBlock = { ...tableBlock, @@ -524,7 +518,95 @@ export const tableSpacingPartialLayout: Layout = { ], }; -export const TABLE_PARTIAL_FRAGMENT_Y_EXPORT = TABLE_PARTIAL_FRAGMENT_Y; +// Table cell spacing.after — selectionToRects test +// Two paragraphs: p1 has spacing.after, p2 is the selection target. +// Tests that p2's rect Y is offset by p1's effective spacing.after. +export const TABLE_SPACING_AFTER = 15; +export const TABLE_SPACING_AFTER_PADDING_BOTTOM = 10; +const TABLE_SPACING_AFTER_FRAGMENT_Y = 50; +const SPACING_AFTER_EFFECTIVE = TABLE_SPACING_AFTER - TABLE_SPACING_AFTER_PADDING_BOTTOM; + +export const tableSpacingAfterBlock: FlowBlock = { + ...tableBlock, + id: 'table-spacing-after', + rows: [ + { + ...tableBlock.rows[0], + cells: [ + { + ...tableBlock.rows[0].cells[0], + attrs: { padding: { top: 0, bottom: TABLE_SPACING_AFTER_PADDING_BOTTOM, left: 4, right: 4 } }, + blocks: [ + { + ...tableParagraph, + id: 'p1', + runs: [{ ...tableParagraph.runs[0], text: 'First', pmStart: 1, pmEnd: 6 }], + attrs: { spacing: { after: TABLE_SPACING_AFTER } }, + }, + { + ...tableParagraph, + id: 'p2', + runs: [{ ...tableParagraph.runs[0], text: 'Second', pmStart: 7, pmEnd: 13 }], + }, + ], + }, + ], + }, + ], +}; + +const spacingAfterTotalHeight = + TABLE_CELL_LINE_HEIGHT * 2 + SPACING_AFTER_EFFECTIVE + TABLE_SPACING_AFTER_PADDING_BOTTOM; + +export const tableSpacingAfterMeasure: Measure = { + kind: 'table', + rows: [ + { + height: spacingAfterTotalHeight, + cells: [ + { + width: 100, + height: spacingAfterTotalHeight, + gridColumnStart: 0, + blocks: [ + { + kind: 'paragraph', + lines: [{ ...tableParagraphLine, toChar: 5, width: 50, ascent: 12 }], + totalHeight: TABLE_CELL_LINE_HEIGHT, + }, + { + kind: 'paragraph', + lines: [{ ...tableParagraphLine, fromRun: 0, fromChar: 0, toChar: 6, width: 55, ascent: 12 }], + totalHeight: TABLE_CELL_LINE_HEIGHT, + }, + ], + }, + ], + }, + ], + columnWidths: [100], + totalWidth: 100, + totalHeight: spacingAfterTotalHeight, +}; + +export const tableSpacingAfterLayout: Layout = { + ...tableLayout, + pages: [ + { + ...tableLayout.pages[0], + fragments: [ + { + ...tablePageFragment, + blockId: 'table-spacing-after', + x: 20, + y: TABLE_SPACING_AFTER_FRAGMENT_Y, + width: 100, + height: spacingAfterTotalHeight, + }, + ], + }, + ], +}; // Mock data for table with rowspan (SD-1626 / IT-22) // Table structure: diff --git a/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts b/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts index c17615001f..995df0862a 100644 --- a/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts +++ b/packages/layout-engine/layout-bridge/test/selectionToRects.test.ts @@ -20,17 +20,23 @@ import { tableSpacingBeforeBlock, tableSpacingBeforeMeasure, tableSpacingBeforeLayout, - TABLE_SPACING_BEFORE_FRAGMENT_Y, - TABLE_SPACING_BEFORE_SPACING, + TABLE_SPACING_FRAGMENT_Y, + TABLE_SPACING_BEFORE, tableSpacingAbsorbedBlock, tableSpacingAbsorbedMeasure, tableSpacingAbsorbedLayout, - TABLE_ABSORBED_FRAGMENT_Y_EXPORT, - TABLE_ABSORBED_PADDING_TOP_EXPORT, + TABLE_ABSORBED_FRAGMENT_Y, + TABLE_ABSORBED_PADDING_TOP, tableSpacingPartialBlock, tableSpacingPartialMeasure, tableSpacingPartialLayout, - TABLE_PARTIAL_FRAGMENT_Y_EXPORT, + TABLE_PARTIAL_FRAGMENT_Y, + tableSpacingAfterBlock, + tableSpacingAfterMeasure, + tableSpacingAfterLayout, + TABLE_SPACING_AFTER, + TABLE_SPACING_AFTER_PADDING_BOTTOM, + TABLE_CELL_LINE_HEIGHT, } from './mock-data'; import { PageGeometryHelper } from '../src/page-geometry-helper'; @@ -73,7 +79,7 @@ describe('selectionToRects', () => { 9, ); expect(rects).toHaveLength(1); - expect(rects[0].y).toBe(TABLE_SPACING_BEFORE_FRAGMENT_Y + TABLE_SPACING_BEFORE_SPACING); + expect(rects[0].y).toBe(TABLE_SPACING_FRAGMENT_Y + TABLE_SPACING_BEFORE); }); it('uses only excess over paddingTop for first paragraph (Word absorption)', () => { @@ -85,7 +91,7 @@ describe('selectionToRects', () => { 5, ); expect(rects).toHaveLength(1); - expect(rects[0].y).toBe(TABLE_ABSORBED_FRAGMENT_Y_EXPORT + TABLE_ABSORBED_PADDING_TOP_EXPORT); + expect(rects[0].y).toBe(TABLE_ABSORBED_FRAGMENT_Y + TABLE_ABSORBED_PADDING_TOP); }); it('does not add spacing.before to rect Y when block starts mid-paragraph (startLine > 0)', () => { @@ -97,7 +103,26 @@ describe('selectionToRects', () => { 19, ); expect(rects).toHaveLength(1); - expect(rects[0].y).toBe(TABLE_PARTIAL_FRAGMENT_Y_EXPORT); + expect(rects[0].y).toBe(TABLE_PARTIAL_FRAGMENT_Y); + }); + }); + + describe('table cell spacing.after', () => { + it('offsets second paragraph rect Y by first paragraph full spacing.after (non-last block)', () => { + // Select text in p2 (pmStart: 7, pmEnd: 13) + const rects = selectionToRects( + tableSpacingAfterLayout, + [tableSpacingAfterBlock], + [tableSpacingAfterMeasure], + 7, + 13, + ); + expect(rects).toHaveLength(1); + // p1 is NOT the last block, so full spacing.after is used (not absorbed) + // p2's rect Y = fragment.y + padding.top(0) + p1 height (lineHeight + full spacing.after) + const expectedY = + tableSpacingAfterLayout.pages[0].fragments[0].y + (TABLE_CELL_LINE_HEIGHT + TABLE_SPACING_AFTER); + expect(rects[0].y).toBe(expectedY); }); });