fix: preserve image alignment through editor round-trip and render it (#1404)#1576
fix: preserve image alignment through editor round-trip and render it (#1404)#1576marcusbellamyshaw-cell wants to merge 2 commits into
Conversation
…emdash-cms#1404) Image blocks imported from WordPress/Gutenberg carry a horizontal alignment (left/right/center/wide/full), but the Portable Text <-> ProseMirror round-trip dropped it, so re-saving an imported post flattened every aligned image to the default. - core + admin converters now carry `alignment` both directions - ImageNode TipTap schema declares the `alignment` attribute - Image.astro renders `emdash-image--align-*` with matching CSS - round-trip regression tests in core and admin Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 59dba7f 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 |
There was a problem hiding this comment.
Approach
This is the right fix for the right problem. Issue #1404 is a real round-trip data-loss bug: the Gutenberg importer writes alignment onto image blocks, but the admin's Portable Text ↔ ProseMirror converters and the TipTap image-node schema didn't carry the attribute, so opening an imported post in the editor and re-saving flattened every aligned image. The PR threads alignment through both converter directions in both packages, adds it to the TipTap node schema, and renders it in Image.astro with per-alignment CSS. That matches EmDash's architecture (Portable Text is the stored source of truth; Image.astro renders PT directly on publish; the admin keeps its own duplicated converters). It's a bug fix tied to an existing issue, so no prior Discussion is needed.
What I checked
- Data flow end-to-end: Gutenberg
mapAlignmentemits exactlyleft/center/right/wide/full, which matches the core converter'sVALID_IMAGE_ALIGNMENTS— no valid imported value is wrongly dropped. The core converters are only a public-API export; the admin uses its own copies, and publish renders PT directly viaImage.astro, so the changed surfaces (admin converters +Image.astro+ schema) are exactly the ones on the live path. ✔ - Round-trip correctness: PT → PM sets
alignmentin attrs; PM → PT reads it back. Unset →null/undefinedround-trips to omitted (attrStr(null)→undefined;extractImageAlignment→undefined). Tests cover both. ✔ - Sidebar replace path:
ImageDetailPanel.handleMediaSelectcallsonReplacewith anImageAttributesobject that omitsalignment; TipTapupdateAttributesmerges ({...node.attrs, ...attributes}), so replacing an image preservesalignmentrather than wiping it. ✔ - RTL / CSS:
float: inline-start/inline-endis the logical, RTL-safe choice (consistent with AGENTS.md's RTL intent for raw CSS). Astro scoped styles apply to thefigure.class:listcorrectly filters the falsyundefinedcase. Float/fit-content/widthrules are reasonable and degrade gracefully. ✔ - Tests: Both new test files mirror existing conventions exactly (
image-dimensions.test.tsfor core;PortableTextEditor.table.test.tsfor admin, including the_-prefixed export aliases). Both packages excludetests/from typecheck, so the loose indexing in the tests is fine and runs under vitest. ✔ - Changeset: Present for both
emdashand@emdash-cms/adminatpatch. ✔
Conclusion
The fix is correct and complete for the stated goal — no data loss, no security issue, no broken contract. Three findings below, none blocking: the changeset reads like a PR description (AGENTS.md asks for user-facing release notes without internal mechanics), the admin converter lacks the validation the core added (low-impact consistency gap), and the editor preview doesn't reflect alignment (a new editor/publish divergence worth tracking).
Findings
-
[needs fixing]
.changeset/image-alignment-round-trip.md:6Per AGENTS.md, a changeset is release notes a user reads while upgrading — it should describe the observable effect and "leave out internal mechanics (file names, refactors, how you implemented it)." This body reads like a PR description: it names
Image.astro, the "core and admin converters", the "image node schema", and the "Portable Text ↔ ProseMirror round-trip." None of that helps an upgrader decide what changed for them. The fix and effect are correct; only the framing needs to be user-facing.Preserves the horizontal alignment (`left`, `right`, `center`, `wide`, `full`) on images imported from WordPress/Gutenberg. Opening an imported post in the editor and re-saving no longer flattens aligned images back to the default, and aligned images now display as authored on the published page. -
[suggestion]
packages/admin/src/components/PortableTextEditor.tsx:316The core converter validates alignment against the allowed set (
isImageAlignment/extractImageAlignmentinprosemirror-to-portable-text.ts), but the admin's PM → PT path — which is the one that actually persists to storage on save — just casts whatever string sits on the attr:attrStr(attrs.alignment) as PortableTextImageBlock["alignment"]. The PT → PM side a few lines up (alignment: imageBlock.alignment) likewise passes the value through unchecked. the two admin sides stay symmetric with each other but asymmetric with the core. Net effect: an out-of-set alignment value would round-trip and persist through the admin while being silently dropped by the core converter.The Gutenberg importer only ever emits valid values (
mapAlignmentwhitelistsleft/center/right/wide/full), so the practical risk is low — hencesuggestion, not a blocker. But since the admin path is the persist path and the core deliberately added validation as defense-in-depth, mirroringisImageAlignmenthere (and on the PT → PM side) would close the gap and keep the duplicated converters consistent. -
[suggestion]
packages/admin/src/components/editor/ImageNode.tsx:149ImageNodeViewalways renders the image withmx-auto, so an image carryingleft/right/center/wide/fullalignment appears centered in the editor while rendering aligned on the published page. Before this PR both views were centered (alignment was dropped); now the editor and publish views diverge for aligned images, and there is no control in the detail panel orsetImagecommand to view or change the value. That's a reasonable scope boundary for an import-preservation fix (alignment is read-only here), but worth tracking as a follow-up so authors aren't surprised that an image they see centered publishes floated, and so the editor WYSIWYGs the new alignment CSS.
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. |
… notes Drop internal mechanics (converters, image node schema, Image.astro) per AGENTS.md changeset guidance, addressing review feedback on emdash-cms#1576. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the review. Addressed the On the two suggestions:
|
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
What does this PR do?
Image blocks imported from WordPress/Gutenberg carry a horizontal alignment (
left/right/center/wide/full), but the Portable Text ↔ ProseMirror round-trip dropped it — so opening an imported post in the editor and re-saving flattened every aligned image to the default. This carriesalignmentthrough both the core and admin converters and the TipTap image-node schema, and renders it inImage.astrowith matching CSS. Round-trip regression tests added in both packages.Closes #1404
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (targeted: 3 core + 2 admin round-trip tests)pnpm formathas been runemdash+@emdash-cms/admin, both patch)AI-generated code disclosure