Skip to content

fix(admin): make /video open the video picker first, like /image - #3936

Merged
khoinguyenpham04 merged 4 commits into
mainfrom
noah/video-picker-first
Oct 7, 2026
Merged

khoinguyenpham04 merged 4 commits into
mainfrom
noah/video-picker-first

Conversation

@khoinguyenpham04

@khoinguyenpham04 khoinguyenpham04 commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

/video, and Video in the add-block menu, now open the video picker straight away. The video block is added once a video is chosen, and closing the picker adds nothing. This is how /image and /gallery already work.

  • A video chosen from the add-block menu goes where that menu's line was, and one undo removes it.
  • The editor never creates an empty video block now, so the dashed empty state is removed, along with the code that let dropped or pasted files fill it.
  • A video block saved without a video, such as one written through the API, shows as unplayable in the editor with Replace video and Delete video. It still renders nothing on the site.
  • Dropping or pasting a video file anywhere in the text works as before.
  • While a picker is open, the place it will insert at follows changes to the document, such as an upload finishing, so the chosen block still lands where the add-block menu was.

The "Add a video" guide is updated to match, and a patch changeset for @emdash-cms/admin describes the change.

Type of change

  • Bug fix
  • Feature (requires maintainer-approved Discussion)
  • Refactor (no behavior change)
  • Translation
  • Documentation
  • Performance improvement
  • Tests
  • Chore (dependencies, CI, tooling)

Checklist

  • I have read CONTRIBUTING.md
  • pnpm typecheck passes
  • pnpm lint passes
  • pnpm test passes (or targeted tests for my change)
  • pnpm format has been run
  • I have added/updated tests for my changes (if applicable)
  • User-visible strings in the admin UI are wrapped for translation (if applicable). No new strings; the picker reuses "Select video" and "Insert video".
  • I have added and reviewed the user-facing changeset (if this PR changes a published package)
  • New features link to an approved Discussion: not applicable.
  • I have included screenshots below if this PR changes the UI

AI-generated code disclosure

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

Screenshots / test output

Typing /video and pressing Enter opens the video picker. No video block is in the document while it's open:

The Select video picker opened from /video, listing launch-week.mp4, with Cancel and Insert video buttons

Choosing a video adds the block where /video was typed:

The post editor with the chosen video added below the first paragraph, selected, with its caption field and the Replace and Delete buttons

  • The three rewritten tests fail on main (an empty block is added before a video is chosen) and pass here:
    • /video adds the chosen video.
    • Closing the picker adds nothing.
    • A video chosen from the add-block menu goes in that menu's line, and one undo removes it.
  • A new test chooses a video from the add-block menu after an upload finishes while the picker is open. It fails without the position change and passes with it.
  • video-block-editor.test.tsx and image-upload.test.tsx passed 45/45 tests in each of 20 runs, and in each of 6 runs with the CPU slowed 4×.
  • Full admin suite: 3231/3231 tests in 229 files.

/video and Video in the add-block menu now open the video picker straight
away, and the block is added once a video is chosen. Closing the picker
adds nothing. The editor no longer creates empty video blocks, so their
dashed empty state and the drop and paste handling that filled them are
removed. A block saved without a video shows as unplayable, with Replace
and Delete.

While a picker is open, the position it inserts at now follows document
changes, so an upload that finishes meanwhile can't leave it pointing
inside another block.
@changeset-bot

changeset-bot Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 4bbbf9a

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

This PR includes changesets to release 12 packages
Name Type
@emdash-cms/admin Patch
emdash Patch
@emdash-cms/cloudflare Patch
@emdash-cms/plugin-test Patch
@emdash-cms/sandbox-workerd Patch
@emdash-cms/auth Patch
@emdash-cms/blocks Patch
create-emdash Patch
@emdash-cms/gutenberg-to-portable-text Patch
@emdash-cms/x402 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

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
docs 4bbbf9a Oct 06 2026, 10:25 PM

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview URL: https://noah-video-picker-first.try.emdashcms.com, https://noah-video-picker-first-emdash-playground.emdash-cms.workers.dev (commit 4bbbf9a)

This URL reflects your latest Preview deployment

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://b1787830.try.emdashcms.com, https://b1787830-emdash-playground.emdash-cms.workers.dev 4bbbf9a 2026-10-06T22:25:49.104Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://86072169.try.emdashcms.com, https://86072169-emdash-playground.emdash-cms.workers.dev d94aced 2026-10-06T15:34:00.315Z Visit the dashboard ↗
  • Build: Success ✅
  • Deployment: Success ✅

View logs ↗
https://a4ccb3d0.try.emdashcms.com, https://a4ccb3d0-emdash-playground.emdash-cms.workers.dev 714cd3c 2026-10-06T14:40:58.751Z Visit the dashboard ↗

@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Oct 6, 2026

@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.

This change is the right fix: it makes /video behave like /image and /gallery by opening the picker first and only adding the block once a video is chosen. The approach fits the editor’s existing deferred-insertion pattern and the tests cover the important new paths (picker-from-slash, picker-from-gutter, undo-in-one-step, and position tracking while an upload finishes). I read the diff, the changed files, and traced the picker/deferred-position flow through PortableTextEditor.tsx, VideoNode.tsx, and ImageUploadExtension.ts.

