diff --git a/projects/packages/videopress/changelog/add-to-content-header-action b/projects/packages/videopress/changelog/add-to-content-header-action new file mode 100644 index 000000000000..aa89b23455bc --- /dev/null +++ b/projects/packages/videopress/changelog/add-to-content-header-action @@ -0,0 +1,4 @@ +Significance: patch +Type: added + +Video details: move the add-to-content action into the page header, where it can now create a new page as well as a new post. diff --git a/projects/packages/videopress/changelog/video-details-card b/projects/packages/videopress/changelog/video-details-card new file mode 100644 index 000000000000..95d5e7b1971d --- /dev/null +++ b/projects/packages/videopress/changelog/video-details-card @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Video details: group the title, description and chapters into a single card, and show the title in the page heading as you type it. diff --git a/projects/packages/videopress/changelog/video-details-thumbnail-and-subtitles b/projects/packages/videopress/changelog/video-details-thumbnail-and-subtitles new file mode 100644 index 000000000000..3a99ea7eeecf --- /dev/null +++ b/projects/packages/videopress/changelog/video-details-thumbnail-and-subtitles @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Video details: show the current thumbnail beside the control that replaces it, and give subtitles a section of their own. diff --git a/projects/packages/videopress/changelog/widen-video-details-layout b/projects/packages/videopress/changelog/widen-video-details-layout new file mode 100644 index 000000000000..e5fa30516b1d --- /dev/null +++ b/projects/packages/videopress/changelog/widen-video-details-layout @@ -0,0 +1,4 @@ +Significance: patch +Type: changed + +Video details: widen the screen and show the video's settings beside a preview of the video rather than below it. diff --git a/projects/packages/videopress/routes/video/stage.tsx b/projects/packages/videopress/routes/video/stage.tsx index 9afaccb138a6..f98a031ece5c 100644 --- a/projects/packages/videopress/routes/video/stage.tsx +++ b/projects/packages/videopress/routes/video/stage.tsx @@ -6,7 +6,6 @@ import { useCallback, useEffect, useRef, useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Link, useNavigate, useParams } from '@wordpress/route'; import { Stack, Text } from '@wordpress/ui'; -import { addQueryArgs } from '@wordpress/url'; import CaptionManagerModal from '../../src/client/components/caption-manager-modal/lazy'; import { getVideoInfoQueryKeyPrefix } from '../../src/client/components/caption-manager-modal/use-video-tracks'; import QueryClientWrapper from '../../src/dashboard/components/query-client-wrapper'; @@ -15,9 +14,11 @@ import HeaderActions from '../../src/dashboard/components/video-details/header-a import PreviewPlayer from '../../src/dashboard/components/video-details/preview-player'; import PrivacySharingCard from '../../src/dashboard/components/video-details/privacy-sharing-card'; import RatingCard from '../../src/dashboard/components/video-details/rating-card'; +import SubtitlesCard from '../../src/dashboard/components/video-details/subtitles-card'; import ThumbnailCard from '../../src/dashboard/components/video-details/thumbnail-card'; import { useVideoDetailsForm } from '../../src/dashboard/components/video-details/use-video-details-form'; import VideoDetailsCard from '../../src/dashboard/components/video-details/video-details-card'; +import VideoInfoCard from '../../src/dashboard/components/video-details/video-info-card'; import VideoNav from '../../src/dashboard/components/video-nav'; import { useDeleteVideo } from '../../src/dashboard/hooks/use-delete-video'; import { useUpdateChapters } from '../../src/dashboard/hooks/use-update-chapters'; @@ -95,7 +96,6 @@ type EditorProps = { onDelete: () => void; onDownload: () => void; onManageCaptions: () => void; - onAddToNewPost: () => void; chaptersOpen: boolean; setChaptersOpen: ( open: boolean ) => void; }; @@ -107,7 +107,6 @@ const Editor = ( { onDelete, onDownload, onManageCaptions, - onAddToNewPost, chaptersOpen, setChaptersOpen, }: EditorProps ) => { @@ -159,11 +158,30 @@ const Editor = ( { // stylesheet can clamp long video titles in the current-item // crumb (Breadcrumbs' own class names are CSS-module hashes).
- + { /* + * The crumb reads the FORM's title, not the saved record, so + * the page heading tracks what is being typed without + * committing it. Two side benefits over reading + * `video.title`: no old→new flicker when the post-save + * refetch lands, and the 2s processing `refetchInterval` + * can't clobber the crumb mid-edit. + * + * `.trim()` matters. Breadcrumbs only short-circuits on + * `items.length === 0`, so a whitespace-only title would + * render an empty

— and that

is this page's only + * accessible name. + */ } +

} actions={ ) }
- - - - - + { /* + * Placement rule for this screen: the canvas holds what a person + * authors about this video — the words, the still, the captions. + * The right-hand column holds the video itself, the values that + * address it, and the settings picked once from a fixed set. + * + * The split is authoring vs. configuring rather than editable vs. + * read-only, which is why Privacy & sharing and Rating sit beside + * the read-outs: all three are things you set and leave, not + * things you write. + * + * The player used to lead the canvas. It was measured at 502px + * tall on a 1080p display — over half the visible page before a + * single field had been read — while the settings it pushed + * down could not fit their own column and grew a second + * scrollbar with no visible boundary. Those are the same + * problem, and moving one element fixes both. + */ } +
+
+ + + +
+ { /* + * Deliberately a sibling of the canvas rather than the first + * child of the aside: it is placed by grid area, so the stacked + * layout below 1100px can lead with the player while the + * settings stay at the bottom. + */ } + + +
@@ -229,7 +278,7 @@ const StageReady = ( { video }: StageReadyProps ) => { /* * The caption manager runs on its own query client, so the page's caches - * (the thumbnail card's Subtitles row) don't see its changes. Refresh the + * (the info card's Subtitles row) don't see its changes. Refresh the * video info on close to pick up publishes and deletions. */ const closeCaptions = useCallback( () => { @@ -322,20 +371,6 @@ const StageReady = ( { video }: StageReadyProps ) => { } } } onManageCaptions={ () => setCaptionsOpen( true ) } - onAddToNewPost={ () => { - const nonce = - typeof JPVIDEOPRESS_INITIAL_STATE !== 'undefined' - ? JPVIDEOPRESS_INITIAL_STATE?.API?.contentNonce - : undefined; - if ( ! video.guid || ! nonce ) { - return; - } - const url = addQueryArgs( 'post-new.php', { - videopress_guid: video.guid, - _wpnonce: nonce, - } ); - window.open( url, '_blank' ); - } } chaptersOpen={ chaptersOpen } setChaptersOpen={ setChaptersOpen } /> diff --git a/projects/packages/videopress/routes/video/style.scss b/projects/packages/videopress/routes/video/style.scss index d8f7a4eb1925..fb7f04cfda20 100644 --- a/projects/packages/videopress/routes/video/style.scss +++ b/projects/packages/videopress/routes/video/style.scss @@ -1,34 +1,106 @@ -// Browsers clip margin-bottom on the last child of a scroll container, -// so a `margin: 24px auto` here loses the bottom 24px gap when the page -// scrolls all the way down (Rating card ends up flush against the +// Same page container as every other modernized dashboard screen — see +// `.vp-overview` in routes/overview/style.scss, which this deliberately +// mirrors rather than inventing a second measurement. The screen used to cap +// itself at 660px, which made the one screen people write on the narrowest in +// the product. + +// Browsers clip margin-bottom on the last child of a scroll container, so a +// `margin: 24px auto` here loses the bottom 24px gap when the page scrolls all +// the way down (the last inspector card ends up flush against the // JetpackFooter). Using vertical padding instead keeps that 24px inside // `.vp-video-details`'s clientHeight, which the scroll container honors. + +// `inline-size: 100%` is required because the wrapper is a flex child of the +// dashboard tabpanel (column-flex) and its auto inline margins opt it out of +// `align-items: stretch`, leaving it shrink-to-fit with nothing for the +// max-inline-size to cap. The inline padding then keeps the page off +// `#wpbody-content`'s edges, which the admin-page-layout mixin strips of +// `#wpcontent`'s own padding. .vp-video-details { - display: flex; - flex-direction: column; - gap: 24px; - max-width: 660px; + box-sizing: border-box; + inline-size: 100%; + max-inline-size: 1344px; margin-inline: auto; padding-block: 24px; padding-inline: 24px; } -// `width: 100%` is required because the wrapper is a flex child of the -// dashboard tabpanel (column-flex); without an explicit cross-axis size -// a `display: flex` flex item is sized to its content, leaving -// `max-width` unable to extend it. The `> *` rule has the same role for -// inner @wordpress/ui Card.Root children (display: flex). Both gated to -// non-mobile so the mobile breakpoint keeps cards content-sized with -// natural breathing room instead of stretching edge-to-edge of the -// viewport. -@media ( min-width: 600px ) { +// Writing canvas on the left; preview player and reference read-outs stacked +// on the right. The player is a preview, not the subject of the screen — at +// full canvas width it was 502px tall on a 1080p display, over half the +// visible page before a single field had been read. Moving it into the 380px +// column costs it nothing it was using and hands the fold to the settings. +// Same arrangement YouTube Studio uses, and for the same reason. - .vp-video-details { - width: 100%; - } +// Nothing here scrolls on its own. The three settings cards used to live in a +// height-capped sticky column that overflowed on every common viewport, which +// put a second scrollbar on the page with no visual boundary between it and +// the page's own. Splitting the content across both columns took the right +// column to roughly 520px, which fits. - .vp-video-details > * { - width: 100%; +// `minmax(0, 1fr)` rather than `1fr` so the player iframe can't push the +// canvas track past its share and widen the grid beyond the container. +// `align-items: start` leaves each item at its own height rather than +// stretching it to fill the row. + +// Row 2 is `1fr`, not `auto`, and that is load-bearing. The canvas spans both +// rows, and a spanning item's excess height is distributed across whichever +// tracks it covers are intrinsically sized — so with two `auto` rows a canvas +// taller than the right column would push the read-outs halfway down the page +// and open a large gap under the player. A flexible track absorbs that excess +// instead, leaving row 1 at the player's own height. + +// 24px matches the gap between cards within each column, the container +// padding, and every other dashboard screen. +.vp-video-details__layout { + display: grid; + grid-template-columns: minmax(0, 1fr) 380px; + grid-template-rows: auto 1fr; + grid-template-areas: + "canvas preview" + "canvas aside"; + gap: 24px; + align-items: start; +} + +// `min-inline-size: 0` because flex items default to `min-width: auto`, which +// would let a wide child override the canvas track's `minmax(0, …)` floor. +.vp-video-details__canvas, +.vp-video-details__inspector { + display: flex; + flex-direction: column; + gap: 24px; + min-inline-size: 0; +} + +.vp-video-details__canvas { + grid-area: canvas; +} + +.vp-video-details__inspector { + grid-area: aside; +} + +// Below the point where a 380px column and a canvas worth writing in stop +// fitting side by side, everything becomes one column. The areas are +// respecified rather than left to source order: the player is the second grid +// child, so source order alone would stack it below every setting on the page. +// Known limitation, deliberately not fixed here: this keys off VIEWPORT +// width, but the writing column's real width also depends on the wp-admin +// sidebar (160px, 36px folded, 272px on WordPress.com nav unification), so +// the breakpoint lands in the wrong place on narrower screens. Fixing it +// means a container query verified across three sidebar states in two +// hosting contexts; tracked as its own follow-up so a regression can be +// attributed to one change rather than two. +@media (max-width: 1100px) { + + .vp-video-details__layout { + grid-template-columns: minmax(0, 1fr); + grid-template-rows: none; + grid-template-areas: + "preview" + "canvas" + "aside"; } } @@ -38,6 +110,8 @@ // because @wordpress/admin-ui's Breadcrumbs uses hashed CSS-module class // names. Long video titles are clamped so they can't crowd the header's // Save button; the full title remains visible in the Title field below. +// The crumb now tracks the Title field live (see stage.tsx), so this clamp +// is also what stops a long in-progress title crowding Save mid-edit. // Upstream defect this works around: Breadcrumbs ships its own // `li:last-child { flex-shrink: 1; min-width: 0 }` intending exactly this // truncation, but it never engages — the nav and the header's inner Stack @@ -67,12 +141,15 @@ min-height: 320px; } -// Playable preview at the top of the details screen. The frame is pinned +// Playable preview at the head of the right-hand column. The frame is pinned // to 16:9 — the canonical player shape — and the embed letterboxes any // other aspect ratio inside it against the black background, so the page // doesn't reflow based on the video's own dimensions (which the media -// record doesn't expose anyway). +// record doesn't expose anyway). At the column's 380px that is a 214px +// frame; the width comes entirely from the grid track, so there is nothing +// to cap here. .vp-video-details__player { + grid-area: preview; position: relative; width: 100%; aspect-ratio: 16 / 9; @@ -89,39 +166,110 @@ } } -// "Processing" panel shown in place of the player while the VideoPress -// backend is still transcoding the upload; keeps the player's 16:9 slot so -// the page doesn't jump when playback becomes available. +// The Thumbnail card's picker: the current poster followed by the two ways to +// replace it, all at the same 16:9 size so the row reads as one set of +// options rather than an image with buttons stuck to it. -// The action buttons row should size to its content, not stretch across -// the column flex parent. -.vp-video-details__actions { - align-self: flex-start; +// Nothing in the design system draws this. `@wordpress/ui` has no media +// picker, upload tile or dashed container, and there is no `border-style` +// token — so the frame is painted here, from WPDS colour and radius tokens, +// on top of an unstyled `Button` that keeps the real focus and disabled +// behaviour. See thumbnail-tile.tsx for the full survey. +.vp-thumbnail-picker__intro { + display: block; + color: var(--wpds-color-foreground-content-neutral-weak); +} + +// Wraps rather than shrinks: three 168px tiles need ~536px, which the canvas +// has at every width above the 1100px breakpoint, but the stacked layout and +// the WordPress.com sidebar can both take it below that. +.vp-thumbnail-picker { flex-wrap: wrap; } -// The "Update thumbnail" trigger is a @wordpress/components Button (36px -// tall) sitting next to a @wordpress/ui Button, which sizes off the WPDS -// size-lg token (40px). Pin the trigger to the same token so the two -// actions read as one row. (No fallback — the build injects token -// fallbacks automatically.) -.vp-thumbnail-update__trigger .components-button { - height: var(--wpds-dimension-size-lg); +// Shared geometry. Both the poster and the tiles are 16:9 at the same width, +// which is what makes the row scan as a set. +.vp-thumbnail-picker__current, +.vp-thumbnail-tile { + flex-shrink: 0; + box-sizing: border-box; + inline-size: 168px; + aspect-ratio: 16 / 9; + border-radius: var(--wpds-border-radius-md); + overflow: hidden; } -// "FILE NAME" / "UPLOADED ON" caption labels (Figma uses uppercase -// with subdued color so they read as field labels, not body text). -.vp-video-details__meta-label { - color: var(--jp-gray-50, #646970); - text-transform: uppercase; - letter-spacing: 0.04em; - font-size: 11px; +// The poster is content, not an affordance, so it gets a solid edge against +// the tiles' dashed ones. +.vp-thumbnail-picker__current { + position: relative; + display: flex; + align-items: center; + justify-content: center; + border: 1px solid var(--wpds-color-stroke-surface-neutral); + background: var(--wpds-color-background-surface-neutral-weak); + + // Covers both the poster itself and the Skeleton standing in for it while + // a private video's playback token is in flight. + > img, + > div { + display: block; + inline-size: 100%; + block-size: 100%; + object-fit: cover; + } +} + +.vp-thumbnail-picker__empty { + padding-inline: var(--wpds-dimension-padding-sm); + text-align: center; + color: var(--wpds-color-foreground-content-neutral-weak); +} + +// Dashed to read as "empty slot you can fill", which is the same language the +// media library and the block editor's placeholders use. +.vp-thumbnail-tile { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: var(--wpds-dimension-gap-xs); + border: 1px dashed var(--wpds-color-stroke-surface-neutral-strong); + background: transparent; + color: var(--wpds-color-foreground-content-neutral); + cursor: pointer; + + &:hover:not(:disabled) { + border-color: var(--wpds-color-stroke-interactive-brand); + background: var(--wpds-color-background-surface-neutral-weak); + } + + &:disabled { + cursor: default; + opacity: 0.5; + } + + // `Button variant="unstyled"` drops the design system's focus ring along + // with everything else, so put it back rather than leave the tile with + // only the browser default. + &:focus-visible { + outline: 2px solid var(--wpds-color-stroke-focus); + outline-offset: 1px; + } } -// The minimal small button's hover background hugs its text; give it -// breathing room without growing the row height. -.vp-video-details__manage-subtitles { - padding-inline: 8px; +.vp-thumbnail-tile__label { + text-align: center; + padding-inline: var(--wpds-dimension-padding-sm); +} + +// File names and subtitle summaries are arbitrary strings with no guaranteed +// break opportunities — underscores don't create one, so a long name with no +// spaces or hyphens punches out of the card and forces the whole page to +// scroll sideways. `anywhere` rather than `break-word` so it also shrinks the +// element's min-content size, which is what stops the grid track widening. +.vp-video-details__readout { + overflow-wrap: anywhere; } .vp-frame-scrubber { @@ -156,3 +304,28 @@ .vp-frame-scrubber__range { margin-top: 12px; } + +// The current value carried in a collapsed CollapsibleCard header, so the +// summary answers "what is this set to?" without expanding. Weak foreground +// so it reads as secondary to the card title beside it. +.vp-video-details__summary { + color: var(--wpds-color-foreground-content-neutral-weak); + white-space: nowrap; +} + +// The Subtitles card's state line — which languages exist, or "No subtitles +// yet." Weak foreground like the Thumbnail card's intro line above it, and +// wrapping (NOT .vp-video-details__summary, whose nowrap is for collapsed +// card headers) because a long language list must fold, not overflow. +.vp-subtitles__summary { + color: var(--wpds-color-foreground-content-neutral-weak); + overflow-wrap: anywhere; +} + +// Stand-in while the tracks query resolves, sized like a one-line summary so +// the button below doesn't jump when the text lands. +.vp-subtitles__loading { + inline-size: 12rem; + block-size: 1lh; + border-radius: var(--wpds-border-radius-sm); +} diff --git a/projects/packages/videopress/routes/video/test/stage.test.tsx b/projects/packages/videopress/routes/video/test/stage.test.tsx index 182016e31206..721b5b7c24ca 100644 --- a/projects/packages/videopress/routes/video/test/stage.test.tsx +++ b/projects/packages/videopress/routes/video/test/stage.test.tsx @@ -1,4 +1,4 @@ -import { act, render, screen } from '@testing-library/react'; +import { act, render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { useNavigate } from '@wordpress/route'; import { resetFeatures, setFeatures } from '../../../src/dashboard/test-utils/features'; @@ -51,8 +51,27 @@ jest.mock( '@automattic/jetpack-components/admin-page', () => ( { ), } ) ); +// Renders `items` rather than a hardcoded trail, splitting link-vs-

the +// way the real component does (@wordpress/admin-ui breadcrumbs/index.tsx: +// every item but the last is a router link; the last, when it carries no +// `to`, is the page's only

). The live-title cases below read that +// heading, so a hardcoded stub would make them assert nothing. jest.mock( '@wordpress/admin-ui', () => ( { - Breadcrumbs: () => VideoPress, + Breadcrumbs: ( { items }: { items: { label: string; to?: string }[] } ) => { + const last = items[ items.length - 1 ]; + return ( + + ); + }, } ) ); // Variables referenced inside jest.mock() factories must be prefixed with @@ -89,10 +108,22 @@ jest.mock( '../../../src/dashboard/components/video-details/preview-player', () __esModule: true, default: () =>
, } ) ); +jest.mock( '../../../src/dashboard/components/video-details/video-info-card', () => ( { + __esModule: true, + default: () =>
, +} ) ); +// Both carry their own queries — the poster mutation and frame picker in one, +// the tracks fetch in the other — which would otherwise mount against this +// file's catch-all apiFetch handler. VideoDetailsCard stays real; the tests +// type into its fields. jest.mock( '../../../src/dashboard/components/video-details/thumbnail-card', () => ( { __esModule: true, default: () =>
, } ) ); +jest.mock( '../../../src/dashboard/components/video-details/subtitles-card', () => ( { + __esModule: true, + default: () =>
, +} ) ); jest.mock( '../../../src/dashboard/components/video-details/privacy-sharing-card', () => ( { __esModule: true, default: () =>
, @@ -213,6 +244,46 @@ describe( 'video stage', () => { resetFeatures(); } ); + /* + * The layout contract. The player is a grid sibling of the canvas and the + * settings panel, not a child of either, because it is placed by grid area — + * that is what lets the stacked layout below 1100px lead with the player + * while the settings stay last. Nesting it inside the panel would look + * equivalent and silently invert the narrow-viewport order, so pin that it + * is outside. + */ + it( 'keeps the player out of the settings panel', async () => { + await renderReadyStage(); + + const panel = screen.getByRole( 'complementary', { name: 'Video settings' } ); + + expect( within( panel ).getByTestId( 'video-info-card' ) ).toBeInTheDocument(); + expect( within( panel ).queryByTestId( 'preview-player' ) ).not.toBeInTheDocument(); + expect( screen.getByTestId( 'preview-player' ) ).toBeInTheDocument(); + } ); + + /* + * The split is authoring vs. configuring, not editable vs. read-only. The + * canvas holds what a person writes; the panel holds the read-outs plus the + * settings picked once from a fixed set. Named cards rather than a count, so + * putting one in the wrong column fails here. + */ + it( 'groups the settings with the read-outs, and the authoring outside them', async () => { + await renderReadyStage(); + + const panel = screen.getByRole( 'complementary', { name: 'Video settings' } ); + + // Configured once, then left alone. + expect( within( panel ).getByTestId( 'privacy-sharing-card' ) ).toBeInTheDocument(); + expect( within( panel ).getByTestId( 'rating-card' ) ).toBeInTheDocument(); + + // Authored — on the canvas, so present on the page but not in the panel. + expect( screen.getByTestId( 'thumbnail-card' ) ).toBeInTheDocument(); + expect( within( panel ).queryByTestId( 'thumbnail-card' ) ).not.toBeInTheDocument(); + expect( within( panel ).queryByTestId( 'subtitles-card' ) ).not.toBeInTheDocument(); + expect( within( panel ).queryByLabelText( 'Title' ) ).not.toBeInTheDocument(); + } ); + it( 'renders the Details / Editor sub-nav with Details active', async () => { await renderReadyStage(); @@ -311,6 +382,35 @@ describe( 'video stage', () => { expect( mockSuccessNotice ).toHaveBeenCalledWith( 'Video details saved.' ); } ); + // The crumb is the page's

. It reads the form's live value, so it has + // to follow typing — and it must not turn typing into a save. + it( 'tracks the title in the breadcrumb heading as it is typed, without saving', async () => { + const user = userEvent.setup(); + + await renderReadyStage(); + expect( screen.getByRole( 'heading', { level: 1 } ) ).toHaveTextContent( 'My Clip' ); + + await user.type( screen.getByLabelText( 'Title' ), ' 2' ); + + expect( screen.getByRole( 'heading', { level: 1 } ) ).toHaveTextContent( 'My Clip 2' ); + expect( mockUpdateMeta ).not.toHaveBeenCalled(); + } ); + + // An empty label renders an empty

, which would leave the page with no + // accessible name mid-edit; whitespace has to count as empty too. + it( 'falls back to Untitled when the title is cleared or only whitespace', async () => { + const user = userEvent.setup(); + + await renderReadyStage(); + await user.clear( screen.getByLabelText( 'Title' ) ); + + expect( screen.getByRole( 'heading', { level: 1 } ) ).toHaveTextContent( 'Untitled' ); + + await user.type( screen.getByLabelText( 'Title' ), ' ' ); + + expect( screen.getByRole( 'heading', { level: 1 } ) ).toHaveTextContent( 'Untitled' ); + } ); + it( 'skips the chapters sync and shows an error when the meta save fails', async () => { const user = userEvent.setup(); diff --git a/projects/packages/videopress/src/dashboard/components/add-to-content-menu/index.tsx b/projects/packages/videopress/src/dashboard/components/add-to-content-menu/index.tsx new file mode 100644 index 000000000000..897f2a6d7e55 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/add-to-content-menu/index.tsx @@ -0,0 +1,151 @@ +/** + * External dependencies + */ +import { DropdownMenu, MenuGroup, MenuItem } from '@wordpress/components'; +import { __ } from '@wordpress/i18n'; +import { plus, page as pageIcon, post as postIcon } from '@wordpress/icons'; +import { addQueryArgs } from '@wordpress/url'; + +// The two content types this menu can hand a video off to. Anything else +// (adding to an *existing* post or page) is deliberately out of scope. +export type NewContentType = 'post' | 'page'; + +/** + * The nonce the server requires before it will fill a new post with the video. + * + * Read in two places — the render guard and the click handler — because the + * menu must not appear at all when it cannot work. + * + * @return The content nonce, or undefined when the boot payload has none. + */ +export const readContentNonce = (): string | undefined => { + const nonce = + typeof JPVIDEOPRESS_INITIAL_STATE !== 'undefined' + ? JPVIDEOPRESS_INITIAL_STATE?.API?.contentNonce + : undefined; + + return typeof nonce === 'string' && nonce !== '' ? nonce : undefined; +}; + +/** + * Open a brand-new post or page in the block editor with the video already in + * it. The block markup is produced server-side by + * `Block_Editor_Content::videopress_video_block_by_guid()`, which filters + * `default_content` on `post-new.php` when the request carries a GUID and a + * valid `videopress-content-nonce`. Because it hooks `post-new.php`, passing + * `post_type=page` gets page support for free. + * + * Opened in a new tab so the user keeps their place on the dashboard. + * + * @param guid - The VideoPress GUID of the published video. + * @param contentType - Whether to create a new post or a new page. + */ +export const openInNewContent = ( guid: string, contentType: NewContentType ) => { + const nonce = readContentNonce(); + + if ( ! guid || ! nonce ) { + return; + } + + const args: Record< string, string > = { videopress_guid: guid, _wpnonce: nonce }; + if ( contentType === 'page' ) { + args.post_type = 'page'; + } + + window.open( addQueryArgs( 'post-new.php', args ), '_blank' ); +}; + +type Props = { + /** The VideoPress GUID of the video to insert. */ + guid?: string; + /** Accessible label for the trigger; disambiguates the menu when several are on screen. */ + label?: string; + /** + * Trigger size. `compact` matches the dashboard header's action row, which + * is the only place this renders today; `default` is the 40px in-card size. + * + * @default 'default' + */ + size?: 'default' | 'compact'; + className?: string; +}; + +/** + * "Add to a post or page" dropdown for a published video. Mirrors the labelled + * `DropdownMenu` used by the video detail view's ThumbnailUpdateButton so the + * two share the design system's look. + * + * Rendered from the video screen's header actions — the one place it appears + * today. Only renders when the video has both a VideoPress GUID and a content + * nonce: the hand-off is the server-side `videopress_guid` content filter, so + * without either there is no honest VideoPress block to insert, and a menu + * whose items silently open a blank editor is worse than no menu at all. A + * GUID is absent on local attachments and on videos still being registered + * with VideoPress. + * + * NOTE: `add/videopress-upload-onboarding` carries a near-identical copy of + * this file at the same path, differing only in the trigger's `text`. If both + * branches land, reconcile them into one component rather than resolving the + * conflict by picking a string. + * + * @param props - Component props. + * @param props.guid - The VideoPress GUID of the video to insert. + * @param props.label - Accessible label for the trigger. + * @param props.size - Trigger size (`default` or `compact`). + * @param props.className - Extra class for the dropdown root. + * @return The dropdown, or null when no GUID is available. + */ +export default function AddToContentMenu( { guid, label, size, className }: Props ) { + // Both halves of the hand-off have to be present, not just the GUID. The + // server only fills `default_content` when the request carries a valid + // `videopress-content-nonce`, so without it every item in this menu would + // open a blank editor — the silent no-op this component's own docblock + // promises not to ship. + if ( ! guid || ! readContentNonce() ) { + return null; + } + + return ( + + { ( { onClose }: { onClose: () => void } ) => ( + + { + openInNewContent( guid, 'post' ); + onClose(); + } } + > + { __( 'New post', 'jetpack-videopress-pkg' ) } + + { + openInNewContent( guid, 'page' ); + onClose(); + } } + > + { __( 'New page', 'jetpack-videopress-pkg' ) } + + + ) } + + ); +} diff --git a/projects/packages/videopress/src/dashboard/components/add-to-content-menu/test/index.test.tsx b/projects/packages/videopress/src/dashboard/components/add-to-content-menu/test/index.test.tsx new file mode 100644 index 000000000000..97863ff08a09 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/add-to-content-menu/test/index.test.tsx @@ -0,0 +1,77 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import AddToContentMenu from '..'; + +const GUID = 'abc123'; + +/** + * Set (or clear) the boot payload the component reads its nonce from. + * + * @param contentNonce - Nonce to expose, or undefined to simulate its absence. + */ +function setInitialState( contentNonce?: string ) { + ( global as unknown as { JPVIDEOPRESS_INITIAL_STATE?: unknown } ).JPVIDEOPRESS_INITIAL_STATE = + contentNonce === undefined ? { API: {} } : { API: { contentNonce } }; +} + +describe( 'AddToContentMenu', () => { + let openSpy: jest.SpyInstance; + + beforeEach( () => { + setInitialState( 'nonce-123' ); + openSpy = jest.spyOn( window, 'open' ).mockImplementation( () => null ); + } ); + + afterEach( () => { + openSpy.mockRestore(); + delete ( global as unknown as { JPVIDEOPRESS_INITIAL_STATE?: unknown } ) + .JPVIDEOPRESS_INITIAL_STATE; + } ); + + // Both halves of the hand-off have to be present. A menu whose items open a + // blank editor is worse than no menu, so the component renders nothing + // rather than something inert. + it( 'renders nothing without a GUID', () => { + const { container } = render( ); + + expect( container ).toBeEmptyDOMElement(); + } ); + + it( 'renders nothing without a content nonce', () => { + setInitialState( undefined ); + + const { container } = render( ); + + expect( container ).toBeEmptyDOMElement(); + } ); + + it( 'opens a new post with the video and the nonce', async () => { + const user = userEvent.setup(); + render( ); + + await user.click( screen.getByRole( 'button', { name: /add to a post or page/i } ) ); + await user.click( await screen.findByRole( 'menuitem', { name: 'New post' } ) ); + + expect( openSpy ).toHaveBeenCalledTimes( 1 ); + const [ url ] = openSpy.mock.calls[ 0 ]; + expect( url ).toContain( 'post-new.php' ); + expect( url ).toContain( `videopress_guid=${ GUID }` ); + expect( url ).toContain( '_wpnonce=nonce-123' ); + // The post branch must not carry a post_type, or it would create a page. + expect( url ).not.toContain( 'post_type' ); + } ); + + // `post_type=page` works only because the server filters `default_content` + // on post-new.php, which serves both types — worth pinning. + it( 'opens a new page when the page item is chosen', async () => { + const user = userEvent.setup(); + render( ); + + await user.click( screen.getByRole( 'button', { name: /add to a post or page/i } ) ); + await user.click( await screen.findByRole( 'menuitem', { name: 'New page' } ) ); + + const [ url ] = openSpy.mock.calls[ 0 ]; + expect( url ).toContain( 'post_type=page' ); + expect( url ).toContain( `videopress_guid=${ GUID }` ); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/header-actions.tsx b/projects/packages/videopress/src/dashboard/components/video-details/header-actions.tsx index 89a471789505..dff66b198f3d 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/header-actions.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/header-actions.tsx @@ -2,9 +2,12 @@ import { DropdownMenu, MenuGroup, MenuItem } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { download, moreVertical, trash } from '@wordpress/icons'; import { Button, Stack } from '@wordpress/ui'; +import AddToContentMenu from '../add-to-content-menu'; import type { ReactElement } from 'react'; type Props = { + /** GUID of the video, for the add-to-content hand-off. Absent on local items. */ + guid?: string; canSave: boolean; onSave: () => void; onManageCaptions: () => void; @@ -14,12 +17,18 @@ type Props = { /** * Actions slot for the AdminPage header on the Video details screen. - * Renders the primary Save button and the ⋯ menu (Download file / Delete - * video). Save is disabled when the form is clean. Delete uses MenuItem's - * built-in `isDestructive` flag so the design-system surfaces the - * red destructive treatment without a CSS override. + * Renders the add-to-content menu, the primary Save button and the ⋯ menu + * (Download file / Delete video). Save is disabled when the form is clean. + * Delete uses MenuItem's built-in `isDestructive` flag so the design system + * surfaces the red destructive treatment without a CSS override. + * + * "Add to a post or page" sits here rather than in a card because it acts on + * the whole video rather than on any one card's contents, which puts it + * alongside the screen's other whole-video actions. It previously lived at the + * foot of the info card, which is otherwise read-outs. * * @param props - Component props. + * @param props.guid - VideoPress GUID, when the video has one. * @param props.canSave - Whether the form has unsaved changes. * @param props.onSave - Called when the Save button is activated. * @param props.onManageCaptions - Called when "Manage subtitles" is selected. @@ -28,6 +37,7 @@ type Props = { * @return The header-actions element. */ export default function HeaderActions( { + guid, canSave, onSave, onManageCaptions, @@ -36,6 +46,7 @@ export default function HeaderActions( { }: Props ): ReactElement { return ( + diff --git a/projects/packages/videopress/src/dashboard/components/video-details/privacy-sharing-card.tsx b/projects/packages/videopress/src/dashboard/components/video-details/privacy-sharing-card.tsx index 2a07b29af0c4..1d2a9c62dd45 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/privacy-sharing-card.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/privacy-sharing-card.tsx @@ -1,6 +1,6 @@ import { SelectControl, ToggleControl } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { Card, Stack } from '@wordpress/ui'; +import { Card, CollapsibleCard, Stack, Text } from '@wordpress/ui'; import type { LibraryItemPrivacy } from '../../types/library'; import type { ReactElement } from 'react'; @@ -25,6 +25,13 @@ const PRIVACY_OPTIONS: { label: string; value: LibraryItemPrivacy }[] = [ * Form card for privacy and sharing controls: a privacy SelectControl * and two ToggleControls for sharing and downloads. * + * Collapsible, and collapsed by default. These are set-once settings that sit + * beside the read-outs, so leaving all three expanded made the right-hand + * column 400px taller than the canvas it sits next to. The current privacy + * value rides in the header via `CollapsibleCard.HeaderDescription`, so + * collapsing costs no information at a glance — you still see "Public" or + * "Private" without opening anything. + * * @param props - Component props. * @param props.privacy - Current privacy value. * @param props.displayEmbed - Whether the share menu is displayed. @@ -38,12 +45,20 @@ export default function PrivacySharingCard( { allowDownloads, onChange, }: Props ): ReactElement { + const currentPrivacyLabel = + PRIVACY_OPTIONS.find( option => option.value === privacy )?.label ?? ''; + return ( - - - { __( 'Privacy & sharing', 'jetpack-videopress-pkg' ) } - - + + + + { __( 'Privacy & sharing', 'jetpack-videopress-pkg' ) } + + { currentPrivacyLabel } + + + + onChange( { allowDownloads: next } ) } /> - - + + ); } diff --git a/projects/packages/videopress/src/dashboard/components/video-details/rating-card.tsx b/projects/packages/videopress/src/dashboard/components/video-details/rating-card.tsx index e4e0e3abab52..5f739d7e6832 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/rating-card.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/rating-card.tsx @@ -1,6 +1,6 @@ import { RadioControl } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { Card } from '@wordpress/ui'; +import { Card, CollapsibleCard, Stack, Text } from '@wordpress/ui'; import type { VideoRating } from '../../types/library'; import type { ReactElement } from 'react'; @@ -12,6 +12,11 @@ type Props = { /** * Rating radio group. Single tab stop; arrow keys cycle G / PG-13 / R. * + * Collapsible, and collapsed by default — the rating is set once and rarely + * revisited, and the three descriptions make it the tallest card in a column + * that was already outrunning the canvas beside it. The selected rating shows + * in the header, so the collapsed state still answers "what is this rated?". + * * @param props - Component props. * @param props.value - Currently selected rating. * @param props.onChange - Receives the new rating. @@ -19,39 +24,54 @@ type Props = { */ export default function RatingCard( { value, onChange }: Props ): ReactElement { return ( - - - { __( 'Rating', 'jetpack-videopress-pkg' ) } - - + + + + { __( 'Rating', 'jetpack-videopress-pkg' ) } + + { value } + + + + onChange( next as VideoRating ) } + // `description` renders through the same `StyledHelp` that + // `BaseControl` gives every `help` line, so the type matches the + // toggle captions in Privacy & sharing. (Only the type — the + // two controls set different margins around it.) Splitting the + // rating off the sentence also stops a screen reader reading the + // whole explanation as the option's name — it becomes an + // `aria-describedby` instead. options={ [ { - label: __( - 'G — Suitable for all audiences, including children', + label: __( 'G', 'jetpack-videopress-pkg' ), + description: __( + 'Suitable for all audiences, including children.', 'jetpack-videopress-pkg' ), value: 'G', }, { - label: __( - 'PG-13 — May include mild language or mature themes', + label: __( 'PG-13', 'jetpack-videopress-pkg' ), + description: __( + 'May include mild language or mature themes.', 'jetpack-videopress-pkg' ), value: 'PG-13', }, { - label: __( - 'R — May include strong language, violence, or adult content', + label: __( 'R', 'jetpack-videopress-pkg' ), + description: __( + 'May include strong language, violence, or adult content.', 'jetpack-videopress-pkg' ), value: 'R', }, ] } /> - - + + ); } diff --git a/projects/packages/videopress/src/dashboard/components/video-details/subtitles-card.tsx b/projects/packages/videopress/src/dashboard/components/video-details/subtitles-card.tsx new file mode 100644 index 000000000000..0eba08ffa053 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/subtitles-card.tsx @@ -0,0 +1,130 @@ +import { __, _n, sprintf } from '@wordpress/i18n'; +import { Button, Card, Skeleton, Stack, Text } from '@wordpress/ui'; +import { useVideoTracks } from '../../../client/components/caption-manager-modal/use-video-tracks'; +import { + getLanguageDisplayName, + getManualLanguageTagFromTrackKey, +} from '../../../client/lib/video-tracks/language'; +import type { LibraryItem } from '../../types/library'; +import type { ReactElement } from 'react'; + +type Props = { + video: LibraryItem; + onManageSubtitles: () => void; +}; + +// The summary lists this many languages before collapsing into "and N more". +const MAX_SUBTITLE_LANGUAGES_SHOWN = 2; + +/** + * The card proper. Split from the guard below so the tracks query only exists + * for videos that actually have a GUID to query with. + * + * @param props - Component props. + * @param props.video - The current video record. + * @param props.onManageSubtitles - Opens the caption manager. + * @return The card element. + */ +function SubtitlesCardContent( { video, onManageSubtitles }: Props ): ReactElement { + /* + * The media REST item omits `tracks`, so the languages come from the same + * video-info query the caption manager uses. + */ + const { managedTracks, isLoading } = useVideoTracks( { + guid: video.guid, + isOpen: true, + isPrivate: video.isPrivate, + tracks: video.tracks, + } ); + + const subtitleLanguages = [ + ...new Set( + managedTracks + .filter( track => track.kind === 'captions' || track.kind === 'subtitles' ) + .map( + track => + track.label || + getLanguageDisplayName( + getManualLanguageTagFromTrackKey( track.srcLang ) || track.srcLang + ) + ) + ), + ]; + const shownLanguages = subtitleLanguages.slice( 0, MAX_SUBTITLE_LANGUAGES_SHOWN ).join( ', ' ); + const moreLanguagesCount = subtitleLanguages.length - MAX_SUBTITLE_LANGUAGES_SHOWN; + let subtitleSummary = shownLanguages || __( 'No subtitles yet.', 'jetpack-videopress-pkg' ); + if ( moreLanguagesCount > 0 ) { + subtitleSummary = sprintf( + /* translators: 1: list of subtitle language names. 2: how many further languages exist. */ + _n( + '%1$s, and %2$d more', + '%1$s, and %2$d more', + moreLanguagesCount, + 'jetpack-videopress-pkg' + ), + shownLanguages, + moreLanguagesCount + ); + } + + return ( + + + { __( 'Subtitles', 'jetpack-videopress-pkg' ) } + + + { /* + * State first, then the action — the same rhythm as the + * Thumbnail card above this one. The two used to share a row, + * which read as fragments ("None [Manage subtitles]"). + */ } + + { isLoading ? ( + + ) : ( + { subtitleSummary } + ) } + { /* + * The full string, not "Manage". The label used to read + * "Manage" with "Manage subtitles" hidden in an aria-label, + * which left the visible text meaningless the moment it was + * separated from its row label. + */ } + + + + + ); +} + +/** + * Which subtitle languages this video has, and the way in to add or change + * them. + * + * It gets a card on the canvas rather than a row in the reference panel + * because the two ways in before this were both nearly invisible: a + * lowest-emphasis button labelled "Manage" at the foot of the second panel + * card — below the fold on a laptop, and absent entirely without a GUID — and + * an iconless first item inside an unlabelled ⋮ menu. Captions are an + * accessibility feature; hiding them behind a kebab was the wrong call. + * + * The ⋮ menu keeps its entry. This is an addition, not a move: a menu people + * already know is not worth breaking. + * + * Renders nothing without a GUID — the tracks API is keyed by it, so there is + * nothing to read and nothing to manage until the upload completes. + * + * @param props - Component props. + * @param props.video - The current video record. + * @param props.onManageSubtitles - Opens the caption manager. + * @return The card, or null when the video has no GUID yet. + */ +export default function SubtitlesCard( { video, onManageSubtitles }: Props ): ReactElement | null { + if ( ! video.guid ) { + return null; + } + + return ; +} diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/privacy-sharing-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/privacy-sharing-card.test.tsx new file mode 100644 index 000000000000..7d1602d7977a --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/privacy-sharing-card.test.tsx @@ -0,0 +1,77 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import PrivacySharingCard from '../privacy-sharing-card'; + +const renderCard = ( overrides: Partial< Parameters< typeof PrivacySharingCard >[ 0 ] > = {} ) => { + const onChange = jest.fn(); + render( + + ); + return { onChange }; +}; + +const expandCard = async ( user: ReturnType< typeof userEvent.setup > ) => + user.click( screen.getByRole( 'button', { name: /privacy & sharing/i } ) ); + +describe( 'PrivacySharingCard', () => { + /* + * Collapsed by default is a deliberate trade: it keeps the settings column + * from outrunning the canvas beside it. The trade only holds if the + * collapsed header still answers "what is this set to?", so that is the + * part worth pinning. + */ + it( 'starts collapsed with the current privacy value in the header', () => { + renderCard(); + + const trigger = screen.getByRole( 'button', { name: /privacy & sharing/i } ); + + expect( trigger ).toHaveAttribute( 'aria-expanded', 'false' ); + // The summary reaches assistive tech through `aria-describedby`, not as + // text inside the name — HeaderDescription is deliberately aria-hidden + // so it isn't announced twice. + expect( trigger ).toHaveAccessibleDescription( 'Public' ); + } ); + + it( 'reflects a changed privacy value in the collapsed header', () => { + renderCard( { privacy: 'private' } ); + + expect( + screen.getByRole( 'button', { name: /privacy & sharing/i } ) + ).toHaveAccessibleDescription( 'Private' ); + } ); + + it( 'reveals the controls when expanded, and reports edits', async () => { + const user = userEvent.setup(); + const { onChange } = renderCard(); + + await expandCard( user ); + + expect( screen.getByRole( 'button', { name: /privacy & sharing/i } ) ).toHaveAttribute( + 'aria-expanded', + 'true' + ); + + await user.click( screen.getByRole( 'checkbox', { name: 'Allow downloads' } ) ); + expect( onChange ).toHaveBeenCalledWith( { allowDownloads: true } ); + } ); + + /* + * `hiddenUntilFound` is CollapsibleCard.Content's default, and it is what + * makes collapsing safe rather than lossy: the content stays in the DOM + * and the browser's find-in-page expands the card when it matches. Someone + * hunting for "Allow downloads" with Ctrl+F still finds it. + */ + it( 'keeps collapsed content findable rather than unmounting it', () => { + renderCard(); + + expect( + screen.getByText( 'Let viewers download this video to their device.' ) + ).toBeInTheDocument(); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/rating-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/rating-card.test.tsx new file mode 100644 index 000000000000..a13a4c6f7c70 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/rating-card.test.tsx @@ -0,0 +1,48 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import RatingCard from '../rating-card'; +import type { VideoRating } from '../../../types/library'; + +const renderCard = ( value: VideoRating = 'G' ) => { + const onChange = jest.fn(); + render( ); + return { onChange }; +}; + +describe( 'RatingCard', () => { + it( 'starts collapsed with the current rating in the header', () => { + renderCard( 'PG-13' ); + + const trigger = screen.getByRole( 'button', { name: /rating/i } ); + + expect( trigger ).toHaveAttribute( 'aria-expanded', 'false' ); + // Reaches assistive tech via `aria-describedby`; HeaderDescription is + // aria-hidden so it isn't announced twice. + expect( trigger ).toHaveAccessibleDescription( 'PG-13' ); + } ); + + it( 'reveals the options when expanded, and reports a change', async () => { + const user = userEvent.setup(); + const { onChange } = renderCard( 'G' ); + + await user.click( screen.getByRole( 'button', { name: /rating/i } ) ); + + expect( screen.getByRole( 'radio', { name: 'G' } ) ).toBeChecked(); + + await user.click( screen.getByRole( 'radio', { name: 'PG-13' } ) ); + expect( onChange ).toHaveBeenCalledWith( 'PG-13' ); + } ); + + /* + * The descriptions are the reason this card is the tallest in the column, + * and the reason it collapses. They must survive collapsing rather than be + * unmounted — `hiddenUntilFound` keeps them reachable by find-in-page. + */ + it( 'keeps the option descriptions in the document while collapsed', () => { + renderCard(); + + expect( + screen.getByText( 'Suitable for all audiences, including children.' ) + ).toBeInTheDocument(); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/subtitles-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/subtitles-card.test.tsx new file mode 100644 index 000000000000..d0d8b3bd1dad --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/subtitles-card.test.tsx @@ -0,0 +1,104 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { createElement, type ReactNode } from 'react'; +import { makeLibraryItem } from '../../../test-utils/library-item'; +import SubtitlesCard from '../subtitles-card'; + +/* + * Mocked so the card doesn't reach the network; the hook's fetch behavior is + * covered by its own suite. + */ +let mockTracksResult: { managedTracks: unknown[]; isLoading: boolean } = { + managedTracks: [], + isLoading: false, +}; +jest.mock( '../../../../client/components/caption-manager-modal/use-video-tracks', () => ( { + useVideoTracks: () => mockTracksResult, +} ) ); + +const baseVideo = makeLibraryItem( { shortcode: '[videopress abc123]' } ); + +/** + * Minimal React Query wrapper for tests. + * + * @param root0 - Component props. + * @param root0.children - Child elements to render inside the provider. + * @return The QueryClientProvider element. + */ +function wrapper( { children }: { children: ReactNode } ) { + const client = new QueryClient( { + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + } ); + return createElement( QueryClientProvider, { client }, children ); +} + +beforeEach( () => { + mockTracksResult = { managedTracks: [], isLoading: false }; +} ); + +describe( 'SubtitlesCard', () => { + it( 'lists the subtitle languages and opens the manager', async () => { + const user = userEvent.setup(); + mockTracksResult = { + managedTracks: [ + { kind: 'captions', srcLang: 'en-US', label: '', src: 'en.vtt' }, + { kind: 'subtitles', srcLang: 'de', label: 'German', src: 'de.vtt' }, + { kind: 'chapters', srcLang: 'en', label: '', src: 'chapters.vtt' }, + ], + isLoading: false, + }; + const onManageSubtitles = jest.fn(); + render( , { + wrapper, + } ); + + // Chapters are not subtitles; only the caption/subtitle languages show. + expect( screen.getByText( 'English (US), German' ) ).toBeInTheDocument(); + + await user.click( screen.getByRole( 'button', { name: 'Manage subtitles' } ) ); + expect( onManageSubtitles ).toHaveBeenCalledTimes( 1 ); + } ); + + // The button used to read "Manage", with the real string only in an + // aria-label. Once the row left the card that labelled it, the visible + // text had to carry the meaning on its own. + it( 'spells out the action in visible text, not only in the accessible name', () => { + render( , { wrapper } ); + + expect( screen.getByRole( 'button', { name: 'Manage subtitles' } ) ).toHaveTextContent( + 'Manage subtitles' + ); + } ); + + it( 'collapses long language lists into the first two and a count', () => { + mockTracksResult = { + managedTracks: [ + { kind: 'captions', srcLang: 'en-US', label: '', src: '' }, + { kind: 'subtitles', srcLang: 'de', label: '', src: '' }, + { kind: 'subtitles', srcLang: 'fr', label: '', src: '' }, + { kind: 'subtitles', srcLang: 'es', label: '', src: '' }, + ], + isLoading: false, + }; + render( , { wrapper } ); + + expect( screen.getByText( 'English (US), German, and 2 more' ) ).toBeInTheDocument(); + } ); + + it( 'says "No subtitles yet." when the video has no subtitle tracks', () => { + render( , { wrapper } ); + + expect( screen.getByText( 'Subtitles' ) ).toBeInTheDocument(); + expect( screen.getByText( 'No subtitles yet.' ) ).toBeInTheDocument(); + } ); + + it( 'renders nothing for items without a VideoPress GUID', () => { + const { container } = render( + , + { wrapper } + ); + + expect( container ).toBeEmptyDOMElement(); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-card.test.tsx index 99b3d6875bfe..4ba273d58438 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-card.test.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-card.test.tsx @@ -3,26 +3,15 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import apiFetch from '@wordpress/api-fetch'; import { createElement, type ReactNode } from 'react'; +import { makeLibraryItem } from '../../../test-utils/library-item'; import { selectImageFromMediaLibrary } from '../../../utils/select-image-from-media-library'; import ThumbnailCard from '../thumbnail-card'; -import type { LibraryItem } from '../../../types/library'; jest.mock( '@wordpress/api-fetch', () => ( { __esModule: true, default: jest.fn(), } ) ); -/* - * Mocked so the subtitles row doesn't share the apiFetch mock with the poster - * mutation tests; the hook's fetch behavior is covered by its own suite. - */ -let mockTracksResult: { managedTracks: unknown[]; isLoading: boolean } = { - managedTracks: [], - isLoading: false, -}; -jest.mock( '../../../../client/components/caption-manager-modal/use-video-tracks', () => ( { - useVideoTracks: () => mockTracksResult, -} ) ); const mockedApiFetch = apiFetch as unknown as jest.Mock; jest.mock( '../../../utils/select-image-from-media-library', () => ( { @@ -60,29 +49,13 @@ jest.mock( '@automattic/jetpack-components/global-notices', () => ( { } ), } ) ); -const baseVideo: LibraryItem = { - id: '42', - guid: 'abc123', - type: 'videopress', - title: 'My video', - filename: 'movie.mp4', +// `sourceUrl` is what makes the "Select from video" mode available. +const baseVideo = makeLibraryItem( { thumbnailUrl: 'https://example.test/poster.jpg', durationSeconds: 60, - uploadDate: '2026-01-01T00:00:00', - privacy: 'public', - isPrivate: false, - fileSizeBytes: 0, - upload: { status: 'idle', progress: 0 }, - description: '', - rating: 'G', - displayEmbed: true, - allowDownloads: false, shortcode: '[videopress abc123]', sourceUrl: 'https://example.test/movie.mp4', - isProcessing: false, - orientation: null, - tracks: [], -}; +} ); /** * Minimal React Query wrapper for tests. @@ -103,7 +76,6 @@ beforeEach( () => { mockedSelectImage.mockReset(); mockSuccessNotice.mockReset(); mockErrorNotice.mockReset(); - mockTracksResult = { managedTracks: [], isLoading: false }; // Provide window.wp.media so canUploadImage is true for upload-mode tests. ( window as unknown as { wp?: { media?: unknown } } ).wp = { media: jest.fn() }; } ); @@ -113,55 +85,61 @@ afterEach( () => { } ); describe( 'ThumbnailCard — update flow', () => { - it( 'renders the Update thumbnail button when video is editable', () => { - render( - , - { wrapper } + it( 'renders both action tiles when the video is editable', () => { + render( , { wrapper } ); + + expect( screen.getByRole( 'button', { name: /upload image/i } ) ).toBeInTheDocument(); + expect( screen.getByRole( 'button', { name: /select from video/i } ) ).toBeInTheDocument(); + } ); + + /* + * Without a source file there is no footage to scrub, so that tile is + * disabled rather than hidden — the other one still works. + * + * Asserted via `aria-disabled`, not `toBeDisabled()`: `@wordpress/ui`'s + * Button defaults to `focusableWhenDisabled`, so it deliberately omits the + * native attribute and keeps the control reachable by keyboard. That is + * the better pattern — a disabled control a screen-reader user can't land + * on is a control they never learn exists — so pin the behaviour rather + * than override it. + */ + it( 'disables Select from video when the item has no source file', async () => { + const user = userEvent.setup(); + render( , { wrapper } ); + + const disabled = screen.getByRole( 'button', { name: /select from video/i } ); + expect( disabled ).toHaveAttribute( 'aria-disabled', 'true' ); + expect( screen.getByRole( 'button', { name: /upload image/i } ) ).not.toHaveAttribute( + 'aria-disabled', + 'true' ); - expect( screen.getByRole( 'button', { name: /update thumbnail/i } ) ).toBeInTheDocument(); + + // Focusable is not the same as actionable. + await user.click( disabled ); + expect( screen.queryByTestId( 'select-frame-dialog' ) ).not.toBeInTheDocument(); } ); - it( 'hides the Update thumbnail button while the video is processing', () => { - render( - , + it( 'renders nothing while the video is processing', () => { + const { container } = render( + , { wrapper } ); - expect( screen.queryByRole( 'button', { name: /update thumbnail/i } ) ).not.toBeInTheDocument(); + expect( container ).toBeEmptyDOMElement(); } ); - it( 'hides the Update thumbnail button for local (non-VideoPress) items', () => { - render( - , + it( 'renders nothing for local (non-VideoPress) items', () => { + const { container } = render( + , { wrapper } ); - expect( screen.queryByRole( 'button', { name: /update thumbnail/i } ) ).not.toBeInTheDocument(); + expect( container ).toBeEmptyDOMElement(); } ); it( 'frame mode: fires the mutation with at_time + is_millisec, shows a success toast', async () => { const user = userEvent.setup(); mockedApiFetch.mockResolvedValueOnce( {} ); - render( - , - { wrapper } - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /select from video/i } ) ); + render( , { wrapper } ); + await user.click( screen.getByRole( 'button', { name: /select from video/i } ) ); await user.click( screen.getByText( 'confirm-frame' ) ); await waitFor( () => @@ -179,17 +157,9 @@ describe( 'ThumbnailCard — update flow', () => { const user = userEvent.setup(); mockedApiFetch.mockResolvedValueOnce( {} ); mockedSelectImage.mockResolvedValueOnce( { id: 17, url: 'x' } ); - render( - , - { wrapper } - ); + render( , { wrapper } ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /upload image/i } ) ); + await user.click( screen.getByRole( 'button', { name: /upload image/i } ) ); await waitFor( () => expect( mockedApiFetch ).toHaveBeenCalledWith( { @@ -204,17 +174,9 @@ describe( 'ThumbnailCard — update flow', () => { it( 'upload mode: no mutation when the user cancels the media library', async () => { const user = userEvent.setup(); mockedSelectImage.mockResolvedValueOnce( null ); - render( - , - { wrapper } - ); + render( , { wrapper } ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /upload image/i } ) ); + await user.click( screen.getByRole( 'button', { name: /upload image/i } ) ); expect( mockedApiFetch ).not.toHaveBeenCalled(); expect( mockSuccessNotice ).not.toHaveBeenCalled(); @@ -223,16 +185,8 @@ describe( 'ThumbnailCard — update flow', () => { it( 'shows an error toast when the mutation fails', async () => { const user = userEvent.setup(); mockedApiFetch.mockRejectedValueOnce( new Error( 'boom' ) ); - render( - , - { wrapper } - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /select from video/i } ) ); + render( , { wrapper } ); + await user.click( screen.getByRole( 'button', { name: /select from video/i } ) ); await user.click( screen.getByText( 'confirm-frame' ) ); await waitFor( () => expect( mockErrorNotice ).toHaveBeenCalledTimes( 1 ) ); @@ -240,80 +194,52 @@ describe( 'ThumbnailCard — update flow', () => { } ); } ); -describe( 'ThumbnailCard — subtitles row', () => { - it( 'lists the subtitle languages and opens the manager from the Manage action', async () => { - const user = userEvent.setup(); - mockTracksResult = { - managedTracks: [ - { kind: 'captions', srcLang: 'en-US', label: '', src: 'en.vtt' }, - { kind: 'subtitles', srcLang: 'de', label: 'German', src: 'de.vtt' }, - { kind: 'chapters', srcLang: 'en', label: '', src: 'chapters.vtt' }, - ], - isLoading: false, - }; - const onManageSubtitles = jest.fn(); - render( - , - { wrapper } +// Showing the still next to the control that replaces it is the reason this +// is a card rather than the labelled lone button it used to be, so the image +// is worth pinning — particularly the private case, where rendering +// `thumbnailUrl` straight would give a broken image on every private video. +describe( 'ThumbnailCard — current poster', () => { + it( 'renders the poster for a public video', () => { + render( , { wrapper } ); + + expect( screen.getByRole( 'img', { name: /current thumbnail/i } ) ).toHaveAttribute( + 'src', + 'https://example.test/poster.jpg' ); - - // Chapters are not subtitles; only the caption/subtitle languages show. - expect( screen.getByText( 'English (US), German' ) ).toBeInTheDocument(); - - await user.click( screen.getByRole( 'button', { name: 'Manage subtitles' } ) ); - expect( onManageSubtitles ).toHaveBeenCalledTimes( 1 ); + expect( mockedApiFetch ).not.toHaveBeenCalled(); } ); - it( 'collapses long language lists into the first two and a count', () => { - mockTracksResult = { - managedTracks: [ - { kind: 'captions', srcLang: 'en-US', label: '', src: '' }, - { kind: 'subtitles', srcLang: 'de', label: '', src: '' }, - { kind: 'subtitles', srcLang: 'fr', label: '', src: '' }, - { kind: 'subtitles', srcLang: 'es', label: '', src: '' }, - ], - isLoading: false, - }; - render( - , - { wrapper } + it( 'waits for the playback token before rendering a private poster', async () => { + let resolveToken: ( value: { playback_token: string } ) => void = () => {}; + mockedApiFetch.mockReturnValueOnce( + new Promise< { playback_token: string } >( resolve => { + resolveToken = resolve; + } ) ); - expect( screen.getByText( 'English (US), German, and 2 more' ) ).toBeInTheDocument(); - } ); + render( , { wrapper } ); - it( 'shows None when the video has no subtitle tracks', () => { - render( - , - { wrapper } - ); + // No at all while the token is in flight — a src without the + // token would 403 and render as a broken image. + expect( screen.queryByRole( 'img' ) ).not.toBeInTheDocument(); - expect( screen.getByText( 'Subtitles' ) ).toBeInTheDocument(); - expect( screen.getByText( 'None' ) ).toBeInTheDocument(); - } ); + resolveToken( { playback_token: 'tok-123' } ); - it( 'omits the row for items without a VideoPress GUID', () => { - render( - , - { wrapper } + await waitFor( () => + expect( screen.getByRole( 'img', { name: /current thumbnail/i } ) ).toHaveAttribute( + 'src', + 'https://example.test/poster.jpg?metadata_token=tok-123' + ) ); + } ); + + it( 'renders no image when the video has no poster', () => { + render( , { wrapper } ); - expect( screen.queryByText( 'Subtitles' ) ).not.toBeInTheDocument(); + expect( screen.queryByRole( 'img' ) ).not.toBeInTheDocument(); + expect( screen.getByText( /no thumbnail yet/i ) ).toBeInTheDocument(); + // The actions are still there — a video with no poster is exactly the + // one most in need of getting one. + expect( screen.getByRole( 'button', { name: /upload image/i } ) ).toBeInTheDocument(); } ); } ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-update-button.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-update-button.test.tsx deleted file mode 100644 index c0e85aa567df..000000000000 --- a/projects/packages/videopress/src/dashboard/components/video-details/test/thumbnail-update-button.test.tsx +++ /dev/null @@ -1,108 +0,0 @@ -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; -import ThumbnailUpdateButton from '../thumbnail-update-button'; - -describe( 'ThumbnailUpdateButton', () => { - it( 'opens a popover with both items on click', async () => { - const user = userEvent.setup(); - render( - - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - expect( screen.getByRole( 'menuitem', { name: /select from video/i } ) ).toBeInTheDocument(); - expect( screen.getByRole( 'menuitem', { name: /upload image/i } ) ).toBeInTheDocument(); - } ); - - it( 'fires onSelectFromVideo when the matching item is clicked', async () => { - const user = userEvent.setup(); - const onSelectFromVideo = jest.fn(); - render( - - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /select from video/i } ) ); - expect( onSelectFromVideo ).toHaveBeenCalledTimes( 1 ); - expect( - screen.queryByRole( 'menuitem', { name: /select from video/i } ) - ).not.toBeInTheDocument(); - } ); - - it( 'fires onUploadImage when the matching item is clicked', async () => { - const user = userEvent.setup(); - const onUploadImage = jest.fn(); - render( - - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - await user.click( screen.getByRole( 'menuitem', { name: /upload image/i } ) ); - expect( onUploadImage ).toHaveBeenCalledTimes( 1 ); - expect( screen.queryByRole( 'menuitem', { name: /upload image/i } ) ).not.toBeInTheDocument(); - } ); - - it( 'disables Select from video when not allowed', async () => { - const user = userEvent.setup(); - render( - - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - // @wordpress/components MenuItem marks a disabled item with aria-disabled - // (keeping it focusable) rather than the native disabled attribute. - const item = screen.getByRole( 'menuitem', { name: /select from video/i } ); - expect( item ).toHaveAttribute( 'aria-disabled', 'true' ); - } ); - - it( 'disables Upload image when not allowed', async () => { - const user = userEvent.setup(); - render( - - ); - await user.click( screen.getByRole( 'button', { name: /update thumbnail/i } ) ); - expect( screen.getByRole( 'menuitem', { name: /upload image/i } ) ).toHaveAttribute( - 'aria-disabled', - 'true' - ); - } ); - - it( 'disables the trigger when busy', () => { - render( - - ); - expect( screen.getByRole( 'button', { name: /update thumbnail/i } ) ).toBeDisabled(); - } ); -} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/use-video-details-form.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/use-video-details-form.test.tsx new file mode 100644 index 000000000000..c468ce9f4dbf --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/use-video-details-form.test.tsx @@ -0,0 +1,86 @@ +import { act, renderHook } from '@testing-library/react'; +import { makeLibraryItem } from '../../../test-utils/library-item'; +import { useVideoDetailsForm } from '../use-video-details-form'; + +const video = makeLibraryItem( { title: 'My Clip', description: 'First cut' } ); + +describe( 'useVideoDetailsForm', () => { + it( 'starts clean, mirroring the record it was given', () => { + const { result } = renderHook( () => useVideoDetailsForm( video ) ); + + expect( result.current.isDirty ).toBe( false ); + expect( result.current.values.title ).toBe( 'My Clip' ); + expect( result.current.values.description ).toBe( 'First cut' ); + } ); + + it( 'goes dirty on update and stays dirty until reset', () => { + const { result } = renderHook( () => useVideoDetailsForm( video ) ); + + act( () => result.current.update( { title: 'My Clip 2' } ) ); + + expect( result.current.values.title ).toBe( 'My Clip 2' ); + expect( result.current.isDirty ).toBe( true ); + } ); + + // The discard path: reset with no argument restores the last baseline. + it( 'reset() restores the baseline values', () => { + const { result } = renderHook( () => useVideoDetailsForm( video ) ); + + act( () => result.current.update( { title: 'Scratch', rating: 'R' } ) ); + act( () => result.current.reset() ); + + expect( result.current.values.title ).toBe( 'My Clip' ); + expect( result.current.values.rating ).toBe( 'G' ); + expect( result.current.isDirty ).toBe( false ); + } ); + + // The save path: the stage calls reset(values) in updateMeta's onSuccess, + // which has to move the baseline forward rather than undo the edit. + it( 'reset( next ) re-baselines to the saved values', () => { + const { result } = renderHook( () => useVideoDetailsForm( video ) ); + + act( () => result.current.update( { title: 'Saved title' } ) ); + act( () => result.current.reset( result.current.values ) ); + + expect( result.current.values.title ).toBe( 'Saved title' ); + expect( result.current.isDirty ).toBe( false ); + } ); + + /* + * The re-baseline effect keys on `[ video.id ]` alone, which is + * exhaustive-deps hostile and looks like a bug. It isn't: `useVideo` + * refetches every 2s while a video is processing, and each refetch hands + * down a NEW object. Keying on the whole record would wipe whatever the + * user was mid-way through typing. This test locks that in. + */ + it( 'does not clobber in-progress edits when the same video refetches', () => { + const { result, rerender } = renderHook( + ( item: typeof video ) => useVideoDetailsForm( item ), + { + initialProps: video, + } + ); + + act( () => result.current.update( { title: 'Half-typed' } ) ); + rerender( makeLibraryItem( { title: 'My Clip', description: 'First cut' } ) ); + + expect( result.current.values.title ).toBe( 'Half-typed' ); + expect( result.current.isDirty ).toBe( true ); + } ); + + it( 're-baselines when the id changes', () => { + const { result, rerender } = renderHook( + ( item: typeof video ) => useVideoDetailsForm( item ), + { + initialProps: video, + } + ); + + act( () => result.current.update( { title: 'Half-typed' } ) ); + rerender( makeLibraryItem( { id: '43', title: 'Other clip', description: 'Other cut' } ) ); + + expect( result.current.values.title ).toBe( 'Other clip' ); + expect( result.current.values.description ).toBe( 'Other cut' ); + expect( result.current.isDirty ).toBe( false ); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/video-details-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/video-details-card.test.tsx new file mode 100644 index 000000000000..785a42e647b8 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/video-details-card.test.tsx @@ -0,0 +1,102 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { resetFeatures, setFeatures } from '../../../test-utils/features'; +import { makeLibraryItem } from '../../../test-utils/library-item'; +import VideoDetailsCard from '../video-details-card'; + +// ChaptersSummary builds its deep link through useLinkProps. +jest.mock( '@wordpress/route', () => ( { + __esModule: true, + useLinkProps: ( { to }: { to: string } ) => ( { href: to } ), +} ) ); + +const video = makeLibraryItem( { filename: 'holiday-clip.mp4' } ); +const DESCRIPTION = '00:00 Intro\n00:30 Middle'; + +const renderCard = ( overrides: { confirmNavigation?: () => boolean } = {} ) => { + const onChange = jest.fn(); + const onOpenChapters = jest.fn(); + const utils = render( + + ); + return { ...utils, onChange, onOpenChapters }; +}; + +beforeEach( () => { + setFeatures( { chaptersEditor: true } ); +} ); + +afterEach( () => { + resetFeatures(); +} ); + +describe( 'VideoDetailsCard', () => { + it( 'heads the card "Video details"', () => { + renderCard(); + + expect( screen.getByText( 'Video details' ) ).toBeInTheDocument(); + } ); + + /* + * The backstop against a future primitive swap. Both fields are reached by + * accessible name from routes/video/test/stage.test.tsx, so a change that + * drops or renames either label breaks the stage suite in a way that is + * hard to read. Fail here instead, next to the cause. + */ + it( 'exposes Title and Description by their accessible names', () => { + renderCard(); + + expect( screen.getByLabelText( 'Title' ) ).toBeInTheDocument(); + expect( screen.getByLabelText( 'Description' ) ).toBeInTheDocument(); + } ); + + it( 'reports edits to both fields through onChange', async () => { + const user = userEvent.setup(); + const { onChange } = renderCard(); + + await user.type( screen.getByLabelText( 'Title' ), '!' ); + expect( onChange ).toHaveBeenCalledWith( { title: 'My Clip!' } ); + + await user.type( screen.getByLabelText( 'Description' ), '!' ); + expect( onChange ).toHaveBeenCalledWith( { description: `${ DESCRIPTION }!` } ); + } ); + + it( 'renders the chapters summary over the current description', () => { + renderCard(); + + expect( screen.getByText( 'Chapters (2)' ) ).toBeInTheDocument(); + } ); + + // Both moved out in the YouTube Studio pass. The card is free text a + // person writes and Save commits; the file name is a fact about the upload + // and the thumbnail does not go through Save at all. + it( 'no longer carries the file name or the thumbnail control', () => { + renderCard(); + + expect( screen.queryByText( 'File name' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'holiday-clip.mp4' ) ).not.toBeInTheDocument(); + expect( screen.queryByRole( 'button', { name: /update thumbnail/i } ) ).not.toBeInTheDocument(); + } ); + + // confirmNavigation has to keep reaching ChaptersSummary: the deep link is + // the same exit as the Editor sub-nav tab, so without the guard it becomes + // a silent-discard path sitting under the description. + it( 'forwards confirmNavigation to the chapters deep link', () => { + const confirmNavigation = jest.fn( () => false ); + renderCard( { confirmNavigation } ); + + const link = screen.getByRole( 'link', { name: 'Edit chapters in the editor' } ); + const clickEvent = new MouseEvent( 'click', { bubbles: true, cancelable: true } ); + link.dispatchEvent( clickEvent ); + + expect( confirmNavigation ).toHaveBeenCalled(); + expect( clickEvent.defaultPrevented ).toBe( true ); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/test/video-info-card.test.tsx b/projects/packages/videopress/src/dashboard/components/video-details/test/video-info-card.test.tsx new file mode 100644 index 000000000000..32b4bde0c978 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/test/video-info-card.test.tsx @@ -0,0 +1,63 @@ +import { render, screen } from '@testing-library/react'; +import { makeLibraryItem } from '../../../test-utils/library-item'; +import VideoInfoCard from '../video-info-card'; + +// Variables referenced inside jest.mock() factories must be prefixed with "mock" +// (case-insensitive) to satisfy Jest's babel-jest hoisting restrictions. +const mockSuccessNotice = jest.fn(); +const mockErrorNotice = jest.fn(); +jest.mock( '@automattic/jetpack-components/global-notices', () => ( { + useGlobalNotices: () => ( { + createSuccessNotice: mockSuccessNotice, + createErrorNotice: mockErrorNotice, + } ), +} ) ); + +const baseVideo = makeLibraryItem( { + filename: 'holiday-clip.mp4', + shortcode: '[videopress abc123]', + uploadDate: '2026-01-01T00:00:00', +} ); + +beforeEach( () => { + mockSuccessNotice.mockReset(); + mockErrorNotice.mockReset(); +} ); + +describe( 'VideoInfoCard', () => { + it( 'renders the four values that address the video', () => { + render( ); + + expect( screen.getByLabelText( 'Link to video' ) ).toHaveValue( + 'https://videopress.com/v/abc123' + ); + expect( screen.getByLabelText( 'Shortcode' ) ).toHaveValue( '[videopress abc123]' ); + expect( screen.getByText( 'File name' ) ).toBeInTheDocument(); + expect( screen.getByText( 'holiday-clip.mp4' ) ).toBeInTheDocument(); + expect( screen.getByText( 'Uploaded on' ) ).toBeInTheDocument(); + } ); + + // Private videos are served from a different host, and the link is the + // thing people copy out of this card. + it( 'links a private video to video.wordpress.com', () => { + render( ); + + expect( screen.getByLabelText( 'Link to video' ) ).toHaveValue( + 'https://video.wordpress.com/v/abc123' + ); + } ); + + // Both left this card: "Add to a post or page" moved to the page header, + // and Subtitles got a card of its own on the canvas. The copy buttons stay + // — copying a read-out is part of reading it. Neither of the other two + // should quietly reappear here. + it( 'no longer carries the subtitles row or the add-to-content menu', () => { + render( ); + + expect( screen.queryByText( 'Subtitles' ) ).not.toBeInTheDocument(); + expect( screen.queryByRole( 'button', { name: /manage subtitles/i } ) ).not.toBeInTheDocument(); + expect( + screen.queryByRole( 'button', { name: /add to a post or page/i } ) + ).not.toBeInTheDocument(); + } ); +} ); diff --git a/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-card.tsx b/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-card.tsx index 67f03f6b5ad7..d6d33f3f2a17 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-card.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-card.tsx @@ -1,158 +1,51 @@ -import { useGlobalNotices } from '@automattic/jetpack-components/global-notices'; -import { useCopyToClipboard } from '@wordpress/compose'; -import { dateI18n, getSettings as getDateSettings } from '@wordpress/date'; -import { __, _n, sprintf } from '@wordpress/i18n'; -import { copy } from '@wordpress/icons'; -import { Button, Card, IconButton, InputControl, Stack, Text } from '@wordpress/ui'; +import { __ } from '@wordpress/i18n'; +import { media, upload } from '@wordpress/icons'; +import { Card, Field, Skeleton, Stack, Text } from '@wordpress/ui'; import { useState } from 'react'; -import { useVideoTracks } from '../../../client/components/caption-manager-modal/use-video-tracks'; -import { - getLanguageDisplayName, - getManualLanguageTagFromTrackKey, -} from '../../../client/lib/video-tracks/language'; +import { usePosterUrl } from '../../hooks/use-poster-url'; import { useUpdateVideoPoster } from '../../hooks/use-update-video-poster'; import { selectImageFromMediaLibrary } from '../../utils/select-image-from-media-library'; import SelectFrameDialog from './select-frame-dialog'; -import ThumbnailUpdateButton from './thumbnail-update-button'; +import ThumbnailTile from './thumbnail-tile'; import type { LibraryItem } from '../../types/library'; import type { ReactElement } from 'react'; type Props = { video: LibraryItem; - onAddToNewPost: () => void; - onManageSubtitles: () => void; -}; - -const dateSettings = getDateSettings(); - -// The Subtitles row lists this many languages before collapsing into "and N more". -const MAX_SUBTITLE_LANGUAGES_SHOWN = 2; - -const linkForVideo = ( video: LibraryItem ): string => { - const host = video.isPrivate ? 'video.wordpress.com' : 'videopress.com'; - return `https://${ host }/v/${ video.guid || video.id }`; -}; - -/** - * Icon-only button that copies its `text` prop to the clipboard. Uses - * `@wordpress/compose`'s `useCopyToClipboard` (clipboard.js under the hood) - * so it falls back to `document.execCommand('copy')` on non-secure origins — - * the native `navigator.clipboard` API is undefined on plain HTTP, which - * the dev environments here run on. Posts a success snackbar via the - * dashboard's GlobalNotices store on every successful copy. - * - * @param props - Component props. - * @param props.text - The string to write to the clipboard on click. - * @param props.fieldLabel - Human-readable name of the field being copied, - * used in the success snackbar. - * @return The icon-button element. - */ -const CopyIconButton = ( { - text, - fieldLabel, -}: { - text: string; - fieldLabel: string; -} ): ReactElement => { - const { createSuccessNotice } = useGlobalNotices(); - const ref = useCopyToClipboard( text, () => - createSuccessNotice( - sprintf( - /* translators: %s: name of the copied field, e.g. "Link to video". */ - __( '%s copied to clipboard.', 'jetpack-videopress-pkg' ), - fieldLabel - ) - ) - ); - return ( - - ); }; +// A poster can only be replaced on a transcoded VideoPress item: local +// attachments have no poster endpoint, and one still being processed has no +// stable frame to cut from. const canEditThumbnail = ( video: LibraryItem ): boolean => video.type === 'videopress' && ! video.isProcessing && Boolean( video.sourceUrl || video.guid ); /** - * Top-of-page card on the Video details screen (below the preview player). - * Renders the "Update thumbnail" and "Add video to new post" actions, two - * read-only copy fields (Link to video, Shortcode) using InputControl + - * IconButton suffix, and metadata rows (File name, Uploaded on, Subtitles - * with a Manage action). + * The card proper. Split from the guard below so the poster query, the + * mutation and the dialog state only exist while the card is actually on + * screen — a `canEditThumbnail` guard placed after the hooks would change the + * hook count the moment a video finishes processing. * - * @param props - Component props. - * @param props.video - The current video record. - * @param props.onAddToNewPost - Click handler for the secondary action. - * @param props.onManageSubtitles - Opens the caption manager. + * @param props - Component props. + * @param props.video - The current video record. * @return The card element. */ -export default function ThumbnailCard( { - video, - onAddToNewPost, - onManageSubtitles, -}: Props ): ReactElement { - const link = linkForVideo( video ); - - /* - * The media REST item omits `tracks`, so the languages come from the same - * video-info query the caption manager uses. - */ - const { managedTracks, isLoading: isLoadingTracks } = useVideoTracks( { - guid: video.guid ?? '', - isOpen: !! video.guid, - isPrivate: video.isPrivate, - tracks: video.tracks, - } ); - - const subtitleLanguages = [ - ...new Set( - managedTracks - .filter( track => track.kind === 'captions' || track.kind === 'subtitles' ) - .map( - track => - track.label || - getLanguageDisplayName( - getManualLanguageTagFromTrackKey( track.srcLang ) || track.srcLang - ) - ) - ), - ]; - const shownLanguages = subtitleLanguages.slice( 0, MAX_SUBTITLE_LANGUAGES_SHOWN ).join( ', ' ); - const moreLanguagesCount = subtitleLanguages.length - MAX_SUBTITLE_LANGUAGES_SHOWN; - let subtitleSummary = shownLanguages || __( 'None', 'jetpack-videopress-pkg' ); - if ( moreLanguagesCount > 0 ) { - subtitleSummary = sprintf( - /* translators: 1: list of subtitle language names. 2: how many further languages exist. */ - _n( - '%1$s, and %2$d more', - '%1$s, and %2$d more', - moreLanguagesCount, - 'jetpack-videopress-pkg' - ), - shownLanguages, - moreLanguagesCount - ); - } - const { createSuccessNotice, createErrorNotice } = useGlobalNotices(); +function EditableThumbnailCard( { video }: Props ): ReactElement { const updatePoster = useUpdateVideoPoster(); + const posterUrl = usePosterUrl( video ); const [ dialogOpen, setDialogOpen ] = useState( false ); - const notifyResult = { - onSuccess: () => createSuccessNotice( __( 'Thumbnail updated.', 'jetpack-videopress-pkg' ) ), - onError: () => - createErrorNotice( __( 'Failed to update thumbnail.', 'jetpack-videopress-pkg' ) ), - }; + /* + * `usePosterUrl` returns null both when there is no poster at all and + * while a private video's playback token is still being minted — it can't + * tell the caller which. A stored `thumbnailUrl` with no resolved URL is + * the second case, and the only one worth a loading state. + */ + const isPosterPending = Boolean( video.thumbnailUrl ) && ! posterUrl; const handleConfirmFrame = ( atTimeMs: number ) => { setDialogOpen( false ); - updatePoster.mutate( - { id: video.id, guid: video.guid, source: 'frame', atTimeMs }, - notifyResult - ); + updatePoster.mutate( { id: video.id, guid: video.guid, source: 'frame', atTimeMs } ); }; const handleUploadImage = async () => { @@ -160,98 +53,76 @@ export default function ThumbnailCard( { if ( ! attachment ) { return; } - updatePoster.mutate( - { id: video.id, guid: video.guid, source: 'attachment', attachmentId: attachment.id }, - notifyResult - ); + updatePoster.mutate( { + id: video.id, + guid: video.guid, + source: 'attachment', + attachmentId: attachment.id, + } ); }; - const showUpdateButton = canEditThumbnail( video ); - const isUpdating = updatePoster.isPending; + const isBusy = updatePoster.isPending; return ( + + { __( 'Thumbnail', 'jetpack-videopress-pkg' ) } + - - - { showUpdateButton && ( - setDialogOpen( true ) } - onUploadImage={ handleUploadImage } - /> - ) } - - - - - } - /> - - - } - /> - - - - { __( 'File name', 'jetpack-videopress-pkg' ) } - - { video.filename } + { /* + * No Field.Label — the card title already names this. The + * Field.Root/Field.Description pair is kept so the caption gets + * the same treatment as the help text under every other control + * on the page. + */ } + + + { __( 'Pick the still that represents this video.', 'jetpack-videopress-pkg' ) } + + + { /* + * The current poster, shown at the same 16:9 size as the + * two actions so the row reads as a set. Solid-bordered + * rather than dashed: it is content, not an affordance. + */ } +
+ { posterUrl && ( + { + ) } + { isPosterPending && } + { ! posterUrl && ! isPosterPending && ( + + { __( 'No thumbnail yet', 'jetpack-videopress-pkg' ) } + + ) } +
+ + setDialogOpen( true ) } + />
- - - - { __( 'Uploaded on', 'jetpack-videopress-pkg' ) } - - { dateI18n( dateSettings.formats.date, video.uploadDate ) } - - - { video.guid && ( - - - { __( 'Subtitles', 'jetpack-videopress-pkg' ) } - - - - { isLoadingTracks ? __( 'Loading…', 'jetpack-videopress-pkg' ) : subtitleSummary } - - - - - ) } -
+ + { __( + 'Applies immediately — everything else on this page waits for Save.', + 'jetpack-videopress-pkg' + ) } + +
); } + +/** + * The Thumbnail card: the still this video is currently represented by, and + * the two ways to replace it — Upload image (WP media modal) and Select from + * video (frame scrubber) — as a row of 16:9 tiles. + * + * Tiles rather than a dropdown because the choice is visual. Both actions + * produce an image, the current one is sitting right there to compare + * against, and a menu hides all of that behind a label. This is the shape + * YouTube Studio uses for the same two actions. + * + * It is a card of its own rather than a row inside Video details because this + * is the one control on the screen that does NOT go through Save: the poster + * endpoint is keyed by GUID while the meta patch is keyed by attachment id, + * and the mutation polls for up to 60s. Folding it into Save would mean a + * button that either spins for a minute or claims success while the poster is + * still generating. Two honest save models beat one dishonest one, and a card + * boundary says so more clearly than a hairline rule. + * + * Renders nothing when the video can't take a new poster. A disabled control + * would be wrong: there is no user action that would enable it. + * + * @param props - Component props. + * @param props.video - The current video record. + * @return The card, or null when the video can't take a new poster. + */ +export default function ThumbnailCard( { video }: Props ): ReactElement | null { + if ( ! canEditThumbnail( video ) ) { + return null; + } + + return ; +} diff --git a/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-tile.tsx b/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-tile.tsx new file mode 100644 index 000000000000..d54c9a965ff7 --- /dev/null +++ b/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-tile.tsx @@ -0,0 +1,55 @@ +import { Icon } from '@wordpress/components'; +import { Button, Text } from '@wordpress/ui'; +import type { ReactElement } from 'react'; + +type Props = { + icon: Parameters< typeof Icon >[ 0 ][ 'icon' ]; + label: string; + disabled?: boolean; + onClick: () => void; +}; + +/** + * One action tile in the Thumbnail card: a 16:9 dashed frame with an icon + * above a centred label, sized to match the poster preview beside it. + * + * Hand-rolled rather than composed, because the design system has nothing for + * this. `@wordpress/ui` ships no media picker, upload tile or dashed + * container — the only `dashed` rule in the package is inside a Popover + * story — and there is no `border-style` token. `EmptyState` is the closest + * shape (icon → title → description) but it is a centred full-width empty + * state, not a picker, and carries no dashed treatment. `FormFileUpload` + * opens a raw file input, where these two actions need the WP media modal and + * the frame scrubber. `MediaPlaceholder` is the canonical WordPress dashed + * box, but it lives in `@wordpress/block-editor` and is editor-canvas + * furniture. + * + * So the interaction is borrowed and only the surface is local: `Button` with + * `variant="unstyled"` keeps the real + ); +} diff --git a/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-update-button.tsx b/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-update-button.tsx deleted file mode 100644 index 849a3f35cc8a..000000000000 --- a/projects/packages/videopress/src/dashboard/components/video-details/thumbnail-update-button.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import { DropdownMenu, MenuGroup, MenuItem } from '@wordpress/components'; -import { __ } from '@wordpress/i18n'; -import { cloud, media, pencil } from '@wordpress/icons'; -import type { ReactElement } from 'react'; - -type Props = { - canSelectFromVideo: boolean; - canUploadImage: boolean; - isBusy: boolean; - onSelectFromVideo: () => void; - onUploadImage: () => void; -}; - -/** - * Button that opens the thumbnail-update menu with two actions: - * "Select from video" and "Upload image". Uses the same `@wordpress/components` - * DropdownMenu as the page header's ⋯ menu so both menus share the design - * system's look — including MenuItem's default right-aligned icons. Rendered - * inline next to the "Add video to new post" action inside ThumbnailCard. - * - * @param props - Component props. - * @param props.canSelectFromVideo - Whether the frame-picker action is available. - * @param props.canUploadImage - Whether the media-library upload action is available. - * @param props.isBusy - When true, the trigger button is disabled. - * @param props.onSelectFromVideo - Called when the user chooses "Select from video". - * @param props.onUploadImage - Called when the user chooses "Upload image". - * @return The dropdown button element. - */ -export default function ThumbnailUpdateButton( { - canSelectFromVideo, - canUploadImage, - isBusy, - onSelectFromVideo, - onUploadImage, -}: Props ): ReactElement { - return ( - - { ( { onClose } ) => ( - - { - onSelectFromVideo(); - onClose(); - } } - > - { __( 'Select from video', 'jetpack-videopress-pkg' ) } - - { - onUploadImage(); - onClose(); - } } - > - { __( 'Upload image', 'jetpack-videopress-pkg' ) } - - - ) } - - ); -} diff --git a/projects/packages/videopress/src/dashboard/components/video-details/video-details-card.tsx b/projects/packages/videopress/src/dashboard/components/video-details/video-details-card.tsx index 9fe556e37b76..16961aac1c72 100644 --- a/projects/packages/videopress/src/dashboard/components/video-details/video-details-card.tsx +++ b/projects/packages/videopress/src/dashboard/components/video-details/video-details-card.tsx @@ -6,7 +6,7 @@ import type { LibraryItem } from '../../types/library'; import type { ReactElement } from 'react'; type Props = { - video: Pick< LibraryItem, 'id' >; + video: LibraryItem; title: string; description: string; onChange: ( partial: { title?: string; description?: string } ) => void; @@ -15,13 +15,17 @@ type Props = { }; /** - * Form card for the editable text fields: title and description, plus the - * compact chapters summary (count derived from the description, a deep link - * into the Editor tab, and the Chapters help modal link — see - * chapters-summary.tsx). + * The words this video is described by: the title, the description, and the + * chapters that description produces. + * + * Everything here is free text a person writes and Save commits, which is the + * whole membership rule. Two things that used to sit in this card have left + * on that basis — the file name, which is a fact about the upload rather than + * something anyone writes, and the thumbnail control, which does not go + * through Save and now shows the poster it replaces. * * @param props - Component props. - * @param props.video - The video (id for the editor deep link). + * @param props.video - The current video record. * @param props.title - Current title value. * @param props.description - Current description value. * @param props.onChange - Partial-update handler from the form hook. @@ -50,6 +54,20 @@ export default function VideoDetailsCard( { value={ title } onValueChange={ next => onChange( { title: next } ) } /> + { /* + * Deliberately `@wordpress/components`' TextareaControl and + * not `@wordpress/ui`'s Textarea primitive. The two label + * styles already agree — `@wordpress/components` + * src/utils/base-label.ts is 11px / fontWeightEmphasis / + * uppercase, and `@wordpress/ui` + * src/utils/css/field.module.css gives Field.Label + * font-size-xs (11px), font-weight-emphasis, uppercase — so + * there is nothing to fix by swapping, and `@wordpress/ui` + * has no TextareaControl equivalent to swap to. (Its + * Textarea primitive is `use-with-caution`, but so are the + * Field and InputControl in this same Stack, so that is not + * a reason to single it out.) Leave this be. + */ } { + const host = video.isPrivate ? 'video.wordpress.com' : 'videopress.com'; + return `https://${ host }/v/${ video.guid || video.id }`; +}; + +/** + * Icon-only button that copies its `text` prop to the clipboard. Uses + * `@wordpress/compose`'s `useCopyToClipboard` (clipboard.js under the hood) + * so it falls back to `document.execCommand('copy')` on non-secure origins — + * the native `navigator.clipboard` API is undefined on plain HTTP, which + * the dev environments here run on. Posts a success snackbar via the + * dashboard's GlobalNotices store on every successful copy. + * + * @param props - Component props. + * @param props.text - The string to write to the clipboard on click. + * @param props.fieldLabel - Human-readable name of the field being copied, + * used in the success snackbar. + * @return The icon-button element. + */ +const CopyIconButton = ( { + text, + fieldLabel, +}: { + text: string; + fieldLabel: string; +} ): ReactElement => { + const { createSuccessNotice } = useGlobalNotices(); + const ref = useCopyToClipboard( text, () => + createSuccessNotice( + sprintf( + /* translators: %s: name of the copied field, e.g. "Link to video". */ + __( '%s copied to clipboard.', 'jetpack-videopress-pkg' ), + fieldLabel + ) + ) + ); + return ( + + ); +}; + +/** + * The panel beside the player: what this video is addressed by, and where it + * came from. Link to video and Shortcode each carry a copy button; the file + * name and upload date are read-outs. + * + * These four are grouped because they identify the video rather than describe + * it — the same split YouTube Studio draws, where the preview column holds + * the link, the file name and the upload state while every editable field + * sits in the main column. The file name in particular reads as a fact about + * the upload, not as something adjacent to the title a person chooses. + * + * Read-outs only, deliberately. Two things have left this card: "Add to a + * post or page", which is an action and moved to the page header next to + * Save, and Subtitles, which is a feature and got a card of its own on the + * canvas — see subtitles-card.tsx. + * + * @param props - Component props. + * @param props.video - The current video record. + * @return The card element. + */ +export default function VideoInfoCard( { video }: Props ): ReactElement { + const link = linkForVideo( video ); + + return ( + + + { __( 'Video info', 'jetpack-videopress-pkg' ) } + + + + + } + /> + + + } + /> + + { /* + * Read-outs, not controls, so `nativeLabel={ false }` and a + * : there is no form element for a + + + ); +} diff --git a/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts b/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts index 49bfd47f8690..9766596ac6fa 100644 --- a/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts +++ b/projects/packages/videopress/src/dashboard/hooks/use-update-video-poster.ts @@ -1,5 +1,7 @@ +import { useGlobalNotices } from '@automattic/jetpack-components/global-notices'; import { useMutation, useQueryClient } from '@tanstack/react-query'; import apiFetch from '@wordpress/api-fetch'; +import { __ } from '@wordpress/i18n'; import { LIBRARY_ITEM_QUERY_SEGMENT, LIBRARY_QUERY_KEY } from './use-library'; import type { LibraryItem } from '../types/library'; @@ -84,10 +86,19 @@ async function mutationFn( vars: UpdatePosterVars ): Promise< { poster?: string * updates the cached item directly so the UI reflects the new poster without * an extra refetch. * + * Notices fire from the hook rather than from per-call `onSuccess`/`onError` + * options because the caller can legitimately unmount mid-flight: the control + * is gated on `! isProcessing`, and `isProcessing` is itself derived from the + * poster being absent — which is exactly the state this mutation creates while + * it polls. A per-call callback dies with the observer, so the poster would + * change with no feedback at all. Mutation-level callbacks survive it. + * * @return A TanStack Query UseMutationResult for UpdatePosterVars. */ export function useUpdateVideoPoster() { const client = useQueryClient(); + const { createSuccessNotice, createErrorNotice } = useGlobalNotices(); + return useMutation< { poster?: string }, Error, UpdatePosterVars >( { mutationFn, onSuccess: ( { poster }, vars ) => { @@ -98,6 +109,10 @@ export function useUpdateVideoPoster() { ); } client.invalidateQueries( { queryKey: [ LIBRARY_QUERY_KEY ] } ); + createSuccessNotice( __( 'Thumbnail updated.', 'jetpack-videopress-pkg' ) ); + }, + onError: () => { + createErrorNotice( __( 'Failed to update thumbnail.', 'jetpack-videopress-pkg' ) ); }, } ); }