feat(admin): add media_picker BlockKit element#731
Merged
ascorbic merged 5 commits intoemdash-cms:mainfrom Apr 30, 2026
Merged
Conversation
🦋 Changeset detectedLatest commit: 6091cd1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 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 |
Contributor
Scope checkThis PR changes 641 lines across 20 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. |
Contributor
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/blocks
@emdash-cms/cloudflare
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@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-forms
@emdash-cms/plugin-webhook-notifier
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new media_picker Block Kit element across @emdash-cms/blocks (types/builders/validation) and @emdash-cms/admin (rendering in plugin block forms and BlockKit field widgets), plus locale catalog updates and a changeset.
Changes:
- Add
MediaPickerElementto the Block Kit element model, builder API, and shape validation. - Implement
media_pickerUI in the admin: thumbnail preview +MediaPickerModalintegration (PT editor + field widget). - Update admin locale catalogs and bump packages via changeset.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/blocks/src/validation.ts | Recognizes and validates media_picker element shape. |
| packages/blocks/src/types.ts | Adds MediaPickerElement and extends the Element union. |
| packages/blocks/src/index.ts | Re-exports MediaPickerElement. |
| packages/blocks/src/builders.ts | Adds elements.mediaPicker(...) builder. |
| packages/admin/src/components/PortableTextEditor.tsx | Renders media_picker fields inside plugin block forms. |
| packages/admin/src/components/BlockKitFieldWidget.tsx | Adds media_picker widget for non-PT BlockKit field contexts. |
| packages/admin/src/locales/en/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/ar/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/de/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/es-419/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/eu/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/fa/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/fr/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/ja/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/ko/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/pseudo/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/pt-BR/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/zh-CN/messages.po | Extracted new strings / updated references. |
| packages/admin/src/locales/zh-TW/messages.po | Extracted new strings / updated references. |
| .changeset/blockkit-media-picker.md | Publishes the feature via a minor version bump changeset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
18 tasks
drudge
added a commit
to drudge/emdash
that referenced
this pull request
Apr 22, 2026
Adds a `repeater` Block Kit element for plugin block forms: an array-of-objects input with scalar sub-fields, drag-to-reorder (dnd-kit), and collapsible item cards. Plugin blocks can now capture repeating data (FAQ rows, carousel slides, card grids) inline in the portable-text editor. `@emdash-cms/blocks` - `RepeaterElement` + `RepeaterSubField` types (text/number/select/ toggle/checkbox/combobox/date/radio as sub-fields — no nested repeaters, no buttons) - `elements.repeater(...)` builder - Schema validation for the repeater: sub-field type whitelist, non-negative integer `min_items` / `max_items` with `min_items <= max_items` - Exhaustive runtime `renderElement` arm (returns null — repeater is admin-authoring only) `@emdash-cms/admin` - `BlockKitRepeater` + `BlockKitRepeaterItem` rendered inside `PortableTextEditor` - Drag-to-reorder handle, collapsible item cards, Add / Remove controls - Keyboard-accessible item header (real `<button>` with `aria-expanded`, labeled drag handle) - Stable `_key` preservation per item (same pattern as the merged `RepeaterField` fix in emdash-cms#678) - `PluginBlockModal` body grows `max-h-[70vh] overflow-y-auto` so the modal scrolls when a repeater has many rows Split from the original bundled emdash-cms#679 per maintainer review. Image-link piece shipped in emdash-cms#704; `media_picker` follows in emdash-cms#731.
Contributor
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. |
drudge
added a commit
to drudge/emdash
that referenced
this pull request
Apr 23, 2026
Adds a `repeater` Block Kit element for plugin block forms: an array-of-objects input with scalar sub-fields, drag-to-reorder (dnd-kit), and collapsible item cards. Plugin blocks can now capture repeating data (FAQ rows, carousel slides, card grids) inline in the portable-text editor. `@emdash-cms/blocks` - `RepeaterElement` + `RepeaterSubField` types. Sub-fields are restricted to the four scalar inputs the admin widget renders inline: `text_input`, `number_input`, `select`, `toggle`. No nested repeaters, no buttons. - `elements.repeater(...)` builder - Schema validation for the repeater: non-empty `fields` array, sub-field type whitelist, non-negative integer `min_items` / `max_items` with `min_items <= max_items`, and `initial_value` entries must be objects - Exhaustive runtime `renderElement` arm (returns null — repeater is admin-authoring only) `@emdash-cms/admin` - `BlockKitRepeater` + `BlockKitRepeaterItem` rendered inside `PortableTextEditor` - Drag-to-reorder handle, collapsible item cards, Add / Remove controls - Keyboard-accessible item header (real `<button>` with `aria-expanded`, labeled drag handle) - `handleRemove` drops the removed item's key from the `expanded` Set so it doesn't retain stale entries - Stable `_key` preservation per item (same pattern as the merged `RepeaterField` fix in emdash-cms#678) - `PluginBlockModal` body grows `max-h-[70vh] overflow-y-auto` so the modal scrolls when a repeater has many rows Split from the original bundled emdash-cms#679 per maintainer review. Image-link piece shipped in emdash-cms#704; `media_picker` follows in emdash-cms#731.
drudge
added a commit
to drudge/emdash
that referenced
this pull request
Apr 24, 2026
Adds a `repeater` Block Kit element for plugin block forms: an array-of-objects input with scalar sub-fields, drag-to-reorder (dnd-kit), and collapsible item cards. Plugin blocks can now capture repeating data (FAQ rows, carousel slides, card grids) inline in the portable-text editor. `@emdash-cms/blocks` - `RepeaterElement` + `RepeaterSubField` types. Sub-fields are restricted to the four scalar inputs the admin widget renders inline: `text_input`, `number_input`, `select`, `toggle`. No nested repeaters, no buttons. - `elements.repeater(...)` builder - Schema validation for the repeater: non-empty `fields` array, sub-field type whitelist, non-negative integer `min_items` / `max_items` with `min_items <= max_items`, and `initial_value` entries must be objects - Unit tests in `tests/validation.test.ts` covering the new rules (valid repeater, empty fields, disallowed sub-field type, non-integer / inverted min/max, malformed `initial_value`) - Exhaustive runtime `renderElement` arm (returns null — repeater is admin-authoring only) `@emdash-cms/admin` - `BlockKitRepeater` + `BlockKitRepeaterItem` rendered inside `PortableTextEditor` - Drag-to-reorder handle, collapsible item cards, Add / Remove controls - Keyboard-accessible item header (real `<button>` with `aria-expanded`, labeled drag handle) - `handleRemove` drops the removed item's key from the `expanded` Set so it doesn't retain stale entries - Item-card summary trims the first text_input before falling back to "Item N" so whitespace-only values still show the index - Stable `_key` preservation per item (same pattern as the merged `RepeaterField` fix in emdash-cms#678) - `PluginBlockModal` body grows `max-h-[70vh] overflow-y-auto` so the modal scrolls when a repeater has many rows Split from the original bundled emdash-cms#679 per maintainer review. Image-link piece shipped in emdash-cms#704; `media_picker` follows in emdash-cms#731.
drudge
added a commit
to drudge/emdash
that referenced
this pull request
Apr 24, 2026
Adds a `repeater` Block Kit element for plugin block forms: an array-of-objects input with scalar sub-fields, drag-to-reorder (dnd-kit), and collapsible item cards. Plugin blocks can now capture repeating data (FAQ rows, carousel slides, card grids) inline in the portable-text editor. `@emdash-cms/blocks` - `RepeaterElement` + `RepeaterSubField` types. Sub-fields are restricted to the four scalar inputs the admin widget renders inline: `text_input`, `number_input`, `select`, `toggle`. No nested repeaters, no buttons. - `elements.repeater(...)` builder - Schema validation for the repeater: non-empty `fields` array, sub-field type whitelist, non-negative integer `min_items` / `max_items` with `min_items <= max_items`, and `initial_value` entries must be objects - Unit tests in `tests/validation.test.ts` covering the new rules (valid repeater, empty fields, disallowed sub-field type, non-integer / inverted min/max, malformed `initial_value`) - Exhaustive runtime `renderElement` arm (returns null — repeater is admin-authoring only) `@emdash-cms/admin` - `BlockKitRepeater` + `BlockKitRepeaterItem` rendered inside `PortableTextEditor` - Drag-to-reorder handle, collapsible item cards, Add / Remove controls - Keyboard-accessible item header (real `<button>` with `aria-expanded`, labeled drag handle) - `handleRemove` drops the removed item's key from the `expanded` Set so it doesn't retain stale entries - Item-card summary trims the first text_input before falling back to "Item N" so whitespace-only values still show the index - Stable `_key` preservation per item (same pattern as the merged `RepeaterField` fix in emdash-cms#678) - `PluginBlockModal` body grows `max-h-[70vh] overflow-y-auto` so the modal scrolls when a repeater has many rows Split from the original bundled emdash-cms#679 per maintainer review. Image-link piece shipped in emdash-cms#704; `media_picker` follows in emdash-cms#731.
drudge
force-pushed
the
feat/admin-blockkit-media-picker
branch
from
0043612 to
73d8fad
Compare
18 tasks
drudge
force-pushed
the
feat/admin-blockkit-media-picker
branch
from
73d8fad to
25da9c8
Compare
Adds a `media_picker` Block Kit element: a thumbnail preview with a modal library picker and mime-type filter. Usable in plugin block forms (inside `PortableTextEditor`) and as a Block Kit field widget (`BlockKitFieldWidget`). The stored value is the selected asset's URL string, so it is value-compatible with a plain `text_input` — existing content continues to work after swapping. Split from emdash-cms#679 per maintainer request. Image-link landed in emdash-cms#704; repeater landed in the rescoped emdash-cms#679.
Add a `case "media_picker"` arm to keep the Element union exhaustive. The picker is an admin-authoring construct (thumbnail + modal library picker) with no runtime render semantics, so returning null matches the repeater pattern.
drudge
force-pushed
the
feat/admin-blockkit-media-picker
branch
from
25da9c8 to
14410e6
Compare
Collaborator
|
@drudge the Copilot comments are mostly valid, so could you address them? Additionally, |
Contributor
Author
|
Will do @ascorbic! |
…back - Extract shared BlockKitMediaPickerField; both BlockKitFieldWidget and PortableTextEditor now render through it, killing two near-identical copies that risked diverging. - Fix URL-insert local-rewrite bug: MediaPickerModal returns URL-inserted items with id:"" and no provider/storageKey. Treating the absence of provider as "local" rewrote external URLs to a broken /_emdash/api/media/file/ path. Detect local explicitly via provider==="local" || !!storageKey and fall through to item.url. - Validate URLs before previewing: only render <img> for safe http(s) URLs or relative paths starting with "/" (not "//"); fall back to the empty-state placeholder otherwise. Add referrerPolicy="no-referrer" and loading="lazy" on the preview <img>. - Improve a11y on hover-revealed Change/Remove controls: also reveal on group-focus-within, and toggle pointer-events with the same group states so invisible controls don't absorb pointer events. - Restrict mime_type_filter to image MIME types (image/ or image/<sub>), rejecting wildcards like image/* (unsupported by the picker's startsWith filter) and non-image types like video/. - Add validation tests (3 valid + 6 invalid cases) and component tests (11 cases covering empty state, picker open, local pick, URL pick, preview attrs, unsafe-URL fallback, remove).
Contributor
Author
|
@ascorbic — addressed in 6091cd1. Summary of each item:
One Copilot comment was a false positive — it claimed Local checks: |
Collaborator
|
/review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Solid follow-up to the Copilot review — all the substantive items are addressed: shared component extraction, image-only mime_type_filter validation, the URL-insert local-rewrite bug, <img src> scheme guard with referrerPolicy="no-referrer", focus-reachable hover controls, and dedicated tests for both the validator and the field component.
A few small observations below — none blocking.
| // `provider`/`storageKey`, so we cannot infer "local" from absence of | ||
| // `provider` alone — that would rewrite the external URL to a broken | ||
| // `/_emdash/api/media/file/` path. Detect local explicitly. | ||
| const isLocalMedia = item.provider === "local" || !!item.storageKey; |
Contributor
There was a problem hiding this comment.
Minor: this branch implicitly assumes storageKey is only ever set on local items. The MediaItem JSDoc backs that up today ("Storage key for local media... Not present for external URLs."), but if a future provider ever populates storageKey it will silently get rewritten to /_emdash/api/media/file/<key> and break.
A slightly more defensive form keeps today's behavior but doesn't depend on absence:
Suggested change
| const isLocalMedia = item.provider === "local" || !!item.storageKey; | |
| const isLocalMedia = item.provider === "local" || (!item.provider && !!item.storageKey); | |
| const localKey = item.storageKey || item.id; | |
| const nextUrl = isLocalMedia && localKey ? `/_emdash/api/media/file/${localKey}` : item.url; |
Up to you — happy to leave as-is given the documented contract.
| className="max-h-40 w-full rounded-md border border-kumo-line object-contain bg-kumo-muted" | ||
| referrerPolicy="no-referrer" | ||
| loading="lazy" | ||
| /> |
Contributor
There was a problem hiding this comment.
If an existing value passes the scheme/path safety check but the resource isn't actually loadable as an image (e.g. a leftover text_input value that points at a PDF, a deleted asset, or a plain HTML page), the user gets a broken-image icon with no Change/Remove affordance unless they happen to hover/focus the broken <img>. Consider an onError that falls back to the empty-state placeholder so the controls are always reachable. Not critical — fully recoverable today by tabbing to the hidden Remove button — but worth a follow-up.
| <div className="relative group"> | ||
| <img | ||
| src={url} | ||
| alt="" |
Contributor
There was a problem hiding this comment.
Nit: the stored value is just a URL string, so there's no captioning/metadata available, but alt="" marks the image as decorative. For a media-picker preview the image is the content for an SR user. Consider alt={label} (or alt={t\Selected media``}) so screen reader users at least know there's a selected asset and what field it belongs to.
| * Accepts 'image/', 'image/png', 'image/svg+xml'. Rejects 'image/*' | ||
| * (wildcards) and non-image types like 'video/' or 'application/pdf'. | ||
| */ | ||
| const MEDIA_PICKER_MIME_FILTER_RE = /^image\/(?:[A-Za-z0-9][A-Za-z0-9!#$&^_.+-]{0,126})?$/; |
Contributor
There was a problem hiding this comment.
The regex is case-sensitive on image/, but MediaPickerModal does item.mimeType.startsWith(mimeTypeFilter), which is also case-sensitive. RFC 6838 type/subtype names are case-insensitive in practice, so a plugin author writing mime_type_filter: "Image/PNG" will fail validation here, and even if it didn't, would silently match nothing in the modal. Consider documenting (or normalizing) that the filter is expected lowercase, since the validator's the natural place to enforce it.
| (screen.getByText("open").element() as HTMLElement).closest("button")!.click(); | ||
| await expect.element(screen.getByTestId("media-picker-modal")).toBeInTheDocument(); | ||
| (screen.getByText("pick-local").element() as HTMLElement).click(); | ||
| expect(onChange).toHaveBeenCalledWith("hero", "/_emdash/api/media/file/photo.png"); |
Contributor
There was a problem hiding this comment.
Coverage gap: there's no test for an external-provider item (e.g. provider: "cloudflare-images", no storageKey) where the modal returns a fully-formed CDN url. The current pick-local and pick-url cases cover the two boundary conditions, but a third case — provider set to something non-"local" — would lock in the behavior that the rewrite branch only fires for local items, which is the whole point of the bug fix. A one-liner addition.
Contributor
|
Review posted successfully. Summary of feedback (all minor/nice-to-have, nothing blocking):
|
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a
media_pickerBlock Kit element: a thumbnail preview with a modal library picker and mime-type filter. Usable in plugin block forms (insidePortableTextEditor) and as a Block Kit field widget (BlockKitFieldWidget).The stored value is the selected asset's URL string, so it is value-compatible with a plain
text_input— existing content continues to work after swapping.This is the third and final piece of the original #679 split, per @ascorbic's review feedback. See discussion #337 for context.
Type of change
Discussion: #337
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runpnpm locale:extracthas been run (if applicable)AI-generated code disclosure
What's included
@emdash-cms/blocks:MediaPickerElementtype (action_id + label + optionalmime_type_filter/initial_value/placeholder)elements.mediaPicker(...)buildermedia_pickercase to runtimerenderElement(admin-authoring only — returnsnullat runtime)@emdash-cms/admin:BlockKitMediaPickerrendering insidePortableTextEditor(for plugin blocks that use fields)MediaPickerWidgetcase inBlockKitFieldWidget(for plugin field widgets outside the PT editor)MediaPickerModalintegration withmime_type_filter(defaults toimage/)/_emdash/api/media/file/...so previews work pre-CDN<Trans>/tand extracted viapnpm locale:extractemdash,@emdash-cms/admin,@emdash-cms/blocksRelated