refactor(Video): Remove SCSS from Video - #3390
Conversation
|
View your CI Pipeline Execution ↗ for commit 62f29a2 ☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3390 +/- ##
==========================================
- Coverage 90.56% 90.10% -0.47%
==========================================
Files 398 270 -128
Lines 6616 5700 -916
Branches 2153 1925 -228
==========================================
- Hits 5992 5136 -856
+ Misses 615 555 -60
Partials 9 9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Update vidstack-vendor.css to use the webpack ~ module resolution prefix, which tells webpack to resolve imports from node_modules. This ensures the CSS imports are correctly resolved in both Storybook and consumer builds like Codecademy. Fixes module resolution error: "Can't resolve './@vidstack/react/..." when Codecademy's webpack processes the CSS file from the gamut dist output. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
… beak Applying vdsBaseThemeStyles as a styled() argument on VariableProvider scoped every .vds-* selector under the player's generated class, which broke the tooltip beak (rendered as a full diamond). Render the overrides via <Global> instead so they apply at document level, matching the original flat cascade and reaching portaled .vds-* nodes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dreamwasp
left a comment
There was a problem hiding this comment.
on mono, i was seeing that the tayra examples don't have the Closed Caption buttons on the ones on prod do. idk if thats just a weird tayra issue but wanted to mention it
ill look into it but im guessing the captions get inserted from Percipio video data that doesnt work on previews/staging. that would be the same reason i had to force the VidstackPlayer to render in the mono PR |
f3d5d2a to
9f3f3c3
Compare
| "peerDependenciesMeta": { | ||
| "css-loader": { | ||
| "optional": true | ||
| } |
dreamwasp
left a comment
There was a problem hiding this comment.
lgtm, i wonder if there afd any other deps we don't need now that scss is dead
| "babel ./src --out-dir ./dist --extensions \".ts,.tsx\"", | ||
| "tsc --emitDeclarationOnly --project ./tsconfig.lib.json", | ||
| "cpy './src/**/*.{d.ts,scss,svg,png}' ./dist" | ||
| "cpy './src/**/*.{d.ts,scss,css,svg,png}' ./dist" |
There was a problem hiding this comment.
can we get rid of scss here now?
There was a problem hiding this comment.
yes! and the robot found 3 other places we can remove it too
|
📬 Published Alpha Packages:
|
|
🚀 Styleguide deploy preview ready! Preview URL: https://6a709e4cd985cb061fcd5475--gamut-preview.netlify.app |
Overview
The Video component was the last consumer of a standalone SCSS file (
vds_base_theme.scss) for styling the Vidstack player. This PR removes that SCSS dependency, moves the styles to plain CSS, and makes the vendor imports resolve in non-webpack bundlers.Changes:
Video/styles/vds_base_theme.scsswith a plainvidstack-styles.css(vendor@imports + gamut overrides in one file). Player imports it as a side-effect import, same as before.@import: switched@import "~@vidstack/..."(webpack-only~tilde prefix) to a bare@import "@vidstack/...", so the Vidstack theme resolves from node_modules in Vite and other modern bundlers — not just webpack.css-loader >=4.0.0as an optionalpeerDependencyto document that bare@importresolution requires css-loader v4+ (no effect on Vite/other-bundler consumers).VidstackPlayerinto its own folder (lib/VidstackPlayer/index.tsx) and co-locatedvariables.ts+vidstack-styles.csswith it..css(instead of.scss) todist.PR Checklist
Testing instructions
Verify the Vidstack player looks and behaves identically to
main— the goal is a no-visual-change refactor.iframe.vds-youtubeheight override) both before and after playback starts.PR Links and Envs