Skip to content

feat(react): port time slider styling into video skin presets#666

Merged
sampotts merged 1 commit intomainfrom
feat/skins
Mar 1, 2026
Merged

feat(react): port time slider styling into video skin presets#666
sampotts merged 1 commit intomainfrom
feat/skins

Conversation

@sampotts
Copy link
Copy Markdown
Collaborator

@sampotts sampotts commented Mar 1, 2026

Closes #616

Summary

  • Replace temporary spacer placeholders with TimeSlider components in all four React video skin presets (minimal, default, CSS, and Tailwind variants)
  • Rework slider CSS: move focus ring from track to thumb, add fill and buffer sub-elements with CSS custom property sizing, position thumb absolutely via --media-slider-fill
  • Add z-index to overlay so it sits above the video element
  • Pass label and orientation props through React slider roots, use defaultProps from core classes for default step/largeStep values

Test plan

  • pnpm -F @videojs/react typecheck
  • Verify time slider renders and is interactive in both skin variants (pnpm dev)
  • Verify keyboard navigation and focus ring on the slider thumb
  • Verify buffer and fill bars update during playback

🤖 Generated with Claude Code

@netlify
Copy link
Copy Markdown

netlify bot commented Mar 1, 2026

Deploy Preview for vjs10-site ready!

Name Link
🔨 Latest commit fb0f01d
🔍 Latest deploy log https://app.netlify.com/projects/vjs10-site/deploys/69a4b007588efe000873bb62
😎 Deploy Preview https://deploy-preview-666--vjs10-site.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
vjs-10-demo-react Ignored Ignored Mar 1, 2026 9:30pm

Request Review

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 1, 2026

📦 Bundle Size Report

Package Size Diff %
@videojs/core 10.28 kB 0 B ░░░░░░░░ 0%
@videojs/element 1.60 kB 0 B ░░░░░░░░ 0%
@videojs/html 18.36 kB 0 B ░░░░░░░░ 0%
@videojs/icons 3.79 kB 0 B ░░░░░░░░ 0%
@videojs/react 14.90 kB 0 B ░░░░░░░░ 0%
@videojs/store 1.95 kB 0 B ░░░░░░░░ 0%
@videojs/utils 2.81 kB 0 B ░░░░░░░░ 0%

Total: 53.68 kB · 0 B · 0%


Entry Breakdown

Subpath sizes are the additional bytes on top of the root entry point, measured by bundling root + subpath together and subtracting the root-only size.

@videojs/core
Entry Base PR Diff %
. 4.28 kB 4.28 kB 0 B 0%
./dom 6.01 kB 6.01 kB 0 B 0%
total 10.28 kB 10.28 kB 0 B 0%
@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/html
Entry Base PR Diff %
. 15.20 kB 15.20 kB 0 B 0%
./video 1.06 kB 1.06 kB 0 B 0%
./audio 1.06 kB 1.06 kB 0 B 0%
./background 1.05 kB 1.05 kB 0 B 0%
total 18.36 kB 18.36 kB 0 B 0%
@videojs/icons
Entry Base PR Diff %
./react 2.27 kB 2.27 kB 0 B 0%
./html 1.52 kB 1.52 kB 0 B 0%
total 3.79 kB 3.79 kB 0 B 0%
@videojs/store
Entry Base PR Diff %
. 1.29 kB 1.29 kB 0 B 0%
./html 468 B 468 B 0 B 0%
./react 204 B 204 B 0 B 0%
total 1.95 kB 1.95 kB 0 B 0%
@videojs/utils
Entry Base PR Diff %
./array 104 B 104 B 0 B 0%
./dom 928 B 928 B 0 B 0%
./events 227 B 227 B 0 B 0%
./function 261 B 261 B 0 B 0%
./object 119 B 119 B 0 B 0%
./predicate 265 B 265 B 0 B 0%
./string 148 B 148 B 0 B 0%
./style 185 B 185 B 0 B 0%
./time 478 B 478 B 0 B 0%
./number 158 B 158 B 0 B 0%
total 2.81 kB 2.81 kB 0 B 0%

ℹ️ 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.

@sampotts sampotts changed the title feat(packages): add slider elements and integrate time slider into skins feat(react): port time slider styling into video skin presets Mar 1, 2026
@sampotts sampotts merged commit ebb75f5 into main Mar 1, 2026
21 checks passed
@sampotts sampotts deleted the feat/skins branch March 1, 2026 21:34
@github-actions github-actions bot mentioned this pull request Mar 1, 2026
@sampotts sampotts requested a review from Copilot March 1, 2026 23:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Ports the TimeSlider UI/styling into the React video skin presets to replace temporary slider spacers and restore the tech-preview time slider look/behavior (fill/buffer bars, thumb focus ring), plus ensures the overlay scrim layers correctly above the video element.

Changes:

  • Replace placeholder spacer elements with TimeSlider compound components across CSS and Tailwind presets (default + minimal).
  • Rework slider styling to use fill/buffer sub-elements and move focus ring styling to the thumb.
  • Add z-index to .media-overlay so the scrim reliably layers above the video.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/react/src/presets/video/skin.tsx Swap temporary time slider spacer for TimeSlider compound parts in the default CSS preset.
