feat(core): add vimeo media layer and html/react components#1620
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| await this.#loadComplete; | ||
| await this.#player?.exitFullscreen?.(); | ||
| this.#isFullscreen = false; | ||
| } |
There was a problem hiding this comment.
Fullscreen state set unconditionally after optional chain
Low Severity
In requestFullscreen and exitFullscreen, #isFullscreen is unconditionally set after awaiting #loadComplete. If #player becomes null between #loadComplete resolving and execution reaching the assignment (e.g., concurrent destroy() call), the optional chain this.#player?.requestFullscreen?.() evaluates to undefined, the await resolves immediately, and the state flag is incorrectly toggled without an actual fullscreen transition. The PiP methods use try/catch which partially mitigates this, but the fullscreen methods lack any guard.
Reviewed by Cursor Bugbot for commit 9f6b941. Configure here.
luwes
force-pushed
the
refactor/media-layer-architecture
branch
from
907e9f3 to
2d783f0
Compare
|
|
||
| player.on('fullscreenchange', ({ fullscreen }) => { | ||
| this.#isFullscreen = fullscreen; | ||
| }); |
There was a problem hiding this comment.
Missing fullscreenchange event dispatch in Vimeo handler
Medium Severity
The fullscreenchange handler from the Vimeo player updates #isFullscreen but never dispatches a fullscreenchange event on the VimeoMedia EventTarget. This is inconsistent with the enterpictureinpicture and leavepictureinpicture handlers on lines 589–597, which both call this.dispatchEvent(...). Without the event dispatch, the player UI and any external listeners won't be notified when fullscreen state changes via Vimeo's own controls.
Reviewed by Cursor Bugbot for commit 3cd8326. Configure here.
luwes
force-pushed
the
refactor/media-layer-architecture
branch
from
6a4244c to
4ed3564
Compare
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Move mediaPlayedRanges to the current media extension lifecycle. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 02ceb05. Configure here.
| if (!loadOptions) return; | ||
|
|
||
| // Vimeo dispatches an `error` event separately on failure. | ||
| await this.#player.loadVideo(loadOptions).catch(() => {}); |
There was a problem hiding this comment.
loadComplete never settles on failure
High Severity
VimeoMedia gates play(), fullscreen, and PiP on await this.#loadComplete, but that promise is only resolved in #onLoaded. A player error, a failed loadVideo, or load() after resetting the promise when toLoadVideoOptions returns null leaves #loadComplete pending forever, so callers can hang indefinitely.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 02ceb05. Configure here.
1 participant
Fix #1435
Related #1538
Summary
Adds Vimeo playback on the media-layer architecture: a
VimeoMediahost in core, HTML and ReactVimeoVideocomponents, sharedmediaPlayedRangesfor buffered/played ranges on custom media, and sandbox demos.Changes
VimeoMediawraps@vimeo/playerwith anHTMLMediaElement-like surface (src parsing, load, PiP, fullscreen, embedconfig)mediaPlayedRangeshelper for custom elements that lack nativeTimeRangesCustomMediaElementgains iframe template support for Vimeo embedsVimeoVideoweb component and React wrapper with sandbox HTML/React templatesTesting
pnpm -F @videojs/core test src/dom/media/vimeopnpm -F @videojs/core test src/dom/media/media-played-rangesMade with Cursor
Note
Medium Risk
Touches shared CustomMediaElement lifecycle (reconnect, iframe path) used by other custom media; new third-party @vimeo/player dependency and large embed surface area, mitigated by unit tests.
Overview
Adds Vimeo as a first-class media source on the v10 media-layer stack, with sandbox demos and shared infrastructure for iframe embeds.
Core: New
VimeoMediahost wraps@vimeo/player, maps Vimeo events to standard media APIs (play/pause, time, volume, PiP, fullscreen, text tracks), parses ids/URLs (including live events and unlistedhhashes), and builds embed URLs viabuildVimeoIframeSrc. AddsmediaPlayedRangesso iframe hosts expose aplayedTimeRanges-like surface.CustomMediaElementis extended for iframe embeds: lazy/recreatable media host, property upgrade before define, iframeconfighydration fromdata-config, and no track/source sync on iframe.Packages: HTML
vimeo-videoweb component and ReactVimeoVideoattachVimeoMediato an iframe;escapeHtmlsecures iframesrcin templates.@vimeo/playeris a new@videojs/coredependency; build entries include vimeo and media-played-ranges.Sandbox: New
vimeo-videopreset with fixedVIMEO_VIDEO_SRC, HTML/React templates, routing, and UI rules (no Tailwind skin, source picker disabled).Reviewed by Cursor Bugbot for commit 02ceb05. Bugbot is set up for automated code reviews on this repo. Configure here.