From b4d83adb617b76c138ab1de24fd36e6146d9c478 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Fri, 13 Mar 2026 18:20:15 +0200 Subject: [PATCH 1/3] fix: issue with vertical cells merging --- .../helpers/legacy-handle-table-cell-node.js | 59 ++++++-- .../legacy-handle-table-cell-node.test.js | 138 ++++++++++++++++++ .../v3/handlers/w/tr/tr-translator.js | 7 + 3 files changed, 194 insertions(+), 10 deletions(-) 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 a81322d9dc..41c48f4c6a 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 @@ -105,28 +105,27 @@ export function handleTableCellNode({ const rows = table.elements.filter((el) => el.name === 'w:tr'); const currentRowIndex = rows.findIndex((r) => r === row); const remainingRows = rows.slice(currentRowIndex + 1); - - const cellsInRow = row.elements.filter((el) => el.name === 'w:tc'); - let cellIndex = cellsInRow.findIndex((el) => el === node); let rowspan = 1; + const startColumn = Number.isFinite(columnIndex) ? columnIndex : 0; - // Iterate through all remaining rows after the current cell, and find all cells that need to be merged + // Continue the merge by matching cells on the logical table grid, not by raw tc index. + // This keeps vertical merges aligned when rows have gridBefore or different preceding spans. for (let remainingRow of remainingRows) { - const firstCell = remainingRow.elements.findIndex((el) => el.name === 'w:tc'); - const cellAtIndex = remainingRow.elements[firstCell + cellIndex]; + const cellAtColumn = findTableCellAtColumn(remainingRow, startColumn); - if (!cellAtIndex) break; + if (!cellAtColumn) break; - const vMerge = getTableCellVMerge(cellAtIndex); + const vMerge = getTableCellVMerge(cellAtColumn); if (!vMerge || vMerge === 'restart') { // We have reached the end of the vertically merged cells break; } - // This cell is part of a merged cell, merge it (remove it from its row) + // This cell is part of a merged cell. Mark it consumed so the row encoder skips it + // but still advances the column index (grid geometry is preserved). rowspan++; - remainingRow.elements.splice(firstCell + cellIndex, 1); + markTableCellAsVMergeConsumed(cellAtColumn); } attributes['rowspan'] = rowspan; } @@ -256,6 +255,46 @@ const getTableCellVMerge = (node) => { return vMerge.attributes?.['w:val'] || 'continue'; }; +const getGridBefore = (row) => { + const trPr = row.elements?.find((el) => el.name === 'w:trPr'); + const gridBefore = trPr?.elements?.find((el) => el.name === 'w:gridBefore'); + const raw = gridBefore?.attributes?.['w:val']; + const value = typeof raw === 'string' ? parseInt(raw, 10) : raw; + return Number.isFinite(value) && value > 0 ? value : 0; +}; + +const getTableCellGridSpan = (node) => { + if (!node || node.name !== 'w:tc') return 1; + const tcPr = node.elements?.find((el) => el.name === 'w:tcPr'); + const gridSpan = tcPr?.elements?.find((el) => el.name === 'w:gridSpan'); + const raw = gridSpan?.attributes?.['w:val']; + const value = typeof raw === 'string' ? parseInt(raw, 10) : raw; + return Number.isFinite(value) && value > 0 ? value : 1; +}; + +const findTableCellAtColumn = (row, targetColumn) => { + const cells = row.elements?.filter((el) => el.name === 'w:tc') ?? []; + let currentColumn = getGridBefore(row); + + for (const cell of cells) { + const colSpan = getTableCellGridSpan(cell); + if (targetColumn >= currentColumn && targetColumn < currentColumn + colSpan) { + return cell._vMergeConsumed ? null : cell; + } + currentColumn += colSpan; + } + + return null; +}; + +/** Mark a w:tc as consumed by a vertical merge so the row encoder skips it but advances column index. */ +const markTableCellAsVMergeConsumed = (node) => { + if (node?.name === 'w:tc') { + node._vMergeConsumed = true; + node._vMergeConsumedGridSpan = getTableCellGridSpan(node); + } +}; + /** * Process the margins for a table cell * @param {Object} inlineMargins diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js index 0fdaca67a9..bfc6ee9827 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js @@ -136,6 +136,144 @@ describe('legacy-handle-table-cell-node', () => { expect(out.attrs.rowspan).toBe(3); }); + it('resolves vertical merge continuations by logical grid column when rows use gridBefore', () => { + const cellNode = { + name: 'w:tc', + elements: [ + { + name: 'w:tcPr', + elements: [ + { name: 'w:vMerge', attributes: { 'w:val': 'restart' } }, + { name: 'w:shd', attributes: { 'w:fill': '006A72' } }, + ], + }, + { name: 'w:p' }, + ], + }; + + const row1 = { + name: 'w:tr', + elements: [ + { + name: 'w:trPr', + elements: [{ name: 'w:gridBefore', attributes: { 'w:val': '1' } }], + }, + cellNode, + { name: 'w:tc', elements: [{ name: 'w:p' }] }, + ], + }; + + const row2 = { + name: 'w:tr', + elements: [ + { + name: 'w:trPr', + elements: [{ name: 'w:gridBefore', attributes: { 'w:val': '1' } }], + }, + { + name: 'w:tc', + elements: [{ name: 'w:tcPr', elements: [{ name: 'w:vMerge' }] }, { name: 'w:p' }], + }, + { name: 'w:tc', elements: [{ name: 'w:p' }] }, + ], + }; + + const table = { name: 'w:tbl', elements: [row1, row2] }; + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => 'CONTENT') }, + path: [], + editor: createEditorStub(), + }; + + const out = handleTableCellNode({ + params, + node: cellNode, + table, + row: row1, + columnIndex: 1, + columnWidth: null, + allColumnWidths: [90, 100, 110], + _referencedStyles: null, + }); + + expect(out.attrs.background).toEqual({ color: '006A72' }); + expect(out.attrs.rowspan).toBe(2); + const row2Cells = row2.elements.filter((el) => el.name === 'w:tc'); + expect(row2Cells).toHaveLength(2); + expect(row2Cells[0]._vMergeConsumed).toBe(true); + }); + + it('preserves later merge-column alignment after removing an earlier continuation cell', () => { + const firstRestart = { + name: 'w:tc', + elements: [ + { name: 'w:tcPr', elements: [{ name: 'w:vMerge', attributes: { 'w:val': 'restart' } }] }, + { name: 'w:p' }, + ], + }; + const secondRestart = { + name: 'w:tc', + elements: [ + { + name: 'w:tcPr', + elements: [ + { name: 'w:vMerge', attributes: { 'w:val': 'restart' } }, + { name: 'w:shd', attributes: { 'w:fill': '006A72' } }, + ], + }, + { name: 'w:p' }, + ], + }; + const firstContinue = { + name: 'w:tc', + elements: [{ name: 'w:tcPr', elements: [{ name: 'w:vMerge' }] }, { name: 'w:p' }], + }; + const secondContinue = { + name: 'w:tc', + elements: [{ name: 'w:tcPr', elements: [{ name: 'w:vMerge' }] }, { name: 'w:p' }], + }; + + const row1 = { name: 'w:tr', elements: [firstRestart, secondRestart] }; + const row2 = { name: 'w:tr', elements: [firstContinue, secondContinue] }; + const table = { name: 'w:tbl', elements: [row1, row2] }; + const params = { + docx: {}, + nodeListHandler: { handler: vi.fn(() => 'CONTENT') }, + path: [], + editor: createEditorStub(), + }; + + const outFirst = handleTableCellNode({ + params, + node: firstRestart, + table, + row: row1, + columnIndex: 0, + columnWidth: null, + allColumnWidths: [90, 100], + _referencedStyles: null, + }); + + const outSecond = handleTableCellNode({ + params, + node: secondRestart, + table, + row: row1, + columnIndex: 1, + columnWidth: null, + allColumnWidths: [90, 100], + _referencedStyles: null, + }); + + expect(outFirst.attrs.rowspan).toBe(2); + expect(outSecond.attrs.rowspan).toBe(2); + expect(outSecond.attrs.background).toEqual({ color: '006A72' }); + const row2Cells = row2.elements.filter((el) => el.name === 'w:tc'); + expect(row2Cells).toHaveLength(2); + expect(row2Cells.every((tc) => tc._vMergeConsumed)).toBe(true); + }); + it('blends percentage table shading into a solid background color', () => { const cellNode = { name: 'w:tc', elements: [{ name: 'w:p' }] }; const row = { name: 'w:tr', elements: [cellNode] }; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js index de0d9b7096..8183a050f8 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js @@ -102,6 +102,13 @@ const encode = (params, encodedAttrs) => { skipOccupiedColumns(); const startColumn = currentColumnIndex; + + // Cell was consumed by a vertical merge (rowspan) above; skip encoding but preserve column advance + if (node._vMergeConsumed && Number.isFinite(node._vMergeConsumedGridSpan)) { + currentColumnIndex = startColumn + node._vMergeConsumedGridSpan; + return; + } + const columnWidth = gridColumnWidths?.[startColumn] || null; const result = tcTranslator.encode({ From ea88df7906de745524580b58e791c7b32f393bf5 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Mon, 16 Mar 2026 17:20:19 +0200 Subject: [PATCH 2/3] fix: don't consume merged cols twice --- .../w/tc/helpers/legacy-handle-table-cell-node.js | 3 +-- .../super-converter/v3/handlers/w/tr/tr-translator.js | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-) 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 41c48f4c6a..934fe0454e 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 @@ -287,11 +287,10 @@ const findTableCellAtColumn = (row, targetColumn) => { return null; }; -/** Mark a w:tc as consumed by a vertical merge so the row encoder skips it but advances column index. */ +/** Mark a w:tc as consumed by a vertical merge so the row encoder skips it. */ const markTableCellAsVMergeConsumed = (node) => { if (node?.name === 'w:tc') { node._vMergeConsumed = true; - node._vMergeConsumedGridSpan = getTableCellGridSpan(node); } }; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js index 8183a050f8..e8c5d813bc 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.js @@ -103,11 +103,10 @@ const encode = (params, encodedAttrs) => { const startColumn = currentColumnIndex; - // Cell was consumed by a vertical merge (rowspan) above; skip encoding but preserve column advance - if (node._vMergeConsumed && Number.isFinite(node._vMergeConsumedGridSpan)) { - currentColumnIndex = startColumn + node._vMergeConsumedGridSpan; - return; - } + // Cell was consumed by a vertical merge (rowspan) above. + // At this point skipOccupiedColumns() has already advanced past the spanned columns + // using pendingRowSpans, so we just skip encoding without touching currentColumnIndex. + if (node._vMergeConsumed) return; const columnWidth = gridColumnWidths?.[startColumn] || null; From 8779b5c99a68303eeb30058269b563e9a17e2d32 Mon Sep 17 00:00:00 2001 From: VladaHarbour Date: Thu, 19 Mar 2026 15:01:34 +0200 Subject: [PATCH 3/3] chore: add test case for _vMergeConsumed: true --- .../v3/handlers/w/tr/tr-translator.test.js | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js index ca548ffab4..b04b3b331b 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tr/tr-translator.test.js @@ -186,6 +186,51 @@ describe('w:tr translator', () => { expect(trPrTranslator.encode).not.toHaveBeenCalled(); expect(tcTranslator.encode).not.toHaveBeenCalled(); }); + + it('skips vMerge-consumed cells without advancing the next encoded cell column', () => { + const rowWithConsumedCell = { + name: 'w:tr', + elements: [ + { name: 'w:tc', elements: [], _vMergeConsumed: true }, + { name: 'w:tc', elements: [] }, + { name: 'w:tc', elements: [] }, + ], + }; + const params = { + nodes: [rowWithConsumedCell], + extraParams: { + row: rowWithConsumedCell, + columnWidths: [100, 150, 200], + activeRowSpans: [1, 0, 0], + }, + }; + + const result = translator.encode(params, {}); + + expect(tcTranslator.encode).toHaveBeenCalledTimes(2); + expect(tcTranslator.encode).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ + extraParams: expect.objectContaining({ + node: rowWithConsumedCell.elements[1], + columnIndex: 1, + columnWidth: 150, + }), + }), + ); + expect(tcTranslator.encode).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ + extraParams: expect.objectContaining({ + node: rowWithConsumedCell.elements[2], + columnIndex: 2, + columnWidth: 200, + }), + }), + ); + expect(result.content).toHaveLength(2); + expect(result.content.map((cell) => cell.attrs.columnIndex)).toEqual([1, 2]); + }); }); describe('decode', () => {