VideoPress: Modernization Phase 3 — Overview (mocked) - #48682
Merged
Conversation
dhasilva
added
[Status] Needs Review
dhasilva
added
[Package] VideoPress
Coverage tests to be added later
17 tasks
Contributor
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Contributor
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Code Coverage SummaryCoverage changed in 1 file.
10 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
|
dhasilva
force-pushed
the
update/videopress-modernization-phase-3
branch
from
May 11, 2026 16:51
fbdedd4 to
030420c
Compare
Contributor
|
Looking great, @dhasilva! Adding some of the things I noticed: Overview page
Header
Detailed view
Settings
|
dhasilva
force-pushed
the
update/videopress-modernization-phase-4
branch
from
May 13, 2026 20:27
8ca5bee to
3df4a3f
Compare
…iew screen Adds the data layer the modernized Overview tab will consume. A deterministic 730-day raw daily series feeds derivations for KPIs, the trends chart (days / weeks / months), and per-row totals on Most viewed and Top locations. The hook follows useMockLibrary's window-attached singleton pattern so cross-bundle re-mounts share state. Hook signature matches the TanStack Query API Phase 6 will swap in. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three stat cards (Views, Visitors, Watch time) with formatted values and
a signed delta indicator driven by the KpiSummary { current, previousPeriod }
shape from useMockStats. Delta uses a custom span with up/down classes —
@wordpress/ui Badge's `intent` vocabulary doesn't include success/error
semantics; the green/red palette lives in the page stylesheet (Task 7).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mini-table with title -> /video/$id Link rows (reuses Phase 4) and a "See all videos" -> /library footer Link. Renders 5 skeleton rows during the initial mock load so the card height does not collapse. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Mini-table of regional-indicator flag emoji + country name + view count. No row links and no "See all locations" footer — there is no locations route yet; Phase 6 may add one. flag-emoji.ts is a 12-line port of the forms package's helper to avoid a cross-package dependency. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
DropdownMenu pill rendered in DashboardLayout's header actions slot. Four presets: Last 7 / 30 / 90 / 365 days. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@automattic/charts LineChart fed with up to 3 series (Views, Visitors, Previous period) per the chart-compare selection. The responsive LineChart auto-wraps in a GlobalChartsProvider when not already inside one, so no explicit theme provider is needed; we just reserve a 240px parent height. Two right-aligned SelectControls (visually hidden labels) control Compare and Granularity. Bumps the package's top-level package.json with @automattic/charts so the dashboard tree can import LineChart; the route's package.json gets the same dep + its peers in the Phase 3 wiring commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Bumps the route's deps to pull in @automattic/charts, @wordpress/theme,
@wordpress/components, and @wordpress/icons; replaces the placeholder
stage.tsx with the composed Overview screen (KPI row + ViewsTrends +
Most viewed / Top locations); adds the page-level grid, KPI delta
palette, and ranking-table styles in style.scss. The date-range pill
rides DashboardLayout's actions slot so it only shows on /overview.
Also pins LineChart's `withGradientFill={false}` — the chart's TS type
requires it explicitly even though the runtime default is the same.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three issues from the first manual smoke against the Figma:
1. KPI values rendered tiny because @wordpress/ui Text takes `variant`,
not `size`/`weight` — the props were silently ignored and the value
fell back to body-md (~14px). Switching to `variant="heading-2xl"`
plus a 36px override on `.vp-overview__kpi-value` so the value
dominates the card the way the Figma shows it.
2. The up/down arrow icons rendered black instead of inheriting the
delta's green/red. The @wordpress/primitives SVG doesn't set
`fill="currentColor"`, so the path falls back to the browser default;
added `svg { fill: currentColor }` under `.vp-overview__kpi-delta`.
3. Most viewed and Top locations rows were missing the persistent row
background the Figma shows — the SCSS only applied a fill on hover.
Moved the fill onto the base `td` (with radii on first/last cells so
each row reads as a pill) and kept a slightly darker hover state.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small changes prevent the chart from re-measuring itself into oblivion:
- Import @automattic/charts/style.css so the package's
`chart-layout__content svg { display: block; }` rule applies. Without it,
the inline SVG's baseline descender pixel ratchets the measured content
height upward on every render until the y-axis collapses.
- Pass an explicit `height` prop to <LineChart> so the responsive wrapper
has a bounded box to measure against rather than resolving to `100%`.
- Memoise chartData so legend registration doesn't refire on every parent
re-render.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both cards rendered identical div-based ranking chrome (header, percentage-bar row backgrounds, 5-row skeleton, barStyle helper, count formatter). RankingCard now owns that scaffolding. The per-card files describe only what's unique: their row-label renderer and (for Most viewed) the "See all videos" footer. The matching SCSS migration from `.vp-overview__ranking-table` table styles to `.vp-overview__ranking` div styles lands here too so markup and CSS stay together. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Below the wp-admin 782px breakpoint, surrounding chrome no longer provides gutters, so the cards visibly clip the viewport edge. Mirror the Library tab's edge inset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each KPI card on the Overview now acts as a tab. Selecting one switches
the trends chart's primary series, title, and y-axis formatting:
- Views / Visitors plot daily counts; the compare selector offers the
other count metric (vs visitors / vs views) and / or previous period.
- Watch time plots seconds with a duration tick formatter ("8 min",
"1.5 h"); the compare selector hides because no other metric shares
its unit, leaving "previous period" as the only meaningful overlay.
Cards carry semantic state via aria-pressed; visual treatment of the
active state lands in the next commit.
The mock fixture grows three per-day fields (previousPeriodVisitors,
watchTimeSeconds, previousPeriodWatchTimeSeconds) so all three metrics
have data to plot. Real data swaps in during Phase 6.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Promotes the KPI row to a proper W3C tabs pattern instead of three loose buttons: - Row gets role="tablist" + aria-orientation + aria-label. - Each card gets role="tab" with roving tabindex (active = 0, others = -1), aria-selected, aria-controls, and a stable id. Click / Enter / Space activates; Left / Right / Home / End move focus without selecting (manual activation). - Trends card becomes role="tabpanel" with id + aria-labelledby pointing to the active tab. Visual treatment: a 3px bottom indicator using the same --wpds-color-stroke-interactive-neutral-strong token as the @wordpress/ui Tabs primitive, so the active card matches the visual language of the dashboard's existing Overview / Library / Settings tab strip. A faint hover preview cues clickability on inactive cards. The dashboard layout's [role="tabpanel"] selector was over-broad and collapsed our nested chart tabpanel to zero height once siblings filled the column. Scope it to .vp-dashboard-tabs > [role="tabpanel"] so nested tabpanels inside any route stay safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three loading-state polish items the design review surfaced: - Chart card now shows a centered Spinner instead of an empty 240px void while data loads. - KPI cards swap their em-dash placeholder for a sized skeleton block (matches the bottom cards' loading vocabulary and reserves the hero-number's vertical space, so values land in place without a shift when data arrives). - `.vp-overview` claims its parent's full available width regardless of child content size. Without this, flex item shrink-to-content made the loading layout narrower than the loaded one (em dashes produce a smaller intrinsic width than 1,012). Pairing `width: 100%` with `box-sizing: border-box` keeps the column inside the viewport when mobile padding is added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The `./style.css` export pointed only at `./dist/index.css`, which doesn't exist in CI before the package is built. Lint runs without a prior build, so `import '@automattic/charts/style.css'` from another monorepo package failed `import/no-unresolved`. JS exports already dodge this via the `jetpack:src` condition; the CSS export now does the same, with an empty `src/style.css` placeholder and a `default` arm preserving the existing `dist/index.css` resolution for published consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the Visitors KPI with Impressions and the Top locations card with a Top videos by watch time card. The change is driven by the Phase 7 WPCOM-side investigation (see issue #48506): - Unique-visitor counts aren't recoverable from the wpcom video_stats table (no viewer ids or session keys). Impressions is real, returned by stats/video-plays?complete_stats=true, and is a coherent reach metric distinct from Views (which require a play). - Per-video geography isn't collected (video_stats has no country / region columns; stats/country-views is site-wide with no post_id filter). Top videos by watch time reuses the same RankingCard, with per-video watchTimeSeconds drawn from the same endpoint. RankingCard gains a `valueColumnHeader` prop and an optional `formatValue` formatter so it can host both a views-formatted column and a watch-time-formatted column. The chart's "secondary" comparison for Views now references Impressions; for Impressions it references Views. flag-emoji.ts and top-locations-card.tsx are removed. Mock data: per-day impressions run 3.0-5.4x views (loads >> plays); each TOP_VIDEOS entry carries an independent baseWatchSeconds so the two ranking lists differ (e.g. a short explainer ranks high by views but lower by watch time, and vice versa). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Layers defensive resets onto the shared admin-page chrome for wp-build
dashboards that mount `@wordpress/admin-ui` 2.x components like
`<Breadcrumbs>`. The component renders semantic `<nav>/<ul>/<li>/<a>`
which inherits two wp-admin globals that misbehave in the modernized
header:
- `ul li { margin-bottom: 6px }` inflates the breadcrumb `<ul>`'s
bounding box below the text baseline. AdminPage's header Stack
centers the visual slot (JetpackLogo) to the inflated midpoint, so
the logo lands a few pixels below the breadcrumb label.
- `a { text-decoration: underline }` forces an always-on underline on
the linked parent breadcrumb, contradicting `<Link tone="neutral">`,
which is designed to render plain at rest and underline on hover.
Kept as a separate mixin rather than folded into the existing
`jetpack-admin-page-layout` so the 17 non-wp-build callers stay
byte-for-byte unchanged; wp-build dashboards opt in with a one-line
swap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Phase 3 (Overview, mocked) design review touches across the Overview screen, Video details page, and the shared admin chrome: - Overview content max-width 1200px → 1344px, with a constant 24px inline gutter so the page never reads flush to `#wpbody-content`'s edges. KPI hero numbers drop from font-weight 700 to 500. - Page canvas uses a tokenized off-white (`var(--vp-page-bg-canvas, #fcfcfc)`) on `body` and `.jp-admin-page` instead of pure white, so Overview, Library, Settings, and Video details share one surface. - Date selector renders as a secondary + compact toggle to match the design-system `outline` Button (toggleProps on DropdownMenu). - Views trends chart gets a custom `renderTooltip` matching admin-ui's dark BaseTooltip palette. The tooltip now formats values with `formatWatchTime` when the active metric is Watch time, so the axis and tooltip read in the same unit (was: axis "12 min" vs tooltip "720"). - Video details: add a 24px inline gutter (matches Overview). Save button and the More-actions DropdownMenu both size="compact" (32px), so the header row doesn't grow beyond admin-ui's 32px min-height and the JetpackLogo stays aligned with the breadcrumbs on the detail screen. - admin-shell swaps to the new `jetpack-admin-page-layout-wp-build` mixin to neutralize wp-admin's `ul li` and `a` defaults inside `<Breadcrumbs>` (logo / link styling). Tabs strip items keoshi flagged again (per-tab padding, indicator continuity, header height between tabs) are deliberately left for the broader admin-page-layout review tracked in #48586. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
dhasilva
force-pushed
the
update/videopress-modernization-phase-3
branch
from
May 13, 2026 20:58
414dc0b to
9570bf8
Compare
github-actions
Bot
added
[Status] UI Changes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #48506 (Phase 3).
Stacked on #48669 — review that one first; this PR will be rebased onto trunk once #48669 lands. (Yes, Phase 4 was implemented before Phase 3.)
Proposed changes
UI-first build of the modernized Overview screen for the wp-build VideoPress dashboard, behind the existing
rsm_jetpack_ui_modernization_videopressfilter.routes/overview/{stage,style}.tsx— KPI row, "Views trends" chart card, and a row with "Most viewed" + "Top videos by watch time" cards. Wraps inDashboardLayout's shared chrome.--wpds-color-stroke-interactive-neutral-strongtoken@wordpress/uiTabs uses for its own indicator. Rovingtabindex+ ArrowLeft / ArrowRight / Home / End for keyboard nav (manual activation: Enter / Space selects).@automattic/chartsLineChart(responsive variant). The package's CSS is now imported (@automattic/charts/style.css) so thechart-layout__content svg { display: block }rule applies — without it the inline SVG's baseline descender ratchets the measured chart height upward on every render until the data lines collapse out of the visible area.formatWatchTimeutil.RankingCardshared component for "Most viewed" and "Top videos by watch time" (header + 5-row skeleton + percentage-bar row backgrounds via a--vp-row-barCSS variable + per-card value formatter). Each consumer card maps its data to{ key, label, value }and supplies the title / aria-label / column-headers / optional formatter / optional footer. "Most viewed" formats values as integers; "Top videos by watch time" formats values withformatWatchTime.Spinneron the chart canvas, sized skeleton blocks on the KPI hero numbers (matches the bottom cards' loading vocabulary), existing row skeletons on the bottom cards. Layout width and card heights are stable across loading and loaded states.[role="tabpanel"]rule (which gives Tabs.Panel its flex-column behavior) is scoped to.vp-dashboard-tabs > [role="tabpanel"], so nested tabpanels inside individual routes (the Overview's chart card panel) keep their natural sizing.All data is hardcoded in
src/dashboard/fixtures/stats.tsand accessed throughuseMockStats(window-attached singleton viauseSyncExternalStore, mirroringuseMockLibrary's pattern). Real data swaps in during Phase 7 (proxy + wiring) per the updated tracking issue.Mock data has been tuned so the two new metrics make sense: per-day impressions run 3.0–5.4× views (loads >> plays), and each video carries an independent baseWatchSeconds so the "Most viewed" and "Top videos by watch time" rankings differ.
Related product discussion/links
Does this pull request change what data or activity we track or use?
No. UI-only PR with mocked data; nothing user-facing ships until the cutover phase flips the modernization filter.
Testing instructions
rsm_jetpack_ui_modernization_videopress→true).