Skip to content

Commit b0a482f

Browse files
harbournickharbournick
authored
fix(tables): preserve TableGrid defaults and style-driven spacing/bor… (#2230)
* fix(tables): preserve TableGrid defaults and style-driven spacing/border behavior * fix(tables): keep tableProperties internal and honor continuation edges with spaced borders
1 parent 95f634e commit b0a482f

15 files changed

Lines changed: 331 additions & 42 deletions

File tree

‎packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import { renderTableRow } from './renderTableRow.js';
3+
4+
const renderTableCellMock = vi.fn(() => ({ cellElement: document.createElement('div') }));
5+
6+
vi.mock('./renderTableCell.js', () => ({
7+
renderTableCell: (args: unknown) => renderTableCellMock(args),
8+
}));
9+
10+
describe('renderTableRow', () => {
11+
let doc: Document;
12+
let container: HTMLElement;
13+
14+
beforeEach(() => {
15+
doc = document.implementation.createHTMLDocument('table-row');
16+
container = doc.createElement('div');
17+
renderTableCellMock.mockClear();
18+
});
19+
20+
const createDeps = (overrides: Record<string, unknown> = {}) => ({
21+
doc,
22+
container,
23+
rowIndex: 3,
24+
y: 0,
25+
rowMeasure: {
26+
height: 20,
27+
cells: [{ width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }],
28+
},
29+
row: {
30+
id: 'row-1',
31+
cells: [{ id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }],
32+
},
33+
totalRows: 10,
34+
tableBorders: {
35+
top: { style: 'single', width: 1, color: '#000000' },
36+
bottom: { style: 'single', width: 1, color: '#000000' },
37+
left: { style: 'single', width: 1, color: '#000000' },
38+
right: { style: 'single', width: 1, color: '#000000' },
39+
insideH: { style: 'single', width: 1, color: '#111111' },
40+
insideV: { style: 'single', width: 1, color: '#222222' },
41+
},
42+
columnWidths: [100],
43+
allRowHeights: [20, 20, 20, 20, 20, 20, 20, 20, 20, 20],
44+
tableIndent: 0,
45+
context: { sectionIndex: 0, pageIndex: 0, columnIndex: 0 },
46+
renderLine: () => doc.createElement('div'),
47+
applySdtDataset: () => {},
48+
cellSpacingPx: 6,
49+
...overrides,
50+
});
51+
52+
it('does not draw insideH on top edge for continuation fragments with cell spacing', () => {
53+
renderTableRow(createDeps({ continuesFromPrev: true }) as never);
54+
55+
expect(renderTableCellMock).toHaveBeenCalledTimes(1);
56+
const call = renderTableCellMock.mock.calls[0][0] as { borders?: { top?: unknown; bottom?: unknown } };
57+
expect(call.borders?.top).toBeUndefined();
58+
expect(call.borders?.bottom).toBeDefined();
59+
});
60+
61+
it('does not draw insideH on bottom edge before continuation with cell spacing', () => {
62+
renderTableRow(createDeps({ continuesOnNext: true }) as never);
63+
64+
expect(renderTableCellMock).toHaveBeenCalledTimes(1);
65+
const call = renderTableCellMock.mock.calls[0][0] as { borders?: { top?: unknown; bottom?: unknown } };
66+
expect(call.borders?.top).toBeDefined();
67+
expect(call.borders?.bottom).toBeUndefined();
68+
});
69+
});

‎packages/layout-engine/painters/dom/src/table/renderTableRow.ts‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -293,26 +293,50 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => {
293293
right: cellBordersAttr.right,
294294
};
295295
} else if (tableBorders) {
296-
// For continuation handling: treat split boundaries as table edges
297-
const isFirstRow = rowIndex === 0;
298-
const isLastRow = rowIndex === totalRows - 1;
299-
const treatAsFirstRow = isFirstRow || continuesFromPrev;
300-
const treatAsLastRow = isLastRow || continuesOnNext;
296+
if (cellSpacingPx > 0) {
297+
// With cell spacing (border-collapse: separate), the TABLE CONTAINER handles outer
298+
// borders (top/right/bottom/left). Cells only render interior borders (insideH/insideV).
299+
// This prevents double borders: one from the container and one from edge cells.
300+
const isFirstRow = rowIndex === 0;
301+
const isLastRow = rowIndex === totalRows - 1;
302+
const isFirstCol = gridColIndex === 0;
303+
const isLastCol = gridColIndex === totalCols - 1;
304+
const treatAsFirstRow = isFirstRow || continuesFromPrev;
305+
const treatAsLastRow = isLastRow || continuesOnNext;
301306

302-
// Get base borders, then override for continuations
303-
const baseBorders = resolveTableCellBorders(tableBorders, rowIndex, gridColIndex, totalRows, totalCols);
304-
305-
if (baseBorders) {
306307
resolvedBorders = {
307-
// If this is a continuation (continuesFromPrev), use table's top border
308-
top: treatAsFirstRow ? borderValueToSpec(tableBorders.top) : baseBorders.top,
309-
// If this continues on next (continuesOnNext), use table's bottom border
310-
bottom: treatAsLastRow ? borderValueToSpec(tableBorders.bottom) : baseBorders.bottom,
311-
left: baseBorders.left,
312-
right: baseBorders.right,
308+
top: !treatAsFirstRow ? borderValueToSpec(tableBorders.insideH) : undefined,
309+
bottom: !treatAsLastRow ? borderValueToSpec(tableBorders.insideH) : undefined,
310+
left: !isFirstCol ? borderValueToSpec(tableBorders.insideV) : undefined,
311+
right: !isLastCol ? borderValueToSpec(tableBorders.insideV) : undefined,
313312
};
313+
314+
// If all sides are undefined, set resolvedBorders to undefined for cleanliness
315+
if (!resolvedBorders.top && !resolvedBorders.bottom && !resolvedBorders.left && !resolvedBorders.right) {
316+
resolvedBorders = undefined;
317+
}
314318
} else {
315-
resolvedBorders = undefined;
319+
// For continuation handling: treat split boundaries as table edges
320+
const isFirstRow = rowIndex === 0;
321+
const isLastRow = rowIndex === totalRows - 1;
322+
const treatAsFirstRow = isFirstRow || continuesFromPrev;
323+
const treatAsLastRow = isLastRow || continuesOnNext;
324+
325+
// Get base borders, then override for continuations
326+
const baseBorders = resolveTableCellBorders(tableBorders, rowIndex, gridColIndex, totalRows, totalCols);
327+
328+
if (baseBorders) {
329+
resolvedBorders = {
330+
// If this is a continuation (continuesFromPrev), use table's top border
331+
top: treatAsFirstRow ? borderValueToSpec(tableBorders.top) : baseBorders.top,
332+
// If this continues on next (continuesOnNext), use table's bottom border
333+
bottom: treatAsLastRow ? borderValueToSpec(tableBorders.bottom) : baseBorders.bottom,
334+
left: baseBorders.left,
335+
right: baseBorders.right,
336+
};
337+
} else {
338+
resolvedBorders = undefined;
339+
}
316340
}
317341
} else {
318342
resolvedBorders = undefined;

‎packages/layout-engine/pm-adapter/src/converters/table-styles.test.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('hydrateTableStyleAttrs', () => {
3636
borders: { top: { val: 'single', size: 8 } },
3737
cellMargins: { left: { value: 72, type: 'dxa' } },
3838
justification: 'center',
39+
tableCellSpacing: { value: 24, type: 'dxa' },
3940
});
4041

4142
const table = {
@@ -51,6 +52,7 @@ describe('hydrateTableStyleAttrs', () => {
5152
expect(result?.borders).toEqual({ top: { val: 'single', size: 8 } });
5253
expect(result?.justification).toBe('center');
5354
expect(result?.cellPadding?.left).toBeCloseTo((72 / 1440) * 96);
55+
expect(result?.tableCellSpacing).toEqual({ value: 24, type: 'dxa' });
5456
expect(result?.tableWidth).toEqual({ width: 500, type: 'px' });
5557
});
5658

‎packages/layout-engine/pm-adapter/src/converters/table-styles.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export type TableStyleHydration = {
1111
cellPadding?: BoxSpacing;
1212
justification?: string;
1313
tableWidth?: { width?: number; type?: string };
14+
tableCellSpacing?: { value?: number; type?: string };
1415
/**
1516
* Paragraph properties from the table style's w:pPr element.
1617
* Per OOXML spec, these should apply to all paragraphs inside the table
@@ -62,6 +63,9 @@ export const hydrateTableStyleAttrs = (tableNode: PMNode, context?: ConverterCon
6263
if (!hydration.justification && referenced.justification) {
6364
hydration.justification = referenced.justification;
6465
}
66+
if (referenced.tableCellSpacing) {
67+
hydration.tableCellSpacing = referenced.tableCellSpacing as { value?: number; type?: string };
68+
}
6569
}
6670

6771
// Extract paragraph properties (w:pPr) from the table style definition

‎packages/layout-engine/pm-adapter/src/converters/table.test.ts‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,97 @@ describe('table converter', () => {
777777
expect(result.rows[0].cells[0]).toBeDefined();
778778
});
779779

780+
it('skips schema-default cell borders when table-level borders exist', () => {
781+
const schemaDefaultBorders = {
782+
top: { size: 8, color: '000000' },
783+
right: { size: 8, color: '000000' },
784+
bottom: { size: 8, color: '000000' },
785+
left: { size: 8, color: '000000' },
786+
};
787+
788+
const node: PMNode = {
789+
type: 'table',
790+
attrs: {
791+
borders: {
792+
top: { val: 'single', size: 8, color: '000000' },
793+
right: { val: 'single', size: 8, color: '000000' },
794+
bottom: { val: 'single', size: 8, color: '000000' },
795+
left: { val: 'single', size: 8, color: '000000' },
796+
},
797+
},
798+
content: [
799+
{
800+
type: 'tableRow',
801+
content: [
802+
{
803+
type: 'tableCell',
804+
attrs: { borders: schemaDefaultBorders },
805+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Cell' }] }],
806+
},
807+
],
808+
},
809+
],
810+
};
811+
812+
const result = tableNodeToBlock(
813+
node,
814+
mockBlockIdGenerator,
815+
mockPositionMap,
816+
'Arial',
817+
16,
818+
undefined,
819+
undefined,
820+
undefined,
821+
undefined,
822+
mockParagraphConverter,
823+
) as TableBlock;
824+
825+
expect(result.rows[0].cells[0].attrs?.borders).toBeUndefined();
826+
});
827+
828+
it('keeps schema-default cell borders when no table-level borders exist', () => {
829+
const schemaDefaultBorders = {
830+
top: { size: 8, color: '000000' },
831+
right: { size: 8, color: '000000' },
832+
bottom: { size: 8, color: '000000' },
833+
left: { size: 8, color: '000000' },
834+
};
835+
836+
const node: PMNode = {
837+
type: 'table',
838+
content: [
839+
{
840+
type: 'tableRow',
841+
content: [
842+
{
843+
type: 'tableCell',
844+
attrs: { borders: schemaDefaultBorders },
845+
content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Cell' }] }],
846+
},
847+
],
848+
},
849+
],
850+
};
851+
852+
const result = tableNodeToBlock(
853+
node,
854+
mockBlockIdGenerator,
855+
mockPositionMap,
856+
'Arial',
857+
16,
858+
undefined,
859+
undefined,
860+
undefined,
861+
undefined,
862+
mockParagraphConverter,
863+
) as TableBlock;
864+
865+
expect(result.rows[0].cells[0].attrs?.borders).toBeDefined();
866+
expect(result.rows[0].cells[0].attrs?.borders?.top).toEqual(
867+
expect.objectContaining({ style: 'single', color: '#000000' }),
868+
);
869+
});
870+
780871
it('extracts cell padding when present', () => {
781872
const node: PMNode = {
782873
type: 'table',

0 commit comments

Comments
 (0)