Skip to content

Commit c388fa7

Browse files
harbournickharbournick
committed
fix(tables): preserve TableGrid defaults and style-driven spacing/border behavior
1 parent 066b9eb commit c388fa7

14 files changed

Lines changed: 255 additions & 42 deletions

File tree

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

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -293,26 +293,48 @@ 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;
301304

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

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

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ type ParseTableCellArgs = {
8383
context: TableParserDependencies;
8484
defaultCellPadding?: BoxSpacing;
8585
tableProperties?: TableProperties;
86+
/** When true, table-level borders exist (from hydrated style or inline) so schema-default cell borders can be skipped. */
87+
hasTableLevelBorders?: boolean;
8688
};
8789

8890
type ParseTableRowArgs = {
@@ -93,6 +95,8 @@ type ParseTableRowArgs = {
9395
defaultCellPadding?: BoxSpacing;
9496
/** Table style to pass to paragraph converter for style cascade */
9597
tableProperties?: TableProperties;
98+
/** When true, table-level borders exist so schema-default cell borders can be skipped. */
99+
hasTableLevelBorders?: boolean;
96100
};
97101

98102
const isTableRowNode = (node: PMNode): boolean => node.type === 'tableRow' || node.type === 'table_row';
@@ -195,7 +199,17 @@ const normalizeRowHeight = (rowProps?: Record<string, unknown>): NormalizedRowHe
195199
* // Returns: null
196200
*/
197201
const parseTableCell = (args: ParseTableCellArgs): TableCell | null => {
198-
const { cellNode, rowIndex, cellIndex, numCells, numRows, context, defaultCellPadding, tableProperties } = args;
202+
const {
203+
cellNode,
204+
rowIndex,
205+
cellIndex,
206+
numCells,
207+
numRows,
208+
context,
209+
defaultCellPadding,
210+
tableProperties,
211+
hasTableLevelBorders,
212+
} = args;
199213
if (!isTableCellNode(cellNode) || !Array.isArray(cellNode.content)) {
200214
return null;
201215
}
@@ -430,8 +444,25 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => {
430444

431445
const cellAttrs: TableCellAttrs = {};
432446

433-
const borders = extractCellBorders(cellNode.attrs ?? {});
434-
if (borders) cellAttrs.borders = borders;
447+
// Schema-default cell borders (from createCellBorders) have { size, color } without a `val`
448+
// property, while OOXML-imported borders always include `val` (e.g., 'single', 'none').
449+
// When table-level borders exist (from a hydrated style like TableGrid), skip schema defaults
450+
// so the painter's resolveTableCellBorders can distribute table borders to cells properly.
451+
// When NO table-level borders exist, keep schema defaults as the fallback (matching Word's
452+
// default behavior of showing single black borders on new tables).
453+
const rawBorders = cellNode.attrs?.borders as Record<string, Record<string, unknown>> | undefined;
454+
const isSchemaDefaultBorders =
455+
rawBorders &&
456+
typeof rawBorders === 'object' &&
457+
['top', 'right', 'bottom', 'left'].every((side) => {
458+
const b = rawBorders[side];
459+
return b && typeof b === 'object' && !('val' in b);
460+
});
461+
462+
if (!(isSchemaDefaultBorders && hasTableLevelBorders)) {
463+
const borders = extractCellBorders(cellNode.attrs ?? {});
464+
if (borders) cellAttrs.borders = borders;
465+
}
435466

436467
const padding =
437468
extractCellPadding(cellNode.attrs ?? {}) ?? (defaultCellPadding ? { ...defaultCellPadding } : undefined);
@@ -510,7 +541,7 @@ const parseTableCell = (args: ParseTableCellArgs): TableCell | null => {
510541
* // Returns: null
511542
*/
512543
const parseTableRow = (args: ParseTableRowArgs): TableRow | null => {
513-
const { rowNode, rowIndex, context, defaultCellPadding, tableProperties, numRows } = args;
544+
const { rowNode, rowIndex, context, defaultCellPadding, tableProperties, numRows, hasTableLevelBorders } = args;
514545
if (!isTableRowNode(rowNode) || !Array.isArray(rowNode.content)) {
515546
return null;
516547
}
@@ -526,6 +557,7 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => {
526557
tableProperties,
527558
numCells: rowNode?.content?.length || 1,
528559
numRows,
560+
hasTableLevelBorders,
529561
});
530562
if (parsedCell) {
531563
cells.push(parsedCell);
@@ -722,6 +754,22 @@ export function tableNodeToBlock(
722754
const hydratedTableStyle = hydrateTableStyleAttrs(node, converterContext);
723755
const defaultCellPadding = hydratedTableStyle?.cellPadding;
724756

757+
// Pre-compute whether table-level borders exist (from inline or hydrated style).
758+
// When they do, schema-default cell borders can be safely skipped so the painter's
759+
// resolveTableCellBorders distributes table borders to cells.
760+
// When they don't, cell schema defaults are kept as fallback.
761+
const inlineBorders = node.attrs?.borders;
762+
const hasInlineBorders =
763+
inlineBorders &&
764+
typeof inlineBorders === 'object' &&
765+
inlineBorders !== null &&
766+
Object.keys(inlineBorders as Record<string, unknown>).length > 0;
767+
const hasHydratedBorders =
768+
hydratedTableStyle?.borders &&
769+
typeof hydratedTableStyle.borders === 'object' &&
770+
Object.keys(hydratedTableStyle.borders).length > 0;
771+
const hasTableLevelBorders = !!(hasInlineBorders || hasHydratedBorders);
772+
725773
const rows: TableRow[] = [];
726774
node.content.forEach((rowNode, rowIndex) => {
727775
const parsedRow = parseTableRow({
@@ -731,6 +779,7 @@ export function tableNodeToBlock(
731779
context: parserDeps,
732780
defaultCellPadding,
733781
tableProperties: node.attrs?.tableProperties as TableProperties | undefined,
782+
hasTableLevelBorders,
734783
});
735784
if (parsedRow) {
736785
rows.push(parsedRow);
@@ -741,7 +790,12 @@ export function tableNodeToBlock(
741790

742791
const tableAttrs: Record<string, unknown> = {};
743792
const getBorderSource = (): Record<string, unknown> | undefined => {
744-
if (node.attrs?.borders && typeof node.attrs.borders === 'object' && node.attrs.borders !== null) {
793+
if (
794+
node.attrs?.borders &&
795+
typeof node.attrs.borders === 'object' &&
796+
node.attrs.borders !== null &&
797+
Object.keys(node.attrs.borders as Record<string, unknown>).length > 0
798+
) {
745799
return node.attrs.borders as Record<string, unknown>;
746800
}
747801
if (
@@ -763,6 +817,12 @@ export function tableNodeToBlock(
763817

764818
if (node.attrs?.tableCellSpacing !== undefined && node.attrs?.tableCellSpacing !== null) {
765819
tableAttrs.cellSpacing = normalizeCellSpacing(node.attrs.tableCellSpacing);
820+
} else if (hydratedTableStyle?.tableCellSpacing) {
821+
tableAttrs.cellSpacing = normalizeCellSpacing(hydratedTableStyle.tableCellSpacing);
822+
// Cell spacing requires border-collapse: separate
823+
if (!tableAttrs.borderCollapse) {
824+
tableAttrs.borderCollapse = 'separate';
825+
}
766826
}
767827

768828
if (node.attrs?.justification) {

‎packages/super-editor/src/core/commands/insertContent.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ export const insertContent =
3737
});
3838

3939
const jsonContent = processedDoc.toJSON();
40-
const ok = commands.insertContentAt({ from: tr.selection.from, to: tr.selection.to }, jsonContent, options);
40+
const insertionContent =
41+
jsonContent?.type === 'doc' && Array.isArray(jsonContent.content) ? jsonContent.content : jsonContent;
42+
const ok = commands.insertContentAt(
43+
{ from: tr.selection.from, to: tr.selection.to },
44+
insertionContent,
45+
options,
46+
);
4147

4248
// Schedule list migration right after the insert transaction dispatches
4349
if (ok && (options.contentType === 'html' || options.contentType === 'markdown')) {

‎packages/super-editor/src/core/commands/insertContent.test.js‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,7 @@ describe('insertContent', () => {
5555
editor: mockEditor,
5656
});
5757

58-
expect(mockCommands.insertContentAt).toHaveBeenCalledWith(
59-
{ from: 0, to: 10 },
60-
{ type: 'doc', content: [] },
61-
{ contentType: 'html' },
62-
);
58+
expect(mockCommands.insertContentAt).toHaveBeenCalledWith({ from: 0, to: 10 }, [], { contentType: 'html' });
6359

6460
// Should trigger list migration for HTML (microtask)
6561
expect(mockEditor.migrateListsToV2).toHaveBeenCalledTimes(1);
@@ -265,6 +261,24 @@ describe('insertContent (integration) list export', () => {
265261
expect(first.ilvl).toBe('0');
266262
});
267263

264+
it('inserts markdown heading + bold text without creating a table', async () => {
265+
const editor = await setupEditor();
266+
267+
editor.commands.insertContent('# Hello\n\nSome **bold** text', { contentType: 'markdown' });
268+
await Promise.resolve();
269+
270+
const doc = editor.getJSON();
271+
const tableNode = (doc.content || []).find((node) => node?.type === 'table');
272+
273+
expect(tableNode).toBeUndefined();
274+
expect(
275+
doc.content?.some(
276+
(node) => node?.type === 'paragraph' && node?.attrs?.paragraphProperties?.styleId === 'Heading1',
277+
),
278+
).toBe(true);
279+
expect(doc.content?.some((node) => node?.type === 'paragraph')).toBe(true);
280+
});
281+
268282
it('exports unordered list from HTML with numId/ilvl', async () => {
269283
const editor = await setupEditor();
270284
editor.commands.insertContent('<ul><li>Apple</li><li>Banana</li></ul>', { contentType: 'html' });

‎packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ function convertTable(node: MdastTable, ctx: MdastConversionContext): JsonNode {
340340
return {
341341
type: 'table',
342342
attrs: {
343+
tableStyleId: 'TableGrid',
343344
tableProperties: {
344345
tableWidth: {
345346
value: FULL_WIDTH_TABLE_PCT,

0 commit comments

Comments
 (0)