feat(editor): add table support - #1368
Merged
Merged
Conversation
Adds tables to the document editor, built directly on `prosemirror-tables` rather than on the upstream ngx-editor table PR (sibiraj-s/ngx-editor#610). That PR is roughly 70% menu components, and Inkweld already replaced ngx-editor's menu with its own Material toolbar — only the schema and editing plugins are useful here, so the fork stays untouched. Editor: - `tableNodes()` merged into the schema in `extended-schema.ts`, with `cellContent: 'paragraph+'` so cells cannot hold nested tables or headings that no export format renders sensibly, plus an `align` cell attribute for GFM column alignment - `columnResizing` + `tableEditing` + a Tab/Shift-Tab keymap appended to the editor's plugin list - Material toolbar menu covering insert, row/column add and delete, merge and split, header toggles, and delete table - Inserting a table at the end of a document also appends a paragraph, so a trailing table is not a dead end for the cursor Wire format and markdown: - table node names added to `BLOCK_NODE_NAMES`, so an empty cell serializes as `<table_cell></table_cell>` instead of collapsing to a self-closing tag and desynchronising the row - `markdownToXml` parses GFM tables including `:---` / `:---:` / `---:` alignment, escaped pipes, and ragged rows (padded to the header width) - `xmlToMarkdown` emits them again; the round trip is byte-stable - `update_document_content` MCP tool documents both Publish: tables render in Markdown, HTML, EPUB, and Typst/PDF output. Merged cells survive in PDF; the other three flatten a merge into the cell plus empty columns so the grid keeps its shape. Tables are not yet part of the user-configurable publish-styles system. `prosemirror-tables` is pinned in `resolutions` alongside the other ProseMirror packages — a second copy of `prosemirror-model` would break class-identity checks in y-prosemirror and silently stop typing (#1068).
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Two problems, both in tests rather than in the table implementation.
**Toolbar spec crashed CI.** The mock selection defined only `$from` and
`$to`. The real `isInTable()` reads `selection.$head`, so it threw inside
the debounced selection-state update — failing the table tests and leaking
unhandled errors into the overflow tests in the same file. With
`maxForks: 1` in CI the worker died and the run ended in `write EPIPE`
rather than a readable failure list.
`vi.mock('prosemirror-tables', ...)` from `setup-vitest.ts` turned out not
to apply reliably — under coverage instrumentation the real module loads
regardless, which is why the suite passed locally without `--coverage` and
failed in CI. Rather than fight that, the mock is gone and the specs now
run against the real library: every table command bails out through
`isInTable()` (or an equivalent guard) when the selection is not in a
table, so the real implementations are safe against a mock view. Mock
selections gain `$head`/`$anchor` and mock schemas a `cached` object,
because that is what the real code reads. The command assertions check
what the toolbar actually owns — the command runs without throwing and
focus returns to the editor — while the e2e specs cover the mutations.
**Publish table output was untested**, which is most of the coverage gap
Sonar flagged (failing tests also contribute no coverage). Added tests for
the HTML, EPUB and Typst/PDF table paths, plus the schema's `align` cell
attribute. Line coverage on the touched files is now 84–100%.
Two things surfaced while writing them:
- EPUB requires `type: 'text'` on text nodes while HTML only checks for a
`text` property, so the original test data silently rendered empty cells.
Real ProseMirror JSON always sets it; the data was wrong, not the code.
All the table fixtures now use the realistic shape.
- The schema's `align` callbacks are folded into the generated
parseDOM/toDOM rather than exposed on the attr spec, so they are covered
through a real DOM round trip instead — which also pins down that an
align value cannot smuggle extra CSS declarations into the output.
Four code smells SonarCloud raised against the new table code. None change
behaviour.
- `splitTableRow` used `cells[cells.length - 1]`; `.at(-1)` says the same
thing and drops the now-redundant length guards.
- `parseDelimiterRow`'s `!line || !line.includes('|')` collapses to
`!line?.includes('|')`.
- The table plugins went into a second consecutive `plugins.push()`; folded
into the preceding one. Array order — and so plugin precedence — is
unchanged.
- `EpubGeneratorService.getTagAndClass` hit cognitive complexity 22 once the
table branch was added. Its node-tag map is now a module-level constant
rather than an object literal rebuilt on every node, and name resolution
and class-list building are separate helpers.
|
bobbyquantum
added
the
deploy:preview:frontend
Contributor
|
🚀 Frontend preview deployed for #1368 This is a frontend-only preview (no backend). It runs in local/offline mode; point it at an existing server at runtime through the setup flow if needed. On PR close or label removal, the cleanup workflow attempts to delete this branch's Pages deployments; note that Cloudflare keeps the latest deployment for a branch, so the preview URL may remain reachable after cleanup. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds tables to the document editor.
Why not the upstream PR
There is an open table PR on ngx-editor (sibiraj-s/ngx-editor#610, open since June 2025, maintainer last responded a year ago). Roughly 70% of it is menu components — and Inkweld replaced ngx-editor's menu with its own Material toolbar, so those would be dead weight. Only the schema and editing plugins are useful, and both can be wired up on this side, so
@bobbyquantum/ngx-editorstays untouched.Editor
tableNodes()merged into the schema inextended-schema.ts.cellContentis'paragraph+', not the library's suggested'block+'— arbitrary blocks would permit nested tables and headings that no export format renders sensibly. An extraaligncell attribute carries GFM column alignment.columnResizing+tableEditing+ aTab/Shift-Tabkeymap appended to the editor's plugin list.Wire format and markdown
BLOCK_NODE_NAMES, so an empty cell serializes as<table_cell></table_cell>rather than collapsing to a self-closing tag and dropping a column on re-parse.markdownToXmlparses GFM tables::---/:---:/---:alignment, escaped pipes, and ragged rows (padded or truncated to the header width, since ProseMirror requires rectangular rows).xmlToMarkdownemits them again. The round trip is byte-stable.update_document_contentMCP tool documents both formats.Publish
Tables render in all four output formats. Merged cells survive in PDF; Markdown, HTML, and EPUB flatten a merge into the cell plus empty columns so the grid keeps its shape. Column alignment is preserved everywhere.
Tables are not yet part of the user-configurable publish-styles system — they get fixed built-in styling for now. That is a deliberate scope cut; wiring them into
DocNodeKeytouches the style model, presets, and the style editor UI.A note on the dependency
prosemirror-tablesis pinned inresolutionsalongside the other ProseMirror packages. It depends onprosemirror-model,-state, and-view, and a second copy ofprosemirror-modelbreaks class-identity checks in y-prosemirror andEditorView— typing silently stops working, which is the failure mode from #1068. Verified there is still exactly one copy of each after install.Testing
@inkweld/prosemirrorcovering both markdown directions, the round trip, and survival through the Yjs sync pathDocs
User guide gets a Tables section under Formatting, table shortcuts under Keyboard Shortcuts, a toolbar entry, and an export-fidelity note. Developer docs get an Editor Schema section in
architecture.md, andAGENTS.mdgains a checklist of the files that must stay in step when touching tables.Not in this PR
.docxor pasted HTMLOne thing worth knowing
Reloading a document that starts with a table logs
TextSelection endpoint not pointing into a node with inline contentfrom y-prosemirror, which restores the selection at position 0 before the first node. This is pre-existing and not table-specific — a document starting with a horizontal rule does the same thing onmain. ProseMirror recovers and content is unaffected, so I left it alone rather than widening this PR.