Compare commits

...

2 Commits

Author SHA1 Message Date
753338f1be Word-style Tab navigation in tables with an accessible exit (#338)
Some checks failed
CI / Lint, typecheck, test (pull_request) Successful in 6m56s
CI / Auth e2e pack (pull_request) Failing after 2m26s
CI / Import/export fidelity gate (pull_request) Has been skipped
CI / Build container images (pull_request) Successful in 5m47s
Tab used to fall through to the browser's focus navigation everywhere.
Inside tables it now moves cell-wise (Shift-Tab backwards) and appends a
new row from the last cell, Word-style. Outside tables every branch
returns false, so Tab keeps leaving the editor.

Capturing Tab inside tables needs a documented way out (WCAG 2.1.2):
Escape places the cursor after the table -- unlike the arrow keys, which
reach the gap cursor (#335) only from the table's edge cells, it works
from every cell, including from a cell selection. When no textblock
follows the table it falls back to the gap cursor position. The
mechanism is announced to assistive tech via an aria-describedby hint
on the editor surface (visually hidden, de+en).

e2e: cell round trip per Tab/Shift-Tab with typed markers, row append
from the last cell, and the full keyboard-only exit (Escape, then Tab
leaves the editor). The table specs now settle briefly after the insert
-- right after it the collab sync can swallow a click's selection
update, which had the markers landing in stale selections.

Closes #338

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012aoPvnakfBP28nAfijgUY9
2026-08-15 21:06:07 +02:00
d76c731473 Merge and split table cells (#337)
Some checks failed
CI / Lint, typecheck, test (pull_request) Successful in 6m51s
CI / Build container images (pull_request) Successful in 3m57s
CI / Auth e2e pack (pull_request) Failing after 5m22s
CI / Import/export fidelity gate (pull_request) Has been skipped
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012aoPvnakfBP28nAfijgUY9
2026-08-15 20:54:59 +02:00
10 changed files with 250 additions and 4 deletions

View File

@ -79,23 +79,136 @@ test('gap cursor reaches positions before and after a lone table (issue #335)',
// Inserting into the empty page replaces the placeholder paragraph — the
// table really is the only block, which is the situation of issue #335.
await expect(content.locator(':scope > p')).toHaveCount(0);
// Right after the insert the collab sync can still swallow a click's
// selection update; interact only against a settled editor (established
// pattern, see a11y.spec.ts). The typed markers below verify each click
// really placed the cursor where the locator points.
await page.waitForTimeout(500);
// Keyboard only: ArrowUp from the first cell lands on the gap cursor
// before the table; typing there materializes a paragraph.
await content.locator('th').first().click();
await page.keyboard.type('in');
await expect(content.locator('th').first()).toHaveText('in');
await page.keyboard.press('ArrowUp');
await expect(page.locator('.ProseMirror-gapcursor')).toHaveCount(1);
await page.keyboard.type('above');
await expect(content.locator(':scope > :first-child')).toHaveText('above');
// Same for the position after the table.
await content.locator('td').last().click();
await page.keyboard.type('z');
await expect(content.locator('td').last()).toHaveText('z');
await page.keyboard.press('ArrowDown');
await expect(page.locator('.ProseMirror-gapcursor')).toHaveCount(1);
await page.keyboard.type('below');
await expect(content.locator(':scope > :last-child')).toHaveText('below');
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();
// Settle before clicking into cells — see the gap cursor test.
await page.waitForTimeout(500);
// 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('Tab navigates table cells, extends the table, and never traps focus (issue #338)', async ({
browser,
}) => {
const context = await contextForUser(browser, BASE_URL, 'fixture-user');
const { pondSlug, pageSlug } = await createPage(context, `E2E TableTab ${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();
const rows = content.locator('tr');
await expect(rows).toHaveCount(3);
// Settle before clicking into cells — see the gap cursor test.
await page.waitForTimeout(500);
// Tab moves to the next cell, Shift+Tab back. Typed markers prove where
// the cursor really is (the typing assertions also settle the editor
// between keypresses — see the merge test on click/key races).
await content.locator('th').first().click();
await page.keyboard.type('one');
await expect(content.locator('th').first()).toHaveText('one');
await page.keyboard.press('Tab');
await page.keyboard.type('two');
await expect(content.locator('th').nth(1)).toHaveText('two');
await page.keyboard.press('Shift+Tab');
await page.keyboard.type('back');
await expect(content.locator('th').first()).toContainText('back');
// Tab in the last cell appends a row and moves into it (Word behavior).
const lastCell = rows.nth(2).locator('td').nth(2);
await lastCell.click();
await page.keyboard.type('z');
await expect(lastCell).toHaveText('z');
await page.keyboard.press('Tab');
await expect(rows).toHaveCount(4);
await page.keyboard.type('new');
await expect(rows.nth(3).locator('td').first()).toHaveText('new');
// No keyboard trap (WCAG 2.1.2): Escape works from EVERY cell (the gap
// cursor is only reachable per arrow key from edge cells) and places the
// cursor after the table; once outside, Tab leaves the editor entirely.
// The mechanism is announced via the editor's aria-describedby hint.
await page.keyboard.press('Escape');
await expect(page.locator('.ProseMirror-gapcursor')).toHaveCount(1);
await page.keyboard.press('Tab');
await expect(content).not.toBeFocused();
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()}`);

View File

@ -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({
>
×
</ToolbarButton>
<ToolbarButton
label={t('toolbar.table.mergeCells')}
disabled={!state.canMergeCells}
onClick={() => editor.chain().focus().mergeCells().run()}
>
{/* Arrows collapsing onto / leaving a cell border: merge removes
the border between selected cells, split restores it. */}
|
</ToolbarButton>
<ToolbarButton
label={t('toolbar.table.splitCell')}
disabled={!state.canSplitCell}
onClick={() => editor.chain().focus().splitCell().run()}
>
|
</ToolbarButton>
<ToolbarButton
label={t('toolbar.table.toggleHeaderRow')}
disabled={!state.canToggleHeaderRow}

View File

@ -1,4 +1,6 @@
import { Node } from '@tiptap/core';
import { GapCursor } from '@tiptap/pm/gapcursor';
import { Selection } from '@tiptap/pm/state';
import type { Node as PMNode, Schema } from 'prosemirror-model';
import {
addColumnAfter,
@ -8,6 +10,9 @@ import {
deleteColumn,
deleteRow,
deleteTable,
goToNextCell,
mergeCells,
splitCell,
tableEditing,
toggleHeaderRow,
} from 'prosemirror-tables';
@ -34,6 +39,10 @@ declare module '@tiptap/core' {
addRowAfter: () => ReturnType;
deleteRow: () => ReturnType;
deleteTable: () => ReturnType;
mergeCells: () => ReturnType;
splitCell: () => ReturnType;
goToNextCell: () => ReturnType;
goToPreviousCell: () => ReturnType;
toggleHeaderRow: () => ReturnType;
};
}
@ -65,6 +74,37 @@ export const Table = Node.create({
addProseMirrorPlugins() {
return [tableEditing()];
},
addKeyboardShortcuts() {
return {
// Word-style navigation (issue #338): Tab moves cell-wise and appends
// a new row from the last cell. Outside a table every branch returns
// false, so Tab keeps its browser default (focus moves on) and the
// editor is no keyboard trap — from inside a table the arrow keys
// lead out via the gap cursor (#335), then Tab leaves the editor.
Tab: () => {
if (this.editor.commands.goToNextCell()) return true;
if (!this.editor.can().addRowAfter()) return false;
return this.editor.chain().addRowAfter().goToNextCell().run();
},
'Shift-Tab': () => this.editor.commands.goToPreviousCell(),
// The documented exit (aria-describedby hint, #338): the gap cursor is
// only reachable per arrow key from the table's edge cells, so Escape
// is the exit that works from EVERY cell. Falls back to a gap cursor
// when no textblock follows the table (#335 guarantees the position).
Escape: () =>
this.editor.commands.command(({ state, dispatch }) => {
const { $head } = state.selection;
for (let depth = $head.depth; depth > 0; depth -= 1) {
if ($head.node(depth).type.spec.tableRole !== 'table') continue;
const $after = state.doc.resolve($head.after(depth));
const selection = Selection.findFrom($after, 1, true) ?? new GapCursor($after);
if (dispatch) dispatch(state.tr.setSelection(selection).scrollIntoView());
return true;
}
return false;
}),
};
},
addCommands() {
return {
insertTable:
@ -101,6 +141,22 @@ export const Table = Node.create({
() =>
({ state, dispatch }) =>
deleteTable(state, dispatch),
mergeCells:
() =>
({ state, dispatch }) =>
mergeCells(state, dispatch),
splitCell:
() =>
({ state, dispatch }) =>
splitCell(state, dispatch),
goToNextCell:
() =>
({ state, dispatch }) =>
goToNextCell(1)(state, dispatch),
goToPreviousCell:
() =>
({ state, dispatch }) =>
goToNextCell(-1)(state, dispatch),
toggleHeaderRow:
() =>
({ state, dispatch }) =>

View File

@ -227,7 +227,9 @@ function PageEditor({
attributes: {
role: canEdit ? 'textbox' : 'document',
'aria-label': t('contentLabel'),
...(canEdit ? { 'aria-multiline': 'true' } : {}),
...(canEdit
? { 'aria-multiline': 'true', 'aria-describedby': 'editor-keyboard-hint' }
: {}),
},
},
});
@ -361,6 +363,13 @@ function PageEditor({
/>
)}
<EditorContent editor={editor} className="editor-content" />
{/* Referenced via aria-describedby in edit mode: Tab is captured
inside tables (#338), so the way out must be discoverable. */}
{canEdit && (
<p id="editor-keyboard-hint" className="visually-hidden">
{t('keyboardHint')}
</p>
)}
{canEdit && <WikilinkAutocomplete editor={editor} />}
{canEdit && <MentionAutocomplete editor={editor} />}
</div>

View File

@ -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"
},
@ -192,5 +194,6 @@
"due": "Zieldatum",
"start": "Startdatum"
},
"contentLabel": "Seiteninhalt"
"contentLabel": "Seiteninhalt",
"keyboardHint": "In Tabellen wechselt die Tabulatortaste zur nächsten Zelle und legt in der letzten Zelle eine neue Zeile an; Umschalt+Tab geht zurück. Escape stellt den Cursor hinter die Tabelle; außerhalb von Tabellen verlässt die Tabulatortaste den Editor."
}

View File

@ -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"
},
@ -192,5 +194,6 @@
"due": "Due date",
"start": "Start date"
},
"contentLabel": "Page content"
"contentLabel": "Page content",
"keyboardHint": "Inside tables, Tab moves to the next cell and creates a new row from the last cell; Shift+Tab moves back. Escape places the cursor after the table; outside tables, Tab leaves the editor."
}

View File

@ -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(
'<table><tr><td colspan="2"><p>wide</p></td><td rowspan="2"><p>tall</p></td></tr><tr><td><p>a</p></td><td><p>b</p></td></tr></table>',
);
});
it('allowlists link protocols, neutralizing javascript: hrefs', () => {
const safe = markdownToDoc('[go](https://example.org)');
expect(docToHtml(safe)).toContain('href="https://example.org"');

View File

@ -111,7 +111,12 @@ function renderTable(node: Node): string {
out += '<tr>';
row.forEach((cell) => {
const tag = cell.type.name === 'table_header' ? 'th' : 'td';
out += `<${tag}>${renderBlocks(cell)}</${tag}>`;
// 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)}</${tag}>`;
});
out += '</tr>';
});

View File

@ -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({

View File

@ -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);
});