From d76c731473270be33243edcf57e5fcc9905218ac Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Sat, 15 Aug 2026 20:54:59 +0200 Subject: [PATCH] Merge and split table cells (#337) prosemirror-tables already ships mergeCells/splitCell and the schema (tableNodes) already carries colspan/rowspan -- only the controls were missing. Adds the two commands, toolbar buttons whose enabled state follows the selection (merge needs a multi-cell selection, split a merged cell), and de+en labels. Both render paths now carry the spans: docToHtml emits colspan/rowspan (read mode, exports via the HTML path), and the markdown serializer pads a colspan with empty cells so every row keeps the table's column count -- rowspan stays lossy there, GFM cannot express it. e2e drives merge and split through the toolbar; the cell selection is made per Shift+Click because a keypress in the same tick as the preceding click races the editor's post-click rendering (keyboard cell selection itself works, verified interactively with a settled editor). Closes #337 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012aoPvnakfBP28nAfijgUY9 --- apps/web/e2e/editor.spec.ts | 45 +++++++++++++++++++ apps/web/src/editor/Toolbar.tsx | 18 ++++++++ apps/web/src/editor/nodes/table.ts | 12 +++++ packages/shared/i18n/de/editor.json | 2 + packages/shared/i18n/en/editor.json | 2 + .../shared/src/editor-schema/html.test.ts | 20 +++++++++ packages/shared/src/editor-schema/html.ts | 7 ++- .../shared/src/editor-schema/markdown.test.ts | 15 +++++++ packages/shared/src/editor-schema/markdown.ts | 4 ++ 9 files changed, 124 insertions(+), 1 deletion(-) diff --git a/apps/web/e2e/editor.spec.ts b/apps/web/e2e/editor.spec.ts index 025f9a3..fe84d6b 100644 --- a/apps/web/e2e/editor.spec.ts +++ b/apps/web/e2e/editor.spec.ts @@ -96,6 +96,51 @@ test('gap cursor reaches positions before and after a lone table (issue #335)', await context.close(); }); +test('cells can be merged and split from the toolbar (issue #337)', async ({ browser }) => { + const context = await contextForUser(browser, BASE_URL, 'fixture-user'); + const { pondSlug, pageSlug } = await createPage(context, `E2E MergeSplit ${Date.now()}`); + const page = await context.newPage(); + + await page.goto(`/p/${pondSlug}/${pageSlug}`); + await page.getByRole('button', { name: /edit|bearbeiten/i }).click(); + const status = page.locator('.editor-connection'); + await expect(status).toHaveAttribute('data-status', 'connected', { timeout: 10000 }); + + const content = page.locator('.ProseMirror'); + await content.click(); + await page.getByRole('button', { name: /insert table|tabelle einfügen/i }).click(); + await expect(content.locator('table')).toBeVisible(); + + const mergeButton = page.getByRole('button', { name: /merge cells|zellen verbinden/i }); + const splitButton = page.getByRole('button', { name: /split cell|zelle teilen/i }); + await expect(mergeButton).toBeDisabled(); + await expect(splitButton).toBeDisabled(); + + // Extending the selection across the cell border turns it into a cell + // selection (prosemirror-tables), which is what merge operates on. + // Shift+Click, not Shift+ArrowRight: a keypress fired in the same tick as + // the preceding click races the editor's post-click rendering and gets + // dropped — no human types that fast (works fine interactively). + await content.locator('td').first().click(); + await content + .locator('td') + .nth(1) + .click({ modifiers: ['Shift'] }); + await expect(content.locator('.selectedCell')).toHaveCount(2); + await expect(mergeButton).toBeEnabled(); + await mergeButton.click(); + await expect(content.locator('td[colspan="2"]')).toHaveCount(1); + + // Splitting the merged cell restores the row's full cell count. + await content.locator('td[colspan="2"]').click(); + await expect(splitButton).toBeEnabled(); + await splitButton.click(); + await expect(content.locator('td[colspan="2"]')).toHaveCount(0); + await expect(content.locator('tr').nth(1).locator('td')).toHaveCount(3); + + await context.close(); +}); + test('edit mode hides the sidebar; leaving edit mode restores it', async ({ browser }) => { const context = await contextForUser(browser, BASE_URL, 'fixture-user'); const { pondSlug, pageSlug } = await createPage(context, `E2E Sidebar ${Date.now()}`); diff --git a/apps/web/src/editor/Toolbar.tsx b/apps/web/src/editor/Toolbar.tsx index 008b6ea..c5f474e 100644 --- a/apps/web/src/editor/Toolbar.tsx +++ b/apps/web/src/editor/Toolbar.tsx @@ -127,6 +127,8 @@ export function Toolbar({ canAddColumn: e.can().addColumnAfter(), canDeleteColumn: e.can().deleteColumn(), canDeleteTable: e.can().deleteTable(), + canMergeCells: e.can().mergeCells(), + canSplitCell: e.can().splitCell(), canToggleHeaderRow: e.can().toggleHeaderRow(), canUndo: e.can().undo(), canRedo: e.can().redo(), @@ -296,6 +298,22 @@ export function Toolbar({ > ▤× + editor.chain().focus().mergeCells().run()} + > + {/* Arrows collapsing onto / leaving a cell border: merge removes + the border between selected cells, split restores it. */} + →|← + + editor.chain().focus().splitCell().run()} + > + ←|→ + ReturnType; deleteRow: () => ReturnType; deleteTable: () => ReturnType; + mergeCells: () => ReturnType; + splitCell: () => ReturnType; toggleHeaderRow: () => ReturnType; }; } @@ -101,6 +105,14 @@ export const Table = Node.create({ () => ({ state, dispatch }) => deleteTable(state, dispatch), + mergeCells: + () => + ({ state, dispatch }) => + mergeCells(state, dispatch), + splitCell: + () => + ({ state, dispatch }) => + splitCell(state, dispatch), toggleHeaderRow: () => ({ state, dispatch }) => diff --git a/packages/shared/i18n/de/editor.json b/packages/shared/i18n/de/editor.json index e150d7c..b7420bb 100644 --- a/packages/shared/i18n/de/editor.json +++ b/packages/shared/i18n/de/editor.json @@ -76,6 +76,8 @@ "addRowBefore": "Zeile davor einfügen", "addRowAfter": "Zeile danach einfügen", "deleteRow": "Zeile löschen", + "mergeCells": "Zellen verbinden", + "splitCell": "Zelle teilen", "toggleHeaderRow": "Kopfzeile umschalten", "deleteTable": "Tabelle löschen" }, diff --git a/packages/shared/i18n/en/editor.json b/packages/shared/i18n/en/editor.json index bd76c46..aee4a86 100644 --- a/packages/shared/i18n/en/editor.json +++ b/packages/shared/i18n/en/editor.json @@ -76,6 +76,8 @@ "addRowBefore": "Add row before", "addRowAfter": "Add row after", "deleteRow": "Delete row", + "mergeCells": "Merge cells", + "splitCell": "Split cell", "toggleHeaderRow": "Toggle header row", "deleteTable": "Delete table" }, diff --git a/packages/shared/src/editor-schema/html.test.ts b/packages/shared/src/editor-schema/html.test.ts index 542e183..9a4ceeb 100644 --- a/packages/shared/src/editor-schema/html.test.ts +++ b/packages/shared/src/editor-schema/html.test.ts @@ -25,6 +25,26 @@ describe('docToHtml (issue #24)', () => { ); }); + it('keeps the cell spans of merged cells (issue #337)', () => { + // Built directly: markdown cannot express merged cells. + const cell = (text: string, attrs: { colspan?: number; rowspan?: number } | null = null) => + editorSchema.node('table_cell', attrs, [ + editorSchema.node('paragraph', null, [editorSchema.text(text)]), + ]); + const doc = editorSchema.node('doc', null, [ + editorSchema.node('table', null, [ + editorSchema.node('table_row', null, [ + cell('wide', { colspan: 2 }), + cell('tall', { rowspan: 2 }), + ]), + editorSchema.node('table_row', null, [cell('a'), cell('b')]), + ]), + ]); + expect(docToHtml(doc)).toBe( + '

wide

tall

a

b

', + ); + }); + it('allowlists link protocols, neutralizing javascript: hrefs', () => { const safe = markdownToDoc('[go](https://example.org)'); expect(docToHtml(safe)).toContain('href="https://example.org"'); diff --git a/packages/shared/src/editor-schema/html.ts b/packages/shared/src/editor-schema/html.ts index 1455a1f..b3541f6 100644 --- a/packages/shared/src/editor-schema/html.ts +++ b/packages/shared/src/editor-schema/html.ts @@ -111,7 +111,12 @@ function renderTable(node: Node): string { out += ''; row.forEach((cell) => { const tag = cell.type.name === 'table_header' ? 'th' : 'td'; - out += `<${tag}>${renderBlocks(cell)}`; + // Merged cells (issue #337): without the span attributes the read-mode + // table silently loses the merge the editor shows. + const colspan = cell.attrs.colspan as number; + const rowspan = cell.attrs.rowspan as number; + const spans = `${colspan > 1 ? ` colspan="${colspan}"` : ''}${rowspan > 1 ? ` rowspan="${rowspan}"` : ''}`; + out += `<${tag}${spans}>${renderBlocks(cell)}`; }); out += ''; }); diff --git a/packages/shared/src/editor-schema/markdown.test.ts b/packages/shared/src/editor-schema/markdown.test.ts index 014fdd5..41f36a1 100644 --- a/packages/shared/src/editor-schema/markdown.test.ts +++ b/packages/shared/src/editor-schema/markdown.test.ts @@ -148,6 +148,21 @@ describe('markdown round-trip (issue #24)', () => { expect(doc.firstChild?.attrs.data).toEqual({}); }); + it('pads a colspan cell so every row keeps the column count (issue #337)', () => { + // Built directly: markdown cannot express merged cells, so the export + // is lossy by format — but it must stay a well-formed GFM table. + const schema = markdownToDoc('x').type.schema; + const cell = (type: string, text: string, attrs: { colspan?: number } | null = null) => + schema.node(type, attrs, [schema.node('paragraph', null, [schema.text(text)])]); + const doc = schema.node('doc', null, [ + schema.node('table', null, [ + schema.node('table_row', null, [cell('table_header', 'A'), cell('table_header', 'B')]), + schema.node('table_row', null, [cell('table_cell', 'wide', { colspan: 2 })]), + ]), + ]); + expect(docToMarkdown(doc)).toBe('| A | B |\n| --- | --- |\n| wide | |'); + }); + it('escalates the fence when the data contains backticks (issue #76)', () => { const doc = markdownToDoc('```dorfteich-plugin p/t\n{"code":"x"}\n```'); const withTicks = doc.type.schema.nodes.plugin_block!.create({ diff --git a/packages/shared/src/editor-schema/markdown.ts b/packages/shared/src/editor-schema/markdown.ts index f779785..a329fef 100644 --- a/packages/shared/src/editor-schema/markdown.ts +++ b/packages/shared/src/editor-schema/markdown.ts @@ -510,6 +510,10 @@ function renderTable(state: MarkdownSerializerState, node: Node): void { const cells: string[] = []; row.forEach((cell) => { cells.push(cell.textContent.replace(/\|/g, '\\|').replace(/\r?\n/g, ' ').trim()); + // GFM has no cell spans: pad a colspan with empty cells so every row + // keeps the table's column count (rowspan stays lossy — the covered + // rows are simply shorter; issue #337). + for (let extra = 1; extra < (cell.attrs.colspan as number); extra += 1) cells.push(''); }); rows.push(cells); });