Repository navigation
VideoPress: rebuild the first-run and upload experience - #51481
Open
heydemoura wants to merge 41 commits into
Open
heydemoura wants to merge 41 commits into
heydemoura wants to merge 41 commits into
Conversation
heydemoura
added
Enhancement
github-actions
Bot
added
[Package] VideoPress
[Plugin] Jetpack
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Videopress plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 18 files. Only the first 5 are listed here.
17 files are newly checked for coverage. Only the first 5 are listed here.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
Extracts the first-run work from a mixed local branch onto current trunk. - New /upload route: dropzone, real wp/v2/media upload with progress, details step, and a success step with a copyable share link. - Tab order responds to library state so an empty library leads with Upload. - Welcome modal on first visit, dismissal persisted per site and user. - Admin_Menu::enqueue_design_tokens() so --wpds-* tokens resolve on the VideoPress admin page. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Splits the dashboard into two shapes and picks between them from one rule: first run only when nothing is published and the library is empty. - New Home screen (routes/home): a recent-videos rail with upload and add-to-a-post actions, and no KPI tiles. No card may render a fabricated "0 views" — when stats are unknown the slot shows nothing. - Upload stops being a tab once you have videos and becomes a header button, so the action stays one click away instead of disappearing. - Welcome modal rebuilt on @wordpress/ui's Dialog, led by a video of the player rather than a two-tab description of it. Fixes a redirect that ejected users mid-flow: every successful upload invalidates the library query before the details step, which on a connected paid site flipped the count and threw the user to the Library before details and success could render. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
…un state Checkpoint of the in-progress first-run work before the modal is rebuilt to the new Figma design (VideoPress Revival, node 56-12912): Dialog-based modal with intro video band, useSettledFirstRunState() so returning users aren't misrouted while the library count loads, and upload-stage tweaks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The first-run modal now leads with the product itself: the intro film playing in a full-bleed 16:9 band over the brand wireframe artwork, then the pitch on white — "Your Video. Your Player.", three hairline-separated value cards, and a Learn more / Upload a Video footer. - Band background is the committed wireframe artwork on deep green, so the modal never flashes a foreign colour while the film loads and still reads as designed if the video is missing. - IntroVideo prefers a VideoPress GUID (the player selling itself) and falls back to the bundled film; getAssetUrl no longer throws the whole dashboard down on a malformed buildUrl. - Value cards use @wordpress/icons glyphs with the one brand-green accent; columns collapse to a stack below 540px. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Three independent design audits (spacing, typography, DS craft) over the rendered modal, all findings fixed: - Headline and lede now actually render at the spec's 32/40 and 15/24 — Dialog.Title/Description pin their own internal variants and route font-size through the design system's global-css-defense custom properties, so the tokens are applied through both channels. - The cards list dropped the user-agent's 40px list indent that pushed the columns out of alignment with the headline; hairlines are now centered pseudo-elements so all three columns share one content width. - Column collapse is a container query on the modal's own width, not a viewport media query — the popup shrinks long before any phone breakpoint. - 24px rhythm: lede to columns and columns to footer, with the cards-to-footer distance split across the scroll seam so content clears the separator when the body scrolls. - Close button: rendered before the player so it is one Tab away, white focus ring for the dark band, 44px hit target, 16px inset. - Autoplay parity between the embed and the bundled fallback, both gated on prefers-reduced-motion. - Card titles use heading-lg instead of a hand-rolled weight override; middle card copy tightened so all columns hold two lines; "Upload a video" in sentence case; role="list" restores Safari list semantics. - Bundled fallback film updated to the v8 master: 4K-derived, scored, with embedded cover art. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Card 2 promises "move library videos over in one click" — now the footer delivers on it. On sites whose media library holds videos VideoPress could host, the secondary button becomes "Move N videos over" with the real count, deep-linking to the Library pre-filtered to local videos where the existing bulk "Upload to VideoPress" action does the moving. Empty sites keep the "Learn more" link. - New use-onboarding-counts hook: two 1-row count queries (VideoPress / local) following the first-run count idiom; nothing acts until both settle, so the modal never opens with a wrong label. - The modal's gate widens to "no VideoPress videos yet" — the migration pitch is exactly for sites full of local videos. The landing redirect and tab order keep the stricter zero-of-any-videos rule. - The primary CTA now navigates to /upload: with the wider gate the modal can open over Library or Home, where nothing upload-shaped is underneath. - The Library seeds its type filter from the route's ?type=local search param (read via the router — wp-admin encodes app search inside p=). Seed-once; filters stay unpersisted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The one-shot guard was a module variable, but each route ships its own bundle with its own copy of the module — so "once" was really "once per route bundle". Landing on Home burned Home's copy; navigating to the Library loaded a fresh copy that fired the "landing" redirect again and bounced straight back, making the Library unreachable from Home (and swallowing the welcome modal's new deep-link). A window-scoped flag is shared across bundle copies. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
- The File Name fields never saved anywhere (the publish PATCH carries only title/description) — an input that edits nothing is a lie. The filename now renders as static text in the row header. - StepFlow froze its wrapper height at step entry, so content growing within a step (error banners, per-row messages) overflowed the frame. A ResizeObserver now tracks the active card for its whole life. - Copy buttons on the success step used navigator.clipboard directly, which is undefined on plain-HTTP dev sites; they now fall back to the hidden-textarea path the clipboard libraries use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Succeeded uploads now stay in the shared queue carrying their media
result ({ id, guid, src }) until a consumer acknowledges them, instead
of self-deleting after two seconds — which threw away the only handle
anything downstream had on the finished video. This is the substrate
for the upload pill and the upload→edit transition, both of which need
to link a completed upload to /video/:id.
- UploadItem gains `media`, populated from the tus success payload.
- New `acknowledgeUpload( id )` removes settled items only; in-flight
items are untouched (cancellation is a different operation).
- The Library's in-flight row splice already excludes success rows, so
retention adds no duplicate rows there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The /upload flow ran its own plain wp/v2/media XHR — a documented local- dev stand-in that produced attachments the edit screen cannot open and uploads invisible to the rest of the dashboard. On connected sites it now feeds the shared tus queue: the real VideoPress uploader, resumable, observed by the Library's in-flight rows and useFreeTier alike. - Queue items for the current batch render through the existing step UI via a mapped view; when the batch settles, results are snapshotted into flow-local state and acknowledged out of the queue. - Back from details/success acknowledges settled rows; in-flight rows keep uploading (Back is not Cancel). - The XHR path survives solely as the disconnected-site fallback, keyed off the new isWpcomConnected() helper. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Drop one video and the flow cross-fades immediately to the same editing surface the /video route uses — in a draft session: title and description are editable from the first second (client state needs no attachment), the player slot is the upload's stage (progress → processing → a one-time "Your video is live" moment with the share link and Add to a post), and Save enables the moment the real record binds. Typed edits survive the draft→real swap. Failure keeps the form intact with Retry in the player slot. Multi-file drops keep the batch path. - Editor extracted from routes/video into video-details/editor.tsx (+ co-located styles) so both routes render one surface; the video route's behavior is unchanged. - useVideo now polls GUID-less records too — previously a video could gain its GUID and the screen would never notice. - useVideoDetailsForm gains preserve-dirty-on-rebind for the id swap. - Cards that need a GUID render skeletons during the session instead of popping in; the ⋯ menu waits for a real attachment. - markFirstPublish fires when the live moment first shows — this flow's publish moment; there is no separate publish button. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Live testing caught a self-inflicted remount: the FIRST upload on a first-run site flips the first-run state mid-flight (the library count goes 1), the tab order changes, and the flow component remounts — wiping the component-state pointer to the in-flight batch while the window-scoped queue kept uploading without a listener. The edit surface then sat on "Uploading… 0%" forever. Queue items now carry a context tag naming the flow that started them, and the flow adopts its own unacknowledged items from the queue on mount — the queue is the durable truth; component state is just a view of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Second remount casualty from live testing: the attachment id was copied into EditStep state at bind time and the queue row acknowledged away — so when the first-run flip remounted the subtree, the new instance had neither the state nor the row, and the player stage sat on "Uploading… 0%" over an already-finished video. The id is now derived from the queue row on every render, and the row lives for the whole edit session (acknowledged on the flow's exit paths, not at bind). Component state is a view; the queue is the truth. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Design feedback from live review: - The "Get your first video online" tile card duplicated the dropzone beneath it (and its only working tile just opened the same picker); the import tile was a disabled promise. One card, one action. - The welcome modal's cards-to-buttons distance widens from 24px to 32px, split across the scroll seam as before. - The modal sits at 42% rather than dead center — dialog convention weights the empty space toward the bottom, and the band-heavy layout reads better lifted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The dropzone is the page's one action; at full container width it read as a void. 718px per design review, centered. Only the dropzone card narrows — the edit surface that follows it in the same flow keeps the full width. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Design review needs to re-see the modal at will; hand-clearing localStorage is nobody's workflow. The `welcome=1` search param opens the modal past the dismissal flag and the library gates, and forgets the stored dismissal so plain loads behave fresh again afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The stage panel inherits the player frame's width: 100% and adds its own padding — as content-box, the padding landed outside the 380px preview track, hanging the panel 40px past the layout (off-screen on smaller windows). border-box keeps the padding inside the frame. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
On a fresh page load the router only parses search it finds inside the encoded `p` path param, so `p=%2Fhome%3Fwelcome%3D1` never reached useSearch. The raw query string check makes `&welcome=1` on admin.php work — which is also the easier URL to hand around for review. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Dropping 2+ videos now lands on the Library — whose in-flight rows already show per-file progress — with one floating pill (bottom-right, every dashboard screen) carrying the batch: collapsed it reads "Uploading 3 videos — 47%" with a combined bar; expanded it offers per-file cancel/retry and, once settled, "Add details" links that walk the batch through the edit screen. Failed rows never auto-dismiss; dismissing the pill is not cancelling (cancel is per-row, confirmed). - cancelUpload joins the shared queue: pending rows are removed, the active upload aborts through the owning instance's tus handle; a non-owner cancel marks the row and the owner cleans up on its next callback — no zombies, no cross-instance handle passing. - The pill stands down while the /upload edit session is the progress surface (a context-tag suppression rule, not route sniffing). - Free plan: files discarded by the one-video slice now say so at drop time, with the real count — previously they vanished wordlessly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Design review: nobody adds a video to a post from the Home overview — that action lives with the video itself, so the header keeps only Upload video. And a user whose library has been emptied now gets the actual drag-and-drop experience inline on Home instead of a button that bounces to another page: dropping a file starts the shared-queue upload and lands directly in the edit session (the /upload stage now resumes into the edit step when it adopts a single in-flight item). The dropzone itself is extracted to a shared component — one drop surface, one free-plan slice + message, used by /upload and Home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The empty-state Home stacked four upload invitations: the header
button, an EmptyState text block, the dropzone's heading, and the
dropzone's button — under a "Recent videos" label with no recents.
Now the dropzone IS the empty state: the text block and section label
go, its useful promise ("ready to share or drop into a post") moves
into the dropzone sub-copy, and the header's Upload button stands down
until videos exist. Status stays announced via a visually-hidden
heading. The /upload page keeps its own first-run copy.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
…fecycle
Four independent reviews of the upload work found two designer-reported
symptoms sharing three root causes. This fixes all of them.
The handoff (the design truth): a single upload no longer lives out its
life in an editor embedded under the dashboard tab strip. The upload
screen is now a bridge — it shows progress and takes the title you type
— and the moment the video registers with WordPress.com it hands you to
/video/:id, the same dedicated screen the Library opens, carrying your
typed edits and the "your video is live" moment with it. The ~150 lines
where the bridge duplicated that route's save/delete/captions wiring are
gone.
The queue's lifecycle: rows were durable but nothing owned their end, so
a deleted video left a row behind that haunted the pill ("Add details"
to a 404) and let /upload resurrect a dead session as an eternal
"processing" screen. Deletes now reconcile the queue, adoption ignores
settled rows, and every upload session ends by acknowledging its row.
Uploads also gained real per-session identity: cancelling during the
token round trip no longer leaks a headless upload or writes one video's
result onto another's row, and two mounts can no longer dispatch the
same file twice.
The navigation gates: /upload no longer ejects users who own videos —
which silently broke Home's own Upload button and the welcome modal's
CTA. The landing redirect only fires for a genuine landing (a bare URL,
no app path), the tab order freezes for the life of a screen instead of
reshuffling mid-flow, and welcome=1 is consumed once instead of
reopening the modal on every navigation.
Also: deletes confirm before destroying (they are permanent, no trash);
deleting your last video lands on Home, which has the dropzone, rather
than an empty Library; the thumbnail and subtitle cards hold skeletons
through the whole upload-and-processing wait instead of appearing late;
Home's dropzone gets the same card and measure as the upload screen; the
celebration announces itself and takes focus.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The post-upload celebration was junk of my own making. It stacked a raw URL, Copy link, Add to a post or page, and Watch video into the player's slot — and every one of those already existed on the same screen: its Copy link called the same helper as the Link to video field beneath it, its Add to a post or page was the *same component* as the header's (both mounted at once, identically labelled, so screen readers announced two identical triggers), and Watch video existed only to dismiss the celebration. Worse, it didn't overlay the player, it replaced it: at the one moment the product should prove "your video, your player", there was no player. Now the upload's processing stage hands the slot straight back to the player, and a snackbar marks the moment for the flow that earns one — a single upload, not each file of a batch chained through "Add details". Underneath, the share plumbing that produced the duplication: - One link resolver. `linkForVideo` fell back to the attachment id, producing videopress.com/v/<id> — a dead link behind a Copy button that copied it anyway. Home's resolver (null when there's no honest link) wins and moves to utils/video-links.ts; the info card hides the field rather than offering a lie. - The queue's `celebrated` flag and its setter go with the celebration; a row is now consumed by standing on its video's page. Left deliberately: Home's card actions (a list's quick actions, not duplicates on one video), the Privacy & sharing Share toggle, and the player's own share menu — the preview should show what viewers see. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The team already had this one on its list — "transcoded video interrupts playback, forces the user to hit play again" — and our code was causing it: the player slot SWAPPED the embed for the upload stage, so the instant `isProcessing` flipped, the iframe was torn down and rebuilt and whatever was playing stopped dead. Our 2s polling made that flip land promptly, so we made it easier to hit, not harder. The stage now overlays a player that stays mounted for the whole session. The regression test asserts element identity across the transition — anything weaker passes on the old code. Also documents the bundled intro film as a placeholder: its footage and music are stand-ins, not cleared for release, and a fully licensed video replaces it before release. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
The upload card sat at 718px while the free-plan notice above it ran the full 1344px container, so the empty screen read as a wide bar over a narrow card, and neither matched the Settings tab beside it. Both now take the canonical 660px Jetpack settings measure, centered — the width Settings already uses (routes/settings/style.scss). Only these narrow; the edit surface in the same flow still needs the full width. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
Stats, Home and the Upload page all sit in a 1344px column with a 24px gutter; the Library had no container at all, so its listing ran edge to edge and didn't line up with the tabs either side of it. It now takes the same measure, as a flex column with `flex: 1` and `min-block-size: 0` so the DataViews viewport still grows into the tab panel and keeps its pagination pinned to the bottom. The at-limit notice loses its own gutter margin, since the container's padding supplies it now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
This reverts commit d1b17bd.
Three fixes from a walkthrough:
The upgrade notice said two different things depending on which tab you
were on — the free-plan sentence on Home and Stats, a shorter at-limit
line on Library, Settings and Upload. Every surface now shows the same
sentence. The short form survives only where a full sentence doesn't
fit: the disabled-upload tooltips and the rejected-drop toast. Home was
hard-coding its own copy of that tooltip string; it now shares the one
constant.
Opening VideoPress from the WordPress menu lands on a bare URL, which
resolves to the Library because Library owns `/`. The landing redirect
then sends you to Home or Upload, so the Library painted first and was
yanked away — it read as the page loading twice. The body now waits for
that decision, but only on the bare landing and only while the count is
unknown, so a failed count still renders rather than hanging.
Home's free-plan notice only takes the narrow measure when the dropzone
is beneath it. Next to the recents rail it stays full width; centred at
660 over left-aligned cards it just looked misaligned.
(The zoom on that same load is WordPress's own admin view transitions —
`@view-transition { navigation: auto }` from core's
wp-view-transitions-admin-inline-css — not this dashboard.)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CcAb177MwHKkw7D5RPrSvq
An audit of the dashboard turned up 46 things; these are the ones worth fixing on this branch. The rest are pre-existing code outside this PR's diff, or need a decision rather than a patch. The one real bug: `&welcome=1` — the URL in this PR's own testing instructions — did not open the modal on a site that already had videos. A bare admin URL carries no app path, so it resolves to Library and the landing redirect fires; each route is its own bundle, so that navigation remounts the modal, by which point the one-shot param had already been consumed and `isOpen` fell through to the empty-library check. It only appeared to work on an empty site because the modal would have opened there anyway. The window latch is now two values: consumption still drives the one-shot `clearDismissal()`, while a separate flag keeps the modal open for the rest of the page load. A dismissal still sticks — `isDismissed` re-seeds from storage on every mount. Screen readers: * The upload panel wrapped the percentage in `aria-live`, so it spoke a new number on every progress tick. Now only the phase is announced, matching what the upload pill already does; the percentage stays visible. * The video-details cards rendered their titles as plain divs, leaving the screen with one heading. They are h2s now, as the design system's own docblock prescribes. * Both copy buttons in the video-info card were named "Copy"; they now name their field. * The rating radio group and the frame scrubber had no accessible name. * The free-plan notice is permanent, but was being spoken aloud on all five screens on every navigation. Visible fixes: * The same video read `0:34` on Home and `00:34` in the Library. Home had its own duration formatter; deleted in favour of the shared one. * At the free-plan limit, the upload button was styled as unavailable on Library but looked completely live on Home. * Picking a video frame made the *other* thumbnail tile say "Updating…". The busy state now sits on the thing that is actually changing. * Four straight apostrophes among curly ones, all in strings new here. Also corrects a docblock in video-details-card that confidently claimed the two label styles already agree. They do not — measured 600 vs 499 weight and a different label-to-field gap. The comment now says so, and why it is being tolerated for now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Two testers walked the product as customers — one brand new, one returning with a library. These are the things that stopped them. **"Add to a post or page" created an empty draft.** The button opens post-new.php with the video's GUID, and a `default_content` filter is supposed to turn that into a video block. The filter was registered *after* an early return that fires whenever the standalone VideoPress plugin is absent — so on any site running VideoPress through the Jetpack plugin it was never hooked at all, and the editor opened on an untouched empty string. The guard is right for the shortcode registrations beside it, which would otherwise collide with the ones the Jetpack module already makes; this filter has nothing to collide with, so it now registers unconditionally. **Dropping a file painted two screens on top of each other.** The outgoing card is absolutely positioned for the cross-fade, and `inset: 0` pinned its bottom edge too, stretching it to the height of the editor arriving underneath. Its vertically-centred contents drifted into the middle of that editor, so "Drag and drop your video here" printed through the Description field. It now pins only the top and sides and keeps its own height. **At the plan limit the drop zone silently ate files.** Three separate floors — the component dropped its handlers while disabled, and both callers returned early — under a surface still reading "Drag and drop your video here". Refusal now lives in the component, so every drop, click and pick raises the same notice, with the upgrade action the library already uses. The button matches the surface instead of contradicting it, and it explains itself on click rather than only on hover. **A .txt renamed .mp4 uploaded all the way through**, took the free plan's only video slot, and left the thumbnail and subtitle panels loading forever. The Library route already had the right check; it now lives beside the shared drop zone so every entry point gets it. The panels also stop pretending: a video that never resolves now says so instead of animating indefinitely. Polling is unchanged — this is a display bound, not a claim about the backend. **Arriving from the WordPress menu flashed the wrong navigation.** An earlier fix held the body back but not the tab strip, so the first-run tabs still painted for ~400ms and then renamed themselves. Both are now held over the same window. A brand-new user still gets the optimistic first-run strip immediately on every other arrival. **The modal's background artwork never loaded.** The stylesheet asked for `url(images/…)`, but the CSS is injected by JS, so it resolved against /wp-admin/ and 404'd on every load. It now goes through the same build-URL helper as the film, keeping the flat green underneath for when that can't be resolved. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Follow-up to the walkthrough fixes. Testers re-ran the journeys and found three of them incomplete, plus one thing the last round broke. **The file check didn't catch the case it was written for.** It trusted `File.type` — but browsers derive that from the extension, so a text file renamed `.mp4` reports `video/mp4` and passed. It now reads the first 64 bytes and checks the container signature: ISO-BMFF for the mp4/mov family, EBML for WebM and Matroska, and Ogg. It fails open on purpose — an extension with no known signature rides on its extension, and an unreadable file is accepted, because refusing somebody's real video is far worse than accepting a renamed text file. `.avi`, `.wmv` and the MPEG family still have no signature and so still pass on their extension alone; those formats want a follow-up rather than a half-remembered magic number that rejects real footage. An invalid file also now says it is invalid even when you are at the plan limit, instead of blaming the limit for it. **The tab strip stopped guessing.** It was deriving first-run state from a count that reads 0 until it loads, so every in-app arrival painted the first-run tabs and then corrected itself — a rename on one route, a tab appearing and vanishing on another. That guess is now only made when there is nothing better: the first settled count proving a library is not empty is remembered, and later loads answer from it immediately. A brand-new user is unaffected and still gets the first-run strip on the first render, which was the reason the guess existed. The same absence of a record was behind the second bug: the "has published" flag was only ever written by an upload made in this browser, so anyone whose videos predate it looked brand new the moment they deleted their last one. **The welcome modal no longer opens over a running upload.** Emptying your library does legitimately restore the first-run condition, but somebody with a file in the queue has already found the one thing the modal is selling. It stands down for the rest of the page load. Also: the disabled upload button inside the drop zone now looks disabled and explains itself on hover, matching the header buttons rather than contradicting them; refusal notices replace each other instead of stacking; and the landing hold now reads as loading rather than as a page that has finished rendering nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
…ether Last round of fixes from the walkthroughs. **A bare landing no longer paints a screen it is about to leave.** The hold added previously released on three conditions that all flip before the destination paints — the settled count commits a render before the effect that navigates can run, and the redirect rewrites the URL while Library is still the mounted route. So the Library rendered anyway, for about 400ms, complete with a live "Upload video" button, on the one path where Library is never the answer. How the mount arrived is now captured once, so the hold is constant for its life: spinner, then the real destination. A deliberate later click on Library is still never hijacked. The structural fix is for Library to stop owning `/` — there is a note saying so where the next person will find it. **A returning user is recognised from any route.** Both first-run flags were written from components that only some routes mount, so opening a video link in a fresh browser recorded nothing, and deleting that video looked exactly like a first run. They now have one writer, mounted in the wrapper every route shares. The two flags stay separate on purpose: the modal should still greet a site full of local videos with the migration pitch, which "library seen" proves and "has published" does not. **A real WebM is no longer told it isn't a video.** The refusal was right — the backend doesn't accept it — but the message was a lie, and the file picker was offering the formats it would then reject. The picker now offers only what the backend takes, and the two refusals read differently: an unsupported container says which formats to convert to, a file lying about its extension says it isn't a video. **The drop no longer fades two layouts through each other.** Both cards animated over the same window, so through the middle both were painted — and a drop zone and an editor form have no shared line to fade between. The handover is now sequenced: the old card leaves, then the new one arrives, in the same total time. Transitions between similarly-shaped steps keep the cross-fade, which is what it was designed for. Also fixes a test helper that shipped literal NUL bytes, which had been making git treat that file as binary and hiding its diffs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Handover tidy-up, driven by the coverage report for this PR. No behaviour changes — deletions and tests only. **Deleted the parked "Try a sample" block.** `SampleVideoModal` and its six exclusive helpers were referenced by nothing but each other, behind an eslint-disable saying so. Around 250 lines of unreachable code in the first file anyone reads, plus the stylesheet block for an element that no longer exists. Every symbol was checked against the whole package before removal. `routes/upload/stage.tsx` is down from 1742 lines to 1491. **Tested the disconnected-site upload path, which had never once run.** Its tests pinned `isWpcomConnected` to true and never flipped it, and the live walkthroughs all ran against connected sites — so the fallback that uploads through `wp/v2/media` had no coverage from either direction. It is also the path anyone running a local install hits first. It turns out to work: the drop, the upload, the details step and the success step all pass, and so do the failure paths — a network error, a non-2xx with and without a usable server message, and a missing nonce. Six tests, plus a direct one for the connection helper, which reads as untested only because every caller mocks it. **Tested the Library's refusal gates.** The same gates on the upload screen were covered; these were not, on the surface where a silently eaten drop was one of the bugs this branch set out to fix. Seven tests over the not-a-video message, the plan-limit notice and its upgrade action, the partial-selection count, and the header button's refusal. Neutering the four gates fails six of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Sequencing the hand-off removed the double-print, and a live capture confirmed it: not one frame paints both cards. But it exposed the wrapper's height transition, which starts when the step changes and so eases open during the exit — and through a sequenced hand-off both cards are at opacity 0, so what expands is an empty box. Measured on a real drop, the wrapper had reached 654 of 734px with nothing painted in it, turning a card-sized pause into a full-width blank about 470px tall. Delaying the height by the exit duration holds it until the outgoing card has actually gone, then eases it open with the card arriving to fill it. Scoped to the sequenced case: the same-shape steps want the height to track the card already fading in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
The previous commit was measured on a live site and made no difference to a single pixel. Holding the wrapper's height was the wrong lever: the blank the user sees is bounded by the white admin panel behind the flow, not by the wrapper, which only reserves space below itself. A frame diff of before and after came back at 0.00% changed. That change is reverted here along with the constant it needed. The gap was never the height. It was the incoming card being held at opacity 0 until the outgoing one had finished leaving — two timings butted end to end, with ~105ms of nothing between them. So stop animating the outgoing card at all. On a shape-changing handover it is now simply not drawn, exactly as the reduced-motion branch has always done, and the editor fades in immediately over the full duration. A drop is a deliberate act with its own feedback; the dropzone does not need a farewell. One thing moves instead of two, there is no dead band between two clocks to fall into, and "the two cards never co-occupy" is true because one of them was never painted rather than because the timings happen to line up. Same-shape steps keep their cross-fade, which is what it was designed for. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Every entry on this branch was filed against packages/videopress only. A package entry reaches that package's changelog and nowhere else — the plugins that bundle it get, at most, the release tooling's generic "Update package dependencies" line, which never reaches readme.txt. Since users, release posts and support docs all read the plugin changelog, a whole rebuild of the first-run and upload experience would have shipped invisible. Adds entries to both plugins that ship the package, written from their own users' point of view rather than copied: the new first-run and upload-to-edit experience, and the add-to-a-post fix, which is the one change here that a user could have hit as a plain bug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
The entry I hand-wrote used `fixed`, which is valid for a package and for the VideoPress plugin but not for the Jetpack plugin — that one defines its own set, where the equivalent is `bugfix`. Changelogger validity caught it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
Tests only, no behaviour change. The coverage report flagged both files as carrying more uncovered lines than trunk despite higher percentages, and the uncovered lines were not incidental. In the upload queue they were the cancel/abandon guards the file's comments spend paragraphs justifying: a progress or error event arriving for a row the user already cancelled, a tus handle turning up after its session ended, and cancelling in the window before tus has produced a handle at all. Those guards are the reason a cancelled upload can't have its media attributed to the next row in the queue. That is the worst bug this file can produce and the hardest to reproduce, and two people are about to build on it, so it should not have rested on the comments alone. In the dashboard layout it was the guard that keeps the active tab in the order when the order has dropped it — which the comment above it describes as reachable in normal use, because the first successful upload flips the state to `home` and drops `upload` while the user is still standing on it. Without the guard the test renders a blank page, which is exactly what it prevents. Two of the new tests pass whether or not their guard is present, and are labelled as such rather than counted as proof: `onError`'s abandon branch is currently unobservable, because falling through releases the queue anyway and the abort it would call is undefined in every state the store can presently reach. It is defensive, not load-bearing — unlike its counterpart in `onProgress`, which is the real one. If a future change lets a live row be removed while its session still holds a handle, that branch starts mattering and its test starts being able to fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
…icon Two things from a review of the home and upload screens. The recent-video card's picture did nothing. It is the biggest target on the card and the thing people aim at, but only the title underneath opened the video. It opens it now. It stays out of the accessibility tree rather than becoming a second button: the title already goes to the same screen, and two tab stops onto one destination is worse for a keyboard user than one — the thumbnail is decorative either way, since the title is the card's accessible name. The upload icon was a bare glyph where the rest of the dashboard puts this kind of icon in a circle. It now uses the design system's own `EmptyState.Icon`, which is where Jetpack Social gets the same treatment, so the circle, border and fill come from the tokens rather than from a shape drawn here. Its trailing margin is dropped because that spacing is meant for the design system's empty-state column and this surface already sets its rhythm with a flex gap; left in, the two stack and the icon floats away from its heading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FpbCbrkUsQUCDsaLxyMsd4
The branch's trunk merge adapted these files to the free-tier-notice move and reconciled the two AddToContentMenu variants; replaying the branch commit-by-commit onto trunk loses content that lived only in that merge commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… intro film The upload onboarding flow moves from its own route and tab to being the Library's empty state, so the Library is the default tab for everyone and upload is simply what an empty library offers. The landing redirect and its paint-hold machinery dissolve with the tab. The welcome modal no longer ships an mp4 in the plugin bundle: the film plays only once a VideoPress-hosted asset is configured, and the band shows the brand artwork until then. Its lede also stops claiming videos are hosted on the user's own site — they play on the site while VideoPress does the hosting and streaming. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
heydemoura
force-pushed
the
add/videopress-upload-onboarding-2
branch
from
August 24, 2026 20:17
ce48cf3 to
7c11f6d
Compare
This was referenced Aug 24, 2026
Contributor
Author
|
This PR has been split into a stacked train of scoped PRs for review:
The stack's combined tree is byte-identical to this branch's code (only three obsolete changelog entries describing the interim landing-redirect/upload-tab machinery were dropped, since that machinery never ships in the stack). Each stage passes the full |
This branch has not been deployed
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
1 participant
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.
Proposed changes
Supersedes #51268 — this is that branch rebased onto trunk now that the video details page redesign has landed separately (#51480). The three video-details commits were dropped during the rebase (their content is already on trunk); everything else is preserved commit-by-commit:
welcome=1review affordance.On top of the rebased branch, three follow-up changes shape the final UX:
intro-video.tsx).One extra commit restores merge-resolution content that lived only in the branch's trunk-merge commit (free-tier-notice path adaptations in two tests, and the reconciled
AddToContentMenuvariant), which a commit-by-commit rebase necessarily drops.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
jetpack build plugins/jetpack --deps(orplugins/videopress).admin.php?page=jetpack-videopress&welcome=1re-opens it for review. Its "Upload a video" CTA lands on the Library..mp4ships in the build:find projects/packages/videopress/build -name '*.mp4'is empty after a build.jetpack test js packages/videopress— all suites pass.