VideoPress: Modernization Phase 2 — Library tab (DataViews grid + table)#48586
VideoPress: Modernization Phase 2 — Library tab (DataViews grid + table)#48586
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
d149a48 to
8fca223
Compare
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage Summary8 files are newly checked for coverage. Only the first 5 are listed here.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
95d623d to
2b21410
Compare
| @@ -0,0 +1,66 @@ | |||
| import { Button, ProgressBar } from '@wordpress/components'; | |||
There was a problem hiding this comment.
Recommend Button from @wordpress/ui instead.
| <span className="vp-library__thumbnail-duration-badge"> | ||
| { formatDuration( durationSeconds ) } | ||
| </span> |
There was a problem hiding this comment.
Looks like Badge from @wordpress/ui could be suitable here, just not sure what to do with color.
| callback: items => { | ||
| api.deleteItems( items.map( i => i.id ) ); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
UX comment — this and others might benefit from feedback after action, so I'd add a Snackbar notice here.
Would be good to get upstream Gutenberg issues for most of these, particularly to avoid styles in |
| label: __( 'Uploaded', 'jetpack-videopress-pkg' ), | ||
| type: 'datetime', | ||
| getValue: ( { item } ) => item.uploadDate, | ||
| render: ( { item } ) => dateI18n( dateSettings.formats.date, item.uploadDate ), |
There was a problem hiding this comment.
I'm not sure, but format might also use date settings already so you wouldn't need render at all..
| * @param bytes - Total bytes. | ||
| * @return Formatted size. | ||
| */ | ||
| export function formatBytes( bytes: number ): string { |
There was a problem hiding this comment.
Not a biggie, but there might already be a helper around Jetpack for this.
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| gap: 8px; |
There was a problem hiding this comment.
You might be able to use Stack instead of these styles.
| font-size: 16px; | ||
| font-weight: 600; |
There was a problem hiding this comment.
Might need Text component instead.
| align-items: center; | ||
| justify-content: center; | ||
| gap: 6px; | ||
| background: rgba(204, 24, 24, 0.85); |
There was a problem hiding this comment.
Should probably pull style token for "error" background + foreground colors. Or use Notice component?
| display: flex; | ||
| flex-direction: column; | ||
| align-items: center; | ||
| justify-content: center; | ||
| gap: 6px; |
There was a problem hiding this comment.
Might be able to use Stack for all these styles.
| font-size: 12px; | ||
| color: #757575; | ||
| line-height: 1.4; |
There was a problem hiding this comment.
Looks like bit like Text would do this already.
Two layout fixes for the Phase 2 Library tab: * Replace `min-height: calc(100vh - 220px)` on `.vp-library__viewport` with `flex: 1; min-height: 0`, and propagate `display: flex` through Tabs.Root + Tabs.Panel (Base UI primitives default to `display: block`, breaking the flex chain). Pagination footer now flexes to the visible bottom regardless of the chrome above. Targeted via stable ARIA roles, not the obfuscated CSS-in-JS classnames. * Lock `.vp-library__thumbnail` to 32x32 in table view. The DataViews cell wrapper applies `align-items: center`, so an img with non-square intrinsic ratio (e.g. 32x18 for 16:9) collapses the wrapper to its content height, leaving white space above and below. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…selectors
Replace third-party-private classnames with selectors anchored to either
the public `view.type` API or our own class hierarchy:
* `.dataviews-wrapper` → `> *` (the viewport only ever wraps a single
DataViews instance).
* `.dataviews-view-table` → `&__viewport--table`, mirrored from `view.type`
via a new modifier class on the viewport in stage.tsx.
* `.dataviews-view-grid` → `&__viewport--grid` (same mechanism).
* `.boot-layout-container .boot-layout &__thumbnail-image` (specificity
0,2,1) → `&__viewport &__thumbnail &__thumbnail-image` (specificity
0,3,0). Beats wp-build's `img { height: auto }` reset by chaining our
own classes instead of naming the wp-build classes.
Behavior is unchanged; verified live in both grid and table views (img
still 32x32 in table view, pagination still pinned to viewport bottom).
The Library route's SCSS no longer references any @wordpress/dataviews
or wp-build private classnames, so future renames in those packages
won't break this tab.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ref p1HpG7-wSr-p2 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… render DataViews' built-in datetime field type formats values via field.format.datetime + dateI18n internally, so the custom render call is redundant. Drop it and pass the desired format on the field instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns ThumbnailField with the design-system migration already underway across modernized dashboards. variant="secondary" -> variant="outline"; the primary "Retry" relies on the wp/ui Button defaults (solid + brand). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the custom .vp-library__status-pill SCSS (uploading / failed / local) with wp/ui Badge driven by intent (informational / high / none) — colors and dark-mode behaviour now ride the design tokens. The title cell becomes a Stack with a stable className so the badge can still be hidden in grid view. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Move the bespoke flex centering and font sizing on .vp-library__placeholder / __hover-action / __progress / __failed onto wp/ui Stack wrappers, and render the filename cell with Text body-sm. SCSS keeps positioning, background, and truncation — typography and layout now ride the design tokens. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires @automattic/jetpack-components' GlobalNotices/useGlobalNotices into the Library route so Delete and "Make public/private/use site default" emit a success snackbar. Promote/Retry stay silent — the per-card progress overlay already conveys the state. Adds a global-notices sub-path export to the shared package so wp-build dashboards can pull it in alongside admin-page without dragging in the rest of the bundle. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops the `variant="minimal"` on `<Tabs.List>` so we get the canonical 16px per-tab inline padding and no tablist gap. `admin-page-layout` still bumps `[role="tab"]` to 24px for header-edge alignment; the local override walks that back to the lg token (16px) and offsets the tablist by the missing 8px, so the first tab's text edge still sits under the page title (8px tablist + 16px tab = 24px). The hairline stays full-width on `.jp-admin-page-tabs`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drops `isPrimary: true` from the `type` field so it joins `privacy` under the filter icon instead of always showing as a pinned chip. With only one primary filter, the dropdown looked empty and the filter icon read as disabled — now both filters live behind the same affordance. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes the placeholder Save action from the Settings header. The toggle is local-only state for Phase 1; persistence will land in Phase 6, at which point we can settle on the right UX (likely save-on-change rather than a discrete button). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
d9e6c48 to
6bc08c3
Compare
Part of #48506
This PR is intended for visual changes only, for design review. All data is mocked.
This is Phase 2 of the VideoPress dashboard modernization tracking issue (#48506) — the Library tab. UI-only with mocked data, all behind the existing
rsm_jetpack_ui_modernization_videopressfilter (Phase 0). Subsequent phases each open their own PR.Proposed changes
@wordpress/dataviewsv14 grid (default) with a table alternate. Layout switcher in the toolbar; grid is the primary mode for visual browsing, table is dense info.useMockLibrary()(src/dashboard/hooks/use-mock-library.ts) returns 50 programmatically-generated fixtures (~80% VideoPress, ~20% Local) with a 1-second initial-loading window, a 10-second per-item upload simulation, and a deterministic every-5th-upload-fails-at-60% failure path with aRetryrecovery action. The hook signature matches the TanStack Query hook Phase 6 will swap in — Phase 6 is a one-file change.ThumbnailField(src/dashboard/components/Library/ThumbnailField.tsx) layers four states on the DataViews grid card: idle VideoPress (thumbnail + duration badge), idle Local (gray "Local video" placeholder + hover-revealedUpload to VideoPressbutton), uploading (light overlay + percentage +ProgressBar), failed (red overlay +Retrybutton).Local/Uploading 47%/Upload failed) instead of the larger card overlays — the table-cell thumbnail is too small to host the full chrome. Pills only render in the table view; in the grid the per-card overlays already convey the same info.thumbnail(media),title,filename,type(primary filter, Local / VideoPress),uploadDate(default sort, desc),duration,privacy(secondary filter),fileSize. Default visible columns differ between layouts: grid showsfilenamebelow the title; switching to table revealsfilename,duration,fileSize,uploadDate,privacy.Edit details(primary, no-op until Phase 4 wires the details screen), three top-levelMake public/Make private/Use site defaultactions (DataViews has no nested-submenu API in v14),Delete,Upload to VideoPress(Local only),Retry(failed only). Bulk:Trashonly —isEligiblefilters out Local items, so mixed-selection bulk-Trash silently skips them while keeping them selectable (matches the Figma mock).Upload videobutton wires intoAdminPage'sactionsslot, opens a hidden<input type="file" accept="video/*">, and prepends an "uploading" card to the grid on file pick. Uses@wordpress/uiButton size="compact"to match Settings's Save button so there's no layout shift on tab change.DashboardLayoutgains ahideFooterprop. Library opts in (per the design directive: pages with a central DataViews component don't show the JetpackFooter). Overview and Settings keep the footer.@wordpress/dataviews@14.1.0as a direct dep of@automattic/jetpack-videopress(and the library route'spackage.json) so wp-build's externals plugin can resolve it. SCSS pulls in DataViews's stylesheet via@include meta.load-css("@wordpress/dataviews/build-style/style.css")— same pattern Forms uses (projects/packages/forms/routes/shared.scss).Related product discussion/links
Does this pull request change what data or activity we track or use?
No. UI-only changes; all data is mocked client-side, no API calls, no analytics events, no tracked data added or changed.
Testing instructions
wp eval:wp-admin/admin.php?page=jetpack-videopress→ click the Library tab.Edit details,Make public/Make private/Use site default,Delete. Local →Upload to VideoPress. Failed →Retry.Upload to VideoPress→ ~10sProgressBarclimbs → card flips to a regular VideoPress card with a thumbnail.Upload videoand per-card promotions). Every 5th upload session deterministically stalls at ~60% with a redUpload failedoverlay; clickRetry→ restarts and succeeds.Upload video(top-right) → file picker → select any local video file → a newLocal-typed card prepends to the grid inuploadingstate → progresses → resolves into a VideoPress card.Trashin the bulk-action bar → only the VideoPress cards are removed; the Local card remains selected (DataViews'sisEligiblesilently skips ineligible items).Typechip is always visible — toggle toLocal(only Local cards) orVideoPress(only VP cards).Privacyfilter appears (Public/Private/Site default); intersects with the type filter.Title,Filename,Duration,File size,Uploaded,Privacy); status pills appear next to the title forLocal/Uploading XX%/Upload failedrows. Switch back to grid.add_filterand confirm the legacy VideoPress dashboard still loads at the same URL.Out of scope (parked for follow-up)
Tabs.Root+Tabs.Panelbut the chain breaks aboveTabs.RootbecauseAdminPage's wrapper isn't itself a flex container.TODOnote inDashboardLayout/style.scssflags it for Phase 2.5..boot-layout-container .boot-layout img { height: auto }reset is winning specificity even with the matching.boot-layout-container .boot-layoutprefix on our rule. Needs DevTools to pin down the exact override path.class-initial-state.phpseeding — Phase 5 (originally scoped into Phase 1, deferred since neither Phase 1 nor 2 has a client-side consumer).