diff --git a/AGENTS.md b/AGENTS.md index ee838ab40..1f5e2c544 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -408,6 +408,33 @@ Key facts: - Template snapshots reuse the document snapshot dialogs (`CreateSnapshotDialog` accepts no `MAT_DIALOG_DATA`). +### Editor Schema & Tables + +The ProseMirror `Schema` is built in +`frontend/src/app/components/element-ref/extended-schema.ts` from ngx-editor's +base specs + `prosemirror-tables`' `tableNodes()` + Inkweld's own extensions. + +**Never construct the `Schema` inside `packages/inkweld-prosemirror`** — the +shared package returns specs only. Building it there pulls a second copy of +`prosemirror-model` into the bundle, which breaks class-identity checks in +y-prosemirror and `EditorView`; typing into the editor then silently produces +no output (see PR #1068). For the same reason `prosemirror-tables` is pinned +in `frontend/package.json`'s `resolutions` block. + +Table support deliberately lives in Inkweld rather than in the +`@bobbyquantum/ngx-editor` fork: Inkweld replaced ngx-editor's menu with its +own Material toolbar, so the upstream table PR's menu components would be dead +weight. Only the schema and plugins are used. + +**When touching tables, all of these need to stay in step**: + +- `extended-schema.ts` — node specs (`cellContent: 'paragraph+'`, `align` attr) +- `document.service.ts` — `columnResizing` (must precede) + `tableEditing` + Tab keymap +- `packages/inkweld-prosemirror/src/xml/tags.ts` — table names in `BLOCK_NODE_NAMES`, + so empty cells don't collapse to self-closing tags and drop columns +- `markdown-to-xml.ts` / `xml-to-markdown.ts` — GFM table parsing and emission +- `markdown-`, `html-`, `epub-`, `pdf-generator.service.ts` — publish output + ### File Structure - Projects contain documents and elements diff --git a/backend/src/mcp/tools/mutation.tools.ts b/backend/src/mcp/tools/mutation.tools.ts index 0a1a7ee71..8077e923d 100644 --- a/backend/src/mcp/tools/mutation.tools.ts +++ b/backend/src/mcp/tools/mutation.tools.ts @@ -1374,9 +1374,10 @@ registerTool({ title: 'Update Document Content', description: `Replace the prose content of a document element. Supports two input formats: -- "prosemirror_xml" (default): Inkweld's canonical XML format. Use tags like , ,
, ..., etc. +- "prosemirror_xml" (default): Inkweld's canonical XML format. Use tags like , ,
, ..., ...
, etc. Example: Hello world -- "markdown": Standard CommonMark-style Markdown. Lossy formatting (comments, text colors) can be expressed via inline elements. Element references use [label](inkweld://username/slug/element/{id}) links. + Table cells hold block content, so each one wraps its text in a ; an optional align="left|center|right" attribute on a cell sets column alignment. +- "markdown": Standard CommonMark-style Markdown, plus GFM tables (including :--- / :---: / ---: column alignment). Lossy formatting (comments, text colors) can be expressed via inline elements. Element references use [label](inkweld://username/slug/element/{id}) links. The content replaces the entire document. Use get_document_content first to read the current content if you need to make partial edits.`, inputSchema: { diff --git a/docs/site/docs-user-guide/publishing/formats.md b/docs/site/docs-user-guide/publishing/formats.md index 709499aec..cba6b2fcb 100644 --- a/docs/site/docs-user-guide/publishing/formats.md +++ b/docs/site/docs-user-guide/publishing/formats.md @@ -56,6 +56,13 @@ Each export includes: - **Document content** — The documents you added to the plan - **Metadata** — Title, author, language, description +### Tables + +Tables export to every format. Merged cells are the one thing that does not +survive everywhere: PDF keeps them, while Markdown, HTML, and EPUB flatten a +merge into the cell plus empty columns so the grid keeps its shape. Column +alignment is preserved in all four. + ## Client-Side Generation Exports are generated entirely in your browser: diff --git a/docs/site/docs-user-guide/writing/editor.md b/docs/site/docs-user-guide/writing/editor.md index 711c60a2e..fd35baf7a 100644 --- a/docs/site/docs-user-guide/writing/editor.md +++ b/docs/site/docs-user-guide/writing/editor.md @@ -87,6 +87,7 @@ Click the paragraph style dropdown to choose: | Button | Effect | |--------|--------| | Link | Insert or edit a hyperlink | +| Table | Insert a table, or edit the rows and columns of the one you are in | | Horizontal rule | Insert a scene break line | | Clear formatting | Remove all formatting from selection | | Undo | Undo last action (`Ctrl/Cmd + Z`) | diff --git a/docs/site/docs-user-guide/writing/formatting.md b/docs/site/docs-user-guide/writing/formatting.md index d5ee8b750..5d006e74c 100644 --- a/docs/site/docs-user-guide/writing/formatting.md +++ b/docs/site/docs-user-guide/writing/formatting.md @@ -133,6 +133,60 @@ Insert a horizontal rule for: Create with the horizontal rule button in the toolbar. +## Tables + +Tables are useful for reference material that sits alongside your prose — magic +system rules, character stat blocks, timelines of regnal years, translation +glossaries. + +### Inserting a Table + +1. Place the cursor where you want the table +2. Click the **Table** button in the toolbar +3. Choose **Insert table** + +A new table starts as three columns by three rows. The first row is a **header +row**, styled differently from the body and preserved when you export. + +### Editing a Table + +Click into any cell and type. Cells hold ordinary paragraphs, so all the +character formatting above works inside them. + +| Action | How | +|--------|-----| +| Move to the next cell | `Tab` | +| Move to the previous cell | `Shift + Tab` | +| Insert a row or column | Table menu → *Insert row/column* | +| Delete a row or column | Table menu → *Delete row/column* | +| Merge selected cells | Table menu → *Merge cells* | +| Split a merged cell | Table menu → *Split cell* | +| Resize a column | Drag the divider between two column headers | +| Remove the whole table | Table menu → *Delete table* | + +Select several cells by clicking one and dragging across the others — merge and +alignment actions apply to the whole selection. + +:::tip +`Tab` in the last cell does **not** create a new row. Use *Insert row below* +from the table menu when you need more space. +::: + +### Tables and Export + +Tables export to Markdown, HTML, EPUB, and PDF. + +Markdown and EPUB have no way to express merged cells. When you export a table +containing merged cells, the merge is flattened: the cell's text stays in place +and the columns it spanned become empty, so the grid keeps its shape. PDF output +preserves merges. + +:::warning +A table always exports with its first row as the header. If you turn the header +row off in the editor, Markdown export will still treat row one as the header — +that is a limitation of the Markdown table format, not of Inkweld. +::: + ## Links ### Inserting Links diff --git a/docs/site/docs-user-guide/writing/keyboard-shortcuts.md b/docs/site/docs-user-guide/writing/keyboard-shortcuts.md index 95ffc37d3..48fc91927 100644 --- a/docs/site/docs-user-guide/writing/keyboard-shortcuts.md +++ b/docs/site/docs-user-guide/writing/keyboard-shortcuts.md @@ -54,6 +54,18 @@ When inside a list item: | Outdent list item | `Ctrl + [` | `Cmd + [` | | Line break (no new item) | `Shift + Enter` | `Shift + Enter` | +## Table Editing + +When the cursor is inside a table cell: + +| Action | Windows/Linux | macOS | +|--------|---------------|-------| +| Next cell | `Tab` | `Tab` | +| Previous cell | `Shift + Tab` | `Shift + Tab` | + +`Tab` in the last cell does not add a row — use *Insert row below* from the +toolbar's table menu. + ## Navigation | Action | Windows/Linux | macOS | diff --git a/docs/site/docs/developer/architecture.md b/docs/site/docs/developer/architecture.md index 885f90aad..e16d9572b 100644 --- a/docs/site/docs/developer/architecture.md +++ b/docs/site/docs/developer/architecture.md @@ -78,6 +78,48 @@ export class MyComponent { - **WorldbuildingService** - Template/schema system - **AuthService** - Authentication and session management +### Editor Schema + +The ProseMirror schema is assembled in +`frontend/src/app/components/element-ref/extended-schema.ts`. It composes +three sources: + +1. ngx-editor's base nodes and marks (`@bobbyquantum/ngx-editor/schema`) +2. `prosemirror-tables`' `tableNodes()` output +3. Inkweld's own extensions from `@inkweld/prosemirror/schema` — the + `elementRef` node plus the `comment`, `autoReview`, and secure `link` marks + +`new Schema(...)` is constructed **in the frontend**, never inside the shared +package. The shared package returns specs only; building the `Schema` there +would pull a second copy of `prosemirror-model` into the bundle and break the +class-identity checks in y-prosemirror and `EditorView` — typing would +silently stop working. `prosemirror-tables` is pinned in `resolutions` +alongside the other ProseMirror packages for the same reason. + +#### Tables + +Table support is layered on `prosemirror-tables` rather than on ngx-editor's +menu, because Inkweld replaced that menu with its own Material toolbar +(`editor-toolbar.component.ts`). Only the schema and the editing plugins come +from the library: + +- **Schema** — `tableNodes()` with `cellContent: 'paragraph+'`. Cells are + restricted to paragraphs deliberately: allowing arbitrary blocks would + permit nested tables and headings that no export format renders sensibly. + An extra `align` cell attribute carries GFM column alignment. +- **Plugins** — `columnResizing`, `tableEditing`, and a `Tab` / `Shift-Tab` + keymap, appended to the editor's plugin list in `document.service.ts`. + `columnResizing` must be registered before `tableEditing`. +- **Wire format** — table node names are block-level in + `packages/inkweld-prosemirror/src/xml/tags.ts`, so an empty cell serializes + as `` rather than collapsing to a self-closing tag + and desynchronising the row. +- **Markdown** — `markdownToXml` parses GFM tables and `xmlToMarkdown` emits + them, so tables round-trip through the MCP tools and markdown export. +- **Publish** — the HTML, EPUB, and Typst/PDF generators each render tables. + Tables are not yet part of the user-configurable publish-styles system and + currently get fixed built-in styling. + ## Backend (Bun + Hono) ### Technology Stack diff --git a/frontend/bun.lock b/frontend/bun.lock index 478e813e5..c900734f6 100644 --- a/frontend/bun.lock +++ b/frontend/bun.lock @@ -43,6 +43,7 @@ "ngx-image-cropper": "9.1.6", "ngx-input-color": "7.0.1", "prosemirror-history": "1.5.0", + "prosemirror-tables": "1.8.5", "prosemirror-transform": "1.12.0", "prosemirror-view": "1.42.3", "rxjs": "7.8.2", @@ -103,6 +104,7 @@ "prosemirror-model": "1.25.11", "prosemirror-schema-list": "1.5.1", "prosemirror-state": "1.4.4", + "prosemirror-tables": "1.8.5", "prosemirror-transform": "1.12.0", "prosemirror-view": "1.42.3", }, @@ -1583,6 +1585,8 @@ "prosemirror-state": ["prosemirror-state@1.4.4", "", { "dependencies": { "prosemirror-model": "^1.0.0", "prosemirror-transform": "^1.0.0", "prosemirror-view": "^1.27.0" } }, "sha512-6jiYHH2CIGbCfnxdHbXZ12gySFY/fz/ulZE333G6bPqIZ4F+TXo9ifiR86nAHpWnfoNjOb3o5ESi7J8Uz1jXHw=="], + "prosemirror-tables": ["prosemirror-tables@1.8.5", "", { "dependencies": { "prosemirror-keymap": "^1.2.3", "prosemirror-model": "^1.25.4", "prosemirror-state": "^1.4.4", "prosemirror-transform": "^1.10.5", "prosemirror-view": "^1.41.4" } }, "sha512-V/0cDCsHKHe/tfWkeCmthNUcEp1IVO3p6vwN8XtwE9PZQLAZJigbw3QoraAdfJPir4NKJtNvOB8oYGKRl+t0Dw=="], + "prosemirror-transform": ["prosemirror-transform@1.12.0", "", { "dependencies": { "prosemirror-model": "^1.21.0" } }, "sha512-GxboyN4AMIsoHNtz5uf2r2Ru551i5hWeCMD6E2Ib4Eogqoub0NflniaBPVQ4MrGE5yZ8JV9tUHg9qcZTTrcN4w=="], "prosemirror-view": ["prosemirror-view@1.42.3", "", { "dependencies": { "prosemirror-model": "^1.25.8", "prosemirror-state": "^1.0.0", "prosemirror-transform": "^1.1.0" } }, "sha512-oTN7EtH+CpwxU9NrwEYWd0UZ4JUx7l048l5A2Xppm4p/60isZYLnth9QVQmC3VRIvdrIWCxwZSd+Uz791G31/w=="], diff --git a/frontend/e2e/local/editor-tables.spec.ts b/frontend/e2e/local/editor-tables.spec.ts new file mode 100644 index 000000000..da84d0bb1 --- /dev/null +++ b/frontend/e2e/local/editor-tables.spec.ts @@ -0,0 +1,186 @@ +/** + * Editor Table Tests - Local Mode + * + * Covers the table support added on top of `prosemirror-tables`: inserting a + * table from the toolbar, editing cells, the row/column commands, and — the + * part most likely to regress silently — that a table survives a reload, + * which exercises the ProseMirror → canonical XML → Yjs → IndexedDB path. + */ +import { type Page } from '@playwright/test'; + +import { expect, test } from './fixtures'; + +/** + * Open a project, create a fresh document, and leave the editor focused. + */ +async function createDocumentAndFocus( + page: Page, + docName: string +): Promise { + await page.getByTestId('project-card').first().click(); + await expect(page.getByTestId('project-tree')).toBeVisible(); + + const newDocButton = page.getByTestId('create-new-element'); + await expect(newDocButton).toBeVisible(); + await newDocButton.click(); + + await page.getByRole('heading', { name: 'Document', level: 4 }).click(); + + const dialogInput = page.getByLabel('Document Name'); + await dialogInput.waitFor({ state: 'visible' }); + await dialogInput.fill(docName); + await page.getByTestId('create-element-button').click(); + + await expect(page.locator('ngx-editor')).toBeVisible(); + await page.locator('ngx-editor .ProseMirror').click(); +} + +/** Open the toolbar's table menu. */ +async function openTableMenu(page: Page): Promise { + await page.getByTestId('toolbar-table').click(); + await expect(page.getByTestId('table-insert')).toBeVisible(); +} + +/** Insert a default 3x3 table via the toolbar. */ +async function insertTable(page: Page): Promise { + await openTableMenu(page); + await page.getByTestId('table-insert').click(); + await expect(page.locator('ngx-editor .ProseMirror table')).toBeVisible(); +} + +test.describe('Editor Tables', () => { + test('insert: toolbar menu creates a 3x3 table with a header row', async ({ + localPageWithProject: page, + }) => { + await createDocumentAndFocus(page, 'Table Insert Test'); + + const editor = page.locator('ngx-editor .ProseMirror'); + + await test.step('table button is visible in the editor toolbar', async () => { + await expect(page.getByTestId('toolbar-table')).toBeVisible(); + }); + + await test.step('row and column commands are disabled outside a table', async () => { + await openTableMenu(page); + await expect(page.getByTestId('table-add-row-after')).toBeDisabled(); + await expect(page.getByTestId('table-delete')).toBeDisabled(); + await page.keyboard.press('Escape'); + }); + + await test.step('insert places a 3x3 table', async () => { + await insertTable(page); + await expect(editor.locator('table tr')).toHaveCount(3); + // First row is a header row, the other two are body rows. + await expect(editor.locator('table th')).toHaveCount(3); + await expect(editor.locator('table td')).toHaveCount(6); + }); + + await test.step('a paragraph follows the table so writing can continue', async () => { + // Without this a table inserted at the end of a document is a dead + // end — there is no inline position after it to place the cursor. + const lastChild = editor.locator('> *').last(); + await expect(lastChild).not.toHaveJSProperty('tagName', 'TABLE'); + await lastChild.click(); + await page.keyboard.type('After the table'); + await expect(editor).toContainText('After the table'); + }); + + await test.step('commands become enabled once the cursor is in a cell', async () => { + await editor.locator('table th').first().click(); + await openTableMenu(page); + await expect(page.getByTestId('table-add-row-after')).toBeEnabled(); + await expect(page.getByTestId('table-delete')).toBeEnabled(); + await page.keyboard.press('Escape'); + }); + }); + + test('edit: typing, Tab navigation, and row/column commands', async ({ + localPageWithProject: page, + }) => { + await createDocumentAndFocus(page, 'Table Edit Test'); + + const editor = page.locator('ngx-editor .ProseMirror'); + await insertTable(page); + + await test.step('typing into a cell keeps the text in that cell', async () => { + await editor.locator('table th').first().click(); + await page.keyboard.type('Name'); + await expect(editor.locator('table th').first()).toHaveText('Name'); + }); + + await test.step('Tab moves to the next cell', async () => { + await page.keyboard.press('Tab'); + await page.keyboard.type('Age'); + await expect(editor.locator('table th').nth(1)).toHaveText('Age'); + }); + + await test.step('adding a row grows the table', async () => { + await editor.locator('table td').first().click(); + await openTableMenu(page); + await page.getByTestId('table-add-row-after').click(); + await expect(editor.locator('table tr')).toHaveCount(4); + }); + + await test.step('adding a column grows every row', async () => { + await editor.locator('table td').first().click(); + await openTableMenu(page); + await page.getByTestId('table-add-column-after').click(); + await expect(editor.locator('table th')).toHaveCount(4); + await expect( + editor.locator('table tr').first().locator('th') + ).toHaveCount(4); + }); + + await test.step('deleting a row shrinks the table', async () => { + await editor.locator('table td').first().click(); + await openTableMenu(page); + await page.getByTestId('table-delete-row').click(); + await expect(editor.locator('table tr')).toHaveCount(3); + }); + + await test.step('delete table removes it entirely', async () => { + await editor.locator('table th').first().click(); + await openTableMenu(page); + await page.getByTestId('table-delete').click(); + await expect(editor.locator('table')).toHaveCount(0); + }); + }); + + test('persistence: a table with content survives a reload', async ({ + localPageWithProject: page, + }) => { + await createDocumentAndFocus(page, 'Table Persist Test'); + + const editor = page.locator('ngx-editor .ProseMirror'); + await insertTable(page); + + await test.step('fill the header and a body cell', async () => { + await editor.locator('table th').first().click(); + await page.keyboard.type('Character'); + await editor.locator('table td').first().click(); + await page.keyboard.type('Alice'); + + await expect(editor.locator('table th').first()).toHaveText('Character'); + await expect(editor.locator('table td').first()).toHaveText('Alice'); + }); + + await test.step('table and cell content are still there after reload', async () => { + await page.reload(); + await expect(page.locator('ngx-editor')).toBeVisible(); + + const reloaded = page.locator('ngx-editor .ProseMirror'); + await expect(reloaded.locator('table')).toBeVisible(); + + // Shape is preserved: 3 rows, header row intact. + await expect(reloaded.locator('table tr')).toHaveCount(3); + await expect(reloaded.locator('table th')).toHaveCount(3); + await expect(reloaded.locator('table td')).toHaveCount(6); + + // ...and so is the text, including the empty cells around it. + await expect(reloaded.locator('table th').first()).toHaveText( + 'Character' + ); + await expect(reloaded.locator('table td').first()).toHaveText('Alice'); + }); + }); +}); diff --git a/frontend/package.json b/frontend/package.json index 10fb01f83..4fbd8fb1a 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -103,6 +103,7 @@ "ngx-image-cropper": "9.1.6", "ngx-input-color": "7.0.1", "prosemirror-history": "1.5.0", + "prosemirror-tables": "1.8.5", "prosemirror-transform": "1.12.0", "prosemirror-view": "1.42.3", "rxjs": "7.8.2", @@ -161,6 +162,7 @@ "prosemirror-model": "1.25.11", "prosemirror-schema-list": "1.5.1", "prosemirror-state": "1.4.4", + "prosemirror-tables": "1.8.5", "prosemirror-transform": "1.12.0", "prosemirror-view": "1.42.3" }, diff --git a/frontend/public/assets/i18n/en/editor.json b/frontend/public/assets/i18n/en/editor.json index 7f6bfbc5b..481098306 100644 --- a/frontend/public/assets/i18n/en/editor.json +++ b/frontend/public/assets/i18n/en/editor.json @@ -26,6 +26,19 @@ "insertImage": "Insert image (Ctrl+Shift+I)", "insertImageShort": "Insert image", "horizontalRule": "Horizontal rule", + "table": "Table", + "insertTable": "Insert table", + "addRowBefore": "Insert row above", + "addRowAfter": "Insert row below", + "deleteRow": "Delete row", + "addColumnBefore": "Insert column left", + "addColumnAfter": "Insert column right", + "deleteColumn": "Delete column", + "mergeCells": "Merge cells", + "splitCell": "Split cell", + "toggleHeaderRow": "Toggle header row", + "toggleHeaderColumn": "Toggle header column", + "deleteTable": "Delete table", "clearFormatting": "Clear formatting", "moreOptions": "More formatting options", "autoReview": "Auto-Review", diff --git a/frontend/setup-vitest.ts b/frontend/setup-vitest.ts index 5db7d69f1..661b14a7c 100644 --- a/frontend/setup-vitest.ts +++ b/frontend/setup-vitest.ts @@ -353,6 +353,17 @@ vi.mock('@bobbyquantum/ngx-editor', () => { }; }); +// NOTE: prosemirror-tables is deliberately NOT mocked. +// +// `vi.mock()` from this setup file does not reliably apply to it — under +// coverage instrumentation the real module is loaded regardless, which made +// specs pass locally and fail in CI. The real implementation is safe in unit +// tests instead: every table command bails out via `isInTable()` (or an +// equivalent guard) when the selection is not inside a table, which is the +// case for the mock editor views the specs build. Mock selections must +// therefore expose `$head` and `$anchor`, and mock schemas a `cached` object, +// because that is what the real commands read. + // Mock prosemirror-schema-list for list operations (wrapInList, liftListItem, etc.) vi.mock('prosemirror-schema-list', () => ({ wrapInList: diff --git a/frontend/src/app/components/document-element-editor/document-element-editor-tables.scss b/frontend/src/app/components/document-element-editor/document-element-editor-tables.scss new file mode 100644 index 000000000..79227eef5 --- /dev/null +++ b/frontend/src/app/components/document-element-editor/document-element-editor-tables.scss @@ -0,0 +1,101 @@ +// Editor table styles (prosemirror-tables). +// +// Split out of `document-element-editor.component.scss` — that file sits +// close to the 15 kB `anyComponentStyle` budget, and table styling is a +// self-contained concern. Loaded as a second entry in the component's +// `styleUrls`, so it shares the same view encapsulation. + +::ng-deep ngx-editor { + // --------------------------------------------------------------- + // Tables (prosemirror-tables) + // + // `columnResizing` wraps each table in a `.tableWrapper` div and drives + // widths through a , so the table needs `table-layout: fixed` + // for the column widths it writes to take effect. + // --------------------------------------------------------------- + .ProseMirror { + .tableWrapper { + // A wide table scrolls inside its own wrapper rather than forcing + // the whole document to scroll sideways. + overflow-x: auto; + margin: 1em 0; + } + + table { + border-collapse: collapse; + table-layout: fixed; + width: 100%; + overflow: hidden; + + td, + th { + border: 1px solid var(--sys-outline-variant); + padding: 6px 10px; + vertical-align: top; + box-sizing: border-box; + position: relative; + // Without a minimum, dragging a column to zero width makes the + // cell impossible to click back into. + min-width: 2em; + + > *:first-child { + margin-top: 0; + } + + > *:last-child { + margin-bottom: 0; + } + } + + th { + background-color: var(--sys-surface-container-high); + font-weight: 600; + text-align: left; + } + + // GFM column alignment, stored as an `align` attribute on the cell. + td[align='center'], + th[align='center'] { + text-align: center; + } + + td[align='right'], + th[align='right'] { + text-align: right; + } + + td[align='left'], + th[align='left'] { + text-align: left; + } + } + + // Cell selection highlight. Rendered as an overlay so it does not + // disturb the cell's own background or text contrast. + .selectedCell::after { + content: ''; + position: absolute; + inset: 0; + background-color: var(--sys-primary); + opacity: 0.16; + pointer-events: none; + } + + .column-resize-handle { + position: absolute; + right: -2px; + top: 0; + bottom: 0; + width: 4px; + background-color: var(--sys-primary); + pointer-events: none; + z-index: 20; + } + } + + // While a column drag is in progress prosemirror-tables adds this class + // to the editor; the col-resize cursor must win over the text cursor. + .ProseMirror.resize-cursor { + cursor: col-resize; + } +} diff --git a/frontend/src/app/components/document-element-editor/document-element-editor.component.ts b/frontend/src/app/components/document-element-editor/document-element-editor.component.ts index 54f99c0fc..84613d394 100644 --- a/frontend/src/app/components/document-element-editor/document-element-editor.component.ts +++ b/frontend/src/app/components/document-element-editor/document-element-editor.component.ts @@ -105,7 +105,10 @@ import { createMediaUrl } from '../image-paste'; ], templateUrl: './document-element-editor.component.html', changeDetection: ChangeDetectionStrategy.Eager, - styleUrls: ['./document-element-editor.component.scss'], + styleUrls: [ + './document-element-editor.component.scss', + './document-element-editor-tables.scss', + ], }) export class DocumentElementEditorComponent implements OnInit, OnChanges, OnDestroy, AfterViewChecked diff --git a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.html b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.html index f25053d2f..9e44d79b5 100644 --- a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.html +++ b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.html @@ -297,6 +297,13 @@ data-testid="toolbar-image"> image + + + + + + + + + + + + + + + + + + diff --git a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.spec.ts b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.spec.ts index da66315b8..bf70a0936 100644 --- a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.spec.ts +++ b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.spec.ts @@ -26,6 +26,7 @@ describe('EditorToolbarComponent', () => { let mockEditorView: { state: { schema: { + cached: Record; marks: Record; nodes: Record; }; @@ -34,6 +35,8 @@ describe('EditorToolbarComponent', () => { to: number; $from: unknown; $to: unknown; + $head: unknown; + $anchor: unknown; empty: boolean; }; tr: { @@ -45,6 +48,9 @@ describe('EditorToolbarComponent', () => { addMark: Mock; removeMark: Mock; setStoredMarks: Mock; + scrollIntoView: Mock; + insert: Mock; + doc: { lastChild: unknown; content: { size: number } }; docChanged: boolean; }; doc: { nodesBetween: Mock; slice: Mock }; @@ -65,6 +71,17 @@ describe('EditorToolbarComponent', () => { create: vi.fn().mockReturnValue({ type: { name } }), }); + // A minimal ResolvedPos. `node(depth)` returns a node whose spec has no + // `tableRole`, so prosemirror-tables treats the position as outside a + // table — which is what the non-table tests expect. + const createMockResolvedPos = () => ({ + pos: 0, + depth: 0, + marks: () => [], + node: () => ({ type: { name: 'paragraph', spec: {} } }), + blockRange: vi.fn().mockReturnValue(null), + }); + const createMockNode = (name: string) => ({ name, create: vi.fn().mockReturnValue({ type: { name } }), @@ -74,6 +91,9 @@ describe('EditorToolbarComponent', () => { mockEditorView = { state: { schema: { + // prosemirror-tables' tableNodeTypes() memoises on schema.cached, + // so a real ProseMirror Schema always has this object. + cached: {}, marks: { strong: createMockMark('strong'), em: createMockMark('em'), @@ -90,19 +110,22 @@ describe('EditorToolbarComponent', () => { list_item: createMockNode('list_item'), blockquote: createMockNode('blockquote'), horizontal_rule: createMockNode('horizontal_rule'), + table: createMockNode('table'), + table_row: createMockNode('table_row'), + table_cell: createMockNode('table_cell'), + table_header: createMockNode('table_header'), }, }, selection: { from: 0, to: 0, - $from: { - pos: 0, - depth: 0, - marks: () => [], - node: () => ({ type: { name: 'paragraph' } }), - blockRange: vi.fn().mockReturnValue(null), - }, + $from: createMockResolvedPos(), $to: {}, + // A real ProseMirror Selection always exposes $head and $anchor. + // prosemirror-tables' isInTable() reads $head.depth, so leaving + // them out throws inside the debounced selection-state update. + $head: createMockResolvedPos(), + $anchor: createMockResolvedPos(), empty: true, }, tr: { @@ -114,6 +137,11 @@ describe('EditorToolbarComponent', () => { addMark: vi.fn().mockReturnThis(), removeMark: vi.fn().mockReturnThis(), setStoredMarks: vi.fn().mockReturnThis(), + scrollIntoView: vi.fn().mockReturnThis(), + insert: vi.fn().mockReturnThis(), + // The doc left behind after replaceSelectionWith(table); the + // trailing-paragraph guard inspects its last child. + doc: { lastChild: null, content: { size: 10 } }, docChanged: false, }, doc: { @@ -272,6 +300,122 @@ describe('EditorToolbarComponent', () => { }); }); + describe('Tables', () => { + it('should insert a table and scroll it into view', () => { + vi.useFakeTimers(); + component.insertTable(); + vi.runAllTimers(); + expect(mockEditorView.state.tr.replaceSelectionWith).toHaveBeenCalled(); + expect(mockEditorView.state.tr.scrollIntoView).toHaveBeenCalled(); + expect(mockEditorView.dispatch).toHaveBeenCalled(); + expect(mockEditorView.focus).toHaveBeenCalled(); + }); + + it('should build the requested number of rows and columns', () => { + const rowType = mockEditorView.state.schema.nodes['table_row'] as { + create: Mock; + }; + component.insertTable(4, 2); + expect(rowType.create).toHaveBeenCalledTimes(4); + // Every row is built with exactly two cells. + for (const call of rowType.create.mock.calls) { + expect(call[1]).toHaveLength(2); + } + }); + + it('should build the first row from header cells and the rest from body cells', () => { + const headerType = mockEditorView.state.schema.nodes['table_header'] as { + create: Mock; + }; + const cellType = mockEditorView.state.schema.nodes['table_cell'] as { + create: Mock; + }; + component.insertTable(3, 3); + // One header row of 3, two body rows of 3. + expect(headerType.create).toHaveBeenCalledTimes(3); + expect(cellType.create).toHaveBeenCalledTimes(6); + }); + + it('should append a trailing paragraph when the table ends the document', () => { + const tableType = mockEditorView.state.schema.nodes['table']; + const paragraphType = mockEditorView.state.schema.nodes['paragraph'] as { + create: Mock; + }; + // Simulate the table landing as the document's last node. + mockEditorView.state.tr.doc.lastChild = { type: tableType }; + + component.insertTable(); + + expect(mockEditorView.state.tr.insert).toHaveBeenCalledWith( + 10, + expect.anything() + ); + expect(paragraphType.create).toHaveBeenCalled(); + }); + + it('should not append a paragraph when content already follows the table', () => { + mockEditorView.state.tr.doc.lastChild = { + type: mockEditorView.state.schema.nodes['paragraph'], + }; + + component.insertTable(); + + expect(mockEditorView.state.tr.insert).not.toHaveBeenCalled(); + }); + + it('should do nothing when the schema has no table nodes', () => { + delete mockEditorView.state.schema.nodes['table']; + component.insertTable(); + expect(mockEditorView.dispatch).not.toHaveBeenCalled(); + }); + + it('should not insert a table while disabled', () => { + component.disabled = true; + component.insertTable(); + expect(mockEditorView.dispatch).not.toHaveBeenCalled(); + }); + + it.each([ + ['addRowBefore'], + ['addRowAfter'], + ['deleteRow'], + ['addColumnBefore'], + ['addColumnAfter'], + ['deleteColumn'], + ['mergeCells'], + ['splitCell'], + ['toggleHeaderRow'], + ['toggleHeaderColumn'], + ['deleteTable'], + ])('should run the %s command and refocus the editor', name => { + // prosemirror-tables is not mocked: outside a table each command + // correctly reports "not applicable" and dispatches nothing. What the + // toolbar owns is running the command without throwing and handing + // focus back to the editor afterwards. + vi.useFakeTimers(); + expect(() => + (component[name as keyof typeof component] as () => void).call( + component + ) + ).not.toThrow(); + vi.runAllTimers(); + expect(mockEditorView.focus).toHaveBeenCalled(); + }); + + it('should not run table commands while disabled', () => { + component.disabled = true; + component.addRowAfter(); + component.deleteTable(); + // execCommand returns before touching the view at all. + expect(mockEditorView.focus).not.toHaveBeenCalled(); + expect(mockEditorView.dispatch).not.toHaveBeenCalled(); + }); + + it('should report inTable as false outside a table', () => { + expect(component.inTable()).toBe(false); + }); + }); + describe('History', () => { it('should undo', () => { vi.useFakeTimers(); diff --git a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.ts b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.ts index c9eca8f8f..850ab1ab4 100644 --- a/frontend/src/app/components/editor-toolbar/editor-toolbar.component.ts +++ b/frontend/src/app/components/editor-toolbar/editor-toolbar.component.ts @@ -32,6 +32,20 @@ import { redo, undo } from 'prosemirror-history'; import { type MarkType, type NodeType } from 'prosemirror-model'; import { wrapInList } from 'prosemirror-schema-list'; import { type EditorState, type Transaction } from 'prosemirror-state'; +import { + addColumnAfter, + addColumnBefore, + addRowAfter, + addRowBefore, + deleteColumn, + deleteRow, + deleteTable, + isInTable, + mergeCells, + splitCell, + toggleHeaderColumn, + toggleHeaderRow, +} from 'prosemirror-tables'; import { type EditorView } from 'prosemirror-view'; import { type Subscription } from 'rxjs'; @@ -132,6 +146,7 @@ export class EditorToolbarComponent implements AfterViewInit, OnDestroy { bulletList: false, orderedList: false, blockquote: false, + inTable: false, }); /** @@ -432,6 +447,9 @@ export class EditorToolbarComponent implements AfterViewInit, OnDestroy { /** Current text alignment */ textAlign = computed(() => this.selectionState().align); + /** True when the selection sits inside a table (gates the table menu items) */ + inTable = computed(() => this.selectionState().inTable); + /** Computed active state for bullet list */ isBulletList = computed(() => this.selectionState().bulletList); @@ -500,6 +518,10 @@ export class EditorToolbarComponent implements AfterViewInit, OnDestroy { ); const blockquote = this.isNodeActive(state, schema.nodes['blockquote']); + // `isInTable` throws if the schema has no table nodes at all, which is + // the case for the plain ngx-editor schema used by some consumers. + const inTable = schema.nodes['table'] ? isInTable(state) : false; + this.selectionState.set({ bold, italic, @@ -512,6 +534,7 @@ export class EditorToolbarComponent implements AfterViewInit, OnDestroy { bulletList, orderedList, blockquote, + inTable, }); }, 50); } @@ -818,6 +841,123 @@ export class EditorToolbarComponent implements AfterViewInit, OnDestroy { this.refocusEditor(); } + // ========== Table Commands ========== + + /** Default size for a freshly inserted table: a header row plus two body rows. */ + private static readonly DEFAULT_TABLE_ROWS = 3; + private static readonly DEFAULT_TABLE_COLS = 3; + + /** + * Insert a table at the cursor. + * + * The first row is built from `table_header` cells so the table has a + * usable header out of the box and so it survives a markdown round trip + * (a GFM table's first row is always its header). + */ + insertTable( + rows = EditorToolbarComponent.DEFAULT_TABLE_ROWS, + cols = EditorToolbarComponent.DEFAULT_TABLE_COLS + ): void { + if (this.disabled) return; + + const view = this.editor?.view; + if (!view) return; + + const { state, dispatch } = view; + const { schema } = state; + const tableType = schema.nodes['table']; + const rowType = schema.nodes['table_row']; + const cellType = schema.nodes['table_cell']; + const headerType = schema.nodes['table_header']; + const paragraphType = schema.nodes['paragraph']; + + if (!tableType || !rowType || !cellType || !headerType || !paragraphType) { + return; + } + + const buildRow = (header: boolean) => + rowType.create( + null, + Array.from({ length: cols }, () => + (header ? headerType : cellType).create(null, paragraphType.create()) + ) + ); + + const table = tableType.create( + null, + Array.from({ length: rows }, (_, i) => buildRow(i === 0)) + ); + + const tr = state.tr.replaceSelectionWith(table); + + // A table that ends the document is a dead end: there is no inline + // position after it, so the user cannot click or arrow past the table + // to keep writing. Append a paragraph whenever the insert leaves the + // table as the last node. + const { doc } = tr; + if (doc.lastChild?.type === tableType) { + tr.insert(doc.content.size, paragraphType.create()); + } + + dispatch(tr.scrollIntoView()); + this.refocusEditor(); + } + + /** Insert a row above the current one */ + addRowBefore(): void { + this.execCommand(addRowBefore); + } + + /** Insert a row below the current one */ + addRowAfter(): void { + this.execCommand(addRowAfter); + } + + /** Delete the row containing the selection */ + deleteRow(): void { + this.execCommand(deleteRow); + } + + /** Insert a column to the left of the current one */ + addColumnBefore(): void { + this.execCommand(addColumnBefore); + } + + /** Insert a column to the right of the current one */ + addColumnAfter(): void { + this.execCommand(addColumnAfter); + } + + /** Delete the column containing the selection */ + deleteColumn(): void { + this.execCommand(deleteColumn); + } + + /** Merge the selected cells into one */ + mergeCells(): void { + this.execCommand(mergeCells); + } + + /** Split a merged cell back into its constituent cells */ + splitCell(): void { + this.execCommand(splitCell); + } + + /** Toggle the first row between header and body cells */ + toggleHeaderRow(): void { + this.execCommand(toggleHeaderRow); + } + + /** Toggle the first column between header and body cells */ + toggleHeaderColumn(): void { + this.execCommand(toggleHeaderColumn); + } + + /** Delete the whole table */ + deleteTable(): void { + this.execCommand(deleteTable); + } + /** Clear formatting from selection */ clearFormatting(): void { if (this.disabled) return; diff --git a/frontend/src/app/components/element-ref/extended-schema.spec.ts b/frontend/src/app/components/element-ref/extended-schema.spec.ts index 1e9cf3d94..a5b524b00 100644 --- a/frontend/src/app/components/element-ref/extended-schema.spec.ts +++ b/frontend/src/app/components/element-ref/extended-schema.spec.ts @@ -1,4 +1,4 @@ -import { Schema } from 'prosemirror-model'; +import { DOMParser, DOMSerializer, Schema } from 'prosemirror-model'; import { describe, expect, it } from 'vitest'; import { @@ -34,6 +34,107 @@ describe('extended-schema', () => { }); }); + describe('table nodes', () => { + const schema = createExtendedSchema(); + + it('should include the prosemirror-tables node types', () => { + expect(schema.nodes['table']).toBeDefined(); + expect(schema.nodes['table_row']).toBeDefined(); + expect(schema.nodes['table_cell']).toBeDefined(); + expect(schema.nodes['table_header']).toBeDefined(); + }); + + it('should restrict cell content to paragraphs', () => { + // `block+` would allow nested tables, headings and lists, none of which + // the publish pipeline can render sensibly. + expect(schema.nodes['table_cell'].spec.content).toBe('paragraph+'); + expect(schema.nodes['table_header'].spec.content).toBe('paragraph+'); + }); + + it('should put tables in the block group so they nest under doc', () => { + expect(schema.nodes['table'].spec.group).toContain('block'); + }); + + it('should declare the align attribute on cells, defaulting to null', () => { + for (const name of ['table_cell', 'table_header']) { + const attrs = schema.nodes[name].spec.attrs!; + expect(attrs['align']).toBeDefined(); + expect(attrs['align'].default).toBeNull(); + } + }); + + describe('align attribute', () => { + // prosemirror-tables folds `cellAttributes` into the generated + // parseDOM/toDOM rather than exposing the callbacks on the attr spec, + // so alignment is verified through an actual DOM round trip. + const parser = DOMParser.fromSchema(schema); + const serializer = DOMSerializer.fromSchema(schema); + + /** Parse a fragment and return the first cell's attrs. */ + function parseCellAttrs(cellHtml: string): Record { + const host = document.createElement('div'); + host.innerHTML = `
${cellHtml}
`; + const doc = parser.parse(host); + + let attrs: Record | null = null; + doc.descendants(node => { + if (attrs) return false; + if ( + node.type.name === 'table_cell' || + node.type.name === 'table_header' + ) { + attrs = node.attrs; + return false; + } + return true; + }); + expect(attrs).not.toBeNull(); + return attrs!; + } + + /** Serialize a cell with the given align attr and return its outerHTML. */ + function serializeCell(align: unknown): string { + const cell = schema.nodes['table_cell'].create( + { align }, + schema.nodes['paragraph'].create() + ); + const dom = serializer.serializeNode(cell) as HTMLElement; + return dom.outerHTML; + } + + it('should read alignment from an inline text-align style', () => { + expect( + parseCellAttrs('x')['align'] + ).toBe('center'); + }); + + it('should fall back to the legacy align attribute', () => { + expect(parseCellAttrs('x')['align']).toBe( + 'right' + ); + }); + + it('should leave align null when the cell has no alignment', () => { + expect(parseCellAttrs('x')['align']).toBeNull(); + }); + + it('should write a text-align style for known values', () => { + expect(serializeCell('center')).toContain('text-align: center'); + }); + + it.each([[null], ['justify'], ['center; color: red']])( + 'should not emit a style for the unsupported align value %p', + value => { + expect(serializeCell(value)).not.toContain('text-align'); + } + ); + + it('should not let an align value inject extra declarations', () => { + expect(serializeCell('center; color: red')).not.toContain('color: red'); + }); + }); + }); + describe('link mark spec', () => { const schema = createExtendedSchema(); const linkMarkType = schema.marks['link']; diff --git a/frontend/src/app/components/element-ref/extended-schema.ts b/frontend/src/app/components/element-ref/extended-schema.ts index f34887012..0a5dfb4d1 100644 --- a/frontend/src/app/components/element-ref/extended-schema.ts +++ b/frontend/src/app/components/element-ref/extended-schema.ts @@ -18,14 +18,63 @@ import { marks, nodes } from '@bobbyquantum/ngx-editor/schema'; import { createExtendedSchemaSpec } from '@inkweld/prosemirror/schema'; import { Schema } from 'prosemirror-model'; +import { tableNodes } from 'prosemirror-tables'; + +/** + * Table node specs from `prosemirror-tables`. + * + * `cellContent` is deliberately `'paragraph+'` rather than the library's + * suggested `'block+'`. Allowing arbitrary blocks in a cell permits nested + * tables, headings and lists, none of which the publish pipeline + * (markdown / HTML / EPUB / Typst-PDF) can render sensibly. Restricting + * cells to paragraphs keeps every export path total. + * + * `colwidth` is stored as an array (e.g. `[120, 240]`) and survives the + * canonical XML wire format because `parseAttrValue` JSON-parses any + * attribute whose serialized form starts with `[`. + * + * The extra `align` cell attribute carries GFM column alignment + * (`| :--- | :---: | ---: |`). It lives on the cell rather than on the + * paragraph inside it because alignment in a markdown table is a property + * of the column, and because the publish pipeline does not currently read + * ngx-editor's paragraph-level `align` attribute at all — riding on it + * would silently drop alignment from every export. + */ +const inkweldTableNodes = tableNodes({ + tableGroup: 'block', + cellContent: 'paragraph+', + cellAttributes: { + align: { + default: null, + getFromDOM(dom: HTMLElement) { + return dom.style.textAlign || dom.getAttribute('align') || null; + }, + setDOMAttr(value: unknown, attrs: Record) { + if (value === 'left' || value === 'center' || value === 'right') { + attrs['style'] = + `${(attrs['style'] as string) ?? ''}text-align: ${value};`; + } + }, + }, + }, +}); /** * Build the Inkweld editor schema by merging ngx-editor's base specs with * the shared Inkweld extensions, then constructing a `Schema` with the * frontend's own copy of `prosemirror-model`. + * + * Table nodes are merged into the base set here rather than inside the + * shared package: `tableNodes()` is a frontend-only concern (the backend + * XML parser/serializer works off node *names*, not a `Schema`), and + * keeping the call on this side of the boundary means the shared package + * never gains a `prosemirror-tables` dependency. */ export function buildInkweldSchema(): Schema { - const spec = createExtendedSchemaSpec({ baseNodes: nodes, baseMarks: marks }); + const spec = createExtendedSchemaSpec({ + baseNodes: { ...nodes, ...inkweldTableNodes }, + baseMarks: marks, + }); return new Schema(spec); } diff --git a/frontend/src/app/services/project/document.service.ts b/frontend/src/app/services/project/document.service.ts index 7e229f9f0..eacebfd01 100644 --- a/frontend/src/app/services/project/document.service.ts +++ b/frontend/src/app/services/project/document.service.ts @@ -32,8 +32,10 @@ import { rateLimitBackoff, withJitter, } from '@services/sync/access-denial'; +import { keymap } from 'prosemirror-keymap'; import { type Node as ProseMirrorModelNode } from 'prosemirror-model'; import { Plugin, PluginKey } from 'prosemirror-state'; +import { columnResizing, goToNextCell, tableEditing } from 'prosemirror-tables'; import { Decoration, DecorationSet } from 'prosemirror-view'; import { Observable, Subject } from 'rxjs'; import { IndexeddbPersistence, storeState } from 'y-indexeddb'; @@ -1138,7 +1140,24 @@ export class DocumentService { this.autoReviewApi.clickEvent.set({ attrs, coords }); }, }); - plugins.push(autoReviewPlugin); + // Table editing: cell selection, row/column commands and drag-to-resize. + // + // `columnResizing` must precede `tableEditing` — it installs the `table` + // node view that draws the resize handles, and `tableEditing` expects + // that view to already be in place when it handles a drag. + // + // Tab / Shift-Tab move between cells. The keymap comes *before* the + // table plugins so it wins over their internal handlers, and it is a + // no-op outside a table, leaving Tab free for its normal behaviour. + plugins.push( + autoReviewPlugin, + keymap({ + Tab: goToNextCell(1), + 'Shift-Tab': goToNextCell(-1), + }), + columnResizing({}), + tableEditing() + ); // Reconfigure state with new plugins - this triggers ySyncPlugin's init() // which binds Yjs content to ProseMirror diff --git a/frontend/src/app/services/publish/epub-generator.service.spec.ts b/frontend/src/app/services/publish/epub-generator.service.spec.ts index 4e171f7c3..36c398045 100644 --- a/frontend/src/app/services/publish/epub-generator.service.spec.ts +++ b/frontend/src/app/services/publish/epub-generator.service.spec.ts @@ -2,6 +2,7 @@ import { provideZonelessChangeDetection, signal } from '@angular/core'; import { TestBed } from '@angular/core/testing'; import { type Element, ElementType, type Project } from '@inkweld/index'; import { createDefaultPublishStyles } from '@models/publish-style'; +import JSZip from 'jszip'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { translocoTestProvider } from '../../../testing/transloco-test-provider'; @@ -617,6 +618,102 @@ describe('EpubGeneratorService', () => { expect(result.success).toBe(true); }); + it('should convert tables, including header cells and column alignment', async () => { + documentServiceMock.getDocumentContent.mockResolvedValue([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [ + { + type: 'table_header', + attrs: { colspan: 1 }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Name' }], + }, + ], + }, + { + type: 'table_header', + attrs: { colspan: 1, align: 'right' }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Age' }], + }, + ], + }, + ], + }, + { + type: 'table_row', + content: [ + { + type: 'table_cell', + attrs: { colspan: 1 }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Alice' }], + }, + ], + }, + { + type: 'table_cell', + attrs: { colspan: 1, align: 'center' }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: '30' }], + }, + ], + }, + ], + }, + ], + }, + ]); + + const result = await service.generateEpub(mockPlan); + expect(result.success).toBe(true); + + // Find the chapter document by content rather than by index — an EPUB + // also contains nav/cover XHTML whose order is not guaranteed. + const zip = await new JSZip().loadAsync(result.file!); + const candidates = zip.file(/\.xhtml$/); + const bodies = await Promise.all(candidates.map(f => f.async('string'))); + const xhtml = bodies.find(b => b.includes(''); + expect(xhtml).toContain(''); + expect(xhtml).toContain(' { + const result = await service.generateEpub(mockPlan); + expect(result.success).toBe(true); + + const zip = await new JSZip().loadAsync(result.file!); + const css = zip.file(/\.css$/)[0]; + expect(css).toBeTruthy(); + const styles = await css.async('string'); + + expect(styles).toContain('.ink-doc-table'); + expect(styles).toContain('.ink-doc-align-center'); + }); + it('should convert blockquote nodes', async () => { documentServiceMock.getDocumentContent.mockResolvedValue([ { diff --git a/frontend/src/app/services/publish/epub-generator.service.ts b/frontend/src/app/services/publish/epub-generator.service.ts index a22310c1c..f36c8a215 100644 --- a/frontend/src/app/services/publish/epub-generator.service.ts +++ b/frontend/src/app/services/publish/epub-generator.service.ts @@ -112,6 +112,78 @@ interface TocEntry { children?: TocEntry[]; } +/** Cell node names that can carry GFM column alignment. */ +const TABLE_CELL_NAMES = new Set([ + 'table_cell', + 'tablecell', + 'table_header', + 'tableheader', +]); + +/** + * Map a table cell's `align` attribute to a CSS class. + * + * Alignment becomes a class rather than an inline `style` attribute so that + * attacker-controlled document JSON can never inject arbitrary CSS; only the + * three known values produce a class at all. + */ +function tableAlignClass(node: ProseMirrorNode): string | null { + if (typeof node !== 'object' || !node || !('attrs' in node)) return null; + const attrs = node['attrs'] as Record | null; + const align = attrs?.['align']; + if (align === 'left' || align === 'center' || align === 'right') { + return `ink-doc-align-${align}`; + } + return null; +} + +/** ProseMirror node name → EPUB tag and class. */ +const EPUB_NODE_TAG_MAP: Record = { + paragraph: { tag: 'p', cls: 'ink-doc-paragraph' }, + blockquote: { tag: 'blockquote', cls: 'ink-doc-blockquote' }, + bullet_list: { tag: 'ul', cls: 'ink-doc-bullet-list' }, + bulletlist: { tag: 'ul', cls: 'ink-doc-bullet-list' }, + ordered_list: { tag: 'ol', cls: 'ink-doc-ordered-list' }, + orderedlist: { tag: 'ol', cls: 'ink-doc-ordered-list' }, + list_item: { tag: 'li', cls: 'ink-doc-list-item' }, + listitem: { tag: 'li', cls: 'ink-doc-list-item' }, + hard_break: { tag: 'br', cls: '' }, + horizontal_rule: { tag: 'hr', cls: 'ink-doc-horizontal-rule' }, + code_block: { tag: 'pre', cls: 'ink-doc-code-block' }, + codeblock: { tag: 'pre', cls: 'ink-doc-code-block' }, + image: { tag: 'img', cls: 'ink-doc-image' }, + figure: { tag: 'figure', cls: 'ink-doc-figure' }, + caption: { tag: 'figcaption', cls: 'ink-doc-caption' }, + table: { tag: 'table', cls: 'ink-doc-table' }, + table_row: { tag: 'tr', cls: 'ink-doc-table-row' }, + tablerow: { tag: 'tr', cls: 'ink-doc-table-row' }, + table_cell: { tag: 'td', cls: 'ink-doc-table-cell' }, + tablecell: { tag: 'td', cls: 'ink-doc-table-cell' }, + table_header: { tag: 'th', cls: 'ink-doc-table-header' }, + tableheader: { tag: 'th', cls: 'ink-doc-table-header' }, +}; + +/** The node's type name, lower-cased. Accepts both Yjs and ProseMirror shapes. */ +function epubNodeName(node: object): string { + if ('nodeName' in node) return String(node.nodeName).toLowerCase(); + if ('type' in node) return String(node.type).toLowerCase(); + return ''; +} + +/** Base class for a node, plus a column-alignment class for table cells. */ +function epubClassNames( + cls: string, + lower: string, + node: ProseMirrorNode +): string[] { + const classNames = cls ? [cls] : []; + if (TABLE_CELL_NAMES.has(lower)) { + const align = tableAlignClass(node); + if (align) classNames.push(align); + } + return classNames; +} + /** * Service for generating EPUB files client-side. * @@ -719,48 +791,28 @@ export class EpubGeneratorService { tagName: string; classNames: string[]; } { - const typeMap: Record = { - paragraph: { tag: 'p', cls: 'ink-doc-paragraph' }, - blockquote: { tag: 'blockquote', cls: 'ink-doc-blockquote' }, - bullet_list: { tag: 'ul', cls: 'ink-doc-bullet-list' }, - bulletlist: { tag: 'ul', cls: 'ink-doc-bullet-list' }, - ordered_list: { tag: 'ol', cls: 'ink-doc-ordered-list' }, - orderedlist: { tag: 'ol', cls: 'ink-doc-ordered-list' }, - list_item: { tag: 'li', cls: 'ink-doc-list-item' }, - listitem: { tag: 'li', cls: 'ink-doc-list-item' }, - hard_break: { tag: 'br', cls: '' }, - horizontal_rule: { tag: 'hr', cls: 'ink-doc-horizontal-rule' }, - code_block: { tag: 'pre', cls: 'ink-doc-code-block' }, - codeblock: { tag: 'pre', cls: 'ink-doc-code-block' }, - image: { tag: 'img', cls: 'ink-doc-image' }, - figure: { tag: 'figure', cls: 'ink-doc-figure' }, - caption: { tag: 'figcaption', cls: 'ink-doc-caption' }, - }; + if (typeof node !== 'object' || !node) { + return { tagName: 'span', classNames: [] }; + } - if (typeof node === 'object' && node) { - let name = ''; - if ('nodeName' in node) name = String(node.nodeName); - else if ('type' in node) name = String(node.type); - const lower = name.toLowerCase(); - if (lower === 'heading') { - const attrs = - 'attrs' in node ? (node['attrs'] as Record) : null; - const level = clampLevel(Number(attrs?.['level'] ?? 1)); - return { - tagName: `h${level}`, - classNames: [`ink-doc-heading-${level}`], - }; - } - const mapped = typeMap[lower]; - if (mapped) { - return { - tagName: mapped.tag, - classNames: mapped.cls ? [mapped.cls] : [], - }; - } - return { tagName: lower || 'div', classNames: [] }; + const lower = epubNodeName(node); + if (lower === 'heading') { + const attrs = + 'attrs' in node ? (node['attrs'] as Record) : null; + const level = clampLevel(Number(attrs?.['level'] ?? 1)); + return { + tagName: `h${level}`, + classNames: [`ink-doc-heading-${level}`], + }; } - return { tagName: 'span', classNames: [] }; + + const mapped = EPUB_NODE_TAG_MAP[lower]; + if (!mapped) return { tagName: lower || 'div', classNames: [] }; + + return { + tagName: mapped.tag, + classNames: epubClassNames(mapped.cls, lower, node), + }; } private getAttributes(node: ProseMirrorNode): string { diff --git a/frontend/src/app/services/publish/html-generator.service.spec.ts b/frontend/src/app/services/publish/html-generator.service.spec.ts index 586401203..2590c740d 100644 --- a/frontend/src/app/services/publish/html-generator.service.spec.ts +++ b/frontend/src/app/services/publish/html-generator.service.spec.ts @@ -730,6 +730,137 @@ describe('HtmlGeneratorService', () => { expect(text).toContain('

'); }); + it('should convert tables, including header cells and column alignment', async () => { + documentServiceMock.getDocumentContent.mockResolvedValue([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [ + { + type: 'table_header', + attrs: { colspan: 1 }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Name' }], + }, + ], + }, + { + type: 'table_header', + attrs: { colspan: 1, align: 'right' }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Age' }], + }, + ], + }, + ], + }, + { + type: 'table_row', + content: [ + { + type: 'table_cell', + attrs: { colspan: 1 }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'Alice' }], + }, + ], + }, + { + type: 'table_cell', + attrs: { colspan: 1, align: 'center' }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: '30' }], + }, + ], + }, + ], + }, + ], + }, + ]); + + const planWithElement: PublishPlan = { + ...mockPlan, + items: [ + { + id: 'item-1', + type: PublishPlanItemType.Element, + elementId: 'doc-1', + includeChildren: false, + isChapter: true, + }, + ], + }; + + const result = await service.generateHtml(planWithElement); + + expect(result.success).toBe(true); + const text = await result.file!.text(); + expect(text).toContain(''); + expect(text).toContain(''); + expect(text).toContain('
'); + expect(text).toContain('ink-doc-table-header ink-doc-align-right'); + expect(text).toContain('ink-doc-table-cell ink-doc-align-center'); + expect(text).toContain('Alice'); + }); + + it('should not emit an alignment class for an unrecognised align value', async () => { + documentServiceMock.getDocumentContent.mockResolvedValue([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [ + { + type: 'table_cell', + // Only left/center/right map to a class; anything else is + // ignored so document JSON cannot inject arbitrary CSS. + attrs: { colspan: 1, align: 'justify; color: red' }, + content: [ + { + type: 'paragraph', + content: [{ type: 'text', text: 'x' }], + }, + ], + }, + ], + }, + ], + }, + ]); + + const planWithElement: PublishPlan = { + ...mockPlan, + items: [ + { + id: 'item-1', + type: PublishPlanItemType.Element, + elementId: 'doc-1', + includeChildren: false, + isChapter: true, + }, + ], + }; + + const result = await service.generateHtml(planWithElement); + + expect(result.success).toBe(true); + const text = await result.file!.text(); + expect(text).toContain(''); + expect(text).not.toContain('color: red'); + }); + it('should convert blockquote nodes', async () => { documentServiceMock.getDocumentContent.mockResolvedValue([ { diff --git a/frontend/src/app/services/publish/html-generator.service.ts b/frontend/src/app/services/publish/html-generator.service.ts index 4aca66801..ee2a69274 100644 --- a/frontend/src/app/services/publish/html-generator.service.ts +++ b/frontend/src/app/services/publish/html-generator.service.ts @@ -66,6 +66,23 @@ export interface HtmlResult { type ProseMirrorNode = string | { [key: string]: unknown } | ProseMirrorNode[] | null | undefined; +/** + * Map a table cell's `align` attribute to a CSS class. + * + * Alignment becomes a class rather than an inline `style` attribute so that + * attacker-controlled document JSON can never inject arbitrary CSS; only the + * three known values produce a class at all. + */ +function alignClass(node: ProseMirrorNode): string | null { + if (typeof node !== 'object' || !node || !('attrs' in node)) return null; + const attrs = node['attrs'] as Record | null; + const align = attrs?.['align']; + if (align === 'left' || align === 'center' || align === 'right') { + return `ink-doc-align-${align}`; + } + return null; +} + /** * HTML Generator Service * @@ -615,8 +632,23 @@ export class HtmlGeneratorService { image: { tag: 'img', cls: 'ink-doc-image' }, figure: { tag: 'figure', cls: 'ink-doc-figure' }, caption: { tag: 'figcaption', cls: 'ink-doc-caption' }, + table: { tag: 'table', cls: 'ink-doc-table' }, + table_row: { tag: 'tr', cls: 'ink-doc-table-row' }, + tablerow: { tag: 'tr', cls: 'ink-doc-table-row' }, + table_cell: { tag: 'td', cls: 'ink-doc-table-cell' }, + tablecell: { tag: 'td', cls: 'ink-doc-table-cell' }, + table_header: { tag: 'th', cls: 'ink-doc-table-header' }, + tableheader: { tag: 'th', cls: 'ink-doc-table-header' }, }; + /** Cell node names that carry GFM column alignment. */ + private static readonly TABLE_CELL_NAMES = new Set([ + 'table_cell', + 'tablecell', + 'table_header', + 'tableheader', + ]); + private getTagAndClass(node: ProseMirrorNode): { tagName: string; classNames: string[]; @@ -636,10 +668,15 @@ export class HtmlGeneratorService { } const mapped = HtmlGeneratorService.NODE_TAG_MAP[lower]; if (mapped) { - return { - tagName: mapped.tag, - classNames: mapped.cls ? [mapped.cls] : [], - }; + const classNames = mapped.cls ? [mapped.cls] : []; + // Column alignment travels as a class rather than a style attribute: + // node attributes come from document JSON and are never emitted + // verbatim (see the note on unknown node types below). + if (HtmlGeneratorService.TABLE_CELL_NAMES.has(lower)) { + const align = alignClass(node); + if (align) classNames.push(align); + } + return { tagName: mapped.tag, classNames }; } // Unknown node types render as a neutral container so a malicious // document JSON cannot smuggle in attacker-controlled tags like diff --git a/frontend/src/app/services/publish/markdown-generator.service.spec.ts b/frontend/src/app/services/publish/markdown-generator.service.spec.ts index a0976b608..130ebc3f4 100644 --- a/frontend/src/app/services/publish/markdown-generator.service.spec.ts +++ b/frontend/src/app/services/publish/markdown-generator.service.spec.ts @@ -732,6 +732,55 @@ describe('MarkdownGeneratorService', () => { expect(text).toContain('### H3 Heading'); }); + it('should convert tables to GFM, preserving header and alignment', async () => { + const cell = (text: string, align?: string) => ({ + type: 'table_cell', + attrs: align ? { align, colspan: 1 } : { colspan: 1 }, + content: [{ type: 'paragraph', content: [{ type: 'text', text }] }], + }); + const header = (text: string, align?: string) => ({ + ...cell(text, align), + type: 'table_header', + }); + + documentServiceMock.getDocumentContent.mockResolvedValue([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [header('Name'), header('Age', 'right')], + }, + { + type: 'table_row', + content: [cell('Alice'), cell('30', 'right')], + }, + ], + }, + ]); + + const planWithElement: PublishPlan = { + ...mockPlan, + items: [ + { + id: 'item-1', + type: PublishPlanItemType.Element, + elementId: 'doc-1', + includeChildren: false, + isChapter: true, + }, + ], + }; + + const result = await service.generateMarkdown(planWithElement); + + expect(result.success).toBe(true); + const text = await result.file!.text(); + expect(text).toContain('| Name | Age |'); + expect(text).toContain('| --- | ---: |'); + expect(text).toContain('| Alice | 30 |'); + }); + it('should convert blockquote nodes', async () => { documentServiceMock.getDocumentContent.mockResolvedValue([ { diff --git a/frontend/src/app/services/publish/markdown-generator.service.ts b/frontend/src/app/services/publish/markdown-generator.service.ts index cee86525e..658237cee 100644 --- a/frontend/src/app/services/publish/markdown-generator.service.ts +++ b/frontend/src/app/services/publish/markdown-generator.service.ts @@ -525,6 +525,9 @@ export class MarkdownGeneratorService { orderedlist: 'ordered_list', list_item: 'list_item', listitem: 'list_item', + table: 'table', + table_row: 'table_row', + tablerow: 'table_row', }; const wrapper = simpleWrappers[name]; if (wrapper) return `<${wrapper}>${inner}`; @@ -539,6 +542,12 @@ export class MarkdownGeneratorService { case 'code_block': case 'codeblock': return this.renderCodeBlockXml(node, inner); + case 'table_cell': + case 'tablecell': + return this.renderTableCellXml(node, inner, 'table_cell'); + case 'table_header': + case 'tableheader': + return this.renderTableCellXml(node, inner, 'table_header'); case 'image': return this.renderImageXml(node); case 'horizontal_rule': @@ -563,6 +572,28 @@ export class MarkdownGeneratorService { } } + /** + * Serialize a table cell, preserving the attributes `xmlToMarkdown` needs: + * `align` drives the GFM delimiter row, and `colspan` is expanded into + * padding cells so the emitted rows stay rectangular. + */ + private renderTableCellXml( + node: ProseMirrorNode, + inner: string, + tag: 'table_cell' | 'table_header' + ): string { + const attrs = (node as Record)['attrs'] as + Record | undefined; + + const align = this.safeStringAttr(attrs, 'align'); + const alignAttr = align ? ` align="${this.escapeXmlAttr(align)}"` : ''; + + const colspan = this.getAttr(node, 'colspan', 1); + const colspanAttr = colspan > 1 ? ` colspan="${colspan}"` : ''; + + return `<${tag}${alignAttr}${colspanAttr}>${inner}`; + } + private renderCodeBlockXml(node: ProseMirrorNode, inner: string): string { const attrs = (node as Record)['attrs'] as Record | undefined; diff --git a/frontend/src/app/services/publish/pdf-generator.service.spec.ts b/frontend/src/app/services/publish/pdf-generator.service.spec.ts index e84ea47e2..208ea6b03 100644 --- a/frontend/src/app/services/publish/pdf-generator.service.spec.ts +++ b/frontend/src/app/services/publish/pdf-generator.service.spec.ts @@ -386,6 +386,134 @@ describe('PdfGeneratorService', () => { }); }); + describe('table rendering', () => { + /** Run a plan through the generator and return the Typst markup it produced. */ + async function markupFor(content: unknown[]): Promise { + documentServiceMock.getDocumentContent.mockResolvedValue(content); + + const planWithElement: PublishPlan = { + ...mockPlan, + items: [ + { + id: 'item-1', + type: PublishPlanItemType.Element, + elementId: 'doc-1', + includeChildren: false, + isChapter: true, + }, + ], + }; + + const result = await service.generatePdf(planWithElement); + expect(result.success).toBe(true); + + const call = ( + $typstSnippet.pdf as unknown as { mock: { calls: unknown[][] } } + ).mock.calls.at(-1)!; + return (call[0] as { mainContent: string }).mainContent; + } + + const cell = (text: string, type = 'table_cell', attrs = {}) => ({ + type, + attrs: { colspan: 1, ...attrs }, + content: [{ type: 'paragraph', content: [{ type: 'text', text }] }], + }); + + it('should emit a Typst table with the right column count', async () => { + const markup = await markupFor([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [ + cell('Name', 'table_header'), + cell('Age', 'table_header'), + ], + }, + { + type: 'table_row', + content: [cell('Alice'), cell('30')], + }, + ], + }, + ]); + + expect(markup).toContain('#table('); + expect(markup).toContain('columns: 2'); + expect(markup).toContain('Alice'); + }); + + it('should wrap header cells in strong', async () => { + const markup = await markupFor([ + { + type: 'table', + content: [ + { type: 'table_row', content: [cell('Name', 'table_header')] }, + ], + }, + ]); + + expect(markup).toContain('[#strong[Name]]'); + }); + + it('should express a merged cell as table.cell(colspan:)', async () => { + const markup = await markupFor([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [cell('Wide', 'table_cell', { colspan: 2 })], + }, + { type: 'table_row', content: [cell('a'), cell('b')] }, + ], + }, + ]); + + expect(markup).toContain('table.cell(colspan: 2)[Wide]'); + expect(markup).toContain('columns: 2'); + }); + + it('should pad a short row so the next row starts in column one', async () => { + const markup = await markupFor([ + { + type: 'table', + content: [ + { type: 'table_row', content: [cell('a'), cell('b')] }, + { type: 'table_row', content: [cell('c')] }, + ], + }, + ]); + + // The short row gains one empty cell to fill the second column. + expect(markup).toContain('columns: 2'); + expect(markup).toContain('[c],\n []'); + }); + + it('should clamp an absurd colspan rather than emitting it verbatim', async () => { + const markup = await markupFor([ + { + type: 'table', + content: [ + { + type: 'table_row', + content: [cell('x', 'table_cell', { colspan: 10_000 })], + }, + ], + }, + ]); + + expect(markup).toContain('table.cell(colspan: 64)'); + expect(markup).not.toContain('colspan: 10000'); + }); + + it('should emit nothing for a table with no rows', async () => { + const markup = await markupFor([{ type: 'table', content: [] }]); + expect(markup).not.toContain('#table('); + }); + }); + describe('complete$', () => { it('should emit result when generation completes', async () => { const results: PdfResult[] = []; diff --git a/frontend/src/app/services/publish/pdf-generator.service.ts b/frontend/src/app/services/publish/pdf-generator.service.ts index 93a9a4318..5a0239c10 100644 --- a/frontend/src/app/services/publish/pdf-generator.service.ts +++ b/frontend/src/app/services/publish/pdf-generator.service.ts @@ -826,12 +826,87 @@ export class PdfGeneratorService { break; } + case 'table': + ctx.markup += this.renderTypstTable(children); + break; + + // Rows and cells are consumed by `renderTypstTable`; if one is + // reached on its own (a malformed document) fall through to the + // default child-walking behaviour rather than emitting stray markup. + default: // Process children for unknown nodes children.forEach(c => this.nodeToTypst(c, ctx)); } } + /** + * Render a table as Typst's `#table(...)`. + * + * Typst wants a flat list of cells plus an explicit column count, so the + * rows are flattened here. Ragged rows are padded to the widest row — + * Typst fills a short final row silently, which would shift every cell + * after the gap into the wrong column. + * + * Merged cells become `#table.cell(colspan: n)[…]`, which Typst supports + * directly, so unlike the markdown path no padding is needed for them. + */ + private renderTypstTable(rows: ProseMirrorNode[]): string { + const grid = rows + .filter(row => this.getNodeName(row).startsWith('table_row')) + .map(row => + this.getChildren(row).filter(cell => + this.getNodeName(cell).startsWith('table_') + ) + ); + + if (grid.length === 0) return ''; + + const columns = Math.max( + ...grid.map(row => + row.reduce((sum, cell) => sum + this.tableCellSpan(cell), 0) + ) + ); + if (columns === 0) return ''; + + const cells: string[] = []; + for (const row of grid) { + let used = 0; + for (const cell of row) { + const span = this.tableCellSpan(cell); + const body = this.extractTypstText(cell).trim(); + + // A Typst content block; header cells are wrapped in `strong`. + const content = + this.getNodeName(cell) === 'table_header' + ? `[#strong[${body}]]` + : `[${body}]`; + + cells.push( + span > 1 ? `table.cell(colspan: ${span})${content}` : content + ); + used += span; + } + // Pad the row out so the next row starts in column one. + for (; used < columns; used++) cells.push('[]'); + } + + return ( + `#table(\n columns: ${columns},\n stroke: 0.5pt + gray,\n inset: 6pt,\n ` + + `${cells.join(',\n ')},\n)\n\n` + ); + } + + /** A cell's `colspan`, clamped to a sane range. */ + private tableCellSpan(cell: ProseMirrorNode): number { + const attrs = (cell as Record)['attrs'] as + Record | undefined; + const raw = attrs?.['colspan']; + const n = typeof raw === 'number' ? raw : Number(raw); + if (!Number.isFinite(n) || n < 1) return 1; + return Math.min(64, Math.trunc(n)); + } + private extractTypstText(node: ProseMirrorNode): string { if (!node) return ''; diff --git a/frontend/src/app/services/publish/publish-css-emitter.service.ts b/frontend/src/app/services/publish/publish-css-emitter.service.ts index 0ac068355..5198c022e 100644 --- a/frontend/src/app/services/publish/publish-css-emitter.service.ts +++ b/frontend/src/app/services/publish/publish-css-emitter.service.ts @@ -212,6 +212,15 @@ ${indent}}`; .concat(boxStyleDecls(bm.box, { epub })) .join(' ')} }`, '.ink-page-break { page-break-after: always; }', + // Tables are not yet part of the user-configurable publish styles, + // so they get a neutral built-in treatment that reads correctly in + // both HTML output and e-readers. + '.ink-doc-table { border-collapse: collapse; width: 100%; margin: 1em 0; }', + '.ink-doc-table-cell, .ink-doc-table-header { border: 1px solid #999; padding: 0.35em 0.6em; vertical-align: top; }', + '.ink-doc-table-header { font-weight: bold; text-align: left; }', + '.ink-doc-align-left { text-align: left; }', + '.ink-doc-align-center { text-align: center; }', + '.ink-doc-align-right { text-align: right; }', ]; return rules.join('\n'); } diff --git a/packages/inkweld-prosemirror/src/markdown/markdown-to-xml.ts b/packages/inkweld-prosemirror/src/markdown/markdown-to-xml.ts index 667d39124..1566963ce 100644 --- a/packages/inkweld-prosemirror/src/markdown/markdown-to-xml.ts +++ b/packages/inkweld-prosemirror/src/markdown/markdown-to-xml.ts @@ -11,6 +11,7 @@ * - Bullet lists (`-`, `*`, `+`) and ordered lists (`1.`, `1)`) * - Fenced code blocks (` ``` ` and `~~~`) with optional language * - Thematic breaks (`---`, `***`, `___`) + * - GFM tables, including per-column alignment (`:---`, `:---:`, `---:`) * * Supported inline constructs * --------------------------- @@ -117,12 +118,16 @@ function defaultDecodeElementRefHref(href: string): Record | nu // Block-level parser // --------------------------------------------------------------------------- +/** GFM column alignment; `null` means "not specified". */ +type TableAlign = 'left' | 'center' | 'right' | null; + type Block = | { kind: 'heading'; level: number; text: string } | { kind: 'paragraph'; text: string } | { kind: 'blockquote'; children: Block[] } | { kind: 'list'; ordered: boolean; start: number; items: Block[][] } | { kind: 'code'; lang: string; content: string } + | { kind: 'table'; align: TableAlign[]; header: string[]; rows: string[][] } | { kind: 'hr' }; function parseBlocks(input: string): Block[] { @@ -199,6 +204,14 @@ function tryParseBlock(lines: string[], i: number, out: Block[]): number { return end - i; } + // GFM table (header row + delimiter row). Checked before setext so that a + // single-column table's `---` delimiter is not mistaken for an underline. + const table = tryParseTable(lines, i); + if (table) { + out.push(table.table); + return table.end - i; + } + // Setext heading? (current line is paragraph text; next line is === or ---) const setext = lookaheadSetext(lines, i); if (setext) { @@ -212,14 +225,14 @@ function tryParseBlock(lines: string[], i: number, out: Block[]): number { function collectParagraph(lines: string[], start: number): { end: number; paragraph: Block } { const paraLines = [lines[start]]; let j = start + 1; - while (j < lines.length && !isParagraphTerminator(lines[j])) { + while (j < lines.length && !isParagraphTerminator(lines[j], lines[j + 1])) { paraLines.push(lines[j]); j++; } return { end: j, paragraph: { kind: 'paragraph', text: paraLines.join('\n') } }; } -function isParagraphTerminator(line: string): boolean { +function isParagraphTerminator(line: string, next?: string): boolean { if (line.trim() === '') return true; if (matchFenceOpen(line)) return true; if (isThematicBreak(line)) return true; @@ -228,6 +241,11 @@ function isParagraphTerminator(line: string): boolean { if (matchListMarker(line)) return true; // Setext underline ends the paragraph (and is handled by lookaheadSetext). if (/^\s{0,3}(=+|-+)\s*$/.test(line)) return true; + // A table header row (this line) followed by a delimiter row (the next) + // starts a table, so the running paragraph must stop here. + if (isTableHeaderRow(line) && next !== undefined && parseDelimiterRow(next)) { + return true; + } return false; } @@ -275,6 +293,124 @@ function lookaheadSetext(lines: string[], i: number): { level: number } | null { return null; } +// --------------------------------------------------------------------------- +// GFM tables +// --------------------------------------------------------------------------- + +/** + * Split one table row into raw cell strings. + * + * Leading and trailing pipes are optional in GFM and are stripped when + * present. A pipe preceded by a backslash is an escaped literal and does + * not split the row; the backslash is consumed so the cell text carries a + * bare `|`. + */ +function splitTableRow(line: string): string[] { + const cells: string[] = []; + let cur = ''; + let escaped = false; + + for (const ch of line.trim()) { + if (escaped) { + // Only `\|` is a table-level escape; every other backslash pair is + // left intact for the inline parser to interpret. + cur += ch === '|' ? '|' : `\\${ch}`; + escaped = false; + continue; + } + if (ch === '\\') { + escaped = true; + continue; + } + if (ch === '|') { + cells.push(cur); + cur = ''; + continue; + } + cur += ch; + } + if (escaped) cur += '\\'; + cells.push(cur); + + // Strip the empty leading/trailing cells produced by outer pipes. + if (cells.at(0)?.trim() === '') cells.shift(); + if (cells.at(-1)?.trim() === '') cells.pop(); + + return cells.map((c) => c.trim()); +} + +/** + * Parse a GFM delimiter row (`| --- | :---: |`) into per-column alignment. + * Returns `null` when the line is not a delimiter row. + */ +function parseDelimiterRow(line: string): TableAlign[] | null { + if (!line?.includes('|')) return null; + const cells = splitTableRow(line); + if (cells.length === 0) return null; + + const align: TableAlign[] = []; + for (const cell of cells) { + // NOSONAR(typescript:S5852) - linear: anchored, bounded, no nested quantifiers. + const m = /^(:?)-+(:?)$/.exec(cell); // NOSONAR + if (!m) return null; + const left = m[1] === ':'; + const right = m[2] === ':'; + if (left && right) align.push('center'); + else if (right) align.push('right'); + else if (left) align.push('left'); + else align.push(null); + } + return align; +} + +/** A candidate header row must contain at least one unescaped pipe. */ +function isTableHeaderRow(line: string): boolean { + if (line.trim() === '') return false; + return /(^|[^\\])\|/.test(line); +} + +/** + * Try to parse a GFM table starting at `lines[i]`. + * + * Rows whose cell count differs from the header are normalised: extra + * cells are dropped and missing cells are padded with empty strings, so + * every row in the emitted table has identical length. ProseMirror's table + * model requires rectangular rows, and a ragged table would otherwise fail + * schema validation when the document is opened. + */ +function tryParseTable( + lines: string[], + i: number +): { end: number; table: Block } | null { + const headerLine = lines[i]; + const delimiterLine = lines[i + 1]; + if (!isTableHeaderRow(headerLine)) return null; + if (delimiterLine === undefined) return null; + + const align = parseDelimiterRow(delimiterLine); + if (!align) return null; + + const header = splitTableRow(headerLine); + if (header.length === 0) return null; + // GFM requires the delimiter row to match the header's column count. + if (align.length !== header.length) return null; + + const rows: string[][] = []; + let j = i + 2; + while (j < lines.length) { + const line = lines[j]; + if (line.trim() === '') break; + if (!isTableHeaderRow(line)) break; + const cells = splitTableRow(line); + // Normalise to the header width. + while (cells.length < header.length) cells.push(''); + rows.push(cells.slice(0, header.length)); + j++; + } + + return { end: j, table: { kind: 'table', align, header, rows } }; +} + function collectBlockquote(lines: string[], start: number): { end: number; inner: string } { const inner: string[] = []; let i = start; @@ -481,6 +617,8 @@ function renderBlock(block: Block, ctx: ParseContext): string { return renderListBlock(block, ctx); case 'code': return renderCodeBlock(block); + case 'table': + return renderTableBlock(block, ctx); case 'hr': return ''; } @@ -501,6 +639,34 @@ function renderListBlock( return `<${tag}${attrs}>${itemsXml}`; } +function renderTableBlock( + block: Extract, + ctx: ParseContext +): string { + const cell = (text: string, align: TableAlign, header: boolean): string => { + const tag = header ? 'table_header' : 'table_cell'; + const alignAttr = align ? ` align="${align}"` : ''; + // Cell content is `paragraph+`, so even an empty cell needs one + // paragraph to satisfy the schema. + return `<${tag}${alignAttr}>${renderInline(text, ctx)}`; + }; + + const headerRow = `${block.header + .map((text, col) => cell(text, block.align[col] ?? null, true)) + .join('')}`; + + const bodyRows = block.rows + .map( + (row) => + `${row + .map((text, col) => cell(text, block.align[col] ?? null, false)) + .join('')}` + ) + .join(''); + + return `${headerRow}${bodyRows}
`; +} + function renderCodeBlock(block: Extract): string { const langAttr = block.lang ? ` lang="${escapeXmlAttr(block.lang)}"` : ''; return `${escapeXmlText(block.content)}`; diff --git a/packages/inkweld-prosemirror/src/markdown/xml-to-markdown.ts b/packages/inkweld-prosemirror/src/markdown/xml-to-markdown.ts index 88a2abe18..fdaa6258e 100644 --- a/packages/inkweld-prosemirror/src/markdown/xml-to-markdown.ts +++ b/packages/inkweld-prosemirror/src/markdown/xml-to-markdown.ts @@ -131,6 +131,8 @@ function renderBlockElement(node: AstElement, ctx: RenderContext): string { return '---'; case 'image': return renderImage(node); + case 'table': + return renderTable(node, ctx); default: // Unknown block: best-effort render its children. if (node.children.length === 0) return ''; @@ -138,6 +140,115 @@ function renderBlockElement(node: AstElement, ctx: RenderContext): string { } } +/** + * Render a `` as a GFM table. + * + * Lossy by necessity — GFM tables have no way to express merged cells or + * column widths: + * - `colspan > 1` is expanded into trailing empty cells so the grid + * stays rectangular and every row keeps the same column count. + * - `rowspan` is ignored; the cell's text appears only in the row that + * declares it. + * - `colwidth` is dropped. + * + * Column alignment is taken from the first row's cells, matching GFM, + * where alignment is a property of the column rather than the cell. + */ +function renderTable(node: AstElement, ctx: RenderContext): string { + const rows = node.children.filter( + (c): c is AstElement => c.type === 'element' && isTableRow(c.name) + ); + if (rows.length === 0) return ''; + + const grid = rows.map((row) => + row.children + .filter((c): c is AstElement => c.type === 'element' && isTableCell(c.name)) + .flatMap((cell) => { + const text = renderTableCell(cell, ctx); + const span = cellSpan(cell.attrs['colspan']); + // First slot carries the text; the rest pad the row out. + return [text, ...Array.from({ length: span - 1 }, () => '')]; + }) + ); + + const width = Math.max(...grid.map((r) => r.length)); + if (width === 0) return ''; + for (const row of grid) { + while (row.length < width) row.push(''); + } + + const alignRow = rows[0].children + .filter((c): c is AstElement => c.type === 'element' && isTableCell(c.name)) + .flatMap((cell) => { + const span = cellSpan(cell.attrs['colspan']); + const align = stringAttr(cell.attrs, 'align'); + return Array.from({ length: span }, () => align); + }); + + const delimiter = Array.from({ length: width }, (_, i) => { + switch (alignRow[i]) { + case 'left': + return ':---'; + case 'center': + return ':---:'; + case 'right': + return '---:'; + default: + return '---'; + } + }); + + const line = (cells: string[]): string => `| ${cells.join(' | ')} |`; + + return [line(grid[0]), line(delimiter), ...grid.slice(1).map(line)].join('\n'); +} + +function isTableRow(name: string): boolean { + return name === 'table_row' || name === 'tableRow' || name === 'tr'; +} + +function isTableCell(name: string): boolean { + return ( + name === 'table_cell' || + name === 'tableCell' || + name === 'td' || + name === 'table_header' || + name === 'tableHeader' || + name === 'th' + ); +} + +function cellSpan(value: unknown): number { + const n = typeof value === 'number' ? value : Number(value); + if (!Number.isFinite(n) || n < 1) return 1; + return Math.min(64, Math.trunc(n)); +} + +/** + * Render one cell's content to a single markdown line. A cell holds + * `paragraph+`; multiple paragraphs are joined with `
` because a GFM + * cell cannot contain a line break. Pipes are escaped so they do not split + * the row on re-parse. + */ +function renderTableCell(cell: AstElement, ctx: RenderContext): string { + const blocks = cell.children.filter((c): c is AstElement => c.type === 'element'); + + // A well-formed cell wraps its content in `paragraph+`. Render each + // paragraph's *children* so the wrapper itself is never emitted as an + // inline node — an empty paragraph would otherwise surface as a + // `` placeholder. Only a cell with no block + // children at all falls back to rendering the cell's own children. + const text = + blocks.length > 0 + ? blocks + .map((block) => renderInlineNodes(block.children, ctx).trim()) + .filter((t) => t !== '') + .join('
') + : renderInlineNodes(cell.children, ctx).trim(); + + return text.replaceAll('|', String.raw`\|`).replaceAll('\n', ' '); +} + function clampHeadingLevel(value: unknown): number { const n = typeof value === 'number' ? value : Number(value); if (!Number.isFinite(n)) return 2; diff --git a/packages/inkweld-prosemirror/src/xml/tags.ts b/packages/inkweld-prosemirror/src/xml/tags.ts index a83217275..3a938d77b 100644 --- a/packages/inkweld-prosemirror/src/xml/tags.ts +++ b/packages/inkweld-prosemirror/src/xml/tags.ts @@ -51,6 +51,9 @@ export const NODE_TAG_ALIASES: Record = { numbered_list: 'ordered_list', ol: 'ordered_list', ul: 'bullet_list', + tr: 'table_row', + td: 'table_cell', + th: 'table_header', }; /** @@ -66,4 +69,11 @@ export const BLOCK_NODE_NAMES = new Set([ 'code_block', 'listItem', 'list_item', + // Table nodes must never collapse to a self-closing form: an empty + // `` would be dropped on re-parse, silently changing the + // shape of the table and desynchronising row lengths. + 'table', + 'table_row', + 'table_cell', + 'table_header', ]); diff --git a/packages/inkweld-prosemirror/test/markdown-tables.spec.ts b/packages/inkweld-prosemirror/test/markdown-tables.spec.ts new file mode 100644 index 000000000..8faa8951f --- /dev/null +++ b/packages/inkweld-prosemirror/test/markdown-tables.spec.ts @@ -0,0 +1,171 @@ +/** + * GFM table support in the markdown ⇄ canonical-XML converters. + * + * Tables reach Inkweld from three directions — the MCP mutation tools, + * document import, and the publish pipeline's markdown output — so both + * directions and the round-trip between them are covered here. + */ +import * as Y from 'yjs'; +import { describe, expect, it } from 'vitest'; + +import { markdownToXml, xmlToMarkdown } from '../src/markdown'; +import { applyXmlToYjsFragment, serializeYjsFragmentToXml } from '../src/xml'; + +const SIMPLE = ['| A | B |', '| --- | --- |', '| 1 | 2 |'].join('\n'); + +// --------------------------------------------------------------------------- +// markdownToXml +// --------------------------------------------------------------------------- + +describe('markdownToXml — tables', () => { + it('converts a header row to table_header and body rows to table_cell', () => { + const xml = markdownToXml(SIMPLE); + expect(xml).toBe( + '
' + + 'A' + + 'B' + + '1' + + '2' + + '
' + ); + }); + + it('maps delimiter-row colons to per-column alignment', () => { + const xml = markdownToXml('| L | C | R | D |\n| :-- | :-: | --: | --- |\n| 1 | 2 | 3 | 4 |'); + expect(xml).toContain(''); + expect(xml).toContain(''); + expect(xml).toContain(''); + // An undecorated column carries no align attribute at all. + expect(xml).toContain('D'); + }); + + it('pads short rows and truncates long ones to the header width', () => { + const xml = markdownToXml('| A | B | C |\n| --- | --- | --- |\n| 1 |\n| 1 | 2 | 3 | 4 |'); + const rows = xml.split('').filter((r) => r.includes('')); + // Header + 2 body rows, each exactly 3 cells wide. + expect(rows).toHaveLength(3); + for (const row of rows.slice(1)) { + expect(row.split(' { + const xml = markdownToXml('| A | B |\n| --- | --- |\n| 1 |'); + expect(xml).toContain(''); + }); + + it('treats a backslash-escaped pipe as literal cell text, not a separator', () => { + const xml = markdownToXml('| A | B |\n| --- | --- |\n| C \\| D | 2 |'); + expect(xml).toContain('C | D'); + expect(xml).not.toContain('C '); + }); + + it('parses inline marks inside cells', () => { + const xml = markdownToXml('| A |\n| --- |\n| **b** and `c` |'); + expect(xml).toContain('b'); + expect(xml).toContain('c'); + }); + + it('starts a table immediately after a paragraph with no blank line', () => { + const xml = markdownToXml(`Intro\n${SIMPLE}`); + expect(xml).toBe(`Intro${markdownToXml(SIMPLE)}`); + }); + + it('still reads --- as a setext underline when no pipes are present', () => { + expect(markdownToXml('Title\n---')).toBe('Title'); + }); + + it('still reads a standalone --- as a thematic break', () => { + expect(markdownToXml('a\n\n---\n\nb')).toContain(''); + }); + + it('rejects a delimiter row whose column count differs from the header', () => { + const xml = markdownToXml('| A | B |\n| --- |\n| 1 | 2 |'); + expect(xml).not.toContain(''); + }); + + it('rejects a pipe-less line as a header row', () => { + expect(markdownToXml('A\n---:\n')).not.toContain('
'); + }); +}); + +// --------------------------------------------------------------------------- +// xmlToMarkdown +// --------------------------------------------------------------------------- + +describe('xmlToMarkdown — tables', () => { + it('emits a GFM table with a delimiter row', () => { + expect(xmlToMarkdown(markdownToXml(SIMPLE))).toBe(SIMPLE); + }); + + it('re-escapes pipes that appear in cell text', () => { + const md = xmlToMarkdown( + '
A | B
' + ); + expect(md).toContain(String.raw`A \| B`); + }); + + it('expands colspan into trailing empty cells to keep rows rectangular', () => { + const md = xmlToMarkdown( + '' + + 'Wide' + + '1' + + '2' + + '
' + ); + const widths = md.split('\n').map((l) => l.split('|').length); + expect(new Set(widths).size).toBe(1); + }); + + it('joins multiple paragraphs in one cell with
', () => { + const md = xmlToMarkdown( + 'one' + + 'two
' + ); + expect(md).toContain('one
two'); + }); + + it('accepts the tr/td/th tag aliases', () => { + const md = xmlToMarkdown('' + '
A
1
'); + expect(md).toBe('| A |\n| --- |\n| 1 |'); + }); + + it('renders an empty table as an empty string rather than a stray delimiter', () => { + expect(xmlToMarkdown('
')).toBe(''); + }); +}); + +// --------------------------------------------------------------------------- +// Round trip +// --------------------------------------------------------------------------- + +describe('table round trip', () => { + it('is byte-stable across markdown → xml → markdown → xml', () => { + const md = [ + '| Name | Role | Age |', + '| :--- | :---: | ---: |', + '| Alice | **Admin** | 30 |', + String.raw`| C \| D | ` + '`code`' + ' | 1 |', + ].join('\n'); + + const once = xmlToMarkdown(markdownToXml(md)); + expect(once).toBe(md); + expect(xmlToMarkdown(markdownToXml(once))).toBe(once); + }); + + it('survives the Yjs sync path without collapsing empty cells', () => { + // An empty cell must round-trip as a real element. If it collapsed to a + // self-closing it would be dropped on re-parse, leaving + // the row one column short and the table ragged. + const xml = markdownToXml('| A | B |\n| --- | --- |\n| 1 |'); + + const doc = new Y.Doc(); + const fragment = doc.getXmlFragment('prosemirror'); + applyXmlToYjsFragment(Y, doc, fragment, xml); + + const serialized = serializeYjsFragmentToXml(Y, fragment); + expect(serialized).toBe(xml); + // Both cells still present after the round trip; the second is empty. + expect(xmlToMarkdown(serialized).split('\n')[2]).toBe('| 1 | |'); + }); +});