fix(core): preserve text alignment in portable text converters (#1201) - #1573
Conversation
…h-cms#1201) The rich-text editor ships `@tiptap/extension-text-align` (configured for paragraph and heading), but `prosemirrorToPortableText` built each block with only `_type`/`_key`/`style`/`children`/`markDefs` and never read `node.attrs.textAlign`, so any alignment set from the toolbar was silently dropped on save. The reverse converter had no handling either, so it could not round-trip. Add an optional `textAlign` to `PortableTextTextBlock`, forward a meaningful (non-default) alignment from paragraph/heading nodes, and restore it back into ProseMirror attrs. Default (left/unset) alignment is omitted so existing content is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 6a5d5e3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
github-actions
Bot
added
area/core
size/M
review/needs-review
![emdashbot[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approach
The approach is sound and fits EmDash's conventions. The rich-text editor ships @tiptap/extension-text-align configured for paragraph/heading, but neither converter touched node.attrs.textAlign, so toolbar alignment was dropped on save and couldn't round-trip. This PR adds an optional textAlign to PortableTextTextBlock, extracts a non-default alignment into Portable Text (extractTextAlign), and restores it into ProseMirror attrs on load. Default/left is omitted, so existing stored content stays byte-for-byte unchanged (the textAlign: undefined value is dropped by JSON.stringify). That's an additive, backwards-compatible fix — the right shape for a pre-1.0 bug fix. Changeset targets emdash at patch and is written as release notes, not a commit message. Good.
What I checked
- Traced both converters end to end and confirmed the paragraph/heading round-trip is correct in both directions, including the default-omitted case (verified
extractTextAlignrejects"left"/unset and the PT→PM side guardstextAlign !== "left"). - Confirmed the
TextAlign.configure({ types: ["heading", "paragraph"] })claim againstPortableTextEditor.tsx:2237, and that the align toolbar (PortableTextEditor.tsx:3040) is rendered unconditionally. - Checked the changeset package name (
emdash) againstpackages/core/package.jsonand the changeset wording against sibling changesets — correct. - Confirmed
pnpm typecheckexcludestests/and vitest is transpile-only, so the test'spm.content[0].attrs?access pattern (matching the existinghtml-block-round-trip.test.ts) won't trip typecheck. - Considered the validation asymmetry (PT→PM forwards any non-
"left"string; PM→PT sanitizes to center/right/justify) — it's benign: the type constrains the value and one round-trip through PM normalizes it, so I'm not flagging it.
Conclusion
Implementation is correct and well-tested for the stated scope (top-level paragraphs and headings). One real gap: the same silent drop remains for paragraphs nested inside blockquotes and list items — paths the editor's align toolbar also reaches — in both conversion directions. Flagged below. Not a blocker (no regression vs. current behavior, no security/data-integrity-on-existing-content issue), so comment rather than request_changes.
| _type: "block", | ||
| _key: generateKey(), | ||
| style: "normal", | ||
| textAlign: extractTextAlign(node), |
There was a problem hiding this comment.
[needs fixing] convertParagraph (here) and convertHeading (line 170) now forward textAlign, but the two other paths that build a text block from an inner paragraph node still drop it. convertBlockquote (line 266) and convertListItem (line 213) push their Portable Text blocks without calling extractTextAlign, so alignment set on a paragraph inside a blockquote or list item is silently lost on save.
This is reachable in the editor: the align toolbar is rendered unconditionally (packages/admin/src/components/PortableTextEditor.tsx:3040) and TextAlign.configure({ types: ["heading", "paragraph"] }) (PortableTextEditor.tsx:2237) applies the attr to paragraph nodes — including the paragraphs nested inside blockquotes and list items. A user can center-align a blockquote paragraph, save, and have it snap back to left on reload: the exact bug this PR fixes, one node path over.
The reverse direction has the matching gap, so even a hand-edited aligned block won't round-trip: convertTextBlock's blockquote case ignores alignAttr (portable-text-to-prosemirror.ts:176), and convertListItem (portable-text-to-prosemirror.ts:239) doesn't forward item.textAlign onto its inner paragraph.
Mirroring convertParagraph in convertBlockquote closes the save side:
blocks.push({
_type: "block",
_key: generateKey(),
style: "blockquote",
textAlign: extractTextAlign(child),
children,
markDefs: markDefs.length > 0 ? markDefs : undefined,
});The same textAlign: extractTextAlign(child) in convertListItem (line 213), plus applying alignAttr in the two reverse-direction spots, would make alignment round-trip consistently across all paragraph-bearing nodes.
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
added
review/awaiting-author
|
Thanks, @marcusbellamyshaw-cell! Closing as the text alignment fix landed in #1396, which has merged. Appreciate the contribution. |
2 participants
What does this PR do?
The rich-text editor ships
@tiptap/extension-text-align(configured for paragraph and heading), butprosemirrorToPortableTextbuilt each text block with only_type/_key/style/children/markDefsand never readnode.attrs.textAlign— so alignment set from the toolbar was silently dropped on save. The reverse converter had no handling either, so a hand-edited aligned block could not round-trip.This adds an optional
textAligntoPortableTextTextBlock, forwards a meaningful (non-default) alignment from paragraph/heading nodes into Portable Text, and restores it back into ProseMirror attrs on load. Default (left/unset) alignment is omitted so existing content is byte-for-byte unchanged.Closes #1201
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure
Screenshots / test output
text-align-round-trip.test.ts— 3 cases (paragraph round-trip, heading round-trip, default-left omitted). Full converter suite: 18 passing.