I did not find any logic bugs, regressions in drop/paste handling, authorization, localization, or changeset problems. The only issues are two stale comments left over from the removed empty-block UI that now describe behavior that no longer exists.

Comment thread packages/admin/src/components/editor/VideoNode.tsx Outdated
Comment thread packages/admin/src/components/editor/VideoNode.tsx Outdated
@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Oct 6, 2026

@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.

This is the right change: it aligns /video and the gutter Video item with the existing /image and /gallery picker-first behavior, removes the now-orphan empty-block UI, and keeps API-created empty blocks visible as unplayable. The implementation matches the described behavior and the updated tests cover /video insertion, picker cancellation, gutter insertion/undo, and the position-follow behavior during in-flight uploads.

I reviewed the diff, the affected source files (PortableTextEditor.tsx, VideoNode.tsx, ImageUploadExtension.ts), the test changes, and the updated changeset and docs. All user-facing strings remain wrapped in Lingui, RTL-safe utilities are used, and no new logged-out queries or SQL are introduced. The only issue I found is a stale comment in VideoNode.tsx that still refers to the removed empty-block placeholder.

Comment thread packages/admin/src/components/editor/VideoNode.tsx Outdated
@pkg-pr-new

pkg-pr-new Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

@emdash-cms/admin

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

@emdash-cms/auth

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

@emdash-cms/auth-atproto

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

@emdash-cms/blocks

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

@emdash-cms/cloudflare

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

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

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

emdash

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

create-emdash

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

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

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

@emdash-cms/plugin-cli

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

@emdash-cms/plugin-test

npm i https://pkg.pr.new/@emdash-cms/plugin-test@3936

@emdash-cms/plugin-types

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

@emdash-cms/registry-client

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

@emdash-cms/registry-lexicons

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

@emdash-cms/registry-loader

npm i https://pkg.pr.new/@emdash-cms/registry-loader@3936

@emdash-cms/registry-moderation

npm i https://pkg.pr.new/@emdash-cms/registry-moderation@3936

@emdash-cms/registry-verification

npm i https://pkg.pr.new/@emdash-cms/registry-verification@3936

upgrade-emdash

npm i https://pkg.pr.new/upgrade-emdash@3936

@emdash-cms/sandbox-workerd

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

@emdash-cms/x402

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

@emdash-cms/plugin-ai-moderation

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

@emdash-cms/plugin-atproto

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

@emdash-cms/plugin-audit-log

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

@emdash-cms/plugin-color

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

@emdash-cms/plugin-embeds

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

@emdash-cms/plugin-field-kit

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

@emdash-cms/plugin-forms

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

@emdash-cms/plugin-webhook-notifier

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

commit: 4bbbf9a

@github-actions github-actions Bot added the review/awaiting-author Reviewed; waiting on the author to respond label Oct 6, 2026
The mock insert button now prevents mousedown like the real one, and the
tests focus the editor first, so the picker never records the button as
the element to return focus to.
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Scope check

This PR changes 669 lines across 7 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 github-actions Bot added review/needs-rereview New commits since the last review and removed review/awaiting-author Reviewed; waiting on the author to respond labels Oct 6, 2026
@khoinguyenpham04 khoinguyenpham04 added the bot:review Trigger an emdashbot code review on this PR label Oct 6, 2026

@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.

The author addressed all prior review findings. The JSDoc at the top of VideoNode.tsx now correctly reads "without asset when it has no video," and the figcaption comment now clarifies that the inline-size containment is for the caption textarea's placeholder text, not the removed dashed empty-block placeholder.

This is still the right change: picker-first /video and gutter Video insertion now behave the same as /image and /gallery. The implementation correctly avoids creating an empty video block, keeps API-created asset-less blocks visible as unplayable in the editor, and preserves the existing drop/paste upload behavior while removing the now-orphan empty-video UI.

I re-checked the new transaction-follow effect that maps pendingBlockInsertPosRef through in-flight uploads, the handleVideoSelect insertion path, the ImageUploadExtension simplification, the updated tests covering picker cancellation, gutter insertion/undo, and the upload-during-picker scenario, plus the updated docs and changeset. All user-facing strings are Lingui-wrapped, layout stays RTL-safe, and no new logged-out queries, database code, or AGENTS.md convention violations were introduced. The updated test suite covers the new behavior and the rewritten assertions match the described semantics.

No blockers remain.

@emdashbot emdashbot Bot removed the bot:review Trigger an emdashbot code review on this PR label Oct 6, 2026
@github-actions github-actions Bot added review/approved Approved; no new commits since and removed review/needs-rereview New commits since the last review labels Oct 6, 2026
# Conflicts:
#	.changeset/editor-video-block.md
@khoinguyenpham04
khoinguyenpham04 merged commit 0e8811d into main Oct 7, 2026
43 checks passed
@khoinguyenpham04
khoinguyenpham04 deleted the noah/video-picker-first branch October 7, 2026 12:47
@emdashbot emdashbot Bot mentioned this pull request Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants