Skip to content

fix: preserve image alignment through editor round-trip and render it (#1404)#1576

Open
marcusbellamyshaw-cell wants to merge 2 commits into
emdash-cms:mainfrom
Emdash-Bug-Testing:fix/1404-image-alignment
Open

fix: preserve image alignment through editor round-trip and render it (#1404)#1576
marcusbellamyshaw-cell wants to merge 2 commits into
emdash-cms:mainfrom
Emdash-Bug-Testing:fix/1404-image-alignment

Conversation

@marcusbellamyshaw-cell

Copy link
Copy Markdown
Contributor

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 carries alignment through both the core and admin converters and the TipTap image-node schema, and renders it in Image.astro with matching CSS. Round-trip regression tests added in both packages.

Closes #1404

Type of change

  • Bug fix

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (targeted: 3 core + 2 admin round-trip tests)
  • pnpm format has been run
  • I have added/updated tests for my changes
  • User-visible strings wrapped for translation — n/a, no new admin UI strings
  • I have added a changeset (emdash + @emdash-cms/admin, both patch)

AI-generated code disclosure

  • This PR includes AI-generated code — model/tool: Claude Opus 4.8 ultracode

…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-bot

changeset-bot Bot commented Jun 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 59dba7f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
emdash Patch
@emdash-cms/admin Patch
@emdash-cms/cloudflare Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/fixture-perf-site Patch
@emdash-cms/perf-demo-site Patch
@emdash-cms/cache-demo-site Patch
@emdash-cms/do-demo-site Patch
@emdash-cms/do-solo-demo-site Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 Patch
create-emdash Patch
@emdash-cms/auth-atproto Patch
@emdash-cms/plugin-embeds Patch

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

@emdashbot emdashbot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 mapAlignment emits exactly left/center/right/wide/full, which matches the core converter's VALID_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 via Image.astro, so the changed surfaces (admin converters + Image.astro + schema) are exactly the ones on the live path. ✔
  • Round-trip correctness: PT → PM sets alignment in attrs; PM → PT reads it back. Unset → null/undefined round-trips to omitted (attrStr(null)undefined; extractImageAlignmentundefined). Tests cover both. ✔
  • Sidebar replace path: ImageDetailPanel.handleMediaSelect calls onReplace with an ImageAttributes object that omits alignment; TipTap updateAttributes merges ({...node.attrs, ...attributes}), so replacing an image preserves alignment rather than wiping it. ✔
  • RTL / CSS: float: inline-start / inline-end is the logical, RTL-safe choice (consistent with AGENTS.md's RTL intent for raw CSS). Astro scoped styles apply to the figure. class:list correctly filters the falsy undefined case. Float/fit-content/width rules are reasonable and degrade gracefully. ✔
  • Tests: Both new test files mirror existing conventions exactly (image-dimensions.test.ts for core; PortableTextEditor.table.test.ts for admin, including the _-prefixed export aliases). Both packages exclude tests/ from typecheck, so the loose indexing in the tests is fine and runs under vitest. ✔
  • Changeset: Present for both emdash and @emdash-cms/admin at patch. ✔

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:6

    Per 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:316

    The core converter validates alignment against the allowed set (isImageAlignment / extractImageAlignment in prosemirror-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 (mapAlignment whitelists left/center/right/wide/full), so the practical risk is low — hence suggestion, not a blocker. But since the admin path is the persist path and the core deliberately added validation as defense-in-depth, mirroring isImageAlignment here (and on the PT → PM side) would close the gap and keep the duplicated converters consistent.

  • [suggestion] packages/admin/src/components/editor/ImageNode.tsx:149

    ImageNodeView always renders the image with mx-auto, so an image carrying left/right/center/wide/full alignment 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 or setImage command 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.

@github-actions

Copy link
Copy Markdown
Contributor

Overlapping PRs

This 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 github-actions Bot added review/awaiting-author Reviewed; waiting on the author to respond and removed review/needs-review No maintainer or bot review yet labels Jun 22, 2026
… 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>
@github-actions github-actions Bot added review/needs-rereview Author pushed changes since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Jun 22, 2026
@marcusbellamyshaw-cell

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Addressed the [needs fixing] item: rewrote .changeset/image-alignment-round-trip.md as user-facing release notes, dropping the internal mechanics (converters, image node schema, Image.astro) per AGENTS.md guidance (59dba7f).

On the two suggestions:

  • Admin converter validation symmetry — leaving as-is for now. The Gutenberg importer (mapAlignment) only ever emits the valid set, so there's no live path that persists an out-of-set value; mirroring isImageAlignment in the admin would be defense-in-depth on a value that can't currently occur, and I'd rather keep this PR scoped to the round-trip fix.
  • Editor preview not reflecting alignment — agreed this is a worthwhile follow-up (WYSIWYG + a control to view/change alignment), but it's a new editor feature beyond this import-preservation fix, so I'll track it separately rather than expand scope here.

@pkg-pr-new

pkg-pr-new Bot commented Jun 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

npm i https://pkg.pr.new/@emdash-cms/admin@1576

@emdash-cms/auth

npm i https://pkg.pr.new/@emdash-cms/auth@1576

@emdash-cms/auth-atproto

npm i https://pkg.pr.new/@emdash-cms/auth-atproto@1576

@emdash-cms/blocks

npm i https://pkg.pr.new/@emdash-cms/blocks@1576

@emdash-cms/cloudflare

npm i https://pkg.pr.new/@emdash-cms/cloudflare@1576

@emdash-cms/contentful-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/contentful-to-portable-text@1576

emdash

npm i https://pkg.pr.new/emdash@1576

create-emdash

npm i https://pkg.pr.new/create-emdash@1576

@emdash-cms/gutenberg-to-portable-text

npm i https://pkg.pr.new/@emdash-cms/gutenberg-to-portable-text@1576

@emdash-cms/plugin-cli

npm i https://pkg.pr.new/@emdash-cms/plugin-cli@1576

@emdash-cms/plugin-types

npm i https://pkg.pr.new/@emdash-cms/plugin-types@1576

@emdash-cms/registry-client

npm i https://pkg.pr.new/@emdash-cms/registry-client@1576

@emdash-cms/registry-lexicons

npm i https://pkg.pr.new/@emdash-cms/registry-lexicons@1576

@emdash-cms/sandbox-workerd

npm i https://pkg.pr.new/@emdash-cms/sandbox-workerd@1576

@emdash-cms/x402

npm i https://pkg.pr.new/@emdash-cms/x402@1576

@emdash-cms/plugin-ai-moderation

npm i https://pkg.pr.new/@emdash-cms/plugin-ai-moderation@1576

@emdash-cms/plugin-atproto

npm i https://pkg.pr.new/@emdash-cms/plugin-atproto@1576

@emdash-cms/plugin-audit-log

npm i https://pkg.pr.new/@emdash-cms/plugin-audit-log@1576

@emdash-cms/plugin-color

npm i https://pkg.pr.new/@emdash-cms/plugin-color@1576

@emdash-cms/plugin-embeds

npm i https://pkg.pr.new/@emdash-cms/plugin-embeds@1576

@emdash-cms/plugin-field-kit

npm i https://pkg.pr.new/@emdash-cms/plugin-field-kit@1576

@emdash-cms/plugin-forms

npm i https://pkg.pr.new/@emdash-cms/plugin-forms@1576

@emdash-cms/plugin-webhook-notifier

npm i https://pkg.pr.new/@emdash-cms/plugin-webhook-notifier@1576

commit: 59dba7f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Image alignment from WordPress import is dropped at render and not editable in the admin editor

1 participant