Skip to content

Commit c7efa85

Browse files
fix: before paragraph spacing inside table cells (#1842)
* fix: before paragraph spacing inside table cells * fix: remove extra code in layout bridge * fix: extend test cases and remove unused code * fix: failing test * fix: consider padding for the first block in line * fix: add helper/ improve test coverage * 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) --------- Co-authored-by: Caio Pizzol <caio@harbourshare.com>
1 parent 5f54480 commit c7efa85

10 files changed

Lines changed: 857 additions & 42 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ export type { TabStop };
77
// Export table contracts
88
export { OOXML_PCT_DIVISOR, type TableWidthAttr, type TableColumnSpec } from './engines/tables.js';
99

10+
export { effectiveTableCellSpacing } from './table-cell-spacing.js';
11+
1012
// Export justify utilities
1113
export {
1214
shouldApplyJustify,
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { effectiveTableCellSpacing } from './table-cell-spacing.js';
3+
4+
describe('effectiveTableCellSpacing', () => {
5+
it('returns 0 when spacing is undefined', () => {
6+
expect(effectiveTableCellSpacing(undefined, false, 0)).toBe(0);
7+
expect(effectiveTableCellSpacing(undefined, true, 10)).toBe(0);
8+
});
9+
10+
it('returns 0 when spacing is <= 0', () => {
11+
expect(effectiveTableCellSpacing(0, false, 0)).toBe(0);
12+
expect(effectiveTableCellSpacing(-5, true, 0)).toBe(0);
13+
});
14+
15+
it('returns full spacing when not at boundary', () => {
16+
expect(effectiveTableCellSpacing(20, false, 10)).toBe(20);
17+
expect(effectiveTableCellSpacing(20, false, 0)).toBe(20);
18+
});
19+
20+
it('returns excess over padding when at boundary', () => {
21+
expect(effectiveTableCellSpacing(20, true, 10)).toBe(10);
22+
expect(effectiveTableCellSpacing(20, true, 0)).toBe(20);
23+
expect(effectiveTableCellSpacing(10, true, 10)).toBe(0);
24+
expect(effectiveTableCellSpacing(5, true, 10)).toBe(0);
25+
});
26+
});
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* Effective paragraph spacing in table cells.
3+
*
4+
* Word absorbs the first paragraph's spacing.before into the cell's top padding,
5+
* and the last paragraph's spacing.after into the cell's bottom padding.
6+
* This helper returns the amount to add to height/position: at a boundary,
7+
* only the excess of spacing over padding; otherwise the full spacing.
8+
*
9+
* Use for both spacing.before (isBoundary = first block, padding = paddingTop)
10+
* and spacing.after (isBoundary = last block, padding = paddingBottom).
11+
*/
12+
export function effectiveTableCellSpacing(spacing: number | undefined, isBoundary: boolean, padding: number): number {
13+
if (typeof spacing !== 'number' || spacing <= 0) {
14+
return 0;
15+
}
16+
return isBoundary ? Math.max(0, spacing - padding) : spacing;
17+
}

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import type {
1313
ParagraphBlock,
1414
ParagraphMeasure,
1515
} from '@superdoc/contracts';
16-
import { computeLinePmRange as computeLinePmRangeUnified } from '@superdoc/contracts';
16+
import { computeLinePmRange as computeLinePmRangeUnified, effectiveTableCellSpacing } from '@superdoc/contracts';
1717
import { charOffsetToPm, findCharacterAtX, measureCharacterX } from './text-measurement.js';
1818
import { clickToPositionDom, findPageElement } from './dom-mapping.js';
1919
import {
@@ -1695,10 +1695,12 @@ export function selectionToRects(
16951695
if (typeof totalHeight === 'number' && totalHeight > height) {
16961696
height = totalHeight;
16971697
}
1698-
const spacingAfter = (paraBlock.attrs as { spacing?: { after?: number } } | undefined)?.spacing?.after;
1699-
if (typeof spacingAfter === 'number' && spacingAfter > 0) {
1700-
height += spacingAfter;
1701-
}
1698+
const isFirstBlock = i === 0;
1699+
const isLastBlock = i === cellBlocks.length - 1;
1700+
const spacingBefore = (paraBlock as ParagraphBlock).attrs?.spacing?.before;
1701+
height += effectiveTableCellSpacing(spacingBefore, isFirstBlock, padding.top);
1702+
const spacingAfter = (paraBlock as ParagraphBlock).attrs?.spacing?.after;
1703+
height += effectiveTableCellSpacing(spacingAfter, isLastBlock, padding.bottom);
17021704
}
17031705

17041706
renderedBlocks.push({ block: paraBlock, measure: paraMeasure, startLine, endLine, height });
@@ -1718,7 +1720,7 @@ export function selectionToRects(
17181720

17191721
let blockTopCursor = padding.top + verticalOffset;
17201722

1721-
renderedBlocks.forEach((info) => {
1723+
renderedBlocks.forEach((info, blockIndex) => {
17221724
const paragraphMarkerWidth = info.measure.marker?.markerWidth ?? 0;
17231725
// List items in table cells are also rendered with left alignment
17241726
const cellIsListItem = isListItem(paragraphMarkerWidth, info.block);
@@ -1731,6 +1733,11 @@ export function selectionToRects(
17311733

17321734
const intersectingLines = findLinesIntersectingRange(info.block, info.measure, from, to);
17331735

1736+
// Match renderer: spacing.before is only applied when rendering from the start of the block (startLine === 0).
1737+
const rawSpacingBefore = (info.block as ParagraphBlock).attrs?.spacing?.before;
1738+
const effectiveSpacingBeforePx =
1739+
info.startLine === 0 ? effectiveTableCellSpacing(rawSpacingBefore, blockIndex === 0, padding.top) : 0;
1740+
17341741
intersectingLines.forEach(({ line, index }) => {
17351742
if (index < info.startLine || index >= info.endLine) {
17361743
return;
@@ -1768,7 +1775,8 @@ export function selectionToRects(
17681775
);
17691776
const lineOffset =
17701777
lineHeightBeforeIndex(info.measure, index) - lineHeightBeforeIndex(info.measure, info.startLine);
1771-
const rectY = fragment.y + contentOffsetY + rowOffset + blockTopCursor + lineOffset;
1778+
const rectY =
1779+
fragment.y + contentOffsetY + rowOffset + blockTopCursor + effectiveSpacingBeforePx + lineOffset;
17721780

17731781
rects.push({
17741782
x: rectX,

0 commit comments

Comments
 (0)