Conversation
alexvcasillas
added
the
Open Source
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: eaccb2f The changes in this PR will be included in the next version bump. This PR includes changesets to release 72 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@bdbch I think the |
There was a problem hiding this comment.
Pull request overview
Extends @tiptap/extension-table’s tableCell and tableHeader node schemas to preserve cell-level styling (background, vertical alignment, and per-side border width/style/color) through HTML parsing and JSON round-trips.
Changes:
- Added shared attribute factories for background color, vertical-align, and 12 border properties.
- Registered the new attributes on
TableCellandTableHeader. - Added Vitest coverage (unit tests for factories + integration tests for
<td>/<th>) and a changeset.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/extension-table/src/utilities/create-vertical-align-attribute.ts | Adds a vertical-align attribute factory with value normalization. |
| packages/extension-table/src/utilities/create-border-attributes.ts | Adds a border attribute factory generating 12 per-side border attrs. |
| packages/extension-table/src/utilities/create-background-attribute.ts | Adds a background-color attribute factory. |
| packages/extension-table/src/header/table-header.ts | Registers new styling attributes on tableHeader. |
| packages/extension-table/src/cell/table-cell.ts | Registers new styling attributes on tableCell. |
| packages/extension-table/tests/utilities/create-vertical-align-attribute.spec.ts | Unit tests for vertical-align attribute parsing/rendering. |
| packages/extension-table/tests/utilities/create-border-attributes.spec.ts | Unit tests for border attribute generation and parse/render behavior. |
| packages/extension-table/tests/utilities/create-background-attribute.spec.ts | Unit tests for background attribute parsing/rendering. |
| packages/extension-table/tests/tableHeaderAttributes.spec.ts | Integration tests ensuring <th> attrs parse/render/round-trip. |
| packages/extension-table/tests/tableCellAttributes.spec.ts | Integration tests ensuring <td> attrs parse/render/round-trip. |
| .zed/settings.json | Adds Zed editor settings (currently invalid JSON due to trailing commas). |
| .changeset/shy-queens-itch.md | Declares a minor bump for the new user-facing table cell/header attributes. |
bdbch
left a comment
bdbch
left a comment
There was a problem hiding this comment.
Nice direction overall, but I think there are still a couple of preservation issues in the HTML path and one stray editor file in the PR.
| * | ||
| * @param value - A CSS length string (e.g. "1px", "2") | ||
| * @return The numeric pixel value, or null if unparseable | ||
| */ |
There was a problem hiding this comment.
This feels a bit too lossy for imported HTML. parseFloat() drops the original unit, and then renderHTML() always writes back px, so something like 0.5rem, 1em, or thin either changes meaning or gets dropped. If the goal here is preserving cell styling end to end, I think width probably needs to keep the original CSS token instead of normalizing everything into a number.
| } | ||
| return { style: `border-${cssSide}-style: ${attributes[styleKey]}` } | ||
| }, | ||
| } |
There was a problem hiding this comment.
I think this has the same round-trip problem the background helper already avoids. Reading color from element.style will usually normalize #000000 into rgb(0, 0, 0), so HTML import/export is still lossy here. I'd probably parse the raw style attribute first, same as createBackgroundAttribute(), and only fall back to the CSSOM value if needed.
| @@ -0,0 +1,23 @@ | |||
| { | |||
There was a problem hiding this comment.
Looks like this is just a local editor config file. Can we drop it from the PR? It doesn't seem related to the feature and it'll just add noise/churn for everyone else.
Changes Overview
Extends the
tableCellandtableHeadernode schemas in@tiptap/extension-tablewith 14 new attributes to support cell-level styling:background,verticalAlign, and all 12 border properties(
borderTopWidth,borderTopStyle,borderTopColor,borderBottomWidth,borderBottomStyle,borderBottomColor,borderLeftWidth,borderLeftStyle,borderLeftColor,borderRightWidth,borderRightStyle,borderRightColor).Implementation Approach
Following the existing
createAlignAttribute()factory pattern, three new shared utility factories were created:createBorderAttributes()— Returns all 12 border attribute definitions. Each parses from the element's computed inline style (e.g.element.style.borderTopWidth) and renders back as individual inlinestyle properties.
createBackgroundAttribute()— Parsesbackground-colorfrom inline styles and renders asbackground-color: <value>.createVerticalAlignAttribute()— Parsesvertical-alignfrom inline styles, normalizes totop | middle | bottom(rejects invalid values), and renders asvertical-align: <value>.Both
tableCellandtableHeaderspread these factories into theiraddAttributes()alongside the existingcolspan,rowspan,colwidth, andalignattributes. All new attributes default tonullandare enabled by default — no configuration needed.
Testing Done
79 tests across 8 test files (72 new, 7 existing):
tableCellAttributes.spec.ts(21 tests) — Full integration tests for<td>: parse, render, default null, JSON round-trip, and coexistence with existing attrs for background, verticalAlign, and allborders.
tableHeaderAttributes.spec.ts(21 tests) — Same coverage for<th>.utilities/create-border-attributes.spec.ts(18 tests) — Unit tests for the factory: creates 12 attrs, correct defaults, parse/render for width (numeric), style (string), and color (string) on allsides.
utilities/create-background-attribute.spec.ts(7 tests) — Unit tests: default, parse hex/rgb/empty, render value/null/undefined.utilities/create-vertical-align-attribute.spec.ts(10 tests) — Unit tests: default, parse all valid values/invalid/empty, render all values/null/undefined.Verification Steps
cd oss && pnpm vitest run packages/extension-table— all 79 tests passTableKit, calleditor.commands.setContent(json)with a JSON doc containingtableCellnodes that haveborderTopWidth: 1, borderTopStyle: "solid", background: "#ECF0E9"— verifythe
<td>elements render with the corresponding inline styleseditor.getJSON()— verify the border/background/verticalAlign attributes round-trip correctlyAdditional Notes
These attributes match what the Tiptap Convert DOCX import API returns on
tableCellnodes (borderTopWidth,borderTopStyle,borderTopColor,background,verticalAlign, etc.). Previously ProseMirrorsilently dropped these attrs during
setContentbecause the schema didn't define them. With this change, DOCX table borders and cell styling are preserved end-to-end.Checklist
Related Issues
N/A — Part of the experimental CSS export style mapping feature (
feat/experimental-css-export-style-mapping).