refactor: unify use-case, skin, and install-method pickers into shared option builders#1757
Draft
decepulis wants to merge 1 commit into
Draft
refactor: unify use-case, skin, and install-method pickers into shared option builders#1757decepulis wants to merge 1 commit into
decepulis wants to merge 1 commit into
Conversation
…d option builders Extends the renderer-options.ts pattern from #1732 to the three remaining installation pickers so the docs UI and @videojs/cli stay in lockstep — one source of truth per picker, no drift. - Add usecase-options.ts, skin-options.ts, and install-method-options.ts (label map + buildOptions) alongside renderer-options.ts. - UI pickers (UseCasePicker, SkinPicker, HTMLInstallTabs) and the CLI prompt consume the shared builders; icons and tab panels are mapped on top. - Derive the CLI's --install-method flag validation from the shared label map. - Mirror the new modules in the CLI site-modules shim and the tsdown/vitest aliases, and assert their presence in site-aliases.test.ts. - Reconcile drifted labels: skin "none" -> "None (headless)" (was "No Skin" in the UI); install CDN tab -> "CDN" (was lowercase "cdn"). Closes #1751. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TMRTUmrr3wSHjY2y1ys2f4
✅ Deploy Preview for vjs10-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📦 Bundle Size Report🎨 @videojs/html — no changesPresets (7)
Media (10)
Players (5)
Skins (30)
UI Components (38)
Sizes are marginal over the root entry point. ⚛️ @videojs/react — no changesPresets (7)
Media (9)
Skins (27)
UI Components (32)
Sizes are marginal over the root entry point. 🧩 @videojs/core — no changesEntries (14)
🏷️ @videojs/element — no changesEntries (2)
📦 @videojs/store — no changesEntries (3)
🔧 @videojs/utils — no changesEntries (10)
📦 @videojs/spf — no changesEntries (4)
ℹ️ How to interpretJS sizes are initial static graph totals (minified + brotli). Lazy dynamic chunks are shown separately when present.
Run |
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
2 participants
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.
Closes #1751. Follow-up from #1732.
#1732 introduced
site/src/utils/installation/renderer-options.ts— a shared, pure module (label map +buildOptions) that the docs UI picker and the@videojs/cliprompt both consume so the renderer options can't drift. This PR extends that pattern to the three remaining installation pickers: use case, skin, and install method.What's in here
Three new shared modules alongside
renderer-options.ts, each exporting a label map + abuildOptions-style function:usecase-options.tsUSE_CASE_LABELS,buildOptions()UseCasePicker.tsx, CLI preset promptskin-options.tsSKIN_LABELS,buildOptions(useCase)SkinPicker.tsx, CLI skin promptinstall-method-options.tsINSTALL_METHOD_LABELS,buildOptions({ includeCdn })HTMLInstallTabsClient.tsx, CLI install-method promptpackages/cli/src/utils/prompts.ts) replacesPRESET_OPTIONS,skinOptionsForUseCase, andinstallMethodOptionswith thin wrappers over the shared builders. The non-interactive--install-methodvalidation indocs.tsnow derives its accepted values fromINSTALL_METHOD_LABELS(mirroring how--mediaalready derives fromRENDERER_LABELS).site-modules.d.ts, aliased intsdown.config.ts+vitest.config.ts, and asserted insite-aliases.test.ts.Reconciled label drift
The issue flagged the skin "none" label as drifted. Reconciled, plus one more I found while wiring the tabs — both are one-line flips if you'd prefer the other direction:
No SkinNone (headless)None (headless)cdnCDNCDNI picked
None (headless)(more descriptive for the dev audience — explains the consequence) andCDN(acronym convention, already the CLI's label). Happy to flip either.Note
The skin builder returns a single fixed
Defaultoption forbackground-video— matching the CLI today and the fact that the UI hides the skin picker entirely for that use case (SkinPickerSectionClientreturnsnull).Tests & verification
__tests__/{usecase,skin,install-method}-options.test.ts), mirroringrenderer-options.test.ts.tsdownbundle builds (new aliases + shim resolve);pnpm check:workspacepasses.Note
pnpm typecheckreports ~510 errors inpackages/{html,react,store}on this branch and on the base commit — a pre-existingtsgobuild-state issue in a fresh checkout, unrelated to this diff. The changed files (site/,packages/cli/) add zero type errors.🤖 Generated with Claude Code
https://claude.ai/code/session_01TMRTUmrr3wSHjY2y1ys2f4
Generated by Claude Code