refactor(skins): consolidate design system tokens and utilities - #2581
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. |
@videojs/core
@videojs/element
@videojs/html
@videojs/media
@videojs/react
@videojs/spf
@videojs/store
@videojs/utils
commit: |
📦 Bundle Size Report🎨 @videojs/html — 30 small size changes
Presets (7)
Media (18)
Extensions (2)
Players (5)
Skins (29)
UI Components (62)
⚛️ @videojs/react — 21 small size changes
Presets (7)
Media (22)
Extensions (2)
Players (5)
Skins (18)
UI Components (39)
🧩 @videojs/core — no changesEntries (76)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (13)
📦 @videojs/media
Entries (23)
📦 @videojs/spf — no changesEntries (10)
ℹ️ How to interpretEach entry is independently bundled, minified, and brotli-compressed. Initial size includes its static import graph; lazy dynamic chunks are reported separately. Entries are not additive because their dependency graphs overlap. Preset rows represent realistic combined bundles. Changes of 300 B or less across initial, lazy, and total size are collapsed, not discarded. Run |
sampotts
left a comment
There was a problem hiding this comment.
Just a quick thought on this - I've not done a super thorough review...
I’m concerned we may be sharing too much styling between the skins. While this works well for the two existing skins, it could make future skins harder to build, particularly if they need to break the mould, as we’d have to untangle those shared assumptions.
My preference would be to keep each skin largely self-contained, sharing only clear base or reset styles. For example, something like shadow-media-thumb introduces a specific visual choice into the shared layer. If a future skin uses a different thumb shadow, that abstraction starts to break down.
I can see why sharing these styles makes sense given the similarities between the current skins, but I worry it could constrain us in unexpected ways later.
| 'before:absolute before:inset-0 before:bg-black/35 before:backdrop-blur-sm', | ||
| 'not-data-visible:[--media-spinner-animation:none] data-visible:grid motion-reduce:[--media-spinner-animation:none]', | ||
| 'pointer-events-none absolute inset-0 hidden place-content-center text-media-controls-foreground', | ||
| 'before:absolute before:inset-0 before:bg-media-scrim/35 before:backdrop-filter-media-scrim', |
There was a problem hiding this comment.
nit: we should probably be consistent and stick with "backdrop" rather than "scrim"
806eff5 to
7f26a6f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7f26a6f. Configure here.
7f26a6f to
e899b71
Compare
e899b71 to
ac5819f
Compare
ac5819f to
3ab2572
Compare
3ab2572 to
6383208
Compare
Move reduced-transparency, high-contrast, and forced-colors handling into theme tokens layered as base.theme and base.preferences so preference overrides win without per-rule variants. Add scrim, frame border, gradient, surface, popup radius, duration, breakpoint, icon, and slider offset tokens, and route focus rings through --media-ring. Replace copied surface, focus ring, time toggle, indicator, and controls recipes with flat surface-media, surface-media-inset, and focus-ring-media utilities plus shared time, indicator, and controls style modules. Remove dead ring utilities and redundant overrides. Live audio controls now share the audio controls stylesheet, so the Vite workflow test declares the expected stylesheet per skin.
Replace the repeated mask and limit-nudge arbitrary properties with flat mask-media-volume, mask-media-volume-open, and nudge-media utilities so the recipes stay static for Tailwind source scanning while remaining shareable across skins.
…gistry Read Tailwind-only at-rules from the theme source text instead of Lightning CSS, which cannot parse Tailwind's --value() syntax. The registry theme item now carries @custom-variant blocks, which the opaque variant in shipped Tailwind sources depended on, and accepts functional @Utility names for the Shadcn css field.
Emit the theme item for the html registry too and make every html skin depend on it, importing it before the skin's own scoped rules. Skin stylesheets no longer repeat the tokens, resets, and preset layers, so installing several skins ships them once.
Add a catalog describing every shared utility, variant, and computed theme key, with a test that keeps it complete against the Tailwind source, and generate the registry theme item's docs from that catalog and the token descriptions. Group the theme tokens and comment the shared Tailwind source for contributors, since compiled output drops comments.
Expose durations, delays, hidden-state scale, blur, and offsets, thumb and tooltip shadows, and the menu item and player radii as Tailwind theme keys so rules use named utilities such as duration-media-fast and scale-media-hidden instead of the variable shorthand. Named utilities fail to compile when a token is renamed and appear in the registry theme and its docs. The shorthand remains for runtime values such as slider positions.
Duration, delay, hidden-state, shadow, text-shadow, backdrop-filter, and menu item radius tokens now read the same as the theme keys that alias them, for example --media-duration-menu behind duration-media-menu and --media-hidden-indicator-scale behind scale-media-hidden-indicator. Property families lead with the property, hidden-state values lead with hidden, and geometry stays subject first.
The default popup translate distance read the scale unit directly, so it ignored --media-scale in fullscreen while the Minimal override already used the scaled spacing unit. Both now derive from --media-spacing and resolve to the same 8px at scale one.
…gh tokens Dialog backdrops, the buffering and playback status scrims, and the audio error dialog blurred with literal utilities that reduced transparency never reached. They now read --media-backdrop-filter-dialog and --media-backdrop-filter-scrim through backdrop-filter-media-*, which preferences switch off with the surface filter. Menu submenu blur, the audio error dialog scale and blur, the Minimal indicator duration, the slider preview hidden state, and the seek indicator entry offset used literals that reduced motion did not collapse. They now use the hidden-state tokens plus --media-duration-indicator, --media-hidden-preview-*, and --media-hidden-seek-offset. Slider tracking transitions read --media-duration-slider, which reduced motion sets to zero, so the slider modules drop their motion-safe prefixes and follow the same token policy as the rest of the system.
media-transitioning matches an element in its starting or ending style, replacing the repeated data-starting-style and data-ending-style pairs and the arbitrary :is() selector that indicators, dialogs, popups, menus, and the seek indicator carried. media-anchored wraps the CSS anchor positioning support check the menu highlight repeated seven times. Both export through the registry theme like the existing variants.
…ariant The variant now also matches [data-highlighted], so menu items and their icons use it instead of separate hover and data-highlighted utilities. Hover highlighting is therefore gated on a hover-capable pointer, which removes the sticky highlight touch devices showed after a tap.
Dialog radius, width, and maximum width and the controls bar radius move from per-theme literals in style modules into --media-dialog-* and --media-controls-radius tokens behind rounded-media-dialog, w-media-dialog-width, max-w-media-dialog, and rounded-media-controls, so the Minimal differences live in the theme files. The hairline the Minimal audio bar drew by hand is now shadow-media-hairline, and the tooltip shadow gains a default so shadow-media-tooltip resolves in every theme. Values are unchanged at scale one; the bar radius and dialog width now follow --media-scale.
Menu items use Tailwind's ease-in-out instead of an arbitrary timing-function property, and the resizable settings menu popup reads transition-media-menu-resize instead of two arbitrary properties listing six durations. The vars test now asserts that every internal token a theme key aliases is declared in a theme file, which would have caught the tooltip shadow that only Minimal defined. The registry theme test removes its temporary consumer directory after loading it.
media-anchored:data-highlighted:bg-transparent and media-highlighted:highlight-media share a specificity, so Tailwind's variant declaration order decides which wins. Record that next to the variant.
src/styles/tailwind.css had no importer since the playground moved to tailwind.compiler.css and the candidate manifest, and scanning raw style modules is what the manifest replaced. The --color-media-accent-text theme key had no call site because highlight-media reads the token directly. tailwind-merge and @playwright/test were declared but never used by this package.
Walk from one skin's component tree through shared components, tokens, the build, and the playground, then map every folder, explain the style layers, the three Tailwind entry files and who consumes them, the generated outputs, and the package commands.
With the unused scanning entry gone, the design system file takes the plain name. The compiler and dev entries, the registry theme item, the catalog tests, the README, and the component skill point at the new path.
--color-media-accent aliased --media-primary and --color-media-control-hover aliased --media-accent, so the class names disagreed with the tokens behind them. bg-media-accent now means the accent token and bg-media-primary the primary token; the slider fill, volume indicator fill, and menu badge move to the matching class. Compiled output is unchanged.
The manifest lives in the Vite cache directory, which the Vite watcher ignores along with node_modules, so Tailwind kept the CSS it compiled before style modules recorded their utilities. In the skins playground that dropped classes such as gap-2.5 from the default video controls until a stylesheet was touched. The style plugin now re-includes the manifest through server.watch.ignored, and it carries the candidates of an earlier session forward so a restart never shrinks the manifest before modules record again.
The live video skins open a captions menu from the captions button, and that trigger lost its tooltip when the menu started rendering the real captions button. Wrap the trigger in the shared button tooltip so both targets label it from the button, with the captions shortcut, like every other control. The Vite contract test now expects the tooltip instead of asserting its absence.
Tailwind skips gitignored directories, and app/_generated is ignored, so the React skins the setup task installs there were never scanned; the Tailwind sandbox pages rendered without any skin utilities while the HTML tree already had an explicit source. List the components directory the same way.
React Fast Refresh treated dev/main.tsx as a boundary, so whenever workspace dist files were rebuilt while the playground ran, hundreds of module updates re-executed the entry and invalidated the player context module mid-render, throwing "usePlayerContext must be used within a Player" before Vite gave up and reloaded. Excluding the entry from refresh lets those updates fall through to one full reload while skin component edits keep hot refreshing.
…tails The playground's copy button now produces a markdown report with the URL, branch and commit, every option, browser, viewport, the live state of the preferences the theme reacts to, and the last runtime errors, and shows it inline when the clipboard is unavailable. Preference badges above the player update as DevTools emulation flips them, so testers know which mode they are looking at.
The package test script type checks src with unchecked index access, where the untyped array from map made the alias values possibly undefined. CI failed there while the workspace typecheck, which does not include the tests, passed.
compare=styles renders the selected skin twice, CSS above Tailwind, sharing the media, width, and every other option, so parity regressions between the two styling outputs show up in one view instead of two tabs. Works for both the React and HTML targets.
dir=rtl sets the document direction so right-to-left control order, menus, and slider previews can be checked without editing the page.
6383208 to
159089a
Compare

