From 8b8af107b4bbcd7a2496e86104e183c08501d618 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Fri, 27 Feb 2026 12:01:03 -0800 Subject: [PATCH] fix(document-api): insert table cell --- .../tables-adapter.regressions.test.ts | 74 +++++++++--- .../document-api-adapters/tables-adapter.ts | 109 ++++++++++++++++-- .../tests/tables/all-commands.ts | 99 +++++++++++++++- 3 files changed, 251 insertions(+), 31 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 55f7f05f07..202bdab2da 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 @@ -196,6 +196,7 @@ function makeTableEditor(): Editor { const tr = { delete: vi.fn().mockReturnThis(), insert: vi.fn().mockReturnThis(), + replaceWith: vi.fn().mockReturnThis(), setNodeMarkup: vi.fn().mockReturnThis(), setMeta: vi.fn().mockReturnThis(), mapping: { @@ -213,13 +214,58 @@ function makeTableEditor(): Editor { schema: { nodes: { tableCell: { - createAndFill: vi.fn(() => - createNode('tableCell', [mockParagraph], { - attrs: { colspan: 1, rowspan: 1 }, + createAndFill: vi.fn((attrs: Record = {}, content?: unknown) => { + const children = Array.isArray(content) + ? (content as ProseMirrorNode[]) + : content + ? ([content] as ProseMirrorNode[]) + : [mockParagraph]; + return createNode('tableCell', children, { + attrs: { colspan: 1, rowspan: 1, ...attrs }, isBlock: true, inlineContent: false, - }), - ), + }); + }), + }, + tableRow: { + createAndFill: vi.fn((attrs: Record = {}, content?: unknown) => { + const children = Array.isArray(content) + ? (content as ProseMirrorNode[]) + : content + ? ([content] as ProseMirrorNode[]) + : []; + return createNode('tableRow', children, { + attrs, + isBlock: true, + inlineContent: false, + }); + }), + create: vi.fn((attrs: Record = {}, content?: unknown) => { + const children = Array.isArray(content) + ? (content as ProseMirrorNode[]) + : content + ? ([content] as ProseMirrorNode[]) + : []; + return createNode('tableRow', children, { + attrs, + isBlock: true, + inlineContent: false, + }); + }), + }, + table: { + create: vi.fn((attrs: Record = {}, content?: unknown) => { + const children = Array.isArray(content) + ? (content as ProseMirrorNode[]) + : content + ? ([content] as ProseMirrorNode[]) + : []; + return createNode('table', children, { + attrs, + isBlock: true, + inlineContent: false, + }); + }), }, }, }, @@ -240,21 +286,19 @@ function getTableGridUpdateAttrs(tr: { setNodeMarkup: ReturnType } } describe('tables-adapter regressions', () => { - it('uses target-cell row coordinates for shiftRight insert on non-first cells', () => { + it('preserves shiftRight data by rebuilding the table instead of deleting the row tail cell', () => { const editor = makeTableEditor(); - const tr = editor.state.tr as unknown as { delete: ReturnType }; + const tr = editor.state.tr as unknown as { + delete: ReturnType; + replaceWith: ReturnType; + }; const tableNode = editor.state.doc.nodeAt(0) as ProseMirrorNode; - const map = TableMap.get(tableNode); - - const targetRowIndex = 1; - const lastCellInRowOffset = map.map[targetRowIndex * map.width + (map.width - 1)]!; - const lastCellNode = tableNode.nodeAt(lastCellInRowOffset) as ProseMirrorNode; - const expectedStart = 1 + lastCellInRowOffset; - const expectedEnd = expectedStart + lastCellNode.nodeSize; const result = tablesInsertCellAdapter(editor, { nodeId: 'cell-4', mode: 'shiftRight' }); expect(result.success).toBe(true); - expect(tr.delete).toHaveBeenCalledWith(expectedStart, expectedEnd); + expect(tr.delete).not.toHaveBeenCalled(); + expect(tr.insert).toHaveBeenCalled(); + expect(tr.replaceWith).toHaveBeenCalledWith(0, expect.any(Number), expect.anything()); }); it('inserts shiftDown cells in the same column of the next row', () => { 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 bb457ca18b..1b5f9ee179 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -1584,8 +1584,9 @@ export function tablesConvertToTextAdapter( /** * tables.insertCell — insert a cell at a resolved position, shifting existing cells. * - * `shiftRight`: inserts a new cell before the target cell in its row; the last cell - * in that row is removed to maintain column count. + * `shiftRight`: inserts a new cell before the target and cascades overflow cells to + * subsequent rows in row-major order. If needed, appends a new trailing row so + * existing cell content is preserved without dropping the rightmost value. * * `shiftDown`: inserts a new cell at the same column in the row below (creating a row * if needed). The last cell of the target column is removed to maintain row count. @@ -1613,18 +1614,102 @@ export function tablesInsertCellAdapter( const schema = editor.state.schema; if (input.mode === 'shiftRight') { - // Remove the last cell in this row, then insert a new cell before the target cell. - const lastColIdx = rowIndex * map.width + (map.width - 1); - const lastCellPos = map.map[lastColIdx]; - const lastCell = tableNode.nodeAt(lastCellPos); - if (lastCell) { - tr.delete(tableStart + lastCellPos, tableStart + lastCellPos + lastCell.nodeSize); + const slotCount = map.width * map.height; + const uniqueOffsets = new Set(map.map); + if (uniqueOffsets.size !== slotCount) { + return toTableFailure( + 'INVALID_TARGET', + 'Cell insertion with shiftRight is not supported for merged cells in this version.', + ); } - // Insert new empty cell at the target position (mapped after deletion). - const newCell = schema.nodes.tableCell.createAndFill()!; - const mappedCellPos = tr.mapping.map(cellPos); - tr.insert(mappedCellPos, newCell); + const makeEmptyCell = (preferHeader: boolean = false): import('prosemirror-model').Node => { + const candidateType = preferHeader + ? (schema.nodes.tableHeader ?? schema.nodes.tableCell) + : schema.nodes.tableCell; + return ( + candidateType.createAndFill({ + sdBlockId: uuidv4(), + paraId: generateParaId(), + }) ?? candidateType.createAndFill()! + ); + }; + + // Append one empty overflow row first so we can shift without dropping + // the row-tail content. + const overflowRowCells: import('prosemirror-model').Node[] = []; + for (let col = 0; col < map.width; col++) { + const templateOffset = map.map[(map.height - 1) * map.width + col]!; + const templateCell = tableNode.nodeAt(templateOffset); + overflowRowCells.push(makeEmptyCell(templateCell?.type.name === 'tableHeader')); + } + + const templateRowAttrs = (tableNode.child(Math.max(0, map.height - 1)).attrs as Record) ?? {}; + const overflowRowAttrs = { + ...templateRowAttrs, + sdBlockId: uuidv4(), + paraId: generateParaId(), + }; + const overflowRow = + schema.nodes.tableRow.createAndFill(overflowRowAttrs, overflowRowCells) ?? + schema.nodes.tableRow.create(overflowRowAttrs, overflowRowCells); + if (!overflowRow) { + return toTableFailure('INVALID_TARGET', 'Cell insertion could not construct an overflow row.'); + } + + tr.insert(tablePos + tableNode.nodeSize - 1, overflowRow); + + const expandedTableNode = tr.doc.nodeAt(tablePos); + if (!expandedTableNode || expandedTableNode.type.name !== 'table') { + return toTableFailure('INVALID_TARGET', 'Cell insertion could not locate expanded table state.'); + } + + const expandedMap = TableMap.get(expandedTableNode); + const expandedSlotCount = expandedMap.width * expandedMap.height; + if (new Set(expandedMap.map).size !== expandedSlotCount) { + return toTableFailure( + 'INVALID_TARGET', + 'Cell insertion with shiftRight produced an unsupported merged-table shape.', + ); + } + + const rowMajorCells: import('prosemirror-model').Node[] = []; + for (let i = 0; i < expandedSlotCount; i++) { + const offset = expandedMap.map[i]!; + const cell = expandedTableNode.nodeAt(offset); + if (!cell) { + return toTableFailure('INVALID_TARGET', 'Cell insertion could not resolve expanded table cells.'); + } + rowMajorCells.push(cell); + } + + const targetLinearIndex = rowIndex * expandedMap.width + columnIndex; + const targetOffset = expandedMap.map[targetLinearIndex]!; + const targetCell = expandedTableNode.nodeAt(targetOffset); + + rowMajorCells.splice(targetLinearIndex, 0, makeEmptyCell(targetCell?.type.name === 'tableHeader')); + rowMajorCells.pop(); + + const rebuiltRows: import('prosemirror-model').Node[] = []; + const rebuiltRowCount = rowMajorCells.length / expandedMap.width; + for (let rebuiltRowIndex = 0; rebuiltRowIndex < rebuiltRowCount; rebuiltRowIndex++) { + const sourceRow = expandedTableNode.child(rebuiltRowIndex); + const rowAttrs = ((sourceRow.attrs as Record) ?? {}) as Record; + + const rowCells = rowMajorCells.slice( + rebuiltRowIndex * expandedMap.width, + (rebuiltRowIndex + 1) * expandedMap.width, + ); + const rebuiltRow = + schema.nodes.tableRow.createAndFill(rowAttrs, rowCells) ?? schema.nodes.tableRow.create(rowAttrs, rowCells); + if (!rebuiltRow) { + return toTableFailure('INVALID_TARGET', 'Cell insertion could not construct a replacement row.'); + } + rebuiltRows.push(rebuiltRow); + } + + const rebuiltTable = schema.nodes.table.create(expandedTableNode.attrs, rebuiltRows); + tr.replaceWith(tablePos, tablePos + expandedTableNode.nodeSize, rebuiltTable); } else { // shiftDown: remove the last cell in this column, insert new cell at the same // column in the row below the target so cells shift downward within the column. diff --git a/tests/doc-api-stories/tests/tables/all-commands.ts b/tests/doc-api-stories/tests/tables/all-commands.ts index 31920ba34a..57c4876fb4 100644 --- a/tests/doc-api-stories/tests/tables/all-commands.ts +++ b/tests/doc-api-stories/tests/tables/all-commands.ts @@ -75,6 +75,9 @@ describe('document-api story: all table commands', () => { const clearContentsTableBySession = new Map(); const clearStyleTableBySession = new Map(); const convertToTextTableBySession = new Map(); + const insertCellBySession = new Map(); + const insertCellTableBySession = new Map(); + const insertCellInitialRowsBySession = new Map(); const deleteCellBySession = new Map(); function makeSessionId(prefix: string): string { @@ -557,10 +560,98 @@ describe('document-api story: all table commands', () => { }, { operationId: 'tables.insertCell', - setup: 'table', - run: async (sessionId, fixture) => { - const f = requireFixture('tables.insertCell', fixture); - return unwrap(await api.doc.tables.insertCell({ sessionId, nodeId: f.cellNodeId, mode: 'shiftRight' })); + setup: 'blank', + prepare: async (sessionId) => { + await api.doc.insert({ sessionId, value: 'Apple\tBanana\tMango' }); + + for (const rowText of ['Orange\tGrape\tKiwi', 'Pear\tPeach\tPlum']) { + const createRowResult = unwrap( + await api.doc.create.paragraph({ + sessionId, + at: { kind: 'documentEnd' }, + text: rowText, + }), + ); + if (createRowResult?.success !== true) { + const code = createRowResult?.failure?.code ?? 'UNKNOWN'; + throw new Error(`tables.insertCell setup failed while creating row paragraph (code: ${code}).`); + } + } + + const paragraphNodeId = await firstNodeId(sessionId, 'paragraph'); + const convertResult = unwrap( + await api.doc.tables.convertFromText({ + sessionId, + nodeId: paragraphNodeId, + delimiter: 'tab', + }), + ); + assertMutationSuccess('tables.convertFromText', convertResult); + + const tableNodeId = convertResult?.table?.nodeId; + if (!tableNodeId) { + throw new Error('tables.insertCell setup failed: converted table nodeId was not returned.'); + } + + const cellsResult = unwrap(await api.doc.tables.getCells({ sessionId, nodeId: tableNodeId, rowIndex: 0 })); + const firstCellNodeId = cellsResult?.cells?.find( + (cell: any) => cell?.rowIndex === 0 && cell?.columnIndex === 0, + )?.nodeId; + if (!firstCellNodeId) { + throw new Error('tables.insertCell setup failed: first-row first-column cell was not found.'); + } + + const tableInfo = unwrap(await api.doc.tables.get({ sessionId, nodeId: tableNodeId })); + if (typeof tableInfo?.rows !== 'number' || tableInfo.rows < 1) { + throw new Error('tables.insertCell setup failed: initial table row count could not be determined.'); + } + + insertCellBySession.set(sessionId, firstCellNodeId); + insertCellTableBySession.set(sessionId, tableNodeId); + insertCellInitialRowsBySession.set(sessionId, tableInfo.rows); + }, + run: async (sessionId) => { + const cellNodeId = insertCellBySession.get(sessionId); + const tableNodeId = insertCellTableBySession.get(sessionId); + const initialRows = insertCellInitialRowsBySession.get(sessionId); + if (!cellNodeId) { + throw new Error('tables.insertCell setup failed: prepared cell nodeId was not found.'); + } + if (!tableNodeId) { + throw new Error('tables.insertCell setup failed: prepared table nodeId was not found.'); + } + if (typeof initialRows !== 'number') { + throw new Error('tables.insertCell setup failed: prepared initial row count was not found.'); + } + insertCellBySession.delete(sessionId); + insertCellTableBySession.delete(sessionId); + insertCellInitialRowsBySession.delete(sessionId); + + const result = unwrap( + await api.doc.tables.insertCell({ sessionId, nodeId: cellNodeId, mode: 'shiftRight' }), + ); + assertMutationSuccess('tables.insertCell', result); + + const tableResult = unwrap(await api.doc.tables.get({ sessionId, nodeId: tableNodeId })); + if (tableResult?.rows !== initialRows + 1) { + throw new Error( + `tables.insertCell expected row count to grow by 1 after overflow-preserving shiftRight, received ${tableResult?.rows} from initial ${initialRows}.`, + ); + } + + const mangoResult = unwrap( + await api.doc.query.match({ + sessionId, + select: { type: 'text', pattern: 'Mango', caseSensitive: true }, + require: 'first', + }), + ); + const mangoMatch = mangoResult?.items?.[0]; + if (!mangoMatch) { + throw new Error('tables.insertCell expected to preserve rightmost cell content "Mango" after shiftRight.'); + } + + return result; }, }, {