Repository navigation
[lexical-mdast][lexical-website] Bug Fix: Give a new table cell the alignment of its column - #9485
conghuydever wants to merge 7 commits into
Conversation
…lignment of its column MdastTableExtension imports a GFM column alignment as the format of the paragraph in each cell, and exports it from the header cell. A cell that @lexical/table adds later, in an inserted row or split from a merged cell, came with a plain paragraph. So a new row was drawn left-aligned in a column the Markdown aligns to the center or right. Now a TableNode transform gives the paragraphs with no format in each new body cell the alignment of the cell's column, read from the header cell as export reads it. A TableCellNode transform marks the table when a new cell appears, because splitting a merged cell doesn't mark it. Cells that were there before, paragraphs with a format of their own, and the header row are left as they are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
![meta-code-review[bot]](https://avatars.githubusercontent.com/in/5334748?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for the follow-up to #9478. For rows that @lexical/table inserts, the fix does what the description says: in the mdast dev example, a row inserted under a :-: or ---: column now renders centered or right-aligned like the rows around it. The six new tests fail on main and pass here, and Markdown import and export output is unchanged.
1. Loading a saved editor state re-aligns its tables (please fix). packages/lexical-mdast/src/MdastTableExtension.ts, L734-748
isNew treats a cell as new when the last committed editor state does not contain it, and the TableNode transform asks that of every table it runs on. In the current lexical, calling setEditorState() with a state from parseEditorState() marks every node dirty and runs transforms (the _parsed branch of LexicalEditor.setEditorState), and parsed nodes get fresh keys. So the whole loaded document counts as new. The same goes for a JSON $initialEditorState (InitialStateExtension applies it after register) and for a table pasted from Lexical clipboard data.
As a result, on load, every body-cell paragraph with no format, in a column whose header is aligned, takes the header's alignment. To reproduce: in the mdast example, align a header cell with Table > Align center, copy the #doc= link with Share JSON and open it. The body cells come back centered, and editorState.toJSON() is different after the load. That contradicts the PR's own statement that "Cells that were already there are not changed", and the new docs sentence, which says the rule is for "a body cell added in the editor".
Suggested fix: align only inside a table that is already in the committed state (see the inline suggestion). With that change all 67 cases in MdastTable.test.ts still pass, a load round-trips unchanged, and importing a table runs the TableNode transforms once instead of twice. A unit test that saves and reloads such a table would cover this.
2. Add a changeset. CONTRIBUTING.md (Changesets) and .changeset/README.md require one for every user-facing change to a published package, and a fix that changes editing behavior also needs a Verify: paragraph. main added this requirement after the branch was created. For example:
---
'@lexical/mdast': patch
---
[fix] `MdastTableExtension` gives a table cell added in the editor (an inserted row, or a cell split from a merged cell) its column's GFM alignment
Verify: rows inserted into, and cells unmerged in, tables whose columns are aligned in Markdown.3. To consider: columns aligned from their header cell. The mdast example aligns a column by aligning its header cell ("Table > Align center in a header cell aligns the column" in dev-examples/mdast-editor/src/__tests__/browser/TableEditing.test.ts), which formats only the header paragraph. If you then insert a row, only the new row is aligned, so the column renders mixed (left, centered, left) while the Markdown writes it :-:. Should a new row follow the header, or the body rows next to it? That is for maintainers to decide and doesn't need to block this PR.
As you note, a row inserted above the header row is out of scope; it behaves as it does on main.
| // A cell that isn't in the last committed editor state is new. | ||
| const isNew = (cell: TableCellNode) => | ||
| !editor.getEditorState()._nodeMap.has(cell.getKey()); | ||
| return mergeRegister( | ||
| // Splitting a merged cell adds cells to rows without marking the table, | ||
| // so a new cell marks it. | ||
| editor.registerNodeTransform(TableCellNode, cell => { | ||
| const tableNode = isNew(cell) ? $findTableNode(cell) : null; | ||
| if (tableNode) { | ||
| tableNode.markDirty(); | ||
| } | ||
| }), | ||
| editor.registerNodeTransform(TableNode, tableNode => | ||
| $alignNewCells(tableNode, isNew), | ||
| ), |
There was a problem hiding this comment.
must_fix: isNew treats every cell of a table that arrives whole as new. In the current lexical, setEditorState(parseEditorState(json)) marks every node dirty and runs transforms (the _parsed branch of LexicalEditor.setEditorState), and parsed nodes get fresh keys. So loading a saved document, applying a JSON $initialEditorState, or pasting a table gives every unformatted body-cell paragraph in an aligned column the header's alignment, and toJSON() no longer round-trips. Leaving tables that are new as a whole alone fixes this, and all 67 MdastTable.test.ts cases still pass:
// A node that isn't in the last committed editor state is new.
const isNew = (node: LexicalNode) =>
!editor.getEditorState()._nodeMap.has(node.getKey());
return mergeRegister(
editor.registerNodeTransform(TableCellNode, cell => {
const tableNode = isNew(cell) ? $findTableNode(cell) : null;
if (tableNode && !isNew(tableNode)) {
tableNode.markDirty();
}
}),
// A table that is new as a whole (imported, loaded or pasted) keeps
// the formats it came with.
editor.registerNodeTransform(TableNode, tableNode => {
if (!isNew(tableNode)) {
$alignNewCells(tableNode, isNew);
}
}),
);There was a problem hiding this comment.
Fixed in c351656, as suggested. A table that is new as a whole (imported, loaded or pasted) now keeps the formats it came with, and a new cell only marks a table that was already in the editor.
New tests cover the three ways a table arrives whole: setEditorState(parseEditorState(json)), a JSON $initialEditorState, and a pasted table. All three failed before the fix (the unformatted body cell in the :-: column came back center) and pass now.
… table as they are Loading a saved editor state marks every node dirty and gives it a new key, so every cell of a loaded, pasted or imported table counted as new and took its column's alignment. Now only a table that was already in the editor aligns its new cells. Add tests for a reload, a JSON initial editor state and a paste, and a changeset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. All points are handled in c351656:
Checks with these changes on top of the current |
There was a problem hiding this comment.
Thanks, this looks good. Giving a new body cell its column's alignment fixes the mismatch between what the editor draws and what the Markdown writes, and the follow-up commit makes loaded, initial and pasted tables keep their stored formats, with tests for each in MdastTable.test.ts. The suggestions below are optional and non-blocking.
- A column aligned from its header cell renders mixed after an insert (design question). The mdast dev example aligns a column from its header cell (Table > Align in the header, as
dev-examples/mdast-editor/src/Editor.tsxdescribes), which formats only the header's paragraph. After that, an inserted row follows the header while the existing body rows stay at the start, though the Markdown writes the whole column aligned. If leaving existing cells alone stays the rule, aligning a header could also restyle that column's body cells (for example in the example'sapplyTableAction), so the editor matches the Markdown either way. - Changeset wording for paste (
.changeset/mdast-table-new-cell-alignment.md, line 7). Pasting a table into an existing table grows it with$insertTableRowAtNode, so the cells of the rows the paste adds are new and their unformatted pasted paragraphs take the column alignment, while pasted cells that land on existing rows keep no format. TheVerify:line says a pasted table keeps the formats it comes with; noting that rows a paste adds to an existing table take their column's alignment would match what users see. - Test plan output. The Test plan still shows the earlier run (six cases, 67 tests in the file, 291 in the package); the head has nine cases (70 in the file) and 294 in the package. AGENTS.md (Commits and Pull Requests) asks for the real Before and After output of the command that was run, so pasting the current output would bring it up to date.
|
|
||
| [fix] `MdastTableExtension` gives a table cell added in the editor (an inserted row, or a cell split from a merged cell) its column's GFM alignment | ||
|
|
||
| Verify: rows inserted into, and cells unmerged in, tables whose columns are aligned in Markdown. A table that is imported, loaded from a saved editor state or pasted keeps the formats it comes with. |
There was a problem hiding this comment.
Optional: pasting a table into an existing table grows it with $insertTableRowAtNode, so the pasted paragraphs (with no format) in the rows the paste adds take their column's alignment, while pasted cells that land on existing rows keep none. Saying that rows a paste adds to an existing table take the column's alignment would match what users see.
There was a problem hiding this comment.
Done in 472d562. The Verify: line now says that a table pasted outside a table keeps the formats it comes with, and that when a table is pasted into an existing table, the rows the paste adds take their column's alignment, while the pasted cells that land on existing rows keep the formats they come with.
I checked it first: pasting a two-row table at the first body row of the aligned test table left that existing row unformatted (['', '', '', '']) and gave the added row its column alignments (['left', 'center', 'right', '']).
…ng into an existing table does ## Description The changeset said that a pasted table keeps the formats it comes with. That holds for a table pasted outside a table. When a table is pasted into an existing table, @lexical/table adds the rows it needs, so the cells of those rows are new and take their column's alignment, while the pasted cells that land on existing rows keep their own formats. The Verify paragraph now says so. ## Changeset Updates .changeset/mdast-table-new-cell-alignment.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the approval. On the three suggestions:
|
There was a problem hiding this comment.
Thanks, this looks good. A cell that @lexical/table adds to an aligned column now looks like the rest of its column, tables that arrive whole keep their stored formats, and the new tests in MdastTable.test.ts cover inserts above and below, a split merged cell, and the load, initial-state and paste cases. The suggestions below are optional and non-blocking.
- Rebuild the grid only for tables that gained a cell (
packages/lexical-mdast/src/MdastTableExtension.ts, lines 749-753). TheTableNodetransform calls$alignNewCells, which builds the whole grid, every time a table already in the editor is marked dirty, so a column-width change, a striping or frozen-row toggle and a row delete pay for it too. On a 2000-row, 6-column aligned table withTableExtension,setColWidthswent from about 2 ms to about 16 ms, and a row insert from about 25 ms to 60-73 ms. Nothing here runs per keystroke, so this is not urgent. TheTableCellNodetransform already sees every new cell; recording its table's key there for the running update, the way@lexical/tablerecords rows that lost cells (rowsLostinLexicalTablePluginHelpers.ts), would let the table transform skip tables with no new cell. Verify:wording for a paste onto merged cells (.changeset/mdast-table-new-cell-alignment.md, line 7). Pasting a table into a table first splits any merged cell in the target area ($insertTableIntoGridcalls$unmergeCellNode), and the split-off cells are new, so pasted cells that land in them take the column's alignment even on existing rows. For example, pasting an unformatted 2×2 table onto a merged b2:c3 of| :- | :-: | -: | - |leaves the first pasted cell unformatted and aligns the other three. CONTRIBUTING.md (Changesets) asks theVerify:paragraph to say what to test, so adding "or onto cells the paste splits from a merged cell" next to "the rows the paste adds" would match what people see.
Leaving the restyling of a column's existing body cells, when its header cell is aligned, to a separate PR makes sense; a maintainer may want that follow-up tracked.
|
|
||
| [fix] `MdastTableExtension` gives a table cell added in the editor (an inserted row, or a cell split from a merged cell) its column's GFM alignment | ||
|
|
||
| Verify: rows inserted into, and cells unmerged in, tables whose columns are aligned in Markdown. A table that is imported, loaded from a saved editor state, or pasted outside a table keeps the formats it comes with. When a table is pasted into an existing table, the rows the paste adds take their column's alignment, while the pasted cells that land on existing rows keep the formats they come with. |
There was a problem hiding this comment.
Optional: a paste onto merged cells splits them first ($insertTableIntoGrid → $unmergeCellNode), and the split-off cells are new, so pasted cells landing there take the column's alignment even on existing rows (pasting an unformatted 2×2 table onto a merged b2:c3 of | :- | :-: | -: | - | aligns three of the four pasted cells). Adding "or onto cells the paste splits from a merged cell" next to "the rows the paste adds" would match what people see.
| editor.registerNodeTransform(TableNode, tableNode => { | ||
| if (!isNew(tableNode)) { | ||
| $alignNewCells(tableNode, isNew); | ||
| } | ||
| }), |
There was a problem hiding this comment.
Optional: this builds the whole grid every time a table already in the editor is marked dirty, not only when it gains a cell, so a column-width change, a striping or frozen-row toggle and a row delete pay for it too (on a 2000×6 aligned table, setColWidths went from about 2 ms to about 16 ms). Recording the table's key in the TableCellNode transform above for the running update, as @lexical/table does with rowsLost, would let this skip tables with no new cell.
Description
Follow-up to #9478.
MdastTableExtensionimports a GFM column's alignment as the format of the paragraph in each cell of the column, and exports a column's alignment from its header cell. A cell that@lexical/tableadds later comes with a plain paragraph:$insertTableRowAtSelectionand$insertTableRowAtNodemake each new cell with$createParagraphNode(), and so does$unmergeCellNodefor the cells it splits off. So in a column written:-:, a new row is drawn left-aligned while the Markdown still says center. After a reload it is centered, because import puts the alignment on every cell.For example, in
insert a row below
1: the new paragraphs have the format'', so the new cell inbis drawn left-aligned, and the export still writes:-:forb.This change registers two transforms in
MdastTableExtension:TableNodetransform gives each paragraph with no format in a new body cell the alignment of the cell's column. It reads it from the header cell with the same$tableGridand$cellFormatthe export uses, so merged cells count as they do in export.TableCellNodetransform marks the table dirty when a new cell appears in a table that was already there. Splitting a merged cell adds cells to existing rows without marking the table, so the table transform would not run for it otherwise.A node is new when it is not in the last committed editor state (
editor.getEditorState()._nodeMap, as@lexical/historyandMdastShortcutsalready read it). So:setEditorState(parseEditorState(json))or a JSON$initialEditorState, and one pasted outside a table. Loading a saved state marks every node dirty and gives it a new key, so without this every cell of a loaded table would count as new.@lexical/tableadds the rows it needs with$insertTableRowAtNode. The cells of those rows are new, so they take their column's alignment, while the pasted cells that land on existing rows keep the formats they came with.The grid is built once per table in a transform pass, not once per cell. Importing a 2000-row, 6-column aligned table took about 441 ms with this change and 400 ms without, and inserting a row into it about 29 ms and 9 ms (median of 5 runs, unit test environment, macOS).
Out of scope: a row inserted above the header row becomes the header row, and its cells have no format, so the export drops the column alignments. That is how it works today, and this change does not touch it.
markdown-mdast.mdgets one sentence about it.AI disclosure: the code, tests and this description were prepared with AI assistance.
Changeset
.changeset/mdast-table-new-cell-alignment.md:@lexical/mdastpatch,[fix], with aVerify:paragraph.Test plan
New cases in
packages/lexical-mdast/src/__tests__/unit/MdastTable.test.ts("a new body cell"), on a table aligned| :- | :-: | -: | - |:$mergeCells, then$unmergeCellNode) gets its column's format.:-:column keeps it unformatted when it is loaded withsetEditorState(parseEditorState(json)), given as a JSON$initialEditorState, or pasted.Before
The new tests with
MdastTableExtension.tsfrommain(e0f13db):The last two fail on
mainbecause the new row is unaligned there. The load, initial state and paste cases pass onmain, since nothing is aligned there; they failed on this PR's first version (817fdd7), where the unformatted cell came backcenter. With this change but without theTableCellNodetransform, only the split case fails.After
The one
test-unitfailure isENOTEMPTYfromfs.rmSyncwhilechangelog.test.tsremoves its own temp folder. Run alone, it fails on and off, a different case each time, and this PR changes no file underscripts/.pnpm run tscis clean, andpnpm exec eslintonpackages/lexical-mdast/src,pnpm exec prettier --checkandpnpm run check-changesetsare clean.Tested on macOS with Node 24.21.0. Browser tests, E2E and the playground were not run.
🤖 Generated with Claude Code