feat(react): implement default and minimal video skins#550
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📦 Bundle Size Report
Total: 37.72 kB · +2.11 kB · +5.9% Entry BreakdownSubpath sizes are the additional bytes on top of the root entry point, measured by bundling root + subpath together and subtracting the root-only size.
|
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
3.09 kB | 3.09 kB | +4 B | +0.1% | 🔺 |
./dom |
2.75 kB | 2.75 kB | +9 B | +0.3% | 🔺 |
| total | 5.84 kB | 5.85 kB | +13 B | +0.2% |
@videojs/element
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
817 B | 817 B | 0 B | 0% | ✅ |
./context |
823 B | 823 B | 0 B | 0% | ✅ |
| total | 1.60 kB | 1.60 kB | 0 B | 0% |
@videojs/icons
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
./react |
4.77 kB | 2.10 kB | -2.67 kB | -56.0% | 🔽 |
./html |
1.64 kB | 1.37 kB | -277 B | -16.5% | 🔽 |
| total | 6.40 kB | 3.46 kB | -2.94 kB | -45.9% |
@videojs/react
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
7.99 kB | 8.00 kB | +13 B | +0.2% | 🔺 |
./audio |
266 B | 255 B | -11 B | -4.1% | 🔽 |
./background |
40 B | 34 B | -6 B | -15.0% | 🔽 |
./video |
264 B | 5.17 kB | +4.92 kB | +1907.2% | 🔴 |
| total | 8.55 kB | 13.46 kB | +4.91 kB | +57.5% |
@videojs/store
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
. |
1.29 kB | 1.29 kB | 0 B | 0% | ✅ |
./html |
468 B | 468 B | 0 B | 0% | ✅ |
./react |
199 B | 199 B | 0 B | 0% | ✅ |
| total | 1.94 kB | 1.94 kB | 0 B | 0% |
@videojs/utils
| Entry | Base | PR | Diff | % | |
|---|---|---|---|---|---|
./array |
104 B | 104 B | 0 B | 0% | ✅ |
./dom |
684 B | 684 B | 0 B | 0% | ✅ |
./events |
227 B | 227 B | 0 B | 0% | ✅ |
./function |
197 B | 197 B | 0 B | 0% | ✅ |
./object |
119 B | 119 B | 0 B | 0% | ✅ |
./predicate |
265 B | 265 B | 0 B | 0% | ✅ |
./string |
110 B | 110 B | 0 B | 0% | ✅ |
./style |
63 B | 185 B | +122 B | +193.7% | 🔴 |
./time |
478 B | 478 B | 0 B | 0% | ✅ |
./number |
158 B | 158 B | 0 B | 0% | ✅ |
| total | 2.35 kB | 2.47 kB | +122 B | +5.1% |
ℹ️ How to interpret
Sizes are minified + brotli, measured with esbuild.
Package totals are computed as root size + marginal subpath costs.
Subpath marginal cost = (root + subpath bundled together) − root alone.
| Icon | Meaning |
|---|---|
| ✅ | No change |
| 🔺 | Increased ≤ 10% |
| 🔴 | Increased > 10% |
| 🔽 | Decreased |
| 🆕 | New (no baseline) |
Run pnpm size locally to check current sizes.
There was a problem hiding this comment.
Pull request overview
This PR implements the default and minimal video skins for the React player, introducing fully functional UI controls with proper theming and accessibility. The implementation includes comprehensive enhancements to utilities, icons, and React components to support the new skin system.
Changes:
- Implemented
VideoSkinandMinimalVideoSkincomponents with complete control layouts (play/pause, seek, mute, PiP, fullscreen, buffering) - Enhanced
cn()utility to support clsx-like object syntax for conditional class names - Added new icons (pip, restart, seek) and updated existing SVG icons with refined designs
- Added barrel exports for React UI components and fixed type/ref composition issues
- Updated site demos and documentation to use new skin components
Reviewed changes
Copilot reviewed 25 out of 46 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/style/cn.ts | Enhanced to support object syntax for conditional classes |
| packages/utils/src/style/tests/cn.test.ts | Comprehensive tests for new cn() functionality |
| packages/react/src/utils/types.ts | Fixed RenderFunction to allow null returns |
| packages/react/src/utils/use-composed-refs.ts | Fixed to only return cleanup when React 19+ callbacks present |
| packages/react/src/presets/video/skin.tsx | Default video skin implementation with glassmorphic design |
| packages/react/src/presets/video/skin.css | Comprehensive styles for default skin with accessibility features |
| packages/react/src/presets/video/minimal-skin.tsx | Minimal video skin implementation |
| packages/react/src/presets/video/minimal-skin.css | Styles for minimal skin with container queries |
| packages/react/src/presets/types.ts | Enhanced BaseSkinProps with className and style support |
| packages/react/src/ui/*/index.ts | Barrel exports for UI components (7 files) |
| packages/icons/scripts/build.ts | Added SVGO optimization, watch mode, and jsxRuntime config |
| packages/icons/package.json | Added build:watch and dev scripts |
| packages/icons/src/assets//.svg | Updated 20 SVG icons with new designs |
| packages/core/src/core/ui/pip-button/pip-button-core.ts | Improved accessibility labels |
| site/src/examples/react/*/Demo.tsx | Updated demos to use new skin components (2 files) |
| site/src/content/docs/concepts/skins.mdx | Updated documentation examples |
| site/src/components/frames/Minimal.astro | Simplified container styling |
| site/src/components/FilmGrain/FilmGrain.tsx | Adjusted z-index for proper layering |
| packages/react/package.json | Added @videojs/icons dependency |
| pnpm-lock.yaml | Updated lockfile with new dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| className={twMerge( | ||
| clsx( | ||
| 'absolute w-full h-full top-0 left-0 pointer-events-none mix-blend-overlay z-10', | ||
| 'absolute w-full h-full top-0 left-0 pointer-events-none mix-blend-overlay z-0', |
There was a problem hiding this comment.
The z-index is changed from z-10 to z-0. This change might affect the layering of the film grain effect. Without seeing the broader context of the page layout, it's unclear if this change is intentional or if it could cause the film grain to be covered by other elements. Please verify that the film grain effect is still visible as intended on the docs pages.
There was a problem hiding this comment.
Yeah check carefully for regressions, here! Iirc there was a reason I chose z-10 instead of z-0.... but... could be wrong.
My Demo component displays players at z-20 to combat this 😂
There was a problem hiding this comment.
Sorry, yeah I should have checked with you. The issue with z-10 was it caused the grain effect to bleed into the players (in Safari at least) and z-0 seemed to allow the effect on the background of the site. Maybe it's due to a classname change that's causing the z-20 to not work.
There was a problem hiding this comment.
I've used the ContentWidth.astro instead and setting relative z-20 on it 👍🏼
There was a problem hiding this comment.
Thanks. Awkward, but effective!
There was a problem hiding this comment.
We have a different frame that provides unstyled functionality -- site/src/components/frames/ContentWidth.astro
There was a problem hiding this comment.
Ah ok cool. I'll use that and revert this change. I just felt it looked a little odd as the container had its own radius which didn't match the players and I wasn't sure we needed a background and border for these demos.
sampotts
changed the title
Summary
Implement the default and minimal video skins for the React player, wiring up all UI controls (play, pause, seek, mute, PiP, fullscreen, buffering indicator) with proper icon sets and CSS styling.
Closes #539
Changes
VideoSkinandMinimalVideoSkincomponents with full control layouts including play/pause, seek forward/back, mute/volume, PiP, fullscreen, time display, and buffering indicatorcn()utility to support object syntax for conditional class names (likeclsx)RenderFunctiontype to allow returningnullfor conditional renderingcomposeRefsto only return cleanup when React 19+ cleanup callbacks are presentreact-previewimportsImplementation details
media-icon--hidden) driven by component statecn()enhancement acceptsRecord<string, unknown>values where truthy values include the key as a class namefill="currentColor"for theme-able iconsBaseSkinPropsextended withclassNameandstylefor skin customizationTesting
cn()changes covered by new tests inpackages/utils/src/style/tests/cn.test.tspnpm devand verify both default and minimal skins render controls correctly