fix(react): expose slider thumbnail parts - #2633
Closed
mihar-22 wants to merge 12 commits into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for vjs-registry ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
@videojs/cdn
@videojs/core
@videojs/element
@videojs/html
@videojs/media
@videojs/react
@videojs/spf
@videojs/store
@videojs/utils
@videojs/cloudflare-video
@videojs/dash-video
@videojs/hlsjs-video
@videojs/mux-audio
@videojs/mux-video
@videojs/native-hls-video
@videojs/shaka-video
@videojs/spotify-audio
@videojs/tiktok-video
@videojs/twitch-video
@videojs/vimeo-video
@videojs/wistia-video
@videojs/youtube-video
@videojs/google-cast
@videojs/mux-data
commit: |
Contributor
📦 Bundle Size Report🎨 @videojs/html — 11 small size changes
Presets (7)
Media (18)
Players (5)
Skins (29)
UI Components (62)
⚛️ @videojs/react
Small changes (8, ≤ 300 B)
Presets (7)
Media (22)
Extensions (2)
Players (5)
Skins (18)
UI Components (39)
🧩 @videojs/core — 2 small size changes
Entries (76)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (13)
📦 @videojs/cdn — no changes📦 @videojs/cloudflare-video — no changes📦 @videojs/dash-video — no changes📦 @videojs/google-cast — no changes📦 @videojs/hlsjs-video — no changes📦 @videojs/media — no changesEntries (3)
📦 @videojs/mux — no changes📦 @videojs/mux-audio — no changesEntries (2)
📦 @videojs/mux-data — no changes📦 @videojs/mux-video — no changesEntries (2)
📦 @videojs/native-hls-video — no changes📦 @videojs/shaka-video — no changes📦 @videojs/spf — no changesEntries (7)
📦 @videojs/spotify-audio — no changes📦 @videojs/tiktok-video — no changes📦 @videojs/twitch-video — no changes📦 @videojs/vimeo-video — no changes📦 @videojs/wistia-video — no changesEntries (2)
📦 @videojs/youtube-video — no changesℹ️ 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 |
![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 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 430a701. Configure here.
Match the Slider.Thumbnail.Image part name already used by the VJSC schema so every image part reads *.Image. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move the crossorigin resolution rule onto ThumbnailCore so the HTML element and React image share it, and replace the React context's ref plus connect/disconnect callbacks with a single callback ref owned by the root. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Compose the forwarded ref with the root's image ref once, so React no longer detaches and reattaches the image on every render. When a renderer does hand the same settled image back, `connect()` now only notifies when the checked state actually changed, which stops the render loop Bugbot reported. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The convenience component accepted root children through its props type but its JSX child replaced them, so overlays never mounted. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The React target now mounts `Slider.Thumbnail.Root` as the thumbnail host with a plain `<img>` image part, but the styles still sized the image and read loading from a descendant. The root carries the size limits and the gradient overlay, reads its own `data-loading`, and the spinner sits above the overlay; the HTML root is still a wrapper around `<media-slider-thumbnail>`, so its descendant selectors and Shadow DOM image variant stay for that target. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…try skins Shadcn's `utils` registry item for Tailwind v4 projects now re-exports `cn` from the new `cn` package. The install fixture's `--overwrite` let that item replace the fixture helper, and the Sandbox copied it without installing the package, so Vite failed to resolve `cn` and the sandbox build broke. Write the helper built from `clsx` and `tailwind-merge` again before copying the installed lib. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
Closing as superseded by #2566 and #2568. This branch merged the #2566 history and re-implemented the slider parts on top; #2568 carries the same |
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
1 participant
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 #2566
Summary
Expose slider thumbnails as nested React compound parts so
Slider.Thumbnail.RootandSlider.Thumbnail.Imagematch the existing VJSC and skin composition surface.Changes
split the shared React thumbnail implementation into root and image parts while preserving the shared thumbnail behavior
export
Slider.Thumbnail.RootandSlider.Thumbnail.Imagefrom the React slider namespace and update slider thumbnail tests to cover the composed partsupdate the React skin target and the seek preview demo so generated and documented slider thumbnails use the nested part API
size and gate the thumbnail from its root in the skin styles, since the React root is the host now and the image part is a real
<img>; the HTML root is still a wrapper around<media-slider-thumbnail>, so its descendant selectors and Shadow DOM image variant stay, andpackages/skins/src/gaps.mdsays socarry the Sandbox
cnhelper fix from fix(packages): adopt cn for class name merging #2641 so the Vercel deploy builds; it drops out on rebase once fix(packages): adopt cn for class name merging #2641 landsTesting
pnpm -F @videojs/react test src/ui/slider src/ui/thumbnail src/presetspnpm -F @videojs/skins testpnpm typecheckpnpm lintNote
Medium Risk
Breaking React composition API for Thumbnail and slider previews; DOM thumbnail lifecycle changes could affect edge cases around image ref swaps and loading state.
Overview
Splits the React Thumbnail into
Thumbnail.RootandThumbnail.Image, and mirrors that underSlider.Thumbnail.Root/Slider.Thumbnail.Image. The root owns time/thumbnail resolution, sizing, anddata-*state; callers compose the image (and overlays) explicitly, withcrossOrigin,loading, andrenderon the image part.ThumbnailCoregains sharedresolveCrossOriginand separateRootProps/ImageProps. The DOMcreateThumbnailhelper rebinds per-image listeners, addsdisconnectImg, and skips redundantonStateChangewhen a cached image is re-attached after React ref churn.Skins, the React skin target, docs, and demos are updated so slider previews style
data-loadingon the React root while keeping HTMLhas-*fallbacks.SliderThumbnailremains as an alias for the slider root only; apps must addThumbnail.Image(orSlider.Thumbnail.Image) where a preview image is required.Reviewed by Cursor Bugbot for commit a1a596a. Bugbot is set up for automated code reviews on this repo. Configure here.