feat(skin): add pip-enter and pip-exit icons#1015
Merged
Conversation
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Size Report🎨 @videojs/html(no changes) Presets (7)
Media (5)
Players (3)
Skins (16)
UI Components (21)
Sizes are marginal over the root entry point. ⚛️ @videojs/react(no changes) Presets (7)
Media (4)
Skins (14)
UI Components (18)
Sizes are marginal over the root entry point. 🧩 @videojs/core(no changes) Entries (6)
🏷️ @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
There was a problem hiding this comment.
Pull request overview
Adds distinct Picture-in-Picture enter/exit icons and wires them through the HTML/React skins so the PiP control visually reflects current state, while also updating the icons toolchain and aligning some monorepo tool versions.
Changes:
- Introduce
pip-enter/pip-exiticons (default + minimal sets) and update icon exports/docs accordingly. - Update HTML + React video skins (and Tailwind variants) to render both PiP icons and toggle visibility via
data-pip-driven icon-state rules. - Upgrade
svgoto 4.x and aligntsx(and related lockfile entries) across the repo.
Reviewed changes
Copilot reviewed 17 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| site/src/content/docs/concepts/skins.mdx | Doc formatting tweaks; contains a PiP example snippet that should be updated to match new icons/state classes. |
| site/scripts/build-ejected-skins.ts | Updates known icon list for ejected skin builds to use pip-enter/pip-exit. |
| site/package.json | Aligns tsx version used by the site scripts. |
| pnpm-lock.yaml | Lockfile updates for svgo@4, tsx alignment, and related dependency bumps. |
| packages/skins/src/shared/tailwind/icon-state.ts | Adds Tailwind iconState.pip mapping using group-data-pip variants. |
| packages/skins/src/shared/css/video/icon-state.css | Adds CSS selectors to toggle PiP icons based on data-pip and media-button--pip. |
| packages/sandbox/package.json | Aligns tsx/TypeScript versions used by the sandbox. |
| packages/react/src/presets/video/skin.tsx | Replaces single PiP icon with enter/exit icons and adds media-button--pip class. |
| packages/react/src/presets/video/skin.tailwind.tsx | Uses new PiP icons and Tailwind iconState.pip visibility classes. |
| packages/react/src/presets/video/minimal-skin.tsx | Same PiP icon swap and media-button--pip class addition for minimal skin. |
| packages/react/src/presets/video/minimal-skin.tailwind.tsx | Same PiP icon swap using Tailwind iconState.pip. |
| packages/icons/src/assets/minimal/pip-exit.svg | Adds minimal pip-exit SVG asset. |
| packages/icons/src/assets/minimal/pip-enter.svg | Updates minimal pip-enter SVG asset. |
| packages/icons/src/assets/default/pip-exit.svg | Adds default pip-exit SVG asset. |
| packages/icons/src/assets/default/pip-enter.svg | Updates default pip-enter SVG asset. |
| packages/icons/src/assets/default/captions-on.svg | SVG re-optimization change (likely from tooling update). |
| packages/icons/scripts/build.ts | Adjusts SVGO config for v4 upgrade. |
| packages/icons/package.json | Bumps svgo and aligns tsx/React dev deps for icon builds. |
| packages/icons/README.md | Documents the new PiP icon names/exports. |
| packages/html/src/define/video/skin.ts | Updates PiP button template to render enter/exit icons with state classes. |
| packages/html/src/define/video/skin.tailwind.ts | Same update for Tailwind HTML video skin template. |
| packages/html/src/define/video/minimal-skin.ts | Same update for minimal HTML video skin template. |
| packages/html/src/define/video/minimal-skin.tailwind.ts | Same update for Tailwind minimal HTML video skin template. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 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.
Comment on lines
+281
to
282
| <Button {...props} className="media-button--icon "> | ||
| <PipIcon className="media-icon" /> |
Merged
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.
svgoto 4.x.tsxin the monorepo.Original icons are in Figma.
Closes #991