From c7368e3512c0608ebee35e646174ef2a2db01e47 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 27 Feb 2026 11:23:01 -0800 Subject: [PATCH 1/2] fix(document-api): delete table cell fix --- .../tables-adapter.regressions.test.ts | 71 ++++++++++++++++ .../document-api-adapters/tables-adapter.ts | 80 ++++++++++++++++--- .../tests/tables/all-commands.ts | 37 ++++++++- 3 files changed, 174 insertions(+), 14 deletions(-) diff --git a/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts b/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts index 4e4dea34b7..aa85309b15 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts @@ -273,6 +273,77 @@ describe('tables-adapter regressions', () => { expect(tr.insert).toHaveBeenCalledWith(expectedInsertPos, expect.anything()); }); + it('deletes shiftLeft cells without appending a trailing replacement cell', () => { + const editor = makeTableEditor(); + const tr = editor.state.tr as unknown as { + delete: ReturnType; + insert: ReturnType; + setNodeMarkup: ReturnType; + }; + const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode; + const targetCellOffset = TableMap.get(tableNode).map[0]!; + const targetCellNode = tableNode.nodeAt(targetCellOffset) as ProseMirrorNode; + const expectedStart = 1 + targetCellOffset; + const expectedEnd = expectedStart + targetCellNode.nodeSize; + + const result = tablesDeleteCellAdapter(editor, { nodeId: 'cell-1', mode: 'shiftLeft' }); + expect(result.success).toBe(true); + expect(tr.delete).toHaveBeenCalledWith(expectedStart, expectedEnd); + expect(tr.insert).not.toHaveBeenCalled(); + expect(tr.setNodeMarkup).toHaveBeenCalledWith( + expect.any(Number), + null, + expect.objectContaining({ + colspan: 2, + }), + ); + }); + + it('deletes the row trailing cell for shiftLeft without appending a replacement cell', () => { + const editor = makeTableEditor(); + const tr = editor.state.tr as unknown as { + delete: ReturnType; + insert: ReturnType; + setNodeMarkup: ReturnType; + }; + const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode; + const targetCellOffset = TableMap.get(tableNode).map[1]!; + const targetCellNode = tableNode.nodeAt(targetCellOffset) as ProseMirrorNode; + const expectedStart = 1 + targetCellOffset; + const expectedEnd = expectedStart + targetCellNode.nodeSize; + + const result = tablesDeleteCellAdapter(editor, { nodeId: 'cell-2', mode: 'shiftLeft' }); + expect(result.success).toBe(true); + expect(tr.delete).toHaveBeenCalledWith(expectedStart, expectedEnd); + expect(tr.insert).not.toHaveBeenCalled(); + expect(tr.setNodeMarkup).toHaveBeenCalledWith( + expect.any(Number), + null, + expect.objectContaining({ + colspan: 2, + }), + ); + }); + + it('falls back to trailing replacement cell when shiftLeft would widen a vertically merged trailing cell', () => { + const editor = makeTableEditor(); + const tr = editor.state.tr as unknown as { + delete: ReturnType; + insert: ReturnType; + setNodeMarkup: ReturnType; + }; + const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode; + const firstRow = tableNode.child(0) as ProseMirrorNode; + const trailingCell = firstRow.child(1) as unknown as { attrs: Record }; + trailingCell.attrs.rowspan = 2; + trailingCell.attrs.tableCellProperties = { vMerge: 'restart' }; + + const result = tablesDeleteCellAdapter(editor, { nodeId: 'cell-1', mode: 'shiftLeft' }); + expect(result.success).toBe(true); + expect(tr.insert).toHaveBeenCalledWith(expect.any(Number), expect.anything()); + expect(tr.setNodeMarkup).not.toHaveBeenCalled(); + }); + it('rejects paragraph targets for tables.setBorder', () => { const editor = makeTableEditor(); const result = tablesSetBorderAdapter(editor, { diff --git a/packages/super-editor/src/document-api-adapters/tables-adapter.ts b/packages/super-editor/src/document-api-adapters/tables-adapter.ts index ef7dab4688..592c05c2e5 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -1641,8 +1641,9 @@ export function tablesInsertCellAdapter( /** * tables.deleteCell — delete a cell at a resolved position, shifting remaining cells. * - * `shiftLeft`: removes the target cell and appends a new empty cell at the end of - * the row to maintain column count. + * `shiftLeft`: removes the target cell and shifts remaining cells in the row left. + * This reduces the row cell count by one and avoids a synthetic trailing cell unless + * widening the remaining trailing cell would conflict with vertical merges. * * `shiftUp`: removes the target cell and appends a new empty cell at the same column * in the last row to maintain row count. @@ -1656,6 +1657,15 @@ export function tablesDeleteCellAdapter( const resolved = resolveCellLocator(editor, input, 'tables.deleteCell'); const { table, cellPos, cellNode, rowIndex, columnIndex } = resolved; + const row = table.candidate.node.child(rowIndex); + const deletedColspan = Math.max(1, ((cellNode.attrs as Record).colspan as number) || 1); + const deletedColwidth = Array.isArray((cellNode.attrs as Record).colwidth) + ? [...((cellNode.attrs as Record).colwidth as number[])] + : null; + + if (input.mode === 'shiftLeft' && row.childCount <= 1) { + return toTableFailure('NO_OP', 'Cannot shift-left delete the last remaining cell in a row.'); + } if (options?.dryRun) { return buildTableSuccess(table.address); @@ -1673,17 +1683,61 @@ export function tablesDeleteCellAdapter( tr.delete(cellPos, cellPos + cellNode.nodeSize); if (input.mode === 'shiftLeft') { - // Append a new empty cell at the end of this row. - const row = tableNode.child(rowIndex); - const rowEnd = tr.mapping.map(tableStart + map.positionAt(rowIndex, map.width - 1, tableNode)); - const lastCell = row.child(row.childCount - 1); - const insertPos = tr.mapping.map(cellPos + cellNode.nodeSize + lastCell.nodeSize - cellNode.nodeSize); - // Find the end of the row in the mapped doc — simpler: compute row end position. - let rowEndPos = table.candidate.pos + 1; - for (let i = 0; i <= rowIndex; i++) rowEndPos += tableNode.child(i).nodeSize; - const mappedRowEnd = tr.mapping.map(rowEndPos - 1); // -1 to be inside the row closing tag - const newCell = schema.nodes.tableCell.createAndFill()!; - tr.insert(mappedRowEnd, newCell); + // Prefer preserving fewer visual cells by widening the new trailing cell. + // Fall back to a trailing replacement cell when merged geometry would become invalid. + const currentTableNode = tr.doc.nodeAt(tablePos); + if (!currentTableNode || currentTableNode.type.name !== 'table') { + return toTableFailure('INVALID_TARGET', 'Cell deletion could not locate the updated table.'); + } + + const currentRow = currentTableNode.child(rowIndex); + const lastCellIndex = currentRow.childCount - 1; + const lastCell = currentRow.child(lastCellIndex); + const lastAttrs = lastCell.attrs as Record; + const tableCellProperties = (lastAttrs.tableCellProperties ?? {}) as Record; + const lastRowspan = Math.max(1, (lastAttrs.rowspan as number) || 1); + const hasVerticalMerge = tableCellProperties.vMerge != null; + + if (lastRowspan > 1 || hasVerticalMerge) { + // Extending a vertically merged cell can overlap cells in lower rows. + let rowEndPos = tablePos + 1; + for (let i = 0; i <= rowIndex; i++) rowEndPos += currentTableNode.child(i).nodeSize; + const mappedRowEnd = tr.mapping.map(rowEndPos - 1); // -1 to stay inside the row. + const newCell = schema.nodes.tableCell.createAndFill()!; + tr.insert(mappedRowEnd, newCell); + } else { + const lastColspan = Math.max(1, (lastAttrs.colspan as number) || 1); + const nextColspan = lastColspan + deletedColspan; + + const nextTableCellProps = { + ...tableCellProperties, + }; + if (nextColspan > 1) nextTableCellProps.gridSpan = nextColspan; + else delete nextTableCellProps.gridSpan; + + const nextColwidth = Array.isArray(lastAttrs.colwidth) ? [...(lastAttrs.colwidth as number[])] : null; + if (nextColwidth) { + if (deletedColwidth) { + for (const width of deletedColwidth) { + if (nextColwidth.length >= nextColspan) break; + nextColwidth.push(typeof width === 'number' ? width : 0); + } + } + while (nextColwidth.length < nextColspan) nextColwidth.push(0); + } + + let rowOffset = 0; + for (let i = 0; i < rowIndex; i++) rowOffset += currentTableNode.child(i).nodeSize; + let lastCellOffset = rowOffset + 1; + for (let i = 0; i < lastCellIndex; i++) lastCellOffset += currentRow.child(i).nodeSize; + + tr.setNodeMarkup(tableStart + lastCellOffset, null, { + ...lastAttrs, + colspan: nextColspan, + colwidth: nextColwidth, + tableCellProperties: nextTableCellProps, + }); + } } else { // shiftUp: insert a new empty cell at the same column in the last row. const lastRowIndex = map.height - 1; diff --git a/tests/doc-api-stories/tests/tables/all-commands.ts b/tests/doc-api-stories/tests/tables/all-commands.ts index 31920ba34a..16046e6745 100644 --- a/tests/doc-api-stories/tests/tables/all-commands.ts +++ b/tests/doc-api-stories/tests/tables/all-commands.ts @@ -76,6 +76,7 @@ describe('document-api story: all table commands', () => { const clearStyleTableBySession = new Map(); const convertToTextTableBySession = new Map(); const deleteCellBySession = new Map(); + const deleteCellTableBySession = new Map(); function makeSessionId(prefix: string): string { return `${prefix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; @@ -599,20 +600,54 @@ describe('document-api story: all table commands', () => { } const cellsResult = unwrap(await api.doc.tables.getCells({ sessionId, nodeId: tableNodeId })); + const firstRowBefore = Array.isArray(cellsResult?.cells) + ? cellsResult.cells.filter((cell: any) => cell?.rowIndex === 0) + : []; + if (firstRowBefore.length !== 3) { + throw new Error(`tables.deleteCell setup expected 3 cells in first row, received ${firstRowBefore.length}.`); + } const firstCellNodeId = cellsResult?.cells?.[0]?.nodeId; if (!firstCellNodeId) { throw new Error('tables.deleteCell setup failed: no table cell was returned from getCells.'); } deleteCellBySession.set(sessionId, firstCellNodeId); + deleteCellTableBySession.set(sessionId, tableNodeId); }, run: async (sessionId) => { const cellNodeId = deleteCellBySession.get(sessionId); + const tableNodeId = deleteCellTableBySession.get(sessionId); if (!cellNodeId) { throw new Error('tables.deleteCell setup failed: prepared cell nodeId was not found.'); } + if (!tableNodeId) { + throw new Error('tables.deleteCell setup failed: prepared table nodeId was not found.'); + } deleteCellBySession.delete(sessionId); - return unwrap(await api.doc.tables.deleteCell({ sessionId, nodeId: cellNodeId, mode: 'shiftLeft' })); + deleteCellTableBySession.delete(sessionId); + + const result = unwrap( + await api.doc.tables.deleteCell({ sessionId, nodeId: cellNodeId, mode: 'shiftLeft' }), + ); + assertMutationSuccess('tables.deleteCell', result); + + const postCells = unwrap(await api.doc.tables.getCells({ sessionId, nodeId: tableNodeId, rowIndex: 0 })); + const firstRowAfter = Array.isArray(postCells?.cells) + ? postCells.cells.filter((cell: any) => cell?.rowIndex === 0) + : []; + if (firstRowAfter.length !== 2) { + throw new Error( + `tables.deleteCell expected first-row cell count to be 2 after shiftLeft, received ${firstRowAfter.length}.`, + ); + } + const firstRowColumns = firstRowAfter.map((cell: any) => Number(cell?.columnIndex)).sort((a, b) => a - b); + if (firstRowColumns.join(',') !== '0,1') { + throw new Error( + `tables.deleteCell expected first-row column indexes [0,1] after shiftLeft, received [${firstRowColumns.join(',')}].`, + ); + } + + return result; }, }, { From 303dbfd39ba40ab897b2bb2dec6fc5fa9bced55a Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 27 Feb 2026 13:13:35 -0800 Subject: [PATCH 2/2] fix(document-api): remove double-mapping of rowEndPos in shiftLeft vMerge fallback --- .../tables-adapter.regressions.test.ts | 60 +++++++++++++++++++ .../document-api-adapters/tables-adapter.ts | 2 +- 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts b/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts index 26398f25b8..04f96314de 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.regressions.test.ts @@ -357,6 +357,66 @@ describe('tables-adapter regressions', () => { expect(tr.setNodeMarkup).not.toHaveBeenCalled(); }); + it('shiftLeft vMerge fallback inserts at the post-delete row end without double-mapping', () => { + // Regression: rowEndPos was computed from the post-delete doc (tr.doc) but then + // passed through tr.mapping.map() which maps old→new, double-shifting the position. + const editor = makeTableEditor(); + + const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode; + const firstRow = tableNode.child(0) as ProseMirrorNode; + const trailingCell = firstRow.child(1) as unknown as { attrs: Record }; + trailingCell.attrs.rowspan = 2; + trailingCell.attrs.tableCellProperties = { vMerge: 'restart' }; + + const cell1 = firstRow.child(0); + const deletionStart = 2; // absolute position of cell-1 + const deletionSize = cell1.nodeSize; // 9 + + // Build post-delete table: row 0 only contains the vMerge cell. + const postDeleteRow0 = createNode('tableRow', [firstRow.child(1)], { + attrs: { ...(firstRow.attrs as Record) }, + isBlock: true, + inlineContent: false, + }); + const postDeleteTable = createNode('table', [postDeleteRow0, tableNode.child(1)], { + attrs: { ...(tableNode.attrs as Record) }, + isBlock: true, + inlineContent: false, + }); + const postDeleteDoc = createNode('doc', [postDeleteTable], { isBlock: false }); + + const trObj = editor.state.tr as unknown as { + delete: ReturnType; + insert: ReturnType; + mapping: { map: (p: number) => number; maps: unknown[]; slice: () => { map: (p: number) => number } }; + doc: ProseMirrorNode; + }; + + // Swap tr.doc to the post-delete document when delete is called. + trObj.delete = vi.fn(() => { + trObj.doc = postDeleteDoc; + return trObj; + }); + + // Simulate real deletion mapping: positions at or after the deleted range shift left. + trObj.mapping.map = (pos: number) => { + if (pos < deletionStart) return pos; + if (pos < deletionStart + deletionSize) return deletionStart; + return pos - deletionSize; + }; + + const result = tablesDeleteCellAdapter(editor, { nodeId: 'cell-1', mode: 'shiftLeft' }); + expect(result.success).toBe(true); + expect(trObj.insert).toHaveBeenCalled(); + + // Post-delete row 0 nodeSize = 2 + cell-2 size (9) = 11. + // rowEndPos = tablePos(0) + 1 + 11 = 12. + // Correct insert = 12 - 1 = 11 (just inside the row). + // Old buggy code: tr.mapping.map(11) = 11 - 9 = 2 — wrong position! + const insertPos = trObj.insert.mock.calls[0]![0]; + expect(insertPos).toBe(11); + }); + it('keeps table grid widths in sync when distributing columns', () => { const editor = makeTableEditor(); const tr = editor.state.tr as unknown as { setNodeMarkup: ReturnType }; diff --git a/packages/super-editor/src/document-api-adapters/tables-adapter.ts b/packages/super-editor/src/document-api-adapters/tables-adapter.ts index db6f67093d..c6b0f7cdbf 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -1718,7 +1718,7 @@ export function tablesDeleteCellAdapter( // Extending a vertically merged cell can overlap cells in lower rows. let rowEndPos = tablePos + 1; for (let i = 0; i <= rowIndex; i++) rowEndPos += currentTableNode.child(i).nodeSize; - const mappedRowEnd = tr.mapping.map(rowEndPos - 1); // -1 to stay inside the row. + const mappedRowEnd = rowEndPos - 1; // -1 to stay inside the row. No mapping needed — rowEndPos is already in post-delete doc space. const newCell = schema.nodes.tableCell.createAndFill()!; tr.insert(mappedRowEnd, newCell); } else {