From 3600896e4fef5869de8082b5e618ce866d3fff13 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Wed, 18 Mar 2026 21:35:22 -0700 Subject: [PATCH 1/3] feat(document-api): accept table coordinates in unmergeCells --- .../reference/_generated-manifest.json | 2 +- .../reference/tables/unmerge-cells.mdx | 98 ++++++++-- .../src/contract/contract.test.ts | 22 ++- packages/document-api/src/contract/schemas.ts | 28 ++- packages/document-api/src/index.test.ts | 63 ++++++ packages/document-api/src/index.ts | 9 +- packages/document-api/src/tables/tables.ts | 77 ++++++++ .../src/types/table-operations.types.ts | 2 +- .../helpers/table-target-resolver.test.ts | 180 +++++++++++++++++- .../helpers/table-target-resolver.ts | 58 ++++++ .../document-api-adapters/tables-adapter.ts | 15 +- .../tables/table-unmerge-coordinates.spec.ts | 164 ++++++++++++++++ .../tests/tables/all-commands.ts | 83 +++++++- 13 files changed, 765 insertions(+), 36 deletions(-) create mode 100644 tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts diff --git a/apps/docs/document-api/reference/_generated-manifest.json b/apps/docs/document-api/reference/_generated-manifest.json index 709b1c4f18..9be29e6189 100644 --- a/apps/docs/document-api/reference/_generated-manifest.json +++ b/apps/docs/document-api/reference/_generated-manifest.json @@ -976,5 +976,5 @@ } ], "marker": "{/* GENERATED FILE: DO NOT EDIT. Regenerate via `pnpm run docapi:sync`. */}", - "sourceHash": "0ac9ab9c8f464a719722f89f32d725cc3c2079d126fa09d487a90eb7170d474d" + "sourceHash": "210ef0c6bfeddc42f66b487f40619ad824763c112441bc85932a93086b50cd73" } diff --git a/apps/docs/document-api/reference/tables/unmerge-cells.mdx b/apps/docs/document-api/reference/tables/unmerge-cells.mdx index 458b8baea6..784ecd07cc 100644 --- a/apps/docs/document-api/reference/tables/unmerge-cells.mdx +++ b/apps/docs/document-api/reference/tables/unmerge-cells.mdx @@ -30,16 +30,30 @@ Returns a TableMutationResult receipt; reports NO_OP if the cell is not merged. | Field | Type | Required | Description | | --- | --- | --- | --- | -| `target` | TableCellAddress | yes | TableCellAddress | +| `nodeId` | string | no | | +| `target` | TableCellAddress | no | TableCellAddress | +| `target.kind` | `"block"` | no | Constant: `"block"` | +| `target.nodeId` | string | no | | +| `target.nodeType` | `"tableCell"` | no | Constant: `"tableCell"` | + +### Variant 2 (target.nodeType="table") + +| Field | Type | Required | Description | +| --- | --- | --- | --- | +| `columnIndex` | integer | yes | | +| `rowIndex` | integer | yes | | +| `target` | TableAddress | yes | TableAddress | | `target.kind` | `"block"` | yes | Constant: `"block"` | | `target.nodeId` | string | yes | | -| `target.nodeType` | `"tableCell"` | yes | Constant: `"tableCell"` | +| `target.nodeType` | `"table"` | yes | Constant: `"table"` | -### Variant 2 (nodeId) +### Variant 3 (nodeId, rowIndex, columnIndex) | Field | Type | Required | Description | | --- | --- | --- | --- | +| `columnIndex` | integer | yes | | | `nodeId` | string | yes | | +| `rowIndex` | integer | yes | | ### Example request @@ -116,28 +130,76 @@ When present, `result.table` is the follow-up address to reuse after this call. ```json { - "additionalProperties": false, "oneOf": [ { - "required": [ - "target" - ] + "additionalProperties": false, + "oneOf": [ + { + "required": [ + "target" + ] + }, + { + "required": [ + "nodeId" + ] + } + ], + "properties": { + "nodeId": { + "type": "string" + }, + "target": { + "$ref": "#/$defs/TableCellAddress" + } + }, + "type": "object" }, { + "additionalProperties": false, + "properties": { + "columnIndex": { + "minimum": 0, + "type": "integer" + }, + "rowIndex": { + "minimum": 0, + "type": "integer" + }, + "target": { + "$ref": "#/$defs/TableAddress" + } + }, "required": [ - "nodeId" - ] - } - ], - "properties": { - "nodeId": { - "type": "string" + "target", + "rowIndex", + "columnIndex" + ], + "type": "object" }, - "target": { - "$ref": "#/$defs/TableCellAddress" + { + "additionalProperties": false, + "properties": { + "columnIndex": { + "minimum": 0, + "type": "integer" + }, + "nodeId": { + "type": "string" + }, + "rowIndex": { + "minimum": 0, + "type": "integer" + } + }, + "required": [ + "nodeId", + "rowIndex", + "columnIndex" + ], + "type": "object" } - }, - "type": "object" + ] } ``` diff --git a/packages/document-api/src/contract/contract.test.ts b/packages/document-api/src/contract/contract.test.ts index 7bc5c6b39f..0a00a05d46 100644 --- a/packages/document-api/src/contract/contract.test.ts +++ b/packages/document-api/src/contract/contract.test.ts @@ -194,7 +194,7 @@ describe('document-api contract catalog', () => { properties?: { address?: { $ref?: string } }; }; const unmergeInput = schemas.operations['tables.unmergeCells'].input as { - properties?: { target?: { $ref?: string } }; + oneOf?: Array>; }; const setBorderInput = schemas.operations['tables.setBorder'].input as { properties?: { target?: { $ref?: string } }; @@ -205,7 +205,25 @@ describe('document-api contract catalog', () => { expect(tablesGetInput.properties?.target?.$ref).toBe('#/$defs/TableAddress'); expect(tablesGetOutput.properties?.address?.$ref).toBe('#/$defs/TableAddress'); - expect(unmergeInput.properties?.target?.$ref).toBe('#/$defs/TableCellAddress'); + + // unmergeCells input is a oneOf: [cellLocator, tableScopedCellLocator (target), tableScopedCellLocator (nodeId)] + expect(unmergeInput.oneOf).toHaveLength(3); + const [cellBranch, tableTargetBranch, tableNodeIdBranch] = unmergeInput.oneOf as Array<{ + properties?: { target?: { $ref?: string }; nodeId?: unknown; rowIndex?: unknown; columnIndex?: unknown }; + required?: string[]; + }>; + // First branch: direct cell locator (target.$ref → TableCellAddress) + expect(cellBranch.properties?.target?.$ref).toBe('#/$defs/TableCellAddress'); + // Second branch: table-scoped with target (target.$ref → TableAddress + coordinates) + expect(tableTargetBranch.properties?.target?.$ref).toBe('#/$defs/TableAddress'); + expect(tableTargetBranch.required).toContain('rowIndex'); + expect(tableTargetBranch.required).toContain('columnIndex'); + // Third branch: table-scoped with nodeId + coordinates + expect(tableNodeIdBranch.properties?.nodeId).toBeDefined(); + expect(tableNodeIdBranch.required).toContain('nodeId'); + expect(tableNodeIdBranch.required).toContain('rowIndex'); + expect(tableNodeIdBranch.required).toContain('columnIndex'); + expect(setBorderInput.properties?.target?.$ref).toBe('#/$defs/TableOrCellAddress'); expect(insertRowSuccess.properties?.table?.$ref).toBe('#/$defs/TableAddress'); }); diff --git a/packages/document-api/src/contract/schemas.ts b/packages/document-api/src/contract/schemas.ts index 20ab4bcab0..a487778fa6 100644 --- a/packages/document-api/src/contract/schemas.ts +++ b/packages/document-api/src/contract/schemas.ts @@ -1507,6 +1507,32 @@ const cellLocatorSchema: JsonSchema = { oneOf: [{ required: ['target'] }, { required: ['nodeId'] }], }; +/** + * Accepts either a direct cell locator (target/nodeId pointing at a cell) + * or a table-scoped cell locator (target/nodeId pointing at a table + rowIndex + columnIndex). + */ +const cellOrTableScopedCellLocatorSchema: JsonSchema = { + oneOf: [ + cellLocatorSchema, + objectSchema( + { + target: tableAddressSchema, + rowIndex: { type: 'integer', minimum: 0 }, + columnIndex: { type: 'integer', minimum: 0 }, + }, + ['target', 'rowIndex', 'columnIndex'], + ), + objectSchema( + { + nodeId: { type: 'string' }, + rowIndex: { type: 'integer', minimum: 0 }, + columnIndex: { type: 'integer', minimum: 0 }, + }, + ['nodeId', 'rowIndex', 'columnIndex'], + ), + ], +}; + const tableOrCellLocatorSchema: JsonSchema = { ...objectSchema({ target: tableOrCellAddressSchema, @@ -4925,7 +4951,7 @@ const operationSchemas: Record = { failure: tableMutationFailureSchema, }, 'tables.unmergeCells': { - input: cellLocatorSchema, + input: cellOrTableScopedCellLocatorSchema, output: tableMutationResultSchema, success: tableMutationSuccessSchema, failure: tableMutationFailureSchema, diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index 8580e29db2..d0ea23c444 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -2742,6 +2742,69 @@ describe('createDocumentApi', () => { ).not.toThrow(); }); + // -- unmergeCells mixed cell/table-scoped locator validation -- + + it('accepts direct cell nodeId for unmergeCells', () => { + const api = makeApi(); + expect(() => api.tables.unmergeCells({ nodeId: 'cell-1' })).not.toThrow(); + }); + + it('accepts direct cell target for unmergeCells', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'tableCell' as const, nodeId: 'c1' }; + expect(() => api.tables.unmergeCells({ target })).not.toThrow(); + }); + + it('accepts table-scoped locator (nodeId + rowIndex + columnIndex) for unmergeCells', () => { + const api = makeApi(); + expect(() => api.tables.unmergeCells({ nodeId: 'table-1', rowIndex: 0, columnIndex: 0 })).not.toThrow(); + }); + + it('accepts table-scoped locator (target + rowIndex + columnIndex) for unmergeCells', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'table' as const, nodeId: 't1' }; + expect(() => api.tables.unmergeCells({ target, rowIndex: 0, columnIndex: 0 })).not.toThrow(); + }); + + it('treats explicit undefined coordinates as a direct cell call for unmergeCells', () => { + const api = makeApi(); + // { nodeId, rowIndex: undefined, columnIndex: undefined } must pass validation + // as a direct-cell call — the keys exist but the values are absent. + expect(() => + api.tables.unmergeCells({ nodeId: 'cell-1', rowIndex: undefined, columnIndex: undefined } as any), + ).not.toThrow(); + }); + + it('rejects unmergeCells with only rowIndex (missing columnIndex)', () => { + const api = makeApi(); + expect(() => api.tables.unmergeCells({ nodeId: 'table-1', rowIndex: 0 } as any)).toThrow( + /both rowIndex and columnIndex/, + ); + }); + + it('rejects unmergeCells with only columnIndex (missing rowIndex)', () => { + const api = makeApi(); + expect(() => api.tables.unmergeCells({ nodeId: 'table-1', columnIndex: 0 } as any)).toThrow( + /both rowIndex and columnIndex/, + ); + }); + + it('rejects unmergeCells with cell target plus coordinates', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'tableCell' as const, nodeId: 'c1' }; + expect(() => api.tables.unmergeCells({ target, rowIndex: 0, columnIndex: 0 } as any)).toThrow( + /must not be provided when target is a cell node/, + ); + }); + + it('rejects unmergeCells with table target without coordinates', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'table' as const, nodeId: 't1' }; + expect(() => api.tables.unmergeCells({ target } as any)).toThrow( + /rowIndex and columnIndex are required when target is a table/, + ); + }); + // -- create.table locator validation -- it('rejects ambiguous create.table at locator (both target + nodeId)', () => { diff --git a/packages/document-api/src/index.ts b/packages/document-api/src/index.ts index 236de0bd4c..4aebb53f77 100644 --- a/packages/document-api/src/index.ts +++ b/packages/document-api/src/index.ts @@ -311,7 +311,12 @@ import type { DiffApplyInput, DiffApplyOptions, } from './diff/diff.types.js'; -import { executeTableLocatorOp, executeRowLocatorOp, executeDocumentLevelTableOp } from './tables/tables.js'; +import { + executeTableLocatorOp, + executeRowLocatorOp, + executeCellOrTableScopedCellLocatorOp, + executeDocumentLevelTableOp, +} from './tables/tables.js'; import type { ParagraphsAdapter, ParagraphFormatApi, @@ -2321,7 +2326,7 @@ export function createDocumentApi(adapters: DocumentApiAdapters): DocumentApi { ); }, unmergeCells(input, options?) { - return executeTableLocatorOp( + return executeCellOrTableScopedCellLocatorOp( 'tables.unmergeCells', adapters.tables.unmergeCells.bind(adapters.tables), input, diff --git a/packages/document-api/src/tables/tables.ts b/packages/document-api/src/tables/tables.ts index 90c04c2b5a..cfc3842839 100644 --- a/packages/document-api/src/tables/tables.ts +++ b/packages/document-api/src/tables/tables.ts @@ -77,6 +77,73 @@ function validateRowLocator(input: RowLocatorInput, operationName: string): void } } +type CellOrTableScopedCellLocatorInput = { + target?: unknown; + nodeId?: unknown; + rowIndex?: unknown; + columnIndex?: unknown; +}; + +/** + * Returns `true` when the input carries non-`undefined` row + column coordinates, + * meaning it should be resolved as a table-scoped cell locator. + * + * This is the **single source of truth** for the direct-cell vs. table-scoped + * discrimination. Both the public-API validator and the adapter-level resolver + * dispatch must use this function so they agree on the classification. + */ +export function hasTableScopedCellCoordinates(input: CellOrTableScopedCellLocatorInput): boolean { + return input.rowIndex !== undefined && input.columnIndex !== undefined; +} + +/** + * Validates a mixed cell locator: either a direct cell locator (target/nodeId + * pointing at a cell, no coordinates) or a table-scoped cell locator + * (target/nodeId pointing at a table + rowIndex + columnIndex). + * + * Rejects: + * - table target without both coordinates + * - only one of rowIndex / columnIndex + * - cell target plus coordinates + */ +function validateCellOrTableScopedCellLocator(input: CellOrTableScopedCellLocatorInput, operationName: string): void { + validateTableLocator(input, operationName); + + const hasRowIndex = input.rowIndex !== undefined; + const hasColumnIndex = input.columnIndex !== undefined; + + if (hasRowIndex !== hasColumnIndex) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName}: both rowIndex and columnIndex are required when using table-scoped cell targeting. ` + + `Provide both or neither.`, + { fields: ['rowIndex', 'columnIndex'] }, + ); + } + + const hasCoordinates = hasTableScopedCellCoordinates(input); + + // When target is a block address, check that coordinates match the node type. + if (isObjectRecord(input.target) && input.target.kind === 'block') { + if (input.target.nodeType === 'tableCell' && hasCoordinates) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName}: rowIndex/columnIndex must not be provided when target is a cell node. ` + + `Either pass a table target with coordinates, or pass a cell target without coordinates.`, + { fields: ['rowIndex', 'columnIndex'] }, + ); + } + + if (input.target.nodeType === 'table' && !hasCoordinates) { + throw new DocumentApiValidationError( + 'INVALID_TARGET', + `${operationName}: rowIndex and columnIndex are required when target is a table.`, + { fields: ['rowIndex', 'columnIndex'] }, + ); + } + } +} + // --------------------------------------------------------------------------- // Typed execute helpers // --------------------------------------------------------------------------- @@ -105,6 +172,16 @@ export function executeRowLocatorOp( return adapter(input, normalizeMutationOptions(options)); } +export function executeCellOrTableScopedCellLocatorOp( + operationName: string, + adapter: (input: TInput, options?: MutationOptions) => TResult, + input: TInput, + options?: MutationOptions, +): TResult { + validateCellOrTableScopedCellLocator(input, operationName); + return adapter(input, normalizeMutationOptions(options)); +} + /** * Execute a document-level table mutation (no locator validation needed). * Only normalizes MutationOptions. diff --git a/packages/document-api/src/types/table-operations.types.ts b/packages/document-api/src/types/table-operations.types.ts index 299c250c5d..87d908e097 100644 --- a/packages/document-api/src/types/table-operations.types.ts +++ b/packages/document-api/src/types/table-operations.types.ts @@ -285,7 +285,7 @@ export interface TablesDeleteCellInput extends CellLocator { export type TablesMergeCellsInput = MergeRangeLocator; -export type TablesUnmergeCellsInput = CellLocator; +export type TablesUnmergeCellsInput = CellLocator | TableScopedCellLocator; export interface TablesSplitCellInput extends CellLocator { rows: number; diff --git a/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.test.ts b/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.test.ts index ff8998be1d..3d18c2f104 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.test.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.test.ts @@ -1,17 +1,22 @@ import type { Node as ProseMirrorNode } from 'prosemirror-model'; -import { describe, expect, it, vi } from 'vitest'; +import { afterEach, describe, expect, it, vi } from 'vitest'; import type { Editor } from '../../core/Editor.js'; -import { resolveRowLocator, resolveCellLocator } from './table-target-resolver.js'; +import { resolveRowLocator, resolveCellLocator, resolveTableScopedCellLocator } from './table-target-resolver.js'; + +let tableMapOverride: { width: number; height: number; map: number[] } | null = null; vi.mock('prosemirror-tables', () => ({ TableMap: { - get: vi.fn(() => ({ - width: 1, - height: 1, - map: [1], - positionAt: vi.fn(() => 1), - colCount: vi.fn(() => 0), - })), + get: vi.fn(() => { + if (tableMapOverride) return { ...tableMapOverride, positionAt: vi.fn(() => 0), colCount: vi.fn(() => 0) }; + return { + width: 1, + height: 1, + map: [1], + positionAt: vi.fn(() => 1), + colCount: vi.fn(() => 0), + }; + }), }, })); @@ -189,6 +194,163 @@ function makeNestedTableEditor(): Editor { } as unknown as Editor; } +/** + * Build a 2×2 table where cell (0,0) is merged across columns 0–1. + * + * Logical grid: + * row 0: [ mergedCell (colspan=2) ] + * row 1: [ cell-r1c0 ] [ cell-r1c1 ] + * + * TableMap.map for this layout: + * [0, 0, , ] + * Indices 0 and 1 both point to offset 0 (the merged cell). + */ +function makeMergedCellTableEditor(): Editor { + const makeParagraph = (id: string) => + createNode('paragraph', [createNode('text', [], { text: id })], { + attrs: { sdBlockId: `p-${id}`, paraId: `p-${id}`, paragraphProperties: {} }, + isBlock: true, + inlineContent: true, + }); + + // Row 0: single merged cell spanning 2 columns + const mergedCell = createNode('tableCell', [makeParagraph('merged')], { + attrs: { sdBlockId: 'merged-cell', colspan: 2, rowspan: 1 }, + isBlock: true, + inlineContent: false, + }); + const row0 = createNode('tableRow', [mergedCell], { + attrs: { sdBlockId: 'row-0', tableRowProperties: {} }, + isBlock: true, + inlineContent: false, + }); + + // Row 1: two normal cells + const cellR1C0 = createNode('tableCell', [makeParagraph('r1c0')], { + attrs: { sdBlockId: 'cell-r1c0', colspan: 1, rowspan: 1 }, + isBlock: true, + inlineContent: false, + }); + const cellR1C1 = createNode('tableCell', [makeParagraph('r1c1')], { + attrs: { sdBlockId: 'cell-r1c1', colspan: 1, rowspan: 1 }, + isBlock: true, + inlineContent: false, + }); + const row1 = createNode('tableRow', [cellR1C0, cellR1C1], { + attrs: { sdBlockId: 'row-1', tableRowProperties: {} }, + isBlock: true, + inlineContent: false, + }); + + const table = createNode('table', [row0, row1], { + attrs: { sdBlockId: 'table-1', tableProperties: {}, tableGrid: [5000, 5000] }, + isBlock: true, + inlineContent: false, + }); + + const doc = createNode('doc', [table], { isBlock: false }); + + // The merged cell's offset relative to table content start (tablePos + 1): + // row0 starts at offset 0 inside the table content; its content starts at +1; + // the merged cell is at offset 1 inside row0. + // Absolute: tablePos=0, table content start=1, row0 starts at 1, row0 content at 2, + // mergedCell at 2. + // Cell offset relative to table content start: mergedCell offset = 1 + // (row0 node opens at 0, content at 1, mergedCell at 1) + // + // For TableMap: the map stores offsets relative to tablePos + 1 (the table content start). + // We need: row0 content offset = 1 (row0 is at offset 0, opens at 0, content at 1). + // mergedCell offset = 1 (first child of row0 content). + // + // row1 offset = row0.nodeSize = mergedCell.nodeSize + 2 (row wrapper) + // mergedCell.nodeSize = paragraph.nodeSize + 2 = (text.nodeSize + 2) + 2 = "merged".length + 4 = 10 + // row0.nodeSize = 10 + 2 = 12 + // row1 starts at offset 12, content at 13. + // cellR1C0 at 13, cellR1C1 at 13 + cellR1C0.nodeSize + // cellR1C0.nodeSize = para.nodeSize + 2 = ("r1c0".length + 2) + 2 = 8 + // cellR1C1 at 13 + 8 = 21 + + // mergedCell offset (from table content start = 1): + // row0 starts at 0 relative to table content, row0 content at 1, mergedCell at 1 + const mergedCellOffset = 1; + // row1 starts at 12 relative to table content, row1 content at 13 + const cellR1C0Offset = 13; + const cellR1C1Offset = 13 + cellR1C0.nodeSize; + + // TableMap.map for 2×2 grid with merged cell at (0,0)–(0,1): + // [mergedCellOffset, mergedCellOffset, cellR1C0Offset, cellR1C1Offset] + tableMapOverride = { + width: 2, + height: 2, + map: [mergedCellOffset, mergedCellOffset, cellR1C0Offset, cellR1C1Offset], + }; + + const tr = { + delete: vi.fn().mockReturnThis(), + insert: vi.fn().mockReturnThis(), + setNodeMarkup: vi.fn().mockReturnThis(), + setMeta: vi.fn().mockReturnThis(), + mapping: { maps: [] as unknown[], map: (p: number) => p, slice: () => ({ map: (p: number) => p }) }, + doc, + }; + + return { + state: { + doc, + tr, + schema: { nodes: { tableCell: { createAndFill: vi.fn() } } }, + }, + dispatch: vi.fn(), + commands: {}, + can: vi.fn(() => ({})), + schema: { marks: {}, nodes: {} }, + options: {}, + } as unknown as Editor; +} + +describe('resolveTableScopedCellLocator', () => { + afterEach(() => { + tableMapOverride = null; + }); + + it('resolves anchor coordinates when targeting the anchor cell directly', () => { + const editor = makeMergedCellTableEditor(); + const resolved = resolveTableScopedCellLocator(editor, { nodeId: 'table-1', rowIndex: 0, columnIndex: 0 }, 'test'); + + expect(resolved.table.address.nodeId).toBe('table-1'); + expect(resolved.rowIndex).toBe(0); + expect(resolved.columnIndex).toBe(0); + expect(resolved.cellNode.attrs.sdBlockId).toBe('merged-cell'); + }); + + it('canonicalizes non-anchor coordinate to anchor coordinates inside a merged span', () => { + const editor = makeMergedCellTableEditor(); + // Target (0,1) — covered by the merged cell anchored at (0,0). + const resolved = resolveTableScopedCellLocator(editor, { nodeId: 'table-1', rowIndex: 0, columnIndex: 1 }, 'test'); + + // Must return the anchor coordinates (0,0), not the requested (0,1). + expect(resolved.rowIndex).toBe(0); + expect(resolved.columnIndex).toBe(0); + expect(resolved.cellNode.attrs.sdBlockId).toBe('merged-cell'); + }); + + it('resolves an unmerged cell in row 1', () => { + const editor = makeMergedCellTableEditor(); + const resolved = resolveTableScopedCellLocator(editor, { nodeId: 'table-1', rowIndex: 1, columnIndex: 0 }, 'test'); + + expect(resolved.rowIndex).toBe(1); + expect(resolved.columnIndex).toBe(0); + expect(resolved.cellNode.attrs.sdBlockId).toBe('cell-r1c0'); + }); + + it('throws INVALID_TARGET for out-of-bounds coordinates', () => { + const editor = makeMergedCellTableEditor(); + expect(() => + resolveTableScopedCellLocator(editor, { nodeId: 'table-1', rowIndex: 5, columnIndex: 0 }, 'test'), + ).toThrow(/out of bounds/); + }); +}); + describe('table-target-resolver nested tables', () => { it('resolveRowLocator picks the innermost parent table for a nested row', () => { const editor = makeNestedTableEditor(); diff --git a/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.ts b/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.ts index 0a1ad30c44..23fc26bffd 100644 --- a/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.ts +++ b/packages/super-editor/src/document-api-adapters/helpers/table-target-resolver.ts @@ -363,6 +363,64 @@ export function resolveMergeRangeLocator( return { table, startRow, startCol, endRow, endCol }; } +/** + * Resolves a table-scoped cell locator (table target/nodeId + rowIndex + columnIndex) + * to a {@link ResolvedCell}. + * + * If the requested coordinates land inside a merged cell, the returned + * `rowIndex`/`columnIndex` are canonicalized to the merged cell's **anchor** + * (top-left) coordinates. This is critical for callers like `unmergeCells` + * that pass coordinates into `expandMergedCellIntoSingles`. + * + * @throws {DocumentApiAdapterError} Various target/validation errors. + */ +export function resolveTableScopedCellLocator( + editor: Editor, + input: { + target?: TableAddress; + nodeId?: string; + rowIndex: number; + columnIndex: number; + }, + operationName: string, +): ResolvedCell { + const table = resolveTableLocator(editor, input, operationName); + const map = TableMap.get(table.candidate.node); + + if (input.rowIndex < 0 || input.rowIndex >= map.height || input.columnIndex < 0 || input.columnIndex >= map.width) { + throw new DocumentApiAdapterError( + 'INVALID_TARGET', + `${operationName}: cell (${input.rowIndex}, ${input.columnIndex}) is out of bounds (table is ${map.height}×${map.width}).`, + ); + } + + // Look up the cell offset from the table map. For merged cells, multiple + // map indices share the same offset — the anchor is the first occurrence. + const requestedIndex = input.rowIndex * map.width + input.columnIndex; + const cellOffset = map.map[requestedIndex]; + const anchorIndex = map.map.indexOf(cellOffset); + const anchorRow = Math.floor(anchorIndex / map.width); + const anchorCol = anchorIndex % map.width; + + const cellPos = table.candidate.pos + 1 + cellOffset; + const cellNode = table.candidate.node.nodeAt(cellOffset); + + if (!cellNode) { + throw new DocumentApiAdapterError( + 'TARGET_NOT_FOUND', + `${operationName}: cell at (${input.rowIndex}, ${input.columnIndex}) could not be resolved.`, + ); + } + + return { + table, + cellNode, + cellPos, + rowIndex: anchorRow, + columnIndex: anchorCol, + }; +} + // --------------------------------------------------------------------------- // Column resolution // --------------------------------------------------------------------------- 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 3c00d91699..d2a3712001 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -68,6 +68,7 @@ import { resolveRowLocator, resolveColumnLocator, resolveCellLocator, + resolveTableScopedCellLocator, resolveMergeRangeLocator, resolvePostMutationTableAddress, getTableColumnCount, @@ -2359,7 +2360,19 @@ export function tablesUnmergeCellsAdapter( ): TableMutationResult { rejectTrackedMode('tables.unmergeCells', options); - const resolved = resolveCellLocator(editor, input, 'tables.unmergeCells'); + // Discriminate by value, not by key presence. A JS caller may pass + // { nodeId: '…', rowIndex: undefined } — the keys exist but the values + // are absent. This must agree with the public-API validator in tables.ts + // which checks `!== undefined`, not `in`. + const inputRecord = input as Record; + const isTableScoped = inputRecord.rowIndex !== undefined && inputRecord.columnIndex !== undefined; + const resolved = isTableScoped + ? resolveTableScopedCellLocator( + editor, + input as { target?: TableAddress; nodeId?: string; rowIndex: number; columnIndex: number }, + 'tables.unmergeCells', + ) + : resolveCellLocator(editor, input, 'tables.unmergeCells'); const { table, cellPos, cellNode, rowIndex, columnIndex } = resolved; const attrs = cellNode.attrs as Record; diff --git a/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts b/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts new file mode 100644 index 0000000000..35c2ad15ab --- /dev/null +++ b/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts @@ -0,0 +1,164 @@ +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { toolbar: 'full' } }); + +/** + * Helper: insert a 3×3 table, merge (0,0)–(0,1), then return the table's nodeId. + */ +async function setupMergedTable(page: import('@playwright/test').Page): Promise { + // Insert a 3×3 table. + await page.evaluate(() => { + (window as any).editor.commands.insertTable({ rows: 3, cols: 3, withHeaderRow: false }); + }); + + // Find the table and merge cells (0,0)–(0,1). + const tableNodeId = await page.evaluate(() => { + const doc = (window as any).editor.doc; + const tables = doc.find({ select: { type: 'node', nodeType: 'table' }, limit: 1 }); + const items = tables?.items ?? []; + const tableAddress = items[0]?.address; + if (!tableAddress) throw new Error('No table found after insert'); + + const mergeResult = doc.tables.mergeCells({ + target: tableAddress, + start: { rowIndex: 0, columnIndex: 0 }, + end: { rowIndex: 0, columnIndex: 1 }, + }); + if (!mergeResult?.success) { + throw new Error(`mergeCells failed: ${mergeResult?.failure?.code}`); + } + + return tableAddress.nodeId as string; + }); + + return tableNodeId; +} + +/** + * Helper: get cell info for a table. + */ +async function getCellInfo( + page: import('@playwright/test').Page, + tableNodeId: string, +): Promise> { + return page.evaluate((tid) => { + const doc = (window as any).editor.doc; + const result = doc.tables.getCells({ nodeId: tid }); + return (result?.cells ?? []).map((c: any) => ({ + rowIndex: c.rowIndex, + columnIndex: c.columnIndex, + colspan: c.colspan, + rowspan: c.rowspan, + })); + }, tableNodeId); +} + +test('unmergeCells with table-scoped anchor coordinates', async ({ superdoc }) => { + await superdoc.waitForStable(); + + const tableNodeId = await setupMergedTable(superdoc.page); + await superdoc.waitForStable(); + + // Verify merge: cell at (0,0) should have colspan=2. + const cellsBefore = await getCellInfo(superdoc.page, tableNodeId); + const mergedCell = cellsBefore.find((c) => c.rowIndex === 0 && c.columnIndex === 0); + expect(mergedCell?.colspan).toBe(2); + + // Unmerge via table-scoped coordinates — targeting the anchor (0,0). + const result = await superdoc.page.evaluate((tid) => { + return (window as any).editor.doc.tables.unmergeCells({ + nodeId: tid, + rowIndex: 0, + columnIndex: 0, + }); + }, tableNodeId); + expect(result?.success).toBe(true); + await superdoc.waitForStable(); + + // Verify unmerge: cell at (0,0) should now have colspan=1. + const cellsAfter = await getCellInfo(superdoc.page, tableNodeId); + const unmergedCell = cellsAfter.find((c) => c.rowIndex === 0 && c.columnIndex === 0); + expect(unmergedCell?.colspan).toBe(1); +}); + +test('unmergeCells with non-anchor coordinate inside a merged span', async ({ superdoc }) => { + await superdoc.waitForStable(); + + const tableNodeId = await setupMergedTable(superdoc.page); + await superdoc.waitForStable(); + + // Unmerge via table-scoped coordinates — targeting (0,1), a covered + // non-anchor coordinate inside the merged span anchored at (0,0). + // The resolver must canonicalize to the anchor cell. + const result = await superdoc.page.evaluate((tid) => { + return (window as any).editor.doc.tables.unmergeCells({ + nodeId: tid, + rowIndex: 0, + columnIndex: 1, + }); + }, tableNodeId); + expect(result?.success).toBe(true); + await superdoc.waitForStable(); + + // Verify unmerge: cell at (0,0) should now have colspan=1. + const cellsAfter = await getCellInfo(superdoc.page, tableNodeId); + const unmergedCell = cellsAfter.find((c) => c.rowIndex === 0 && c.columnIndex === 0); + expect(unmergedCell?.colspan).toBe(1); +}); + +test('unmergeCells with direct cell nodeId still works', async ({ superdoc }) => { + await superdoc.waitForStable(); + + const tableNodeId = await setupMergedTable(superdoc.page); + await superdoc.waitForStable(); + + // Get the merged cell's nodeId. + const cellNodeId = await superdoc.page.evaluate((tid) => { + const doc = (window as any).editor.doc; + const result = doc.tables.getCells({ nodeId: tid, rowIndex: 0, columnIndex: 0 }); + return result?.cells?.[0]?.nodeId as string; + }, tableNodeId); + + // Unmerge via direct cell nodeId (the original form). + const result = await superdoc.page.evaluate((cid) => { + return (window as any).editor.doc.tables.unmergeCells({ nodeId: cid }); + }, cellNodeId); + expect(result?.success).toBe(true); + await superdoc.waitForStable(); + + // Verify unmerge. + const cellsAfter = await getCellInfo(superdoc.page, tableNodeId); + const unmergedCell = cellsAfter.find((c) => c.rowIndex === 0 && c.columnIndex === 0); + expect(unmergedCell?.colspan).toBe(1); +}); + +test('unmergeCells with out-of-bounds coordinates fails gracefully', async ({ superdoc }) => { + await superdoc.waitForStable(); + + await superdoc.page.evaluate(() => { + (window as any).editor.commands.insertTable({ rows: 2, cols: 2, withHeaderRow: false }); + }); + await superdoc.waitForStable(); + + const tableNodeId = await superdoc.page.evaluate(() => { + const doc = (window as any).editor.doc; + const tables = doc.find({ select: { type: 'node', nodeType: 'table' }, limit: 1 }); + return tables?.items?.[0]?.address?.nodeId as string; + }); + + // Out-of-bounds coordinates should throw or return a failure. + const threw = await superdoc.page.evaluate(async (tid) => { + try { + const result = (window as any).editor.doc.tables.unmergeCells({ + nodeId: tid, + rowIndex: 99, + columnIndex: 99, + }); + return result?.success === false ? 'failure' : 'unexpected-success'; + } catch { + return 'threw'; + } + }, tableNodeId); + + expect(['threw', 'failure']).toContain(threw); +}); diff --git a/tests/doc-api-stories/tests/tables/all-commands.ts b/tests/doc-api-stories/tests/tables/all-commands.ts index 55975b4a88..76a4312bb6 100644 --- a/tests/doc-api-stories/tests/tables/all-commands.ts +++ b/tests/doc-api-stories/tests/tables/all-commands.ts @@ -811,8 +811,17 @@ describe('document-api story: all table commands', () => { assertMutationSuccess('tables.mergeCells', mergeResult); }, run: async (sessionId, fixture) => { + // Use table-scoped coordinates (tableNodeId + rowIndex + columnIndex) + // instead of direct cellNodeId — exercises the new coordinate path. const f = requireFixture('tables.unmergeCells', fixture); - return unwrap(await api.doc.tables.unmergeCells({ sessionId, nodeId: f.cellNodeId })); + return unwrap( + await api.doc.tables.unmergeCells({ + sessionId, + nodeId: f.tableNodeId, + rowIndex: 0, + columnIndex: 0, + }), + ); }, }, { @@ -1281,4 +1290,76 @@ describe('document-api story: all table commands', () => { await saveResult(sessionId, scenario.operationId); }); } + + // ------------------------------------------------------------------------- + // tables.unmergeCells — additional coordinate-path edge cases + // ------------------------------------------------------------------------- + + it('tables.unmergeCells: direct cell nodeId form still works', async () => { + const sessionId = makeSessionId('unmerge-direct-cell'); + const fixture = await setupTableFixture(sessionId); + const f = requireFixture('tables.unmergeCells', fixture); + + // Merge cells (0,0)–(0,1) so we have something to unmerge. + const mergeResult = unwrap( + await api.doc.tables.mergeCells({ + sessionId, + nodeId: f.tableNodeId, + start: { rowIndex: 0, columnIndex: 0 }, + end: { rowIndex: 0, columnIndex: 1 }, + }), + ); + assertMutationSuccess('tables.mergeCells', mergeResult); + + // Unmerge via direct cell nodeId (original form). + const result = unwrap(await api.doc.tables.unmergeCells({ sessionId, nodeId: f.cellNodeId })); + assertMutationSuccess('tables.unmergeCells', result); + }); + + it('tables.unmergeCells: non-anchor coordinate inside a merged span resolves correctly', async () => { + const sessionId = makeSessionId('unmerge-non-anchor'); + const fixture = await setupTableFixture(sessionId); + const f = requireFixture('tables.unmergeCells', fixture); + + // Merge cells (0,0)–(0,1) → creates a merged cell anchored at (0,0). + const mergeResult = unwrap( + await api.doc.tables.mergeCells({ + sessionId, + nodeId: f.tableNodeId, + start: { rowIndex: 0, columnIndex: 0 }, + end: { rowIndex: 0, columnIndex: 1 }, + }), + ); + assertMutationSuccess('tables.mergeCells', mergeResult); + + // Target (0,1) — a covered coordinate inside the merged span, NOT the anchor. + // The resolver must canonicalize this to the anchor cell at (0,0). + const result = unwrap( + await api.doc.tables.unmergeCells({ + sessionId, + nodeId: f.tableNodeId, + rowIndex: 0, + columnIndex: 1, + }), + ); + assertMutationSuccess('tables.unmergeCells', result); + }); + + it('tables.unmergeCells: out-of-bounds coordinates report failure', async () => { + const sessionId = makeSessionId('unmerge-oob'); + const fixture = await setupTableFixture(sessionId); + const f = requireFixture('tables.unmergeCells', fixture); + + // Target a coordinate outside the table bounds. + const result = await api.doc.tables.unmergeCells({ + sessionId, + nodeId: f.tableNodeId, + rowIndex: 99, + columnIndex: 99, + }); + + // Should fail (either thrown error caught or failure result). + const unwrapped = result?.result ?? result; + expect(unwrapped?.success).not.toBe(true); + }); }); From 6ab1d2c2e768a7fb1086e9fe40a3054e87e2d9cc Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Wed, 18 Mar 2026 21:41:53 -0700 Subject: [PATCH 2/3] chore: type fixes --- .../src/document-api-adapters/tables-adapter.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) 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 d2a3712001..97efcfc8e7 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -2347,6 +2347,13 @@ export function tablesMergeCellsAdapter( } } +function isTableScopedUnmergeInput( + input: TablesUnmergeCellsInput, +): input is Extract { + const inputRecord = input as Record; + return inputRecord.rowIndex !== undefined && inputRecord.columnIndex !== undefined; +} + /** * tables.unmergeCells — unmerge a merged cell back into individual cells. * @@ -2364,14 +2371,8 @@ export function tablesUnmergeCellsAdapter( // { nodeId: '…', rowIndex: undefined } — the keys exist but the values // are absent. This must agree with the public-API validator in tables.ts // which checks `!== undefined`, not `in`. - const inputRecord = input as Record; - const isTableScoped = inputRecord.rowIndex !== undefined && inputRecord.columnIndex !== undefined; - const resolved = isTableScoped - ? resolveTableScopedCellLocator( - editor, - input as { target?: TableAddress; nodeId?: string; rowIndex: number; columnIndex: number }, - 'tables.unmergeCells', - ) + const resolved = isTableScopedUnmergeInput(input) + ? resolveTableScopedCellLocator(editor, input, 'tables.unmergeCells') : resolveCellLocator(editor, input, 'tables.unmergeCells'); const { table, cellPos, cellNode, rowIndex, columnIndex } = resolved; From bc948b5273d67573b4fadbc6a9cdd1fd86bc4467 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 08:50:41 -0700 Subject: [PATCH 3/3] fix(document-api): treat null coordinates as absent in unmerge cell locator checks --- packages/document-api/src/index.test.ts | 25 ++++++++++ packages/document-api/src/tables/tables.ts | 15 +++--- .../tables-adapter.regressions.test.ts | 29 ++++++++++++ .../document-api-adapters/tables-adapter.ts | 46 +++++++++++++++---- .../tables/table-unmerge-coordinates.spec.ts | 33 ++++++++++++- .../tests/tables/all-commands.ts | 30 ++++++++++++ 6 files changed, 161 insertions(+), 17 deletions(-) diff --git a/packages/document-api/src/index.test.ts b/packages/document-api/src/index.test.ts index d0ea23c444..2ae6560a36 100644 --- a/packages/document-api/src/index.test.ts +++ b/packages/document-api/src/index.test.ts @@ -2755,6 +2755,12 @@ describe('createDocumentApi', () => { expect(() => api.tables.unmergeCells({ target })).not.toThrow(); }); + it('treats explicit null coordinates as absent for direct cell target on unmergeCells', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'tableCell' as const, nodeId: 'c1' }; + expect(() => api.tables.unmergeCells({ target, rowIndex: null, columnIndex: null } as any)).not.toThrow(); + }); + it('accepts table-scoped locator (nodeId + rowIndex + columnIndex) for unmergeCells', () => { const api = makeApi(); expect(() => api.tables.unmergeCells({ nodeId: 'table-1', rowIndex: 0, columnIndex: 0 })).not.toThrow(); @@ -2805,6 +2811,25 @@ describe('createDocumentApi', () => { ); }); + it('rejects unmergeCells with table target and null coordinates', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'table' as const, nodeId: 't1' }; + expect(() => api.tables.unmergeCells({ target, rowIndex: null, columnIndex: null } as any)).toThrow( + /rowIndex and columnIndex are required when target is a table/, + ); + }); + + it('rejects unmergeCells with table target and mixed null coordinates', () => { + const api = makeApi(); + const target = { kind: 'block' as const, nodeType: 'table' as const, nodeId: 't1' }; + expect(() => api.tables.unmergeCells({ target, rowIndex: null, columnIndex: 0 } as any)).toThrow( + /both rowIndex and columnIndex/, + ); + expect(() => api.tables.unmergeCells({ target, rowIndex: 0, columnIndex: null } as any)).toThrow( + /both rowIndex and columnIndex/, + ); + }); + // -- create.table locator validation -- it('rejects ambiguous create.table at locator (both target + nodeId)', () => { diff --git a/packages/document-api/src/tables/tables.ts b/packages/document-api/src/tables/tables.ts index cfc3842839..5817f4a4a0 100644 --- a/packages/document-api/src/tables/tables.ts +++ b/packages/document-api/src/tables/tables.ts @@ -86,14 +86,15 @@ type CellOrTableScopedCellLocatorInput = { /** * Returns `true` when the input carries non-`undefined` row + column coordinates, - * meaning it should be resolved as a table-scoped cell locator. + * meaning it can participate in table-scoped cell targeting. * - * This is the **single source of truth** for the direct-cell vs. table-scoped - * discrimination. Both the public-API validator and the adapter-level resolver - * dispatch must use this function so they agree on the classification. + * This is the validation-time check for coordinate presence. Adapter-level + * resolution may still refine ambiguous `nodeId` handoffs by resolved node + * type so payloads like `TableCellInfo` from `tables.getCells()` continue to + * work as direct cell locators. */ export function hasTableScopedCellCoordinates(input: CellOrTableScopedCellLocatorInput): boolean { - return input.rowIndex !== undefined && input.columnIndex !== undefined; + return input.rowIndex != null && input.columnIndex != null; } /** @@ -109,8 +110,8 @@ export function hasTableScopedCellCoordinates(input: CellOrTableScopedCellLocato function validateCellOrTableScopedCellLocator(input: CellOrTableScopedCellLocatorInput, operationName: string): void { validateTableLocator(input, operationName); - const hasRowIndex = input.rowIndex !== undefined; - const hasColumnIndex = input.columnIndex !== undefined; + const hasRowIndex = input.rowIndex != null; + const hasColumnIndex = input.columnIndex != null; if (hasRowIndex !== hasColumnIndex) { throw new DocumentApiValidationError( 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 fb8dbd075b..7137a4df9c 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 @@ -13,6 +13,7 @@ import { tablesSetShadingAdapter, tablesSplitCellAdapter, tablesSplitAdapter, + tablesUnmergeCellsAdapter, } from './tables-adapter.js'; vi.mock('prosemirror-tables', () => ({ @@ -681,6 +682,34 @@ describe('tables-adapter regressions', () => { ); }); + it('rejects table nodeId unmerge requests with null coordinates before mutating cell (0,0)', () => { + const editor = makeTableEditor(); + const dispatch = editor.dispatch as unknown as ReturnType; + const leadCell = (editor.state.doc.nodeAt(0)?.child(0).child(0) as unknown as { attrs: Record })!; + leadCell.attrs.colspan = 2; + + expect(() => + tablesUnmergeCellsAdapter(editor, { nodeId: 'table-1', rowIndex: null, columnIndex: null } as any), + ).toThrow(/expected "tableCell"/); + expect(dispatch).not.toHaveBeenCalled(); + }); + + it('rejects table target unmerge requests with null coordinates before mutating cell (0,0)', () => { + const editor = makeTableEditor(); + const dispatch = editor.dispatch as unknown as ReturnType; + const leadCell = (editor.state.doc.nodeAt(0)?.child(0).child(0) as unknown as { attrs: Record })!; + leadCell.attrs.colspan = 2; + + expect(() => + tablesUnmergeCellsAdapter(editor, { + target: { kind: 'block', nodeType: 'table', nodeId: 'table-1' }, + rowIndex: null, + columnIndex: null, + } as any), + ).toThrow(/expected "tableCell"/); + expect(dispatch).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 97efcfc8e7..5bba513aae 100644 --- a/packages/super-editor/src/document-api-adapters/tables-adapter.ts +++ b/packages/super-editor/src/document-api-adapters/tables-adapter.ts @@ -2347,11 +2347,42 @@ export function tablesMergeCellsAdapter( } } -function isTableScopedUnmergeInput( +function hasDefinedUnmergeCoordinates( input: TablesUnmergeCellsInput, ): input is Extract { const inputRecord = input as Record; - return inputRecord.rowIndex !== undefined && inputRecord.columnIndex !== undefined; + return inputRecord.rowIndex != null && inputRecord.columnIndex != null; +} + +function resolveUnmergeInput(editor: Editor, input: TablesUnmergeCellsInput) { + if (!hasDefinedUnmergeCoordinates(input)) { + return resolveCellLocator(editor, input, 'tables.unmergeCells'); + } + + const target = (input as { target?: unknown }).target; + if (target && typeof target === 'object' && !Array.isArray(target)) { + const blockTarget = target as { kind?: unknown; nodeType?: unknown }; + if (blockTarget.kind === 'block' && blockTarget.nodeType === 'table') { + return resolveTableScopedCellLocator(editor, input, 'tables.unmergeCells'); + } + return resolveCellLocator(editor, { target: target as TableCellAddress }, 'tables.unmergeCells'); + } + + const nodeId = (input as { nodeId?: unknown }).nodeId; + if (typeof nodeId === 'string') { + const candidate = findBlockByNodeIdOnly(getBlockIndex(editor), nodeId); + if (!candidate) { + throw new DocumentApiAdapterError('TARGET_NOT_FOUND', 'tables.unmergeCells: target was not found.', { + target: nodeId, + }); + } + + return candidate.nodeType === 'table' + ? resolveTableScopedCellLocator(editor, input, 'tables.unmergeCells') + : resolveCellLocator(editor, { nodeId }, 'tables.unmergeCells'); + } + + return resolveCellLocator(editor, {}, 'tables.unmergeCells'); } /** @@ -2367,13 +2398,10 @@ export function tablesUnmergeCellsAdapter( ): TableMutationResult { rejectTrackedMode('tables.unmergeCells', options); - // Discriminate by value, not by key presence. A JS caller may pass - // { nodeId: '…', rowIndex: undefined } — the keys exist but the values - // are absent. This must agree with the public-API validator in tables.ts - // which checks `!== undefined`, not `in`. - const resolved = isTableScopedUnmergeInput(input) - ? resolveTableScopedCellLocator(editor, input, 'tables.unmergeCells') - : resolveCellLocator(editor, input, 'tables.unmergeCells'); + // Preserve read→write handoff from tables.getCells(): a TableCellInfo carries + // row/column metadata plus a cell nodeId. For nodeId-based inputs, resolve by + // actual node type instead of assuming coordinates always mean "table-scoped". + const resolved = resolveUnmergeInput(editor, input); const { table, cellPos, cellNode, rowIndex, columnIndex } = resolved; const attrs = cellNode.attrs as Record; diff --git a/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts b/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts index 35c2ad15ab..61eed206c6 100644 --- a/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts +++ b/tests/behavior/tests/tables/table-unmerge-coordinates.spec.ts @@ -40,11 +40,12 @@ async function setupMergedTable(page: import('@playwright/test').Page): Promise< async function getCellInfo( page: import('@playwright/test').Page, tableNodeId: string, -): Promise> { +): Promise> { return page.evaluate((tid) => { const doc = (window as any).editor.doc; const result = doc.tables.getCells({ nodeId: tid }); return (result?.cells ?? []).map((c: any) => ({ + nodeId: c.nodeId, rowIndex: c.rowIndex, columnIndex: c.columnIndex, colspan: c.colspan, @@ -132,6 +133,36 @@ test('unmergeCells with direct cell nodeId still works', async ({ superdoc }) => expect(unmergedCell?.colspan).toBe(1); }); +test('unmergeCells accepts TableCellInfo handoff from getCells()', async ({ superdoc }) => { + await superdoc.waitForStable(); + + const tableNodeId = await setupMergedTable(superdoc.page); + await superdoc.waitForStable(); + + const cellInfo = await superdoc.page.evaluate((tid) => { + const doc = (window as any).editor.doc; + return doc.tables.getCells({ nodeId: tid, rowIndex: 0, columnIndex: 0 })?.cells?.[0] ?? null; + }, tableNodeId); + + expect(cellInfo).toMatchObject({ + nodeId: expect.any(String), + rowIndex: 0, + columnIndex: 0, + colspan: 2, + rowspan: 1, + }); + + const result = await superdoc.page.evaluate((payload) => { + return (window as any).editor.doc.tables.unmergeCells(payload); + }, cellInfo); + expect(result?.success).toBe(true); + await superdoc.waitForStable(); + + const cellsAfter = await getCellInfo(superdoc.page, tableNodeId); + const unmergedCell = cellsAfter.find((c) => c.rowIndex === 0 && c.columnIndex === 0); + expect(unmergedCell?.colspan).toBe(1); +}); + test('unmergeCells with out-of-bounds coordinates fails gracefully', async ({ superdoc }) => { await superdoc.waitForStable(); diff --git a/tests/doc-api-stories/tests/tables/all-commands.ts b/tests/doc-api-stories/tests/tables/all-commands.ts index 76a4312bb6..5944ab0858 100644 --- a/tests/doc-api-stories/tests/tables/all-commands.ts +++ b/tests/doc-api-stories/tests/tables/all-commands.ts @@ -1316,6 +1316,36 @@ describe('document-api story: all table commands', () => { assertMutationSuccess('tables.unmergeCells', result); }); + it('tables.unmergeCells: TableCellInfo handoff from tables.getCells still works', async () => { + const sessionId = makeSessionId('unmerge-cell-info-handoff'); + const fixture = await setupTableFixture(sessionId); + const f = requireFixture('tables.unmergeCells', fixture); + + const mergeResult = unwrap( + await api.doc.tables.mergeCells({ + sessionId, + nodeId: f.tableNodeId, + start: { rowIndex: 0, columnIndex: 0 }, + end: { rowIndex: 0, columnIndex: 1 }, + }), + ); + assertMutationSuccess('tables.mergeCells', mergeResult); + + const cellsResult = unwrap( + await api.doc.tables.getCells({ + sessionId, + nodeId: f.tableNodeId, + rowIndex: 0, + columnIndex: 0, + }), + ); + const cellInfo = cellsResult.cells[0]; + expect(cellInfo).toMatchObject({ rowIndex: 0, columnIndex: 0, colspan: 2, rowspan: 1 }); + + const result = unwrap(await api.doc.tables.unmergeCells({ sessionId, ...cellInfo })); + assertMutationSuccess('tables.unmergeCells', result); + }); + it('tables.unmergeCells: non-anchor coordinate inside a merged span resolves correctly', async () => { const sessionId = makeSessionId('unmerge-non-anchor'); const fixture = await setupTableFixture(sessionId);