feat(gallery): Portable Text block editable #1967
Conversation
…ordering, and side-panel configuration in the editor
…cy in GalleryDetailPanel
…mponents and converters
…ails and replacing assets
… view and external settings panel
…lery blocks Preserves asset._type "reference" (the shape the Gutenberg importer emits and the import media pass rewrites) through both converter pairs, with a round-trip test against the exact imported shape. Clicking an image inside the gallery node now opens the sidebar with that image's settings selected, and the panel resyncs its local state when reopened for a different gallery node so edits can't leak between galleries. Adds the release changeset.
…drop to enable click events on thumbnails
🦋 Changeset detectedLatest commit: 2eaa229 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 |
Scope checkThis PR changes 1,296 lines across 13 files. Large PRs are harder to review and more likely to be closed without review. If this scope is intentional, no action needed. A maintainer will review it. If not, please consider splitting this into smaller PRs. See CONTRIBUTING.md for contribution guidelines. |
github-actions
Bot
added
area/admin
area/core
size/XL
review/needs-review
@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: |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
This is the right change for the problem it describes: today WordPress-imported gallery blocks round-trip through prosemirror-to-portable-text into nothing, so opening and saving a migrated post silently destroys its galleries. The PR closes that loop with a symmetric converter mapping plus a real editor experience.
The approach fits EmDash:
- Core side mirrors the existing image/CodeBlock patterns: a new
gallerycase in both converters driven by a sharedsanitizeGalleryImagessanitizer, withasset._type: "reference"preserved so the Gutenberg importer's real output survives. - Admin side follows the established image-node/block-sidebar pattern (
GalleryNode→GallerySidebarPanel→GalleryDetailPanel) and extendsMediaPickerModalwith additive multi-select. - User-facing strings are Lingui-wrapped, Tailwind uses logical properties (
end-*,start-*,border-s), and there is a sensible changeset.
I read the diff, the full changed files, and the relevant existing ImageNode/Gallery.astro/media-utils code. No security, SQL, authorization, or locale-filter issues are introduced by these frontend-only changes.
The only real caveats are polish/convention points:
- Admin UI tests are missing. The diff adds only the core converter round-trip tests. The new gallery node, detail panel, and multi-select picker behavior should have unit/component tests.
- Widget editor nests DndContexts. The gallery panel (with its own drag-and-drop thumbnail grid) is rendered inside the Widgets page-level DndContext, which can conflict.
- Empty gallery uses a hand-rolled
<button>. AGENTS.md asks for Kumo components rather than manually styled buttons. - Gallery images lose provider/LQIP metadata. For parity with
ImageNode, provider and LQIP fields should be carried through. - Panel resync is keyed on
attributesidentity. Stable in current callers, but fragile.
None of these block the stated goal, so this is a comment review.
| @@ -66,6 +66,10 @@ export interface MediaPickerModalProps { | |||
| open: boolean; | |||
There was a problem hiding this comment.
[suggestion] The diff only adds core converter tests for the gallery round-trip. The new multi-select MediaPickerModal behavior, GalleryNode node view, and GalleryDetailPanel interactions are not covered by new tests. Add tests for: confirming multiple items preserves click order, toggling selection, upload-in-multi-select, opening the gallery panel from a node view, thumbnail drag reorder, and a full PT→PM→PT round-trip through the admin's duplicated converters. This guards against the core/admin converters drifting.
| onClose={handleBlockSidebarClose} | ||
| /> | ||
| )} | ||
| {blockSidebarPanel?.type === "gallery" && ( |
There was a problem hiding this comment.
[suggestion] The gallery detail panel is rendered inside the page-level <DndContext> that handles widget drag-and-drop, but the panel itself contains another <DndContext> for thumbnail reordering. Nested DndContexts can compete for pointer events, so thumbnail reorder may be unreliable in the Widgets editor. Either pass inline and render the panel inside the widget card's own layout, or move the {blockSidebarPanel?.type === "gallery" && ...} block outside the page-level <DndContext> so the thumbnail grid has a single context.
| )} | ||
| > | ||
| {images.length === 0 ? ( | ||
| <button |
There was a problem hiding this comment.
[suggestion] The empty-gallery placeholder uses a raw <button> with hand-rolled styling (border-dashed, hover utilities, etc.) instead of a Kumo Button. AGENTS.md asks not to hand-roll buttons. Use Kumo’s <Button variant="outline" ...> (or buttonVariants() if you need a non-button wrapper) so focus, hover, disabled, dark mode, and RTL states stay consistent with the rest of the admin.
| } | ||
|
|
||
| /** Map a picked MediaItem to the gallery's Portable Text image shape. */ | ||
| export function mediaItemToGalleryImage(item: MediaItem): GalleryImage { |
There was a problem hiding this comment.
[suggestion] mediaItemToGalleryImage drops provider, blurhash, dominantColor, and meta from MediaItem. For external-provider images, losing provider means the public renderer cannot fall back to provider-aware resolution if the stored url expires or is missing. For parity with ImageNode, preserve at least provider (and ideally LQIP fields) on GalleryImage and thread them through the converters to Gallery.astro. That also requires extending PortableTextGalleryImage in packages/core/src/content/converters/types.ts and the shared sanitizer.
| // for a DIFFERENT gallery node. Resync or edits would write this panel's | ||
| // stale images into the other node. | ||
| React.useEffect(() => { | ||
| setGallery({ images: attributes.images ?? [], columns: attributes.columns }); |
There was a problem hiding this comment.
[suggestion] The panel resets its local gallery state whenever the attributes object identity changes. Current callers keep the panel object stable, so this works, but a parent that ever re-wraps attrs would clobber sequential edits. Make the resync conditional on a stable gallery/node identifier (e.g., the TipTap node position or the gallery _key) instead of on object identity.
github-actions
Bot
added
review/needs-rereview
…operties to gallery block and improve multi-select media picker behavior.
There was a problem hiding this comment.
This is the right change for the problem it describes, and the follow-up commits fully address the five concerns from the previous review.
What’s fixed since the last review
- Admin UI tests are now present. Three new test files cover the gallery detail panel behavior (selection, caption edits, removal, and the
nodeKeyresync edge case), the multi-select media picker, and the admin-side converter round-trip. - DndContext nesting in Widgets is resolved. The
GalleryDetailPanelis now rendered outside the page-levelDndContext, so its internal thumbnail drag-and-drop cannot conflict with the widget-area sensors. - Empty gallery uses Kumo. The empty-state placeholder is now a
<Button>from@cloudflare/kumo. - Provider and LQIP metadata are preserved through the whole edit round-trip.
mediaItemToGalleryImage, the coresanitizeGalleryImages,GalleryNode, andGallery.astroprops all carryprovider,blurhash, anddominantColor. - Panel resync is keyed by
nodeKey.GalleryDetailPanelonly resets local state when the gallery node identity changes, and there is a regression test proving in-progress edits survive a parent re-render with a fresh attrs object identity.
The converters and new node panel are well-structured and mirror the existing image-node pattern. I did not run the test suite or typecheck (none of that tooling is available here); I rely on the PR checklist for those results.
Remaining polish / convention notes
I have two non-blocking suggestions:
Gallery.astroacceptsblurhashandproviderin its props but only rendersdominantColor. For parity withImage.astro— which turns blurhash into a placeholder via@unpic/placeholderand resolves provider embeds — these fields should be consumed. As written, provider-hosted gallery images might not render the same optimized URLs as standalone images, and stored blurhash values are unused on the public site.GalleryDetailPanelstill has one hand-rolled<button>for the sortable thumbnail grid. AGENTS.md asks for Kumo components; the dnd-kitattributes/listenerscan be forwarded through Kumo’sButton.
There is also a process note: the PR checklist links to …/discussions/00 as the “approved Discussion,” which is clearly a placeholder. That should be replaced with a real discussion or at least the roadmap issue reference before merge if the maintainer enforces the feature-discussion requirement.
None of these block the gallery editing workflow, so this is a comment review and the PR is essentially ready.
| @@ -45,8 +51,11 @@ if (!images.length) { | |||
| id: image.asset._ref, | |||
| }); | |||
| const hasSize = image.width && image.height; | |||
| const lqipStyle = image.dominantColor | |||
There was a problem hiding this comment.
[suggestion] The gallery renderer now accepts blurhash and provider in the image shape (lines 28–31) but only uses dominantColor for the LQIP style. For parity with Image.astro — which renders blurhash placeholders via @unpic/placeholder and resolves provider embeds — Gallery.astro should also consume these fields. As written, stored blurhash values are dead on the public site and provider-hosted gallery images may not render with the same optimized URLs as standalone images.
| const hasSize = image.width && image.height; | |
| const lqipStyle = image.dominantColor | |
| ? `background-color: ${image.dominantColor}` | |
| : undefined; |
Consider adding the blurhash placeholder generation and provider-aware URL resolution that Image.astro already implements.
|
|
||
| return ( | ||
| <div ref={setNodeRef} style={style} className="relative group"> | ||
| <button |
There was a problem hiding this comment.
[suggestion] This sortable thumbnail is a hand-rolled <button> with manual Tailwind classes. AGENTS.md asks to use Kumo components instead of rolling custom buttons; the dnd-kit attributes and listeners can be spread onto Kumo’s Button.
| <button | |
| <Button | |
| type="button" | |
| variant="ghost" | |
| shape="square" | |
| className={cn( | |
| "aspect-square w-full rounded-md border overflow-hidden", | |
| selected && "ring-2 ring-kumo-brand", | |
| )} | |
| onClick={onSelect} | |
| {...attributes} | |
| {...listeners} | |
| > |
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
…ery component and update UI panel button styles
github-actions
Bot
added
review/needs-rereview
2 participants
What does this PR do?
Makes the
galleryPortable Text block, editable in the admin editor. Today the block is import/render-only:@emdash-cms/gutenberg-to-portable-textemits it andGallery.astrorenders it, but there is no TipTap node for it — and worse than invisible, the editor's PM→PT serializer dropped the block entirely, so opening and saving a migrated post silently destroyed its galleries. This PR closes the loop requested in #1436: insert, edit, reorder, and configure galleries from the admin, with WordPress-imported galleries surviving the full edit round-trip.Core:
portable-text-to-prosemirror.ts/prosemirror-to-portable-text.tsgain a symmetricgallerymapping with a shared image sanitizer. Malformed galleries (noimagesarray) keep the visible unknown-block placeholder instead of becoming an empty grid.PortableTextGalleryBlock/PortableTextGalleryImage, preservingasset._type: "reference"— the exact shape the Gutenberg importer emits and the WP-import media pass rewrites — so imported galleries round-trip without loss. Unit tests include a fixture matching the importer's real output (rewritten_ref/url, per-image captions,columns).Admin:
PortableTextEditor.tsx) gets the same two-waygallerymapping. Previously a loaded gallery fell into the plugin-block fallback and a gallery node serialized to nothing.GalleryNode(TipTap node view): WYSIWYG grid preview honoringcolumns, per-image captions, selection overlay with settings/delete, empty-state placeholder. Clicking an individual image opens the sidebar with that image pre-selected (transient selection — never persisted into node attrs).GalleryDetailPanel(sidebar): compact 3-per-row thumbnail grid with drag-and-drop reordering (dnd-kitPointerSensorwith a 6px activation distance so real clicks select instead of being swallowed by drag tracking), multi-select "Add Images", column count, and a per-image settings card (large preview, hover Replace, original dimensions, alt text, caption). Panel state composes sequential edits and resyncs when reopened for a different gallery node, so edits cannot leak across galleries.MediaPickerModalgains an additive multi-select mode (multiple+onSelectMany, selection in click order); single-select behavior is untouched./galleryslash command. Multiple galleries per document are supported. The gallery panel is wired into the content editor, section editor, and widgets editors.Closes #1436
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change) — converter suite (31 tests, 7 new gallery round-trip tests incl. a WordPress-importer-shaped fixture) and the full admin suite (1100 tests)pnpm formathas been runmessages.pochanges included.emdashand@emdash-cms/admin)AI-generated code disclosure
Screenshots / test output
Verified end-to-end in
demos/simple(Node + SQLite):galleryPT block (columns, ordered image refs) — confirmed via the content API and directly in SQLite.[Unknown block type]placeholder, no data loss on save)..emdash-gallerywith the right figure count,--columnsvalue, and captions.