Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📦 Bundle Size Report🎨 @videojs/html
Presets (7)
Media (4)
Players (3)
Skins (16)
UI Components (22)
Sizes are marginal over the root entry point. ⚛️ @videojs/react
Presets (7)
Media (3)
Skins (14)
UI Components (18)
Sizes are marginal over the root entry point. 🧩 @videojs/core(no changes) Entries (5)
🏷️ @videojs/element(no changes) Entries (2)
📦 @videojs/store(no changes) Entries (3)
🔧 @videojs/utils(no changes) Entries (10)
📦 @videojs/spf(no changes) Entries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
Contributor
CI Failure Diagnosis
|
mihar-22
reviewed
Mar 10, 2026
mihar-22
reviewed
Mar 10, 2026
luwes
changed the title
container, use conext to get container
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new cross-platform “gesture” abstraction (core + React + HTML) and wires it into the default/minimal video skins to toggle playback on pointer interactions, supported by a new useMediaContainer hook in React for container-level event binding.
Changes:
- Introduces
GestureCorein@videojs/coreplus initial unit tests. - Adds React
Gesturecomponent and HTMLmedia-gesturecustom element, and includes them in video skin templates/presets. - Extends React player context to track the media container (
useMediaContainer, container registration inContainer).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/ui/gesture/index.ts | Adds a barrel export for the React Gesture component. |
| packages/react/src/ui/gesture/gesture.tsx | Implements React headless Gesture component that binds pointer events to the media container. |
| packages/react/src/testing/mocks.tsx | Updates test helpers to include container/setContainer in PlayerContextValue. |
| packages/react/src/presets/video/skin.tsx | Installs default gesture behavior into the default React video skin. |
| packages/react/src/presets/video/minimal-skin.tsx | Installs default gesture behavior into the minimal React video skin. |
| packages/react/src/player/tests/context.test.tsx | Updates context tests for extended PlayerContextValue shape. |
| packages/react/src/player/create-player.tsx | Provider now tracks container state and exposes it via context. |
| packages/react/src/player/context.tsx | Adds useMediaContainer and container registration lifecycle in Container. |
| packages/react/src/media/tests/video.test.tsx | Updates PlayerContextValue usage to include container fields. |
| packages/react/src/media/tests/audio.test.tsx | Updates PlayerContextValue usage to include container fields. |
| packages/react/src/index.ts | Exports useMediaContainer and React Gesture from the package entrypoint. |
| packages/html/src/ui/gesture/gesture-element.ts | Adds media-gesture custom element that binds to the player container and forwards events to GestureCore. |
| packages/html/src/index.ts | Exports GestureElement from the HTML package entrypoint. |
| packages/html/src/define/video/skin.ts | Registers gesture element and adds <media-gesture> to the default video skin template. |
| packages/html/src/define/video/minimal-skin.ts | Registers gesture element and adds <media-gesture> to the minimal video skin template. |
| packages/html/src/define/ui/gesture.ts | Adds safeDefine(GestureElement) module for side-effect registration. |
| packages/html/src/define/background/skin.ts | Adjusts background skin slot markup (removes redundant slot="media" attribute). |
| packages/core/src/core/ui/gesture/tests/gesture-core.test.ts | Adds initial unit tests for GestureCore behavior. |
| packages/core/src/core/ui/gesture/gesture-core.ts | Introduces GestureCore, allowed gesture types/commands, and pointer-type handling. |
| packages/core/src/core/index.ts | Re-exports GestureCore from the core package entrypoint. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
mihar-22
reviewed
Mar 13, 2026
Member
mihar-22
left a comment
mihar-22
left a comment
There was a problem hiding this comment.
Awesome, thanks for doing this! Main suggestion is creating core/dom for the component, moving pointer type there, and sharing event handling logic with html/react.
Comment on lines
+9
to
+10
| type: GestureType; | ||
| command: GestureCommand; |
Member
There was a problem hiding this comment.
(request): missing jsdoc comments, used by API reference tooling for docs.
Comment on lines
+16
to
+20
| export const PointerTypes = { | ||
| MOUSE: 'mouse', | ||
| PEN: 'pen', | ||
| TOUCH: 'touch', | ||
| } as const; |
Member
There was a problem hiding this comment.
(request): this is handy but more a DOM specific type. Can we move this to core/ui/types, or as per suggestion below have core/dom layer for gesture and move to core/dom/ui/event
Comment on lines
+44
to
+45
| // TODO: Should `pointerType` be a prop that can be configured? | ||
| if (pointerType && pointerType === PointerTypes.MOUSE) { |
Member
There was a problem hiding this comment.
Interesting. I like it and if it makes sense for future gestures then good idea!
(suggestion): if it doesn't make sense as a prop, you could consider moving "pointer type" handling to the DOM layer or framework-layer since it's just a DOM event related guard and not core state management piece.
| this.#media = media; | ||
| } | ||
|
|
||
| handleGesture({ pointerType }: { pointerType: string }): void { |
Member
There was a problem hiding this comment.
(request): stricter type pointerType: PointerType.
Requires creating the type:
type PointerType = PointerTypes[typeof keyof PointerTypes]Not required if guarding is moved to core/dom
Comment on lines
+52
to
+69
| if (changed.has('type') && ALLOWED_GESTURE_TYPES.includes(this.type)) { | ||
| this.#disconnect?.abort(); | ||
| this.#disconnect = new AbortController(); | ||
| const { signal } = this.#disconnect; | ||
|
|
||
| const container = this.#player.value?.target?.container; | ||
| container?.addEventListener( | ||
| this.type, | ||
| (event: PointerEvent) => { | ||
| const target = event.target as Element; | ||
| if (target !== container && !target.localName.endsWith('video')) return; | ||
|
|
||
| this.#core.handleGesture(event); | ||
| }, | ||
| { signal } | ||
| ); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
(suggestion): could move this to core/dom to share with html and react. The pointer types could live there too which would make more sense. Claude should be able to do it by referencing other examples.
Comment on lines
+16
to
+18
| else { | ||
| if (__DEV__) logMissingFeature('Gesture', 'playback'); | ||
| } |
Member
There was a problem hiding this comment.
(nit): could be else if (__DEV__)
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
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.
Todo:
Adds a gesture component for React and HTML.
Has a type and command prop to customize in the future.
(inspired by @mihar-22 https://vidstack.io/docs/player/components/display/gesture/)
Question: should this component also have a pointer type prop?
Gestures behave differently for mouse or touch (tap), example: display click -> play / pause for mouse, show / hide controls for tap