packages/react/src/presets/video/skin.tailwind.tsx Add TimeSlider compound parts and Tailwind-based fill/buffer/thumb styling in the default Tailwind preset.
packages/react/src/presets/video/skin.css Update default CSS preset slider styles (thumb focus ring, fill/buffer elements) and overlay z-index.
packages/react/src/presets/video/minimal-skin.tsx Swap temporary time slider spacer for TimeSlider compound parts in the minimal CSS preset.
packages/react/src/presets/video/minimal-skin.tailwind.tsx Add TimeSlider compound parts and Tailwind-based fill/buffer/thumb styling in the minimal Tailwind preset.
packages/react/src/presets/video/minimal-skin.css Update minimal CSS preset slider styles (thumb focus ring, fill/buffer elements) and overlay z-index.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +310 to +315
<TimeSlider.Fill className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 left-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Tailwind classes use physical left-0 / left-(--media-slider-fill) for fill/buffer and thumb positioning, which will render incorrectly in RTL because the slider core reports percents from the inline-start edge. Prefer logical inset utilities (e.g., start-0 / start-(--media-slider-fill)) and consider adding data-orientation variants if vertical sliders are meant to work.

Suggested change
<TimeSlider.Fill className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 left-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',
<TimeSlider.Fill className="absolute inset-y-0 start-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 start-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 start-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',

Copilot uses AI. Check for mistakes.
Comment on lines 482 to +487
.media-default-skin .media-slider__thumb {
z-index: 10;
position: absolute;
top: 50%;
left: var(--media-slider-fill);
transform: translate(-50%, -50%);
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The slider thumb is positioned with physical left: var(--media-slider-fill), which breaks RTL: the core slider flips percents for RTL, but this CSS always lays out left→right. Use logical positioning (e.g., inset-inline-start) and add an orientation-specific rule so vertical sliders can position the thumb on the block axis.

Copilot uses AI. Check for mistakes.
Comment on lines +517 to +537
/* Shared track fills */
.media-default-skin .media-slider__buffer,
.media-default-skin .media-slider__fill {
position: absolute;
inset-block: 0;
left: 0;
border-radius: inherit;
pointer-events: none;
}

/* Pointer (hover preview) */
.media-default-skin .media-slider__pointer {
/* Buffer */
.media-default-skin .media-slider__buffer {
background-color: oklch(1 0 0 / 0.2);
border-radius: inherit;
width: var(--media-slider-buffer);
transition: width 0.25s ease-out;
}

/* Progress fill */
.media-default-skin .media-slider__progress {
/* Fill */
.media-default-skin .media-slider__fill {
background-color: oklch(1 0 0);
border-radius: inherit;
width: var(--media-slider-fill);
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

media-slider__buffer/media-slider__fill are anchored with left: 0 and grow via width, which also breaks RTL (buffer/fill should originate from the inline-start edge). Consider switching to logical properties (inset-inline-start: 0 + inline-size: var(...)) and adding a vertical-orientation variant (block-start/end + block-size) if vertical sliders are intended to be supported.

Copilot uses AI. Check for mistakes.
Comment on lines 468 to +472
.media-minimal-skin .media-slider__thumb {
position: absolute;
top: 50%;
left: var(--media-slider-fill, 0%);
transform: translate(-50%, -50%);
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The slider thumb uses physical left: var(--media-slider-fill, 0%), which breaks RTL for the same reason as in the default skin (core flips percents in RTL, but this CSS always lays out left→right). Use logical positioning (inset-inline-start) and add a vertical-orientation rule so the thumb can be positioned along the block axis when data-orientation="vertical".

Copilot uses AI. Check for mistakes.
Comment on lines +502 to 523
/* Shared track fills */
.media-minimal-skin .media-slider__buffer,
.media-minimal-skin .media-slider__fill {
position: absolute;
inset-block: 0;
left: 0;
border-radius: inherit;
pointer-events: none;
}

/* Pointer (hidden in minimal skin) */
.media-minimal-skin .media-slider__pointer {
display: none;
/* Buffer */
.media-minimal-skin .media-slider__buffer {
background-color: oklch(1 0 0 / 0.2);
width: var(--media-slider-buffer, 0%);
transition: width 0.25s ease-out;
}

/* Progress fill */
.media-minimal-skin .media-slider__progress {
/* Fill */
.media-minimal-skin .media-slider__fill {
background-color: oklch(1 0 0);
border-radius: inherit;
width: var(--media-slider-fill, 0%);
}
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

media-slider__buffer/media-slider__fill are anchored with left: 0 and grow via width, which will render incorrectly in RTL (the filled region should originate from inline-start). Prefer logical properties (inset-inline-start + inline-size) and add an orientation-aware variant for vertical sliders if vertical is supported.

Copilot uses AI. Check for mistakes.
Comment on lines +317 to +322
<TimeSlider.Fill className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 left-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',
Copy link

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These Tailwind classes use physical left-0 / left-(--media-slider-fill) for the fill/buffer and thumb, which will be incorrect in RTL since the slider core flips percents for RTL. Consider switching to logical inset utilities (e.g., start-0 / start-(--media-slider-fill)), and if vertical orientation is supported add data-orientation variants to position the thumb along the block axis.

Suggested change
<TimeSlider.Fill className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 left-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 left-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',
<TimeSlider.Fill className="absolute inset-y-0 start-0 rounded-[inherit] pointer-events-none bg-white w-(--media-slider-fill)" />
<TimeSlider.Buffer className="absolute inset-y-0 start-0 rounded-[inherit] pointer-events-none bg-white/20 w-(--media-slider-buffer) transition-[width] duration-250 ease-out" />
</TimeSlider.Track>
<TimeSlider.Thumb
className={cn(
'z-10 absolute top-1/2 start-(--media-slider-fill) -translate-x-1/2 -translate-y-1/2',

Copilot uses AI. Check for mistakes.
@github-actions github-actions bot mentioned this pull request Mar 10, 2026
This was referenced Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Skins: Port over time slider styling

2 participants