Stacked on #2580.
Summary
A review pass over the skins design system after the generated-skin parity work. Every value that varies per theme or preference now flows through a
--media-*token with a matching Tailwind theme key, repeated selector patterns became shared utilities and variants, and the parts of the pipeline that kept those from reaching consumers were fixed along the way.Changes
duration-media-fast,scale-media-hidden, androunded-media-controls, and named after those keys. Reduced motion, reduced transparency, and forced colors override tokens in abase.preferencessublayer instead of per-rule variants. That also fixes scrims, dialogs, menu blur, slider previews, and the seek indicator, which still animated or blurred under those preferences.surface-media,focus-ring-media,mask-media-volume,clip-media-*,transition-media-popup, and friends replace repeated recipes.media-compact,media-wide,media-highlighted,media-transitioning, andmedia-anchoredreplace raw container, hover, starting-style, and@supportsprefixes. Menu items gate hover highlighting on hover-capable pointers.vjsc:candidates, keeps it watched so the dev CSS recompiles as modules record, and carries candidates across restarts. The skins playground fully reloads when workspace dist rebuilds reach its entry instead of hot swapping into a broken player context. The sandbox scans its generated Tailwind skins.Testing
pnpm -F @videojs/skins test: unit, build, and registry contracts, including a test that compiles every shipped Tailwind class against the exported registry theme and one that every aliased internal token is declared.pnpm -F vjsc test,pnpm typecheck,pnpm check:workspace, andvp run validate:shadcnfor the three registries.Note
Medium Risk
Broad visual and build-output changes across all skins and Shadcn registry consumers; mitigated by contract tests and token-driven preferences, but regressions in theming or registry install order remain possible.
Overview
This PR recenters skins styling on a shared token and Tailwind design system, then rewires builds, registries, and skin modules to use it instead of duplicated literals and ad hoc breakpoints.
Tokens and CSS layering move motion, shadows, radii, and hidden-state values onto
--media-*variables with@theme inlinealiases (duration-media-*,scale-media-hidden, etc.). Preference handling shifts into abase.preferenceslayer so reduced motion, transparency, and forced colors win without sprinklingmotion-reduce/opaqueon every rule. Preset-specific bases split intobase.video.cssandbase.audio.css, and bundling picks the right entry viaskinBaseStylesheet.Shared Tailwind recipes land in
tailwind.cssas flat@utilityandmedia-*variants (surface-media,media-compact/media-wide,media-transitioning,clip-media-*, …). Style modules and per-skin controls are trimmed to deltas; common chrome moves into layout modules (controls.styles,audio-controls.styles,time.styles,indicator.styles).Registry and packages ship one
_style-themeitem (documented fromvars.ts/utilities.ts); HTML skins import theme CSS before scopedskin.cssand no longer embed full base bundles. A new registry test asserts exported theme utilities/variants match the source and compile every Tailwind class in shipped TSX.Dev and docs: playground gains CSS vs Tailwind compare, RTL, richer copy report, and
tailwind.dev.css+ vjsc candidate manifest scanning; sandbox adds a generated-components@source. README and the create-vjsc-component skill document the pipeline. Smaller fixes include captions menuButtonTooltip, live-audio controls deduplication, and token-driven play-button hidden icons.Reviewed by Cursor Bugbot for commit 159089a. Bugbot is set up for automated code reviews on this repo. Configure here.