refactor(html): SkinMixin -> SkinElement#1159
Merged
Conversation
…xin with base class Move generic shadow DOM style utilities (`createShadowStyle`, `applyShadowStyles`, `ShadowStyle` type) from `@videojs/html` into `@videojs/utils/dom` so they are reusable across packages. Replace the `SkinMixin` pattern with a `SkinElement` base class that directly extends `ReactiveElement`, simplifying the type signatures and inheritance chain for all 8 skin element definitions. Made-with: Cursor
|
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 (7)
Players (3)
Skins (17)
UI Components (22)
Sizes are marginal over the root entry point. ⚛️ @videojs/react
Presets (7)
Media (6)
Skins (14)
UI Components (19)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (8)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (3)
ℹ️ How to interpretAll sizes are standalone totals (minified + brotli).
Run |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors shadow-DOM styling and skin infrastructure by extracting generic shadow style helpers into @videojs/utils/dom and replacing the SkinMixin(ReactiveElement) pattern with a concrete SkinElement base class used by all built-in audio/video skins.
Changes:
- Added
createShadowStyle,applyShadowStyles, andShadowStyleto@videojs/utils/dom, with constructable stylesheet support and<style>fallback. - Introduced
SkinElement extends ReactiveElementto encapsulate shadow root creation, template rendering, and shared/per-skin style application. - Updated all built-in audio/video default/minimal (CSS + tailwind) skins to extend
SkinElementand usecreateShadowStylefor inline CSS.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/dom/shadow-styles.ts | New shared utilities for creating/applying shadow-root styles with constructable-sheet + <style> fallback. |
| packages/utils/src/dom/index.ts | Re-exports the new shadow style utilities from the dom entrypoint. |
| packages/utils/src/dom/tests/shadow-styles.test.ts | Adds unit tests covering constructable stylesheet creation and apply fallback behavior. |
| packages/html/src/define/skin-element.ts | Adds SkinElement base class to replace the removed mixin and centralize skin shadow-root setup. |
| packages/html/src/define/skin-mixin.ts | Removes the prior SkinMixin + createStyles implementation (logic moved/replaced). |
| packages/html/src/define/video/skin.ts | Migrates video default CSS skin to SkinElement + createShadowStyle. |
| packages/html/src/define/video/skin.tailwind.ts | Migrates video default tailwind skin to SkinElement. |
| packages/html/src/define/video/minimal-skin.ts | Migrates video minimal CSS skin to SkinElement + createShadowStyle. |
| packages/html/src/define/video/minimal-skin.tailwind.ts | Migrates video minimal tailwind skin to SkinElement. |
| packages/html/src/define/audio/skin.ts | Migrates audio default CSS skin to SkinElement + createShadowStyle. |
| packages/html/src/define/audio/skin.tailwind.ts | Migrates audio default tailwind skin to SkinElement. |
| packages/html/src/define/audio/minimal-skin.ts | Migrates audio minimal CSS skin to SkinElement + createShadowStyle. |
| packages/html/src/define/audio/minimal-skin.tailwind.ts | Migrates audio minimal tailwind skin to SkinElement. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mihar-22
previously approved these changes
Mar 31, 2026
mihar-22
changed the title
…ities Move ensureRootStyles into a generic ensureGlobalStyle(id, css) utility in @videojs/utils/dom. Add createTemplate(html) for SSR-safe HTMLTemplateElement creation. Replace innerHTML-based template rendering with document.importNode cloning for better performance (parse once, clone per instance). Update SkinElement and all skin definitions to use static template instead of static getTemplateHTML. Also update background/skin.ts to use ensureGlobalStyle. Made-with: Cursor
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
createTemplate() was running at module evaluation via static class fields. If first imported without document, template became null permanently. Move to lazy creation on first construction via WeakMap. Made-with: Cursor
mihar-22
reviewed
Mar 31, 2026
Address review feedback: createTemplate is unrelated to shadow styles, so move it to a dedicated template.ts module with its own test file. Also clean up resolveTemplate to avoid unnecessary ?? undefined. Made-with: Cursor
Replace lazy WeakMap + resolveTemplate() with a simple `static template` field set eagerly at class evaluation via createTemplate(). These are custom elements that only construct in the browser, so lazy SSR guards are unnecessary.
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.
Refs #1155
Summary
Address post-merge review feedback from @mihar-22 on #1155:
@videojs/utils/dom—createShadowStyle,applyShadowStyles,ensureGlobalStyle,createTemplate(each in its own module)SkinMixin(ReactiveElement)pattern with aSkinElementbase class — simpler types, clearer inheritance, no mixin indirectionWeakMap, avoiding permanentnullif the module is imported beforedocumentis availableChanges
@videojs/utils/dom: Newshadow-stylesmodule (createShadowStyle,applyShadowStyles,ensureGlobalStyle,ShadowStyletype) andtemplatemodule (createTemplate) with full test coverage@videojs/html: NewSkinElementbase class replacesSkinMixin; all 8 skin definitions updated to extend it directlybackground/skin.ts: Uses sharedensureGlobalStyleinstead of local helperTesting
pnpm -F @videojs/utils test src/dom/tests/pnpm -F @videojs/html testpnpm -F @videojs/html buildNote
Medium Risk
Refactors how all audio/video skin custom elements render templates and apply shadow-root styles, which could subtly affect styling, slotting, or SSR behavior. Changes are mostly internal abstractions but touch core UI skins across multiple variants.
Overview
Refactors HTML skin definitions to drop the
SkinMixin(ReactiveElement)pattern in favor of a newSkinElementbase class that owns shadow-root creation, shared/root style injection, and template cloning.Extracts shadow DOM styling and template helpers into
@videojs/utils/dom(newshadow-stylesandtemplatemodules), updates skins to usecreateShadowStyle/createTemplate, and replaces the background skin’s local style injection with sharedensureGlobalStyle. Adds unit tests covering the new DOM utilities and their SSR/fallback behavior.Written by Cursor Bugbot for commit d3fe0ca. This will update automatically on new commits. Configure here.