Skip to content

refactor(Video): Remove SCSS from Video - #3390

Merged
aresnik11 merged 18 commits into
mainfrom
ajr-video-no-scss
Aug 3, 2026
Merged

refactor(Video): Remove SCSS from Video#3390
aresnik11 merged 18 commits into
mainfrom
ajr-video-no-scss

Conversation

@aresnik11

@aresnik11 aresnik11 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

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:

  • SCSS → CSS: replaced Video/styles/vds_base_theme.scss with a plain vidstack-styles.css (vendor @imports + gamut overrides in one file). Player imports it as a side-effect import, same as before.
  • Bare @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.
  • Added css-loader >=4.0.0 as an optional peerDependency to document that bare @import resolution requires css-loader v4+ (no effect on Vite/other-bundler consumers).
  • File structure: moved VidstackPlayer into its own folder (lib/VidstackPlayer/index.tsx) and co-located variables.ts +vidstack-styles.css with it.
  • Build: copy .css (instead of .scss) to dist.

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: GMT-1662
  • Version plan added/updated (or not needed)
  • I have run this code to verify it works
  • This PR includes unit tests for the code change
  • This PR includes testing instructions tests for the code change
  • The alpha package of this PR is passing end-to-end tests in all relevant Codecademy repositories

Testing instructions

Verify the Vidstack player looks and behaves identically to main — the goal is a no-visual-change refactor.

  1. Run Storybook and open the Video stories.
  2. For a standard (mp4/HLS) video, confirm:
    • Custom control bar renders with correct button styling and border radius.
    • Play/pause, volume slider, and time slider (scrubber) look correct.
    • Hovering a control shows the tooltip with the correct arrow/caret and enter/exit fade animation.
  3. For a YouTube video, confirm the YouTube logo / copy-link overlay is still cropped out (the iframe.vds-youtube height override) both before and after playback starts.
  4. Confirm the poster/placeholder image and captions/text tracks still render as expected. (Note: not sure why the video only buffers on preview/prod but it seems to work locally)

PR Links and Envs

Repository PR Link
Monolith Monolith PR
Mono Mono PR

@nx-cloud

nx-cloud Bot commented Jun 30, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit 62f29a2


☁️ Nx Cloud last updated this comment at 2026-08-03 13:54:15 UTC

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.10%. Comparing base (52edfc9) to head (62f29a2).
✅ All tests successful. No failed tests found.

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              
Flag Coverage Δ
main ?
pull-request 90.10% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aresnik11 aresnik11 changed the title remove scss refactor(Video): Remove SCSS from Video Jul 6, 2026
aresnik11 and others added 5 commits July 7, 2026 15:32
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>
@aresnik11
aresnik11 marked this pull request as ready for review July 16, 2026 20:09
@aresnik11
aresnik11 requested a review from a team as a code owner July 16, 2026 20:09

@dreamwasp dreamwasp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Comment thread packages/gamut/src/Video/lib/VidstackPlayer.tsx Outdated
@aresnik11

Copy link
Copy Markdown
Contributor Author

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

Comment thread packages/gamut/src/Video/vidstack-vendor.css Outdated
@aresnik11
aresnik11 marked this pull request as draft July 24, 2026 18:39
@aresnik11
aresnik11 force-pushed the ajr-video-no-scss branch from f3d5d2a to 9f3f3c3 Compare July 28, 2026 19:36
@aresnik11
aresnik11 marked this pull request as ready for review July 29, 2026 19:53
Comment on lines +51 to +54
"peerDependenciesMeta": {
"css-loader": {
"optional": true
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

@jakemhiller jakemhiller left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lgtm!

@dreamwasp dreamwasp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, i wonder if there afd any other deps we don't need now that scss is dead

Comment thread packages/gamut/project.json Outdated
"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"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we get rid of scss here now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes! and the robot found 3 other places we can remove it too

@codecademydev

Copy link
Copy Markdown
Collaborator

📬 Published Alpha Packages:

Package Version npm Diff
@codecademy/gamut 72.3.2-alpha.653b9e.0 npm diff
@codecademy/gamut-icons 9.57.11-alpha.653b9e.0 npm diff
@codecademy/gamut-illustrations 0.58.16-alpha.653b9e.0 npm diff
@codecademy/gamut-kit 3.0.12-alpha.653b9e.0 npm diff
@codecademy/gamut-patterns 0.10.35-alpha.653b9e.0 npm diff
@codecademy/gamut-styles 20.0.3-alpha.653b9e.0 npm diff
@codecademy/gamut-tests 6.0.6-alpha.653b9e.0 npm diff
@codecademy/variance 0.26.2-alpha.653b9e.0 npm diff
eslint-plugin-gamut 2.4.4-alpha.653b9e.0 npm diff

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

@aresnik11
aresnik11 merged commit f150234 into main Aug 3, 2026
16 checks passed
@aresnik11
aresnik11 deleted the ajr-video-no-scss branch August 3, 2026 14:05
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.

5 participants