feat: add title display and media metadata state#1603
Conversation
|
@Jerricho93 is attempting to deploy a commit to the Mux Team on Vercel. A member of the Team first needs to authorize it. |
👷 Deploy request for vjs10-site pending review.Visit the deploys page to approve it
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
sampotts
left a comment
There was a problem hiding this comment.
Looking good! Cheers for doing this. We probably want Rahim or Wes to give a once over too but I've left a few comments.
Also, I built and tried the sandbox but couldn't see the title:
https://v10-sandbox-git-fork-jerricho93-pr-title-core-default-mux.vercel.app/?platform=react&styling=css&preset=hls-video&skin=default&source=hls-4&autoplay=0&muted=0&loop=0&preload=metadata
| export const SOURCES = { | ||
| 'hls-1': { | ||
| label: 'HLS - Big Buck Bunny', | ||
| title: 'Big Buck Bunny', |
There was a problem hiding this comment.
A nit and not a blocker but we could probably derive the label from {this.type.toUpperCase()} {this.title} to save the double up.
There was a problem hiding this comment.
Or inversely, just do label.split(' - ')[1] to get the title.
| if (!media) return; | ||
| return store.attach({ media, container }); | ||
| const cleanup = store.attach({ media, container }); | ||
| const s = store.state as { setTitle?: (t: string | null) => void }; |
There was a problem hiding this comment.
nit: can we use state here instead of s? It's a personal thing but I'm not a fan of the single letter variable names (e.g. e for error or event) unless it's inside a simple .map() or .filter() since it makes it a bit harder to understand.
| linear-gradient(to bottom, oklch(0 0 0 / 0.5), oklch(0 0 0 / 0.3) 25%, oklch(0 0 0 / 0)), | ||
| linear-gradient(to top, oklch(0 0 0 / 0.5), oklch(0 0 0 / 0.3) 25%, oklch(0 0 0 / 0)); |
There was a problem hiding this comment.
Couldn't we combine these and just add another stop in? Or we could just go with oklch(0 0 0 / 0.5), oklch(0 0 0 / 0), oklch(0 0 0 / 0.5)
Also, how do we handle if there's no title/metadata? It might look a little weird to have the extra gradient if there's no title/metadata.
| // Default: hidden | ||
| 'opacity-0', | ||
| 'bg-linear-to-t from-black/50 via-black/30 via-25% to-transparent', | ||
| '[background-image:linear-gradient(to_top,oklch(0_0_0_/_0.5),oklch(0_0_0_/_0.3)_25%,oklch(0_0_0_/_0)),linear-gradient(to_bottom,oklch(0_0_0_/_0.5),oklch(0_0_0_/_0.3)_25%,oklch(0_0_0_/_0))]', |
There was a problem hiding this comment.
If we simplify as above, we can revert to using the native Tailwind classnames.
| text-overflow: ellipsis; | ||
| font-size: 1rem; | ||
| font-weight: 500; | ||
| color: var(--media-color-primary, oklch(1 0 0)); |
There was a problem hiding this comment.
Did you try without this? I think the currentColor would be --media-color-primary but I could be wrong!
There was a problem hiding this comment.
.media-title and .media-controls are siblings, so we can't inherit from that. What could be done is defining color in .media-default-skin--video and then both components can inherit from that. Would that be a better solution?
| 'py-3 px-4', | ||
| 'pointer-events-none', | ||
| 'text-base font-medium', | ||
| '[color:var(--media-color-primary,oklch(1_0_0))]', |
There was a problem hiding this comment.
As above re: whether we need this.
| // Default: hidden | ||
| 'opacity-0', | ||
| 'bg-linear-to-t from-black/70 via-black/50 via-[7.5rem] to-transparent', | ||
| '[background-image:linear-gradient(to_top,oklch(0_0_0_/_0.5),oklch(0_0_0_/_0.3)_25%,oklch(0_0_0_/_0)),linear-gradient(to_bottom,oklch(0_0_0_/_0.5),oklch(0_0_0_/_0.3)_25%,oklch(0_0_0_/_0))]', |
There was a problem hiding this comment.
As with the other skin but I guess this crept in?
|
It's looking good. I think we'll want some responsive styles for the font size and padding. On a smaller screen let's go with: font-size: 0.875rem;
padding: 1rem 1.5rem;then using an container query for font-size: 1rem;
padding: 1.5rem 2rem;I'd also like to add a text-shadow to the title to improve the contrast a touch. We have a shadow color based on the current color that we should be able to use here. It might need the color definition moved to the skin though as you suggested. We'd just need to make sure that I also reckon the title should only show when the player is paused rather than every time the controls are visible? What do you think? |
|
@sampotts sure, I can push those changes. One question regarding displaying the title: if we're going to just display it when the video is paused, should the top gradient also be displayed only when paused? Otherwise it may cause the player to look too "dark" on the top side for no specific reason. What do you think? Also, we held off on moving |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Omit 'children' from Title.Value's ValueProps so consumers can't accidentally pass children that would be silently overridden. Add PlayButtonCore test coverage for the setMetadata/title-in-label behavior introduced in the previous commit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
store.detach() resets all feature state to initial values, including
title → null. The previous implementation only called setTitle when
the mediaTitle property changed on the element (HTML) or when title/store
deps changed (React), so a static title would not be restored after a
detach/reattach cycle.
HTML: remove the changed.has('mediaTitle') guard so update() always
syncs the current mediaTitle value on every render cycle.
React: capture title in a ref and re-apply it immediately after
store.attach() so the title is restored without adding it to the
attach effect's dependency array (which would cause extra reattaches
on every title change).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Title should only appear in the visual overlay, not in button labels. Reverts PlayButtonCore.setMetadata(), the metadata selector wiring in PlayButtonElement, and the createMediaButton expansion in PlayButton.tsx. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add useTitle() hook that derives the title from the selected source, and pass it as the title prop to Provider in all React video/audio templates so the title overlay renders correctly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the store detaches, signals are aborted before state resets, so the sync subscription was already removed when title resets to null — leaving the previous title visible in the OS lock-screen controls. Fix: capture navigator.mediaSession at attach time and clear it explicitly in the abort handler. Also adds a metadataFeature test suite covering MediaSession sync, detach, destroy, and reattach. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…overlay Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Default skin overlay: bottom gradient always visible; top darkening added only when a title is present via :has([data-has-title]). CSS and Tailwind aligned (via-black/30 at 25%). - Minimal skin Tailwind overlay reverted to main — changes belong in pr/title-minimal. - Sandbox: title derived from label instead of a redundant title field; remove title field from all sources. - create-player: rename s → state for readability (review fix). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All selectors in title.css were missing the .media-default-skin ancestor prefix, causing styles to apply globally across all skins. Aligned with the convention used by every other component CSS file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tle change Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
d5d413c to
a957b97
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a957b97. Configure here.
| } | ||
|
|
||
| core.setMedia(metadata); | ||
| const state = core.getState(); |
There was a problem hiding this comment.
Title hook needs no memo
Medium Severity
Title.Value uses the imperative Core projection pattern (usePlayer, a TitleCore in useState, and setMedia/getState during render) without the 'use no memo' directive. With React Compiler enabled, that can skip updates and leave the title UI stale when metadata changes asynchronously.
Triggered by learned rule: React hooks with imperative Core projection need 'use no memo' for React Compiler
Reviewed by Cursor Bugbot for commit a957b97. Configure here.


Summary
metadataFeaturetopackages/corewithtitlestate andsetTitle— syncs tonavigator.mediaSession.metadatawhen availableTitleCore,TitleDataAttrs, andselectMetadataselector for runtime-agnostic title logic<media-title>custom element (HTML) andTitle.Valuecomponent (React) that read from the metadata storemedia-titleto default skin templates (HTML and React, CSS and Tailwind variants) — fades in when the player is pausedmedia-titleattribute /titleprop on all player elements — never uses the nativetitle=attribute, avoiding the browser tooltip issue (Bug: Title Hover Issue #791)Test plan
pnpm -F @videojs/core test src/core/ui/titlepnpm -F @videojs/html test src/ui/titlepnpm -F @videojs/react test src/ui/titlepnpm typecheckpnpm lintmedia-titleon HTML player → title fades in when paused, hidden on error, no native browser tooltip on hovertitleprop → same behaviornavigator.mediaSession.metadata.titleis set in Chromepnpm check:workspaceCloses #1123
Note
Medium Risk
Touches shared player store presets, MediaSession integration, and cross-package public APIs (
titleprop,media-title); behavior is well-tested but affects all player variants.Overview
Adds media title as first-class player state and wires it through HTML, React, default skins, and the sandbox.
Core: New
metadataFeatureexposestitle/setTitleon all audio and video feature presets and syncs tonavigator.mediaSession.metadatawhen supported.TitleCore,selectMetadata, and tests cover store behavior and MediaSession lifecycle.UI:
<media-title>andTitle.Valueread metadata from the store (with a11y viadata-has-titleandaria-hidden). Player roots acceptmedia-title(HTML) ortitleon the ReactProvider—not the nativetitleattribute.Skins: Default video/live skins include a top title overlay that shows when controls are visible and playback is paused; overlay gradients adjust when a title is present. CSS and Tailwind title/overlay styles added. Minimal skin title is out of scope per PR notes.
Sandbox: Titles are derived from source labels via
renderPlayerAttrs/useTitleand passed into providers or player markup.Reviewed by Cursor Bugbot for commit a957b97. Bugbot is set up for automated code reviews on this repo. Configure here.