From fd4d2662ea268c6d6f9f75cd3e2222e7320f132b Mon Sep 17 00:00:00 2001 From: rahim Date: Mon, 27 Jul 2026 16:21:40 -0700 Subject: [PATCH] chore(root): refresh agent skills and docs (#1835) --- .agents/plans/README.md | 10 + .agents/skills/build-ui-component/SKILL.md | 31 + .../references/animation.md | 0 .../references/anti-patterns.md | 1 - .../references/collection.md | 1 - .../build-ui-component}/references/html.md | 0 .../references/polymorphism.md | 1 - .../build-ui-component}/references/props.md | 0 .../build-ui-component}/references/react.md | 6 - .../build-ui-component}/references/styling.md | 0 .../references/videojs-element.md | 0 .../build-ui-component}/references/videojs.md | 0 .agents/skills/change-spf-behavior/SKILL.md | 37 + .../change-spf-behavior/references/merge.md | 30 +- .../references/refactor.md | 20 +- .../change-spf-behavior/references/split.md | 32 +- .../change-spf-behavior/references/update.md | 42 +- .agents/skills/commit-pr/SKILL.md | 31 + .../skills/commit-pr}/references/branch.md | 0 .../skills/commit-pr}/references/commit.md | 0 .../skills/commit-pr}/references/pr.md | 0 .../skills/commit-pr}/references/scope.md | 0 .agents/skills/create-issue/SKILL.md | 28 + .agents/skills/create-skill/SKILL.md | 39 + .../create-skill/references/patterns.md | 19 + .../create-skill/references/principles.md | 25 + .../create-skill/references/structure.md | 29 + .agents/skills/create-spf-behavior/SKILL.md | 25 + .../references/workflow.md | 30 +- .agents/skills/design-api/SKILL.md | 34 + .../design-api}/references/anti-patterns.md | 1 - .../design-api}/references/extensibility.md | 0 .../design-api}/references/libraries.md | 0 .../design-api}/references/principles.md | 0 .../skills/design-api}/references/state.md | 0 .../design-api}/references/typescript.md | 0 .../skills/design-api}/references/voices.md | 0 .agents/skills/document-spf-feature/SKILL.md | 23 + .../references/workflow.md | 20 +- .agents/skills/document-spf-use-case/SKILL.md | 23 + .../references/workflow.md | 36 +- .../skills/implement-accessible-ui/SKILL.md | 30 + .../references/anti-patterns.md | 1 - .../references/aria.md | 0 .../references/focus.md | 0 .../references/keyboard.md | 0 .../references/media.md | 0 .../references/react.md | 6 - .agents/skills/implement-spf-feature/SKILL.md | 25 + .../references/workflow.md | 62 +- .../skills/implement-spf-use-case/SKILL.md | 25 + .../references/workflow.md | 88 +- .agents/skills/investigate-issue/SKILL.md | 24 + .agents/skills/maintain-agent-docs/SKILL.md | 35 + .../skills/migrate-css-to-tailwind/SKILL.md | 21 + .../references/migration.md | 30 +- .agents/skills/review-accessibility/SKILL.md | 21 + .../references}/checklist.md | 0 .agents/skills/review-api/SKILL.md | 21 + .../review-api/references}/checklist.md | 17 - .agents/skills/review-branch/SKILL.md | 24 + .agents/skills/review-docs/SKILL.md | 21 + .../skills/review-tailwind-migration/SKILL.md | 21 + .../references}/checklist.md | 9 +- .agents/skills/review-ui-component/SKILL.md | 21 + .../references}/checklist.md | 11 - .../references}/checklists/general.md | 2 - .../references}/checklists/severity.md | 0 .../references}/checklists/videojs.md | 2 +- .agents/skills/write-api-reference/SKILL.md | 44 + .../references/builder-conventions.md | 0 .../references/demo-patterns.md | 0 .../references/mdx-structure.md | 0 .../references/util-conventions.md | 2 +- .agents/skills/write-design-doc/SKILL.md | 27 + .../references/components.md | 8 +- .../write-design-doc}/references/features.md | 6 +- .../write-design-doc}/references/structure.md | 2 +- .../templates/component-basic.md | 0 .../templates/component-compound.md | 0 .../write-design-doc}/templates/decision.md | 0 .../templates/feature-multi.md | 2 +- .../templates/feature-single.md | 0 .agents/skills/write-docs/SKILL.md | 39 + .../write-docs}/patterns/code-examples.md | 2 +- .../skills/write-docs}/patterns/error-docs.md | 0 .../references/component-libraries.md | 6 - .../write-docs}/references/gold-standard.md | 0 .../skills/write-docs}/references/seo.md | 0 .../write-docs}/references/state-tooling.md | 6 - .../write-docs}/references/writing-style.md | 0 .../skills/write-docs}/templates/concept.md | 0 .../skills/write-docs}/templates/how-to.md | 0 .../skills/write-docs}/templates/readme.md | 0 .agents/skills/write-rfc/SKILL.md | 26 + .claude/plans/README.md | 60 - .claude/plans/menus.md | 116 -- .claude/plans/player-api-implementation.md | 1461 -------------- .claude/plans/slider.md | 1747 ----------------- .../spf-text-track-switching-refactor.md | 97 - .../behaviors-and-media-cleanup-assessment.md | 701 ------- .../discrete-signals-and-behavior-objects.md | 463 ----- .claude/plans/spf/signals-poc.md | 262 --- .claude/plans/store/bindings.md | 29 - .claude/plans/store/feature-api-redesign.md | 268 --- .claude/plans/store/reactive.md | 46 - .claude/plans/store/store-v2.md | 1305 ------------ .claude/skills/README.md | 129 -- .claude/skills/api-reference/SKILL.md | 130 -- .claude/skills/api/SKILL.md | 110 -- .claude/skills/api/review/agents.md | 186 -- .claude/skills/api/review/example.md | 314 --- .claude/skills/api/review/templates.md | 223 --- .claude/skills/api/review/workflow.md | 85 - .claude/skills/aria/SKILL.md | 76 - .claude/skills/aria/review/templates.md | 239 --- .claude/skills/aria/review/workflow.md | 92 - .claude/skills/claude-update/SKILL.md | 47 - .claude/skills/commit-pr/SKILL.md | 124 -- .claude/skills/component/SKILL.md | 219 --- .claude/skills/component/review/templates.md | 232 --- .claude/skills/component/review/workflow.md | 108 - .claude/skills/create-issue/SKILL.md | 170 -- .claude/skills/create-skill/SKILL.md | 189 -- .../create-skill/references/patterns.md | 276 --- .../create-skill/references/principles.md | 134 -- .../create-skill/references/structure.md | 240 --- .claude/skills/css-to-tailwind/SKILL.md | 47 - .../skills/css-to-tailwind/review/workflow.md | 46 - .claude/skills/design/SKILL.md | 218 -- .claude/skills/docs/SKILL.md | 207 -- .claude/skills/docs/review/agents.md | 149 -- .claude/skills/docs/review/checklist.md | 59 - .claude/skills/docs/review/templates.md | 151 -- .claude/skills/docs/review/workflow.md | 82 - .claude/skills/gh-issue/SKILL.md | 169 -- .claude/skills/git/SKILL.md | 38 - .claude/skills/review-branch/SKILL.md | 154 -- .claude/skills/rfc/SKILL.md | 81 - .github/workflows/api-reference-sync.yml | 4 +- .github/workflows/changelog-prose.yml | 2 +- .gitignore | 11 +- AGENTS.md | 82 + CLAUDE.md | 564 +----- CONTRIBUTING.md | 48 +- build/scripts/check-workspace.mjs | 379 +++- build/scripts/link-aliases.mjs | 71 +- internal/decisions/README.md | 25 +- .../{ => player}/context-media-discovery.md | 0 .../player-container-separation.md | 0 .../decisions/{ => player}/provider-attach.md | 0 .../end-of-stream-av-skew-recovery.md | 0 .../{ => spf}/mse-timestamp-offset.md | 14 +- internal/decisions/spf/signal-ownership.md | 25 + internal/decisions/store/reactive-state.md | 25 + internal/decisions/{ => ui}/captions.md | 0 .../{ => ui}/gestures-as-components.md | 0 internal/design/README.md | 93 +- .../archive/feature-availability-design.md | 111 -- internal/design/archive/queue-design.md | 429 ---- internal/design/i18n.md | 1022 ---------- internal/design/i18n/architecture.md | 36 + internal/design/media.md | 832 -------- internal/design/media/architecture.md | 34 + internal/design/site/source-url-detection.md | 180 -- internal/design/spf/conventions/behaviors.md | 12 +- internal/design/spf/features/audio-abr.md | 2 +- .../design/spf/features/audio-playback.md | 2 +- internal/design/spf/features/clusters.md | 8 +- .../spf/features/multi-language-audio.md | 8 +- .../spf/features/non-zero-pts-support.md | 6 +- .../design/spf/presentation-timeline-model.md | 19 +- internal/design/spf/signals.md | 2 +- internal/design/spf/use-cases/README.md | 16 +- .../design/spf/use-cases/background-video.md | 2 +- internal/design/ui/gesture.md | 243 +-- internal/design/ui/hotkey.md | 384 +--- internal/design/ui/input-feedback.md | 411 +--- internal/design/ui/live-presets.md | 227 +-- internal/design/ui/menus.md | 838 +------- internal/design/ui/poster-placeholder.md | 166 +- internal/design/ui/slider.md | 177 +- packages/spf/README.md | 5 + packages/spf/docs/explainer.md | 303 --- packages/spf/docs/fundamentals.md | 1213 +----------- packages/spf/docs/hls-engine.md | 444 +---- packages/spf/src/AGENTS.md | 32 + packages/spf/src/CLAUDE.md | 105 +- .../behaviors/dom/recover-end-stall.ts | 2 +- rfc/README.md | 161 +- rfc/player-api.md | 82 + rfc/player-api/api.md | 300 --- rfc/player-api/architecture.md | 182 -- rfc/player-api/decisions.md | 257 --- rfc/player-api/examples.md | 429 ---- rfc/player-api/features.md | 285 --- rfc/player-api/feedback.md | 68 - rfc/player-api/html.md | 378 ---- rfc/player-api/index.md | 112 -- rfc/player-api/primitives.md | 267 --- site/AGENTS.md | 71 + site/CLAUDE.md | 998 +--------- site/README.md | 23 +- site/astro.config.mjs | 1 - site/src/content/docs/how-to/write-guides.mdx | 4 +- .../docs/reference/write-references.mdx | 4 +- 206 files changed, 2199 insertions(+), 22577 deletions(-) create mode 100644 .agents/plans/README.md create mode 100644 .agents/skills/build-ui-component/SKILL.md rename {.claude/skills/component => .agents/skills/build-ui-component}/references/animation.md (100%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/anti-patterns.md (98%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/collection.md (97%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/html.md (100%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/polymorphism.md (97%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/props.md (100%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/react.md (98%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/styling.md (100%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/videojs-element.md (100%) rename {.claude/skills/component => .agents/skills/build-ui-component}/references/videojs.md (100%) create mode 100644 .agents/skills/change-spf-behavior/SKILL.md rename .claude/skills/merge-behaviors/SKILL.md => .agents/skills/change-spf-behavior/references/merge.md (89%) rename .claude/skills/refactor-behavior/SKILL.md => .agents/skills/change-spf-behavior/references/refactor.md (97%) rename .claude/skills/split-behavior/SKILL.md => .agents/skills/change-spf-behavior/references/split.md (92%) rename .claude/skills/spf-update-behavior/SKILL.md => .agents/skills/change-spf-behavior/references/update.md (84%) create mode 100644 .agents/skills/commit-pr/SKILL.md rename {.claude/skills/git => .agents/skills/commit-pr}/references/branch.md (100%) rename {.claude/skills/git => .agents/skills/commit-pr}/references/commit.md (100%) rename {.claude/skills/git => .agents/skills/commit-pr}/references/pr.md (100%) rename {.claude/skills/git => .agents/skills/commit-pr}/references/scope.md (100%) create mode 100644 .agents/skills/create-issue/SKILL.md create mode 100644 .agents/skills/create-skill/SKILL.md create mode 100644 .agents/skills/create-skill/references/patterns.md create mode 100644 .agents/skills/create-skill/references/principles.md create mode 100644 .agents/skills/create-skill/references/structure.md create mode 100644 .agents/skills/create-spf-behavior/SKILL.md rename .claude/skills/spf-create-behavior/SKILL.md => .agents/skills/create-spf-behavior/references/workflow.md (89%) create mode 100644 .agents/skills/design-api/SKILL.md rename {.claude/skills/api => .agents/skills/design-api}/references/anti-patterns.md (98%) rename {.claude/skills/api => .agents/skills/design-api}/references/extensibility.md (100%) rename {.claude/skills/api => .agents/skills/design-api}/references/libraries.md (100%) rename {.claude/skills/api => .agents/skills/design-api}/references/principles.md (100%) rename {.claude/skills/api => .agents/skills/design-api}/references/state.md (100%) rename {.claude/skills/api => .agents/skills/design-api}/references/typescript.md (100%) rename {.claude/skills/api => .agents/skills/design-api}/references/voices.md (100%) create mode 100644 .agents/skills/document-spf-feature/SKILL.md rename .claude/skills/spf-document-feature/SKILL.md => .agents/skills/document-spf-feature/references/workflow.md (97%) create mode 100644 .agents/skills/document-spf-use-case/SKILL.md rename .claude/skills/spf-document-use-case/SKILL.md => .agents/skills/document-spf-use-case/references/workflow.md (96%) create mode 100644 .agents/skills/implement-accessible-ui/SKILL.md rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/anti-patterns.md (98%) rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/aria.md (100%) rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/focus.md (100%) rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/keyboard.md (100%) rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/media.md (100%) rename {.claude/skills/aria => .agents/skills/implement-accessible-ui}/references/react.md (98%) create mode 100644 .agents/skills/implement-spf-feature/SKILL.md rename .claude/skills/spf-implement-feature/SKILL.md => .agents/skills/implement-spf-feature/references/workflow.md (95%) create mode 100644 .agents/skills/implement-spf-use-case/SKILL.md rename .claude/skills/spf-implement-use-case/SKILL.md => .agents/skills/implement-spf-use-case/references/workflow.md (92%) create mode 100644 .agents/skills/investigate-issue/SKILL.md create mode 100644 .agents/skills/maintain-agent-docs/SKILL.md create mode 100644 .agents/skills/migrate-css-to-tailwind/SKILL.md rename {.claude/skills/css-to-tailwind => .agents/skills/migrate-css-to-tailwind}/references/migration.md (74%) create mode 100644 .agents/skills/review-accessibility/SKILL.md rename {.claude/skills/aria/review => .agents/skills/review-accessibility/references}/checklist.md (100%) create mode 100644 .agents/skills/review-api/SKILL.md rename {.claude/skills/api/review => .agents/skills/review-api/references}/checklist.md (73%) create mode 100644 .agents/skills/review-branch/SKILL.md create mode 100644 .agents/skills/review-docs/SKILL.md create mode 100644 .agents/skills/review-tailwind-migration/SKILL.md rename {.claude/skills/css-to-tailwind/review => .agents/skills/review-tailwind-migration/references}/checklist.md (77%) create mode 100644 .agents/skills/review-ui-component/SKILL.md rename {.claude/skills/component/review => .agents/skills/review-ui-component/references}/checklist.md (54%) rename {.claude/skills/component/review => .agents/skills/review-ui-component/references}/checklists/general.md (98%) rename {.claude/skills/component/review => .agents/skills/review-ui-component/references}/checklists/severity.md (100%) rename {.claude/skills/component/review => .agents/skills/review-ui-component/references}/checklists/videojs.md (94%) create mode 100644 .agents/skills/write-api-reference/SKILL.md rename {.claude/skills/api-reference => .agents/skills/write-api-reference}/references/builder-conventions.md (100%) rename {.claude/skills/api-reference => .agents/skills/write-api-reference}/references/demo-patterns.md (100%) rename {.claude/skills/api-reference => .agents/skills/write-api-reference}/references/mdx-structure.md (100%) rename {.claude/skills/api-reference => .agents/skills/write-api-reference}/references/util-conventions.md (98%) create mode 100644 .agents/skills/write-design-doc/SKILL.md rename {.claude/skills/design => .agents/skills/write-design-doc}/references/components.md (94%) rename {.claude/skills/design => .agents/skills/write-design-doc}/references/features.md (92%) rename {.claude/skills/design => .agents/skills/write-design-doc}/references/structure.md (92%) rename {.claude/skills/design => .agents/skills/write-design-doc}/templates/component-basic.md (100%) rename {.claude/skills/design => .agents/skills/write-design-doc}/templates/component-compound.md (100%) rename {.claude/skills/design => .agents/skills/write-design-doc}/templates/decision.md (100%) rename {.claude/skills/design => .agents/skills/write-design-doc}/templates/feature-multi.md (96%) rename {.claude/skills/design => .agents/skills/write-design-doc}/templates/feature-single.md (100%) create mode 100644 .agents/skills/write-docs/SKILL.md rename {.claude/skills/docs => .agents/skills/write-docs}/patterns/code-examples.md (98%) rename {.claude/skills/docs => .agents/skills/write-docs}/patterns/error-docs.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/references/component-libraries.md (98%) rename {.claude/skills/docs => .agents/skills/write-docs}/references/gold-standard.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/references/seo.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/references/state-tooling.md (98%) rename {.claude/skills/docs => .agents/skills/write-docs}/references/writing-style.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/templates/concept.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/templates/how-to.md (100%) rename {.claude/skills/docs => .agents/skills/write-docs}/templates/readme.md (100%) create mode 100644 .agents/skills/write-rfc/SKILL.md delete mode 100644 .claude/plans/README.md delete mode 100644 .claude/plans/menus.md delete mode 100644 .claude/plans/player-api-implementation.md delete mode 100644 .claude/plans/slider.md delete mode 100644 .claude/plans/spf-text-track-switching-refactor.md delete mode 100644 .claude/plans/spf/behaviors-and-media-cleanup-assessment.md delete mode 100644 .claude/plans/spf/discrete-signals-and-behavior-objects.md delete mode 100644 .claude/plans/spf/signals-poc.md delete mode 100644 .claude/plans/store/bindings.md delete mode 100644 .claude/plans/store/feature-api-redesign.md delete mode 100644 .claude/plans/store/reactive.md delete mode 100644 .claude/plans/store/store-v2.md delete mode 100644 .claude/skills/README.md delete mode 100644 .claude/skills/api-reference/SKILL.md delete mode 100644 .claude/skills/api/SKILL.md delete mode 100644 .claude/skills/api/review/agents.md delete mode 100644 .claude/skills/api/review/example.md delete mode 100644 .claude/skills/api/review/templates.md delete mode 100644 .claude/skills/api/review/workflow.md delete mode 100644 .claude/skills/aria/SKILL.md delete mode 100644 .claude/skills/aria/review/templates.md delete mode 100644 .claude/skills/aria/review/workflow.md delete mode 100644 .claude/skills/claude-update/SKILL.md delete mode 100644 .claude/skills/commit-pr/SKILL.md delete mode 100644 .claude/skills/component/SKILL.md delete mode 100644 .claude/skills/component/review/templates.md delete mode 100644 .claude/skills/component/review/workflow.md delete mode 100644 .claude/skills/create-issue/SKILL.md delete mode 100644 .claude/skills/create-skill/SKILL.md delete mode 100644 .claude/skills/create-skill/references/patterns.md delete mode 100644 .claude/skills/create-skill/references/principles.md delete mode 100644 .claude/skills/create-skill/references/structure.md delete mode 100644 .claude/skills/css-to-tailwind/SKILL.md delete mode 100644 .claude/skills/css-to-tailwind/review/workflow.md delete mode 100644 .claude/skills/design/SKILL.md delete mode 100644 .claude/skills/docs/SKILL.md delete mode 100644 .claude/skills/docs/review/agents.md delete mode 100644 .claude/skills/docs/review/checklist.md delete mode 100644 .claude/skills/docs/review/templates.md delete mode 100644 .claude/skills/docs/review/workflow.md delete mode 100644 .claude/skills/gh-issue/SKILL.md delete mode 100644 .claude/skills/git/SKILL.md delete mode 100644 .claude/skills/review-branch/SKILL.md delete mode 100644 .claude/skills/rfc/SKILL.md create mode 100644 AGENTS.md rename internal/decisions/{ => player}/context-media-discovery.md (100%) rename internal/decisions/{ => player}/player-container-separation.md (100%) rename internal/decisions/{ => player}/provider-attach.md (100%) rename internal/decisions/{ => spf}/end-of-stream-av-skew-recovery.md (100%) rename internal/decisions/{ => spf}/mse-timestamp-offset.md (95%) create mode 100644 internal/decisions/spf/signal-ownership.md create mode 100644 internal/decisions/store/reactive-state.md rename internal/decisions/{ => ui}/captions.md (100%) rename internal/decisions/{ => ui}/gestures-as-components.md (100%) delete mode 100644 internal/design/archive/feature-availability-design.md delete mode 100644 internal/design/archive/queue-design.md delete mode 100644 internal/design/i18n.md create mode 100644 internal/design/i18n/architecture.md delete mode 100644 internal/design/media.md create mode 100644 internal/design/media/architecture.md delete mode 100644 internal/design/site/source-url-detection.md delete mode 100644 packages/spf/docs/explainer.md create mode 100644 packages/spf/src/AGENTS.md create mode 100644 rfc/player-api.md delete mode 100644 rfc/player-api/api.md delete mode 100644 rfc/player-api/architecture.md delete mode 100644 rfc/player-api/decisions.md delete mode 100644 rfc/player-api/examples.md delete mode 100644 rfc/player-api/features.md delete mode 100644 rfc/player-api/feedback.md delete mode 100644 rfc/player-api/html.md delete mode 100644 rfc/player-api/index.md delete mode 100644 rfc/player-api/primitives.md create mode 100644 site/AGENTS.md diff --git a/.agents/plans/README.md b/.agents/plans/README.md new file mode 100644 index 00000000..3b70d486 --- /dev/null +++ b/.agents/plans/README.md @@ -0,0 +1,10 @@ +# Temporary implementation plans + +Use this directory for active, non-authoritative working notes when a task is too large to hold in an issue or conversation. + +- Name the owner, status, branch or issue, and next concrete step. +- Link to source code and durable records instead of copying them. +- Do not treat a plan as evidence of current behavior. +- Before merge, delete the plan or move only durable rationale into `internal/decisions/`, `internal/design/`, or `rfc/`. + +Completed task logs, file inventories, pasted code, and stale checklists do not belong in the repository. Git and pull requests preserve implementation history. diff --git a/.agents/skills/build-ui-component/SKILL.md b/.agents/skills/build-ui-component/SKILL.md new file mode 100644 index 00000000..2cdfe2ec --- /dev/null +++ b/.agents/skills/build-ui-component/SKILL.md @@ -0,0 +1,31 @@ +--- +name: build-ui-component +description: Build reusable Video.js HTML and React components. Use for contracts, controlled state, compound APIs, styling hooks, collections, animation, or parity. +--- + +# Component architecture + +Read the core contract, both platform adapters, tests, and nearby components before choosing a pattern. Preserve the separation between runtime-neutral core state and HTML/React bindings. + +## Workflow + +1. Define observable behavior, ownership of state, platform parity, accessibility, and styling contracts. +2. Reuse an adjacent Video.js component shape where possible. +3. Load only relevant material: + - Core component/API patterns: `references/videojs.md` + - HTML controller or element implementation: `references/html.md`, then `references/videojs-element.md` if needed + - React implementation: `references/react.md` + - Props, controlled state, or polymorphism: `references/props.md`, `references/polymorphism.md` + - Collections, animation, or styling: the matching file in `references/` + - Suspected smell: `references/anti-patterns.md` +4. Define and test the accessible interaction contract when semantics or behavior change. +5. Implement behavior in the lowest shared layer and keep adapters thin. +6. Add focused core and platform tests, then verify interactive behavior in the browser when needed. + +Expose meaningful state through stable `data-*` attributes and CSS variables rather than inline animation logic. Do not add controlled/uncontrolled modes, compound structure, or polymorphism unless the use case requires them. + +## Example + +Input: “Build matching HTML and React mute buttons.” + +Output: A shared behavior contract, thin platform adapters, accessible interaction behavior, and focused parity tests. diff --git a/.claude/skills/component/references/animation.md b/.agents/skills/build-ui-component/references/animation.md similarity index 100% rename from .claude/skills/component/references/animation.md rename to .agents/skills/build-ui-component/references/animation.md diff --git a/.claude/skills/component/references/anti-patterns.md b/.agents/skills/build-ui-component/references/anti-patterns.md similarity index 98% rename from .claude/skills/component/references/anti-patterns.md rename to .agents/skills/build-ui-component/references/anti-patterns.md index 8cce8ec6..f2bc1af5 100644 --- a/.claude/skills/component/references/anti-patterns.md +++ b/.agents/skills/build-ui-component/references/anti-patterns.md @@ -321,5 +321,4 @@ const Button = forwardRef(({ children, ...props ## See Also -- [Accessibility Anti-Patterns](../../aria/references/anti-patterns.md) — a11y mistakes - [Polymorphism](polymorphism.md) — correct render customization diff --git a/.claude/skills/component/references/collection.md b/.agents/skills/build-ui-component/references/collection.md similarity index 97% rename from .claude/skills/component/references/collection.md rename to .agents/skills/build-ui-component/references/collection.md index e1844718..fb28edb5 100644 --- a/.claude/skills/component/references/collection.md +++ b/.agents/skills/build-ui-component/references/collection.md @@ -161,5 +161,4 @@ For variable heights, measure items and cache heights. More complex — consider ## See Also -- [Focus Management](../../aria/references/focus.md) — keyboard navigation in collections - [Animation](animation.md) — exit animations for items diff --git a/.claude/skills/component/references/html.md b/.agents/skills/build-ui-component/references/html.md similarity index 100% rename from .claude/skills/component/references/html.md rename to .agents/skills/build-ui-component/references/html.md diff --git a/.claude/skills/component/references/polymorphism.md b/.agents/skills/build-ui-component/references/polymorphism.md similarity index 97% rename from .claude/skills/component/references/polymorphism.md rename to .agents/skills/build-ui-component/references/polymorphism.md index 3465ed12..f56e8a6c 100644 --- a/.claude/skills/component/references/polymorphism.md +++ b/.agents/skills/build-ui-component/references/polymorphism.md @@ -161,5 +161,4 @@ The `as` prop requires complex generic types that slow down the TypeScript langu ## See Also -- [Progressive Disclosure](../../api-design/principles/progressive-disclosure.md) — layered complexity - [Anti-Patterns](anti-patterns.md) — polymorphism pitfalls diff --git a/.claude/skills/component/references/props.md b/.agents/skills/build-ui-component/references/props.md similarity index 100% rename from .claude/skills/component/references/props.md rename to .agents/skills/build-ui-component/references/props.md diff --git a/.claude/skills/component/references/react.md b/.agents/skills/build-ui-component/references/react.md similarity index 98% rename from .claude/skills/component/references/react.md rename to .agents/skills/build-ui-component/references/react.md index 83409612..9c0a6cc6 100644 --- a/.claude/skills/component/references/react.md +++ b/.agents/skills/build-ui-component/references/react.md @@ -309,9 +309,3 @@ export { DialogTrigger as Trigger } from './trigger'; | Roving Focus | [Radix RovingFocus](https://github.com/radix-ui/primitives/tree/main/packages/react/roving-focus) | | Floating | [Floating UI React](https://floating-ui.com/docs/react) | | Merge Props | [Base UI mergeProps](https://github.com/mui/base-ui/blob/master/packages/react/src/merge-props/mergeProps.ts) | - ---- - -## See Also - -- [aria/react.md](../../aria/references/react.md) — React accessibility patterns (focus scope, announcements, a11y testing) diff --git a/.claude/skills/component/references/styling.md b/.agents/skills/build-ui-component/references/styling.md similarity index 100% rename from .claude/skills/component/references/styling.md rename to .agents/skills/build-ui-component/references/styling.md diff --git a/.claude/skills/component/references/videojs-element.md b/.agents/skills/build-ui-component/references/videojs-element.md similarity index 100% rename from .claude/skills/component/references/videojs-element.md rename to .agents/skills/build-ui-component/references/videojs-element.md diff --git a/.claude/skills/component/references/videojs.md b/.agents/skills/build-ui-component/references/videojs.md similarity index 100% rename from .claude/skills/component/references/videojs.md rename to .agents/skills/build-ui-component/references/videojs.md diff --git a/.agents/skills/change-spf-behavior/SKILL.md b/.agents/skills/change-spf-behavior/SKILL.md new file mode 100644 index 00000000..e30c5227 --- /dev/null +++ b/.agents/skills/change-spf-behavior/SKILL.md @@ -0,0 +1,37 @@ +--- +name: change-spf-behavior +description: Change SPF playback-engine behaviors. Use when updating, refactoring, splitting, or merging responsibilities, signals, lifecycle, cleanup, or composition. +--- + +# Change an SPF behavior + +Treat code and tests as implementation truth. Read the behavior, composition site, state/context slots, helpers, and tests, then load only the relevant file from `internal/design/spf/conventions/`. + +Before acting, read the one detailed workflow that matches the requested change: + +- Purpose expands or changes: `references/update.md` +- Purpose stays fixed while implementation improves: `references/refactor.md` +- One responsibility splits by type or concern: `references/split.md` +- Two responsibilities may become one: `references/merge.md` + +1. State the current purpose, intended delta, and observable business rules. +2. Map inputs, writes, lifecycle gates, cleanup, ordering, and source-reset rules. +3. Choose the smallest destination shape that preserves those constraints. +4. Pin observable behavior with tests before changing implementation. +5. Re-read the final behavior without the diff and verify ownership, cleanup, composition, naming, and coverage. + +For a split, name the ownership axis and cross-boundary invariants. For a merge, confirm the result owns one coherent purpose; extract a helper when only an algorithm is shared. Ask for direction when alternatives materially change ownership or public contracts. + +Run the narrow affected tests, then as applicable: + +```bash +pnpm -F @videojs/spf test +pnpm -F @videojs/spf build +pnpm -F @videojs/spf size +``` + +## Example + +Input: “Split source cleanup out of the playback lifecycle behavior.” + +Output: A documented ownership boundary, pinned invariants, focused implementation changes, and proportional SPF checks. diff --git a/.claude/skills/merge-behaviors/SKILL.md b/.agents/skills/change-spf-behavior/references/merge.md similarity index 89% rename from .claude/skills/merge-behaviors/SKILL.md rename to .agents/skills/change-spf-behavior/references/merge.md index f2d49769..d0afe16e 100644 --- a/.claude/skills/merge-behaviors/SKILL.md +++ b/.agents/skills/change-spf-behavior/references/merge.md @@ -1,12 +1,12 @@ --- -name: merge-behaviors +name: change-spf-behavior description: >- Merge two SPF behaviors into one with cleaned-shape-first discipline. Forces per-side standalone analysis and an explicit complexity-driven direction declaration before any combining happens — avoids the "relocated mess" failure mode where the merge anchors on the current merged-file shape rather than the cleaned per-side shapes. Use after - /refactor-behavior's decomposition check has concluded "merge," or to + /change-spf-behavior's decomposition check has concluded "merge," or to redo a merge that landed without the discipline. Triggers: "merge behaviors", "merge these behaviors", "combine behaviors", "redo merge", "merge X into Y". @@ -26,13 +26,13 @@ direction declared from a complexity inventory, then combine. ## Usage ``` -/merge-behaviors +/change-spf-behavior ``` Or for a redo case (a merge that already landed without this discipline): ``` -/merge-behaviors +/change-spf-behavior ``` For the redo case, identify the pre-merge inputs from git history (the @@ -58,17 +58,17 @@ The canonical references — read them first: conditional branches around optional state-scoped work). - [`internal/design/spf/conventions/signals.md`](../../../internal/design/spf/conventions/signals.md) — multi-writer slots, decomposition check. -- [`.claude/skills/refactor-behavior/SKILL.md`](../refactor-behavior/SKILL.md) +- [`.agents/skills/change-spf-behavior/references/refactor.md`](../change-spf-behavior/SKILL.md) — the per-side analysis (Steps 1–4) is borrowed from here. -## When to use this skill vs. /refactor-behavior +## When to use this skill vs. /change-spf-behavior | Situation | Skill | | --- | --- | -| Cleaning up a single behavior file | `/refactor-behavior` | -| Decomposition check (Step 6 of `/refactor-behavior`) concludes "this should merge with X" | `/merge-behaviors` | -| A merge already landed without per-side analysis (suspected "relocated mess") | `/merge-behaviors` | -| Splitting one behavior into two | `/refactor-behavior` (or open a design discussion first) | +| Cleaning up a single behavior file | `/change-spf-behavior` | +| Decomposition check (Step 6 of `/change-spf-behavior`) concludes "this should merge with X" | `/change-spf-behavior` | +| A merge already landed without per-side analysis (suspected "relocated mess") | `/change-spf-behavior` | +| Splitting one behavior into two | `/change-spf-behavior` (or open a design discussion first) | ## Steps (do these in order; do not skip) @@ -82,7 +82,7 @@ Confirm with the user before proceeding if the inputs aren't obvious. ### Step 2 — Per-side standalone analysis -For **each side independently**, run Steps 1–4 of `/refactor-behavior`: +For **each side independently**, run Steps 1–4 of `/change-spf-behavior`: 1. Articulate the purpose (1 sentence). Apply the **purpose-verb diagnostic** from `behaviors.md` Step 1 — note whether the verbs are @@ -92,7 +92,7 @@ For **each side independently**, run Steps 1–4 of `/refactor-behavior`: exist). 4. Pattern selection. -The per-side analysis is exactly what `/refactor-behavior` does up to +The per-side analysis is exactly what `/change-spf-behavior` does up to Step 4; you're running it twice in parallel. **Don't refactor pre-emptively.** If a side's gap analysis is empty or @@ -100,7 +100,7 @@ trivial — the current code already conforms to current conventions — record that finding and move on. The next step (cleaned-shape sketch) takes the current shape, not a projected cleaner one. Only when the gap analysis turned up real issues do you project a cleaned shape; in -that case, recommend (don't perform) `/refactor-behavior` on that side +that case, recommend (don't perform) `/change-spf-behavior` on that side as an optional standalone exercise the user may opt into. ### Step 3 — Cleaned-shape sketch per side @@ -123,7 +123,7 @@ comparable. If a side already conforms (Step 2 found no issues), the sketch describes the current shape. If the side needed projecting, the sketch is the projected post-refactor shape — and the proposal should note -that landing the standalone refactor first (via `/refactor-behavior`) +that landing the standalone refactor first (via `/change-spf-behavior`) is an option the user can choose. ### Step 4 — Complexity inventory + direction declaration @@ -163,7 +163,7 @@ in which fields are populated. ### Step 6 — Convention checks + decomposition note -Same as `/refactor-behavior` Steps 5–6: +Same as `/change-spf-behavior` Steps 5–6: - Setup-shape helper signature (`({ state, config }) => cleanup`)? Helpers parameterized by what *varies between variants* (per-type diff --git a/.claude/skills/refactor-behavior/SKILL.md b/.agents/skills/change-spf-behavior/references/refactor.md similarity index 97% rename from .claude/skills/refactor-behavior/SKILL.md rename to .agents/skills/change-spf-behavior/references/refactor.md index 5870f573..8977e07f 100644 --- a/.claude/skills/refactor-behavior/SKILL.md +++ b/.agents/skills/change-spf-behavior/references/refactor.md @@ -1,5 +1,5 @@ --- -name: refactor-behavior +name: change-spf-behavior description: >- Refactor an existing SPF behavior using purpose-first discipline. Forces articulation of the behavior's purpose and business rules before code @@ -22,7 +22,7 @@ refactors. Steps 3–6 only make sense once the purpose is named. ## Usage ``` -/refactor-behavior +/change-spf-behavior ``` `path` (required): the behavior file to refactor, e.g. @@ -134,11 +134,11 @@ Three categories: conditional branches around optional state-scoped work. **If this refactor is a merge of two behaviors, stop and use -`/merge-behaviors` instead.** A merge is two analyses combined, not +`/change-spf-behavior` instead.** A merge is two analyses combined, not one — the per-side cleaned-shape sketch + complexity-driven direction declaration that merges need don't fit cleanly inside the single-behavior workflow. See `behaviors.md` "Merging two behaviors — -extra discipline" and `.claude/skills/merge-behaviors/SKILL.md`. +extra discipline" and `.agents/skills/change-spf-behavior/references/merge.md`. ### Step 4 — Pattern selection @@ -292,7 +292,7 @@ Before writing the refactor: "Inverse: behaviors that operate uniformly across tracks." - **Downstream consumers operate per-type** → this isn't an in-place fix; this is a **split candidate**. Defer the resolution - to Step 6a — recommend `/split-behavior` rather than rewriting + to Step 6a — recommend `/change-spf-behavior` rather than rewriting the slot map here. Diagnostic: would an audio-only or video-only engine be able to @@ -355,7 +355,7 @@ not split). The distinguishing signals below pull split apart from uniform-aggregate. **Diagnostic — three split-candidate triggers. Any one firing is enough -to recommend `/split-behavior` as the follow-up.** +to recommend `/change-spf-behavior` as the follow-up.** - **Explicit per-type axis declared inline.** A `type FooType = 'video' | 'audio'`, a `KeyByType` map, a `for (const type of types)` @@ -363,7 +363,7 @@ to recommend `/split-behavior` as the follow-up.** per-type specialization was already in mind when the merged form was written — the merged form usually exists because of a *perceived cross-type constraint*. Don't pre-decide the constraint here; surface - it as the invariant `/split-behavior`'s cross-boundary audit will + it as the invariant `/change-spf-behavior`'s cross-boundary audit will evaluate. - **Sibling precedents at the same engine layer.** If per-type- specialized siblings already exist (`resolveVideoTrack`/`Audio`/ @@ -383,7 +383,7 @@ to recommend `/split-behavior` as the follow-up.** iterate `mediaSource.sourceBuffers` or similar aggregates, not when consumers consume per-type slots. -**If any trigger fires**: recommend `/split-behavior` as the follow-up +**If any trigger fires**: recommend `/change-spf-behavior` as the follow-up (don't perform the split inline, and don't pre-decide the cross- boundary constraint — that's the skill's audit step). Per `behaviors.md` "Per-type specialization" (destination shape for per-type splits) and @@ -420,12 +420,12 @@ often slots cleanly into the larger refactor of the *other* writer rather than landing as a standalone change. **If merge is the answer**: don't perform the merge inline. Recommend -`/merge-behaviors` as the follow-up — it operationalizes the per-side +`/change-spf-behavior` as the follow-up — it operationalizes the per-side cleaned-shape sketch + complexity-inventory + direction-declaration discipline that merges need. Per `behaviors.md` "Merging two behaviors — extra discipline." -(The split path is handled in 6a above. Recommend `/split-behavior` if +(The split path is handled in 6a above. Recommend `/change-spf-behavior` if any of the three triggers fire there.) ### Step 7 — Final-shape audit (after writing the change) diff --git a/.claude/skills/split-behavior/SKILL.md b/.agents/skills/change-spf-behavior/references/split.md similarity index 92% rename from .claude/skills/split-behavior/SKILL.md rename to .agents/skills/change-spf-behavior/references/split.md index 48e610a2..3ab9b7de 100644 --- a/.claude/skills/split-behavior/SKILL.md +++ b/.agents/skills/change-spf-behavior/references/split.md @@ -1,5 +1,5 @@ --- -name: split-behavior +name: change-spf-behavior description: >- Split one SPF behavior into N with axis-declared, constraints-audited discipline. Forces explicit axis declaration (per-type horizontal vs. @@ -7,7 +7,7 @@ description: >- the split lands — avoids the failure mode where an apparent per-type-friendly behavior ships a split that quietly drops a cross-type ordering invariant the merged code was enforcing. Use after - /refactor-behavior's decomposition check has concluded "split," or + /change-spf-behavior's decomposition check has concluded "split," or when you've already noticed a behavior wants splitting. Triggers: "split this behavior", "split into per-type", "extract per-type behaviors", "convert to per-type variants", "split behavior". @@ -32,7 +32,7 @@ shape is right) and a cross-boundary constraint audit *before* code ## Usage ``` -/split-behavior +/change-spf-behavior ``` Where `` is the single behavior file you're splitting. The skill @@ -60,18 +60,18 @@ The canonical references — read them first: with conditional branches around optional state-scoped work — the setup-shape helper for a per-type split must not carry per-variant conditionals). -- [`.claude/skills/refactor-behavior/SKILL.md`](../refactor-behavior/SKILL.md) +- [`.agents/skills/change-spf-behavior/references/refactor.md`](../change-spf-behavior/SKILL.md) — the per-side analysis (Steps 1–4) is borrowed from here. -## When to use this skill vs. /refactor-behavior, /merge-behaviors +## When to use this skill vs. /change-spf-behavior, /change-spf-behavior | Situation | Skill | | --- | --- | -| Cleaning up a single behavior file (same shape, same boundaries) | `/refactor-behavior` | -| `/refactor-behavior`'s decomposition check concludes "this should split" | `/split-behavior` | -| Splitting one behavior into per-type variants (video/audio/text) | `/split-behavior` (per-type axis) | -| Splitting one behavior into per-concern behaviors (two disjoint slot clusters) | `/split-behavior` (per-concern axis) | -| Merging two behaviors into one | `/merge-behaviors` | +| Cleaning up a single behavior file (same shape, same boundaries) | `/change-spf-behavior` | +| `/change-spf-behavior`'s decomposition check concludes "this should split" | `/change-spf-behavior` | +| Splitting one behavior into per-type variants (video/audio/text) | `/change-spf-behavior` (per-type axis) | +| Splitting one behavior into per-concern behaviors (two disjoint slot clusters) | `/change-spf-behavior` (per-concern axis) | +| Merging two behaviors into one | `/change-spf-behavior` | ## Steps (do these in order; do not skip) @@ -83,12 +83,12 @@ Confirm with the user if it's not obvious. ### Step 2 — Articulate the current behavior's purpose In one sentence: what is this behavior **for**? Same as -[`refactor-behavior`](../refactor-behavior/SKILL.md) Step 1. +[`change-spf-behavior`](../change-spf-behavior/SKILL.md) Step 1. The purpose establishes the basis for evaluating the split. If the purpose is one coherent thing that doesn't decompose into a per-type or per-concern shape, that's a finding ("the split doesn't earn its -keep — keep as-is or apply `/refactor-behavior` instead"). +keep — keep as-is or apply `/change-spf-behavior` instead"). Pull from the file-level JSDoc if present; if not, that's a doc gap to flag. @@ -153,12 +153,12 @@ may be wrong. post-split unit *would* look like extracted from the current code, not a projected refactored version. If a unit's sketch reveals real issues (closure-mutable state, fight-the-shape sniffs), note them as -follow-up `/refactor-behavior` candidates *for that variant after the +follow-up `/change-spf-behavior` candidates *for that variant after the split lands* — don't bundle the refactor into the split. ### Step 5 — Cross-boundary constraint audit (load-bearing) -This is the discipline that distinguishes `/split-behavior` from the +This is the discipline that distinguishes `/change-spf-behavior` from the pre-existing decomposition guidance. Skipping it is how splits ship latent races. @@ -238,7 +238,7 @@ Based on the axis: ### Step 7 — Convention checks -Same as `/refactor-behavior` Step 5 applied to each post-split unit +Same as `/change-spf-behavior` Step 5 applied to each post-split unit independently: - File placement (DOM-free vs. DOM-bound) per unit. @@ -308,4 +308,4 @@ Three failure modes the order prevents: A side benefit: the per-side sketches double as the design for each post-split unit's standalone refactor (if the user later opts to land -the split as separate `/refactor-behavior` runs on each variant). +the split as separate `/change-spf-behavior` runs on each variant). diff --git a/.claude/skills/spf-update-behavior/SKILL.md b/.agents/skills/change-spf-behavior/references/update.md similarity index 84% rename from .claude/skills/spf-update-behavior/SKILL.md rename to .agents/skills/change-spf-behavior/references/update.md index 7613a1ea..a15e8b3b 100644 --- a/.claude/skills/spf-update-behavior/SKILL.md +++ b/.agents/skills/change-spf-behavior/references/update.md @@ -1,11 +1,11 @@ --- -name: spf-update-behavior +name: change-spf-behavior description: >- Update an existing SPF behavior whose purpose is changing or expanding. - Distinct from /refactor-behavior, which preserves purpose — this skill + Distinct from /change-spf-behavior, which preserves purpose — this skill handles cases where the behavior gains new responsibility (new state slot to react to, new lifecycle phase, new constraint, new code path). Carries - /refactor-behavior's purpose-first discipline applied to the *purpose + /change-spf-behavior's purpose-first discipline applied to the *purpose change*. Triggers: "update behavior", "extend behavior", "modify behavior", "change behavior purpose", "expand behavior responsibility". --- @@ -14,7 +14,7 @@ description: >- Modify an existing SPF behavior whose purpose is **changing or expanding**. The canonical failure mode without this discipline is treating -purpose-changes as refactors — applying `/refactor-behavior`'s +purpose-changes as refactors — applying `/change-spf-behavior`'s preserve-purpose lens to a change that's actually adding responsibility. The discipline distinction matters because: @@ -24,16 +24,16 @@ discipline distinction matters because: owns a new lifecycle phase, applies a new constraint. The behavior's *contract* changes. -This skill is a **stub** scoped for use by `/spf-implement-feature`. Failure- +This skill is a **stub** scoped for use by `/implement-spf-feature`. Failure- mode catalog grows from real use. ## Usage ``` -/spf-update-behavior +/change-spf-behavior ``` -Typically invoked from `/spf-implement-feature`'s Step 6 when a feature +Typically invoked from `/implement-spf-feature`'s Step 6 when a feature implementation requires extending an existing behavior. Can be invoked directly when the user has identified the behavior to update. @@ -44,11 +44,11 @@ directly when the user has identified the behavior to update. update must continue to satisfy - `internal/design/spf/conventions/signals.md` — multi-writer characterization when adding writers to a slot another behavior writes -- `.claude/skills/refactor-behavior/SKILL.md` — the purpose-first discipline +- `.agents/skills/change-spf-behavior/references/refactor.md` — the purpose-first discipline shape this skill mirrors (applied to *purpose change* instead of *preserved purpose*) - The feature doc driving the update (if invoked from - `/spf-implement-feature`) — Step 1 grounds the update in the doc's phase + `/implement-spf-feature`) — Step 1 grounds the update in the doc's phase row or "What's not implemented" entry ## Failure-mode catalog (seeded; grows with use) @@ -74,9 +74,9 @@ directly when the user has identified the behavior to update. canonical leak shape; reorganizing cleanup without preserving order is the canonical lifecycle bug. -- **Conflating with refactor-behavior territory.** If the purpose isn't +- **Conflating with change-spf-behavior territory.** If the purpose isn't actually changing — the behavior's contract stays the same, just the - implementation improves — route to `/refactor-behavior`. The discipline + implementation improves — route to `/change-spf-behavior`. The discipline for purpose-preservation vs purpose-evolution differs; using the wrong skill produces drift in either direction (refactor-as-update bloats the behavior; update-as-refactor silently changes contracts). @@ -97,7 +97,7 @@ The load-bearing setup step. Before any code: observable interface. - **Why is this an *update*, not a *refactor*?** If the answer is "the behavior does the same thing, just differently," **stop and route to - `/refactor-behavior`**. + `/change-spf-behavior`**. **Stop and report to user** with the purpose-change articulation. The user confirms before proceeding. @@ -142,7 +142,7 @@ confirms before proceeding. ### Step 5 — Final-shape audit + commit -Per parent skill (`/spf-implement-feature`), commits are typically batched +Per parent skill (`/implement-spf-feature`), commits are typically batched at the feature-implementation level. If invoked standalone, propose a per-update commit shape. @@ -159,10 +159,10 @@ Audit checklist: ## When this is the wrong skill -- **Behavior's purpose stays the same, just code improves** → `/refactor-behavior` -- **Creating a new behavior** → `/spf-create-behavior` -- **Major restructuring (split or merge)** → `/refactor-behavior` (which - may route to `/split-behavior` or `/merge-behaviors`) +- **Behavior's purpose stays the same, just code improves** → `/change-spf-behavior` +- **Creating a new behavior** → `/create-spf-behavior` +- **Major restructuring (split or merge)** → `/change-spf-behavior` (which + may route to `/change-spf-behavior` or `/change-spf-behavior`) - **Pure config-driven change with no behavior code change** → handle in the feature implementation directly; no behavior-update needed @@ -175,14 +175,14 @@ it. ## Open framing question -The boundary between `/spf-update-behavior` and `/refactor-behavior`-with- +The boundary between `/change-spf-behavior` and `/change-spf-behavior`-with- extension is genuinely open. Per `project_spf_implementation_skills_next` -memory: *"`spf-update-behavior` OR non-trivial updates to `spf-refactor- +memory: *"`change-spf-behavior` OR non-trivial updates to `spf-refactor- behavior` — when an existing behavior needs a feature-implementation change -that isn't a pure refactor. Open which framing — extend refactor-behavior +that isn't a pure refactor. Open which framing — extend change-spf-behavior or add a new skill."* -This skill ships as a separate skill (rather than a refactor-behavior +This skill ships as a separate skill (rather than a change-spf-behavior extension) because the **purposes differ** — refactor preserves; update changes. If usage reveals the discipline is mostly shared, the skills may later merge. For now, the separation is intentional: route by diff --git a/.agents/skills/commit-pr/SKILL.md b/.agents/skills/commit-pr/SKILL.md new file mode 100644 index 00000000..29224ff8 --- /dev/null +++ b/.agents/skills/commit-pr/SKILL.md @@ -0,0 +1,31 @@ +--- +name: commit-pr +description: Commit, push, and create or update a Video.js pull request. Use when the user explicitly requests repository publication or PR preparation. +--- + +# Commit and pull request + +Preserve unrelated user changes. Do not stage or rewrite files you cannot attribute to the requested work. + +## Workflow + +1. Inspect `git status`, staged and unstaged diffs, branch name, and commits since the merge base. +2. Read the relevant reference only when needed: + - Commit wording: `references/commit.md` + - Scope selection: `references/scope.md` + - Branch naming: `references/branch.md` + - PR title/body: `references/pr.md` +3. Split changes into semantic commits when they represent independently reviewable purposes. Never use `git add .`; stage explicit paths or a reviewed set. +4. Run the checks appropriate to each commit before committing. +5. Use a conventional commit accepted by `commitlint.config.js`; treat that config and recent history as the current source of truth. +6. Push only when requested. Check for an existing PR before creating one. +7. Build the PR description from the complete branch diff, not only the last commit. Explain motivation, behavior, verification, and relevant issue links. +8. Report commit hashes, checks, and the PR URL. + +Do not amend, force-push, change an existing PR title, or update an existing PR body without clear user authorization. + +## Example + +Input: “Commit these skill changes and open a draft PR.” + +Output: Intentionally scoped commits, recorded checks, a pushed branch, and the draft PR URL. diff --git a/.claude/skills/git/references/branch.md b/.agents/skills/commit-pr/references/branch.md similarity index 100% rename from .claude/skills/git/references/branch.md rename to .agents/skills/commit-pr/references/branch.md diff --git a/.claude/skills/git/references/commit.md b/.agents/skills/commit-pr/references/commit.md similarity index 100% rename from .claude/skills/git/references/commit.md rename to .agents/skills/commit-pr/references/commit.md diff --git a/.claude/skills/git/references/pr.md b/.agents/skills/commit-pr/references/pr.md similarity index 100% rename from .claude/skills/git/references/pr.md rename to .agents/skills/commit-pr/references/pr.md diff --git a/.claude/skills/git/references/scope.md b/.agents/skills/commit-pr/references/scope.md similarity index 100% rename from .claude/skills/git/references/scope.md rename to .agents/skills/commit-pr/references/scope.md diff --git a/.agents/skills/create-issue/SKILL.md b/.agents/skills/create-issue/SKILL.md new file mode 100644 index 00000000..d9bd3a0b --- /dev/null +++ b/.agents/skills/create-issue/SKILL.md @@ -0,0 +1,28 @@ +--- +name: create-issue +description: Draft or create a videojs/v10 GitHub issue. Use when asked to file a bug, feature, task, or repository issue. +--- + +# Create a GitHub issue + +## Workflow + +1. Gather the observed behavior, expected behavior, motivation, affected surface, reproduction or acceptance criteria, and relevant links. +2. Search open and closed issues for duplicates or useful prior art. +3. Inspect the relevant code or docs when needed to make the issue actionable; do not invent a root cause. +4. Do not add labels or a type prefix. The triage bot owns both so auto-triage has one source of truth. +5. Draft: + - A concise Title Case title without a type prefix. + - Context and user impact. + - Reproduction for bugs, or scope/acceptance criteria for features. + - Relevant implementation notes only when verified. +6. Show the final draft and obtain confirmation before creating the external issue unless the user explicitly authorized immediate creation. +7. Create it without labels using `gh issue create` or the available GitHub connector and return the URL. + +Keep the issue focused on the problem and acceptance boundary. Do not prescribe an unverified implementation. + +## Example + +Input: “Draft an issue for captions disappearing after a source change.” + +Output: A concise title and evidence-backed body with impact, reproduction, expected behavior, and acceptance criteria. diff --git a/.agents/skills/create-skill/SKILL.md b/.agents/skills/create-skill/SKILL.md new file mode 100644 index 00000000..40205dd0 --- /dev/null +++ b/.agents/skills/create-skill/SKILL.md @@ -0,0 +1,39 @@ +--- +name: create-skill +description: Create or restructure a repository skill. Use for repeatable agent workflows, portable metadata, resource layout, or context-budget cleanup. +--- + +# Create a skill + +Create a skill only when a repeatable vertical workflow needs instructions beyond what code, tests, existing docs, and general model capability already provide. + +## Workflow + +1. Collect concrete trigger and non-trigger examples. +2. Check existing skills for overlap; extend or consolidate before adding another metadata entry. +3. Choose one coherent job and a globally unique, verb-first kebab-case name. +4. Create `.agents/skills//SKILL.md` with only portable frontmatter: + + ```yaml + --- + name: + description: + --- + ``` + +5. Put only essential procedure, project-specific gotchas, inputs/outputs, one compact example, and validation in `SKILL.md`. +6. Add a `references/` file only for conditional detail and link it directly with an explicit read condition. Add a tested script only for repeated deterministic work. +7. Do not tell a skill to load a sibling skill. Give related skills precise, overlapping descriptions so hosts can compose them from the task. +8. Run `pnpm link:aliases` to refresh compatibility directory links. +9. Update `AGENTS.md` only when its compact routing index needs the skill. +10. Run `pnpm check:workspace`. + +Read `references/principles.md` for scope/context decisions, `references/structure.md` for non-trivial resource layout, and `references/patterns.md` only when the workflow needs an established pattern. Do not load them by default. + +Create another skill only when it has an independent trigger and useful standalone workflow. Keep supporting knowledge as a reference. Do not add auxiliary changelogs, duplicated project docs, generic tutorials, or variants that belong in one parameterized workflow. + +## Example + +Input: “Create a repeatable workflow for reviewing package export changes.” + +Output: One focused `.agents/skills/review-package-exports/` skill with a precise trigger, compact procedure, example result, and validation loop. diff --git a/.agents/skills/create-skill/references/patterns.md b/.agents/skills/create-skill/references/patterns.md new file mode 100644 index 00000000..adca3093 --- /dev/null +++ b/.agents/skills/create-skill/references/patterns.md @@ -0,0 +1,19 @@ +# Repository skill patterns + +## Focused workflow + +Use one small `SKILL.md` when the task has a stable sequence and little conditional detail, such as issue creation or agent-guidance maintenance. + +## Domain workflow with references + +Keep the default procedure in `SKILL.md` and route optional concerns to named references. Examples: API design, UI component construction, and accessible UI implementation. + +## Composable implementation and review + +Use separate skills when implementation and audit are independently requested. Each owns a standalone workflow and output contract. Their descriptions may overlap so the host composes them automatically when a task needs both, but neither instructs the agent to load its sibling. + +## Output with a stable scaffold + +Keep a template only when the artifact has a repository-defined shape that code does not generate, such as a design record or RFC. + +Avoid skills that only select a persona, mirror a documentation tree, or require loading several sibling skills for every invocation. diff --git a/.agents/skills/create-skill/references/principles.md b/.agents/skills/create-skill/references/principles.md new file mode 100644 index 00000000..58a1c6dc --- /dev/null +++ b/.agents/skills/create-skill/references/principles.md @@ -0,0 +1,25 @@ +# Skill scope and context + +## Create a skill when + +- The user can ask for the workflow directly. +- The workflow recurs and needs repository-specific judgment or sequencing. +- Code, tests, existing docs, and ordinary agent capability do not already make the right path obvious. + +Do not create a skill for a one-off plan, a generic role, a large knowledge dump, or a rule that can be enforced mechanically. + +## Split or reference + +Create a separate skill when the new unit has its own trigger and can produce a useful result independently. Keep material as a reference when it only supports another workflow. + +Implementation and review may be separate skills when users invoke them separately and their procedures differ. Each must stand alone. Give their descriptions enough shared domain language that a host can select both when a task truly needs both; do not make either skill explicitly load the other. + +## Token discipline + +- Metadata is always-on; every new skill adds discovery cost. +- The skill body should contain only the default path, project-specific corrections, and validation. +- Load conditional detail from one directly named reference. +- Point to executable sources rather than copying commands, schemas, APIs, or architecture. +- Delete stale guidance in the same change. + +Match detail to fragility: use exact steps for destructive or order-sensitive work, concise constraints for judgment-heavy work. diff --git a/.agents/skills/create-skill/references/structure.md b/.agents/skills/create-skill/references/structure.md new file mode 100644 index 00000000..098cfa7e --- /dev/null +++ b/.agents/skills/create-skill/references/structure.md @@ -0,0 +1,29 @@ +# Repository skill structure + +Checked-in skills are direct children of the canonical agent catalog: + +```text +.agents/skills/ +└── / + ├── SKILL.md + ├── references/ # optional, conditional detail + ├── scripts/ # optional, deterministic automation + └── assets/ # optional, output inputs +``` + +`pnpm link:aliases` exposes the whole catalog through generated `.claude/skills/` and `.opencode/skills/` directory links. Do not create per-skill links or depend on recursive discovery inside a host's skills directory. + +## Portable metadata + +Use only `name` and `description`. The name must match the immediate parent directory, use verb-first lowercase kebab-case, and be globally unique. Put what the skill does and when it should trigger in the description; the body is unavailable during discovery. + +## Resources + +- Keep `SKILL.md` as the workflow and conditional resource index. +- Include one small input/output example that demonstrates success. +- Link references directly and state when to read each one. +- Keep references one level deep when practical. +- Use a script only for repeated deterministic work; test it directly. +- Keep templates only when exact structure matters and code is not a better generator. + +Run `pnpm check:workspace` to validate naming, metadata, aliases, and context budgets. diff --git a/.agents/skills/create-spf-behavior/SKILL.md b/.agents/skills/create-spf-behavior/SKILL.md new file mode 100644 index 00000000..c8d608fa --- /dev/null +++ b/.agents/skills/create-spf-behavior/SKILL.md @@ -0,0 +1,25 @@ +--- +name: create-spf-behavior +description: Create or extract one SPF playback-engine behavior. Use for a new atomic responsibility with signals, lifecycle, cleanup, composition, and tests. +--- + +# Create an SPF behavior + +Read `references/workflow.md` completely before acting; it contains the detailed SPF creation workflow and failure checks. + +Read adjacent behaviors, the composition site, state/context slots, and `internal/design/spf/conventions/README.md`. Load only the applicable signal, actor, reactor, configuration, or behavior convention. + +1. Define one-sentence purpose, inputs, writes, lifecycle, cleanup, ordering, and source-reset rules. +2. Confirm the responsibility is independently testable and does not hide coordination with another behavior. +3. Add focused failing tests, implement the smallest behavior, and wire it at the owning composition layer. +4. Verify slot ownership, naming, cleanup, boundaries, and final test coverage. + +If existing ownership must change, treat the work as a behavior refactor. If the requested capability spans multiple behaviors, keep this workflow scoped to the one atomic responsibility and surface the broader composition work separately. + +Run the narrow affected SPF tests. Build SPF when public types or exports change, and measure size when a public entry point can grow. + +## Example + +Input: “Extract manifest refresh scheduling into its own behavior.” + +Output: One independently testable responsibility with explicit inputs, writes, lifecycle, cleanup, composition, and proportional verification. diff --git a/.claude/skills/spf-create-behavior/SKILL.md b/.agents/skills/create-spf-behavior/references/workflow.md similarity index 89% rename from .claude/skills/spf-create-behavior/SKILL.md rename to .agents/skills/create-spf-behavior/references/workflow.md index f14815c8..94909a15 100644 --- a/.claude/skills/spf-create-behavior/SKILL.md +++ b/.agents/skills/create-spf-behavior/references/workflow.md @@ -1,12 +1,12 @@ --- -name: spf-create-behavior +name: create-spf-behavior description: >- Create a new SPF behavior with conventions-aligned shape. Walks through - purpose articulation (carries /refactor-behavior's purpose-first discipline), + purpose articulation (carries /change-spf-behavior's purpose-first discipline), signal type choice, slot map design, composition placement, cleanup pattern selection, test placement, and engine wiring. Distinct from - /refactor-behavior (which modifies an existing behavior preserving its - purpose) and /spf-update-behavior (which modifies an existing behavior whose + /change-spf-behavior (which modifies an existing behavior preserving its + purpose) and /change-spf-behavior (which modifies an existing behavior whose purpose is changing). Triggers: "create behavior", "new behavior", "create SPF behavior", "add behavior", "scaffold behavior", "new SPF behavior". --- @@ -21,17 +21,17 @@ purpose — producing a behavior whose slot map drifts, whose composition placement is unclear, or whose cleanup contract doesn't match the project convention. -This skill is a **stub** scoped for use by `/spf-implement-feature`. Failure- +This skill is a **stub** scoped for use by `/implement-spf-feature`. Failure- mode catalog grows from real use; the seeded entries below capture the load-bearing patterns identified at skill-creation time. ## Usage ``` -/spf-create-behavior [] +/create-spf-behavior [] ``` -The skill is typically invoked from `/spf-implement-feature`'s Step 6 when a +The skill is typically invoked from `/implement-spf-feature`'s Step 6 when a chunk requires a new behavior, but can be invoked directly when the user knows the behavior shape upfront. @@ -58,7 +58,7 @@ Required reading before drafting: ## Failure-mode catalog (seeded; grows with use) -- **Purpose-articulation skipped.** Same failure mode as `/refactor-behavior`: +- **Purpose-articulation skipped.** Same failure mode as `/change-spf-behavior`: jumping from "we need a behavior" to `defineBehavior` without naming what the behavior *does* in business terms. Carries the purpose-first discipline. The articulation should answer: what business rule does this @@ -84,7 +84,7 @@ Required reading before drafting: variant-specific (lives only in a live variant, audio-only variant, etc.), it must live as a separate behavior composed into the variant, *not* as a runtime conditional inside an always-on behavior. Same failure mode as - the spf-document-feature catalog entry. + the document-spf-feature catalog entry. - **Tests written after the fact.** TDD discipline: write the test in `tests/.test.ts` first, run it failing, then implement. @@ -94,7 +94,7 @@ Required reading before drafting: ### Step 1 — Articulate purpose -Carry `/refactor-behavior`'s purpose-first discipline forward to new +Carry `/change-spf-behavior`'s purpose-first discipline forward to new behaviors. Before any code: - **What business rule does this behavior implement?** Name it in plain @@ -156,7 +156,7 @@ Per `conventions/signals.md`: ### Step 5 — Final-shape audit + commit -Per the parent skill (`/spf-implement-feature`), commits are typically +Per the parent skill (`/implement-spf-feature`), commits are typically batched at the feature-implementation level, not per-behavior. If invoked standalone, propose a per-behavior commit shape and ask the user to confirm. @@ -170,10 +170,10 @@ Audit checklist: ## When this is the wrong skill -- **Refactoring an existing behavior, purpose preserved** → `/refactor-behavior` -- **Updating an existing behavior, purpose changing** → `/spf-update-behavior` -- **Splitting / merging existing behaviors** → `/split-behavior` / - `/merge-behaviors` (often routed through `/refactor-behavior`) +- **Refactoring an existing behavior, purpose preserved** → `/change-spf-behavior` +- **Updating an existing behavior, purpose changing** → `/change-spf-behavior` +- **Splitting / merging existing behaviors** → `/change-spf-behavior` / + `/change-spf-behavior` (often routed through `/change-spf-behavior`) - **Creating a media-layer or network-layer helper (not a behavior)** → manual for now; future media-layer / network-layer skills will own this diff --git a/.agents/skills/design-api/SKILL.md b/.agents/skills/design-api/SKILL.md new file mode 100644 index 00000000..d7f832c6 --- /dev/null +++ b/.agents/skills/design-api/SKILL.md @@ -0,0 +1,34 @@ +--- +name: design-api +description: Design Video.js TypeScript APIs. Use for public shape, inference, state, extension points, middleware, composition, or ergonomics. +--- + +# API design and DX + +Inspect the current call sites, types, tests, exports, and relevant design records before applying general guidance. Existing local contracts beat generic patterns. + +## Default priorities + +1. Correct observable behavior +2. Type safety and useful inference +3. A small, coherent concept set +4. Consistency with adjacent Video.js APIs +5. Composable escape hatches and tree-shakable output + +Prefer config objects once positional arguments become ambiguous, explicit contracts over hidden coupling, and inference over required annotations. Avoid adding a plugin system when ordinary composition or a narrow extension point solves the problem. + +## Load references conditionally + +- General API tradeoffs: `references/principles.md` +- Type inference or public type shape: `references/typescript.md` +- Store, signal, or state architecture: `references/state.md` +- Middleware, builders, adapters, or lifecycles: `references/extensibility.md` +- Suspected design smell: `references/anti-patterns.md` +- Prior-art comparison: `references/libraries.md` and, only if useful, `references/voices.md` +Do not load every reference by default. + +## Example + +Input: “Design a typed selector API for the store.” + +Output: A concrete public shape with inference behavior, compatibility tradeoffs, extension points, and tests to prove the contract. diff --git a/.claude/skills/api/references/anti-patterns.md b/.agents/skills/design-api/references/anti-patterns.md similarity index 98% rename from .claude/skills/api/references/anti-patterns.md rename to .agents/skills/design-api/references/anti-patterns.md index 3eca1efe..d02e84ab 100644 --- a/.claude/skills/api/references/anti-patterns.md +++ b/.agents/skills/design-api/references/anti-patterns.md @@ -334,4 +334,3 @@ if (query.error) { - [Principles](principles.md) — what to do instead - [TypeScript](typescript.md) — type inference patterns -- [Component Anti-Patterns](../../component/references/anti-patterns.md) — UI component mistakes diff --git a/.claude/skills/api/references/extensibility.md b/.agents/skills/design-api/references/extensibility.md similarity index 100% rename from .claude/skills/api/references/extensibility.md rename to .agents/skills/design-api/references/extensibility.md diff --git a/.claude/skills/api/references/libraries.md b/.agents/skills/design-api/references/libraries.md similarity index 100% rename from .claude/skills/api/references/libraries.md rename to .agents/skills/design-api/references/libraries.md diff --git a/.claude/skills/api/references/principles.md b/.agents/skills/design-api/references/principles.md similarity index 100% rename from .claude/skills/api/references/principles.md rename to .agents/skills/design-api/references/principles.md diff --git a/.claude/skills/api/references/state.md b/.agents/skills/design-api/references/state.md similarity index 100% rename from .claude/skills/api/references/state.md rename to .agents/skills/design-api/references/state.md diff --git a/.claude/skills/api/references/typescript.md b/.agents/skills/design-api/references/typescript.md similarity index 100% rename from .claude/skills/api/references/typescript.md rename to .agents/skills/design-api/references/typescript.md diff --git a/.claude/skills/api/references/voices.md b/.agents/skills/design-api/references/voices.md similarity index 100% rename from .claude/skills/api/references/voices.md rename to .agents/skills/design-api/references/voices.md diff --git a/.agents/skills/document-spf-feature/SKILL.md b/.agents/skills/document-spf-feature/SKILL.md new file mode 100644 index 00000000..c9d7310f --- /dev/null +++ b/.agents/skills/document-spf-feature/SKILL.md @@ -0,0 +1,23 @@ +--- +name: document-spf-feature +description: Document an SPF feature under internal/design/spf/features. Use for capability scope, status, evidence, constraints, or relationships. +--- + +# Document an SPF feature + +Read `references/workflow.md` completely before acting; it contains the detailed registry, decomposition, evidence, cascade, and validation workflow. + +Registry entries guide planning but do not override code. Read `internal/design/spf/features/clusters.md`, a strong neighboring entry, relevant implementation/tests, and linked records. + +1. Classify the engine capability and distinguish it from delivery use cases or implementation conventions. +2. Separate current behavior from proposed direction and decisions still needed. +3. For shipped work, retain decisions, consequences, and current source pointers. For future work, retain scope, boundaries, and evidence required before implementation. +4. Update directly affected entries only when their facts changed; verify links and relationship symmetry. + +Cite repository paths for implemented behavior. Remove phase tables, speculative file inventories, and progress logs once code lands. Do not mark work implemented without code and verification evidence, and do not implement the feature unless requested. + +## Example + +Input: “Document the current depth of discontinuity handling.” + +Output: A compact evidence-backed entry separating implemented behavior, remaining decisions, constraints, and source pointers. diff --git a/.claude/skills/spf-document-feature/SKILL.md b/.agents/skills/document-spf-feature/references/workflow.md similarity index 97% rename from .claude/skills/spf-document-feature/SKILL.md rename to .agents/skills/document-spf-feature/references/workflow.md index 9821626b..2f671601 100644 --- a/.claude/skills/spf-document-feature/SKILL.md +++ b/.agents/skills/document-spf-feature/references/workflow.md @@ -1,5 +1,5 @@ --- -name: spf-document-feature +name: document-spf-feature description: >- Produce or update an entry in the SPF feature registry at internal/design/spf/features/. Triangulates context from multiple sources @@ -31,7 +31,7 @@ consistent rather than drifting. ## Usage ``` -/spf-document-feature [] +/document-spf-feature [] ``` The arg is optional. The skill is also invoked after the user pastes @@ -223,8 +223,8 @@ by itself establish feature-level scope. The decomposition rubric does. This is the load-bearing step. Getting it wrong (misreading historical context as current scope, missing a sister feature, conflating with another feature, inflating an API primitive into a standalone feature) -invalidates everything downstream — same failure shape as -`refactor-behavior`'s Step 1 misdiagnosis. +invalidates everything downstream — the same failure shape as a +purpose-first behavior analysis that starts from implementation instead. ### Step 2 — Discuss to resolve ambiguities @@ -476,7 +476,7 @@ this new draft entails. Common candidates: cluster's docs list. Move from bracketed `[name]` to plain `name` if applicable. - **Skills `README.md`.** If this skill creation is itself the feature - (self-application), add the skill to `.claude/skills/README.md`. + (self-application), add the skill to the compact routing index in `AGENTS.md`. **Discipline for cascade edits:** @@ -562,11 +562,11 @@ by Step 1's gathering. ## When this is the wrong skill -- **You want to refactor an existing behavior** → `/refactor-behavior`. -- **You want to split a per-type behavior** → `/refactor-behavior`'s - Step 6a may route you to `/split-behavior`. -- **You want to merge two behaviors** → `/refactor-behavior`'s Step 3 - routes to `/merge-behaviors`. +- **You want to refactor an existing behavior** → `/change-spf-behavior`. +- **You want to split a per-type behavior** → `/change-spf-behavior`'s + Step 6a may route you to `/change-spf-behavior`. +- **You want to merge two behaviors** → `/change-spf-behavior`'s Step 3 + routes to `/change-spf-behavior`. - **You want to write a design doc for an architecture concern (not a feature)** → `design` skill. Architectural concerns live in `internal/design/spf/` directly, not under `features/`. diff --git a/.agents/skills/document-spf-use-case/SKILL.md b/.agents/skills/document-spf-use-case/SKILL.md new file mode 100644 index 00000000..87483db9 --- /dev/null +++ b/.agents/skills/document-spf-use-case/SKILL.md @@ -0,0 +1,23 @@ +--- +name: document-spf-use-case +description: Document an SPF use-case composition. Use for delivery scenarios, variants, constituent features, status, constraints, or verification. +--- + +# Document an SPF use case + +Read `references/workflow.md` completely before acting; it contains the detailed composition, evidence, cascade, and validation workflow. + +Read `internal/design/spf/use-cases/README.md`, its decomposition rubric, related feature entries, a strong neighboring use case, and current implementation/tests. + +1. Describe the delivery scenario and the feature additions, removals, swaps, or tuning that compose it. +2. Separate current behavior from proposed direction and decisions still needed; label missing constituent features explicitly. +3. For shipped variants, retain composition decisions and current source pointers. For future variants, retain the scenario, boundaries, and evidence required before implementation. +4. Update directly affected entries only when their facts changed; verify links and relationship symmetry. + +Do not duplicate feature definitions, code mechanics, phase tables, or progress logs. When the missing unit is an engine capability, record it in the feature registry rather than hiding it in a delivery scenario. + +## Example + +Input: “Document low-latency live playback as a composed use case.” + +Output: A compact delivery-focused entry naming composition, current status, missing prerequisites, constraints, and source evidence. diff --git a/.claude/skills/spf-document-use-case/SKILL.md b/.agents/skills/document-spf-use-case/references/workflow.md similarity index 96% rename from .claude/skills/spf-document-use-case/SKILL.md rename to .agents/skills/document-spf-use-case/references/workflow.md index 6ffd7905..6b8cd944 100644 --- a/.claude/skills/spf-document-use-case/SKILL.md +++ b/.agents/skills/document-spf-use-case/references/workflow.md @@ -1,5 +1,5 @@ --- -name: spf-document-use-case +name: document-spf-use-case description: >- Produce or update an entry in the SPF use-case-composition registry at internal/design/spf/use-cases/. Triangulates context from multiple sources @@ -34,7 +34,7 @@ internally consistent rather than drifting. ## Usage ``` -/spf-document-use-case [] +/document-spf-use-case [] ``` The arg is optional. The skill is also invoked after the user pastes context, @@ -62,7 +62,7 @@ Read these before drafting: - `internal/design/spf/features/.md` — constituent feature docs supplying the engine capabilities the use case rests on. Step 3 grounding maps the variant's composition against these. -- `.claude/skills/spf-document-feature/SKILL.md` — parallel skill for +- `.agents/skills/document-spf-feature/SKILL.md` — parallel skill for feature docs; consult for the analogous discipline shape and the failure-mode catalog patterns that apply symmetrically. - `internal/design/spf/evaluation-axes.md` — for Phase 3 (optimizations) @@ -90,7 +90,7 @@ actual use. factory composes a different behavior list; behaviors are subtracted / added / swapped at composition time. Signals of middle pattern: behaviors read a new state slot at runtime; all behaviors compose uniformly. When - middle pattern fires, route to `/spf-document-feature`. + middle pattern fires, route to `/document-spf-feature`. - **Feature-vs-use-case framing confusion** — the invocation describes an engine capability (Case-1: "the engine handles this source-shape @@ -114,7 +114,7 @@ actual use. variant-decision-glue behavior, composition-wiring behavior, or single-scenario tuning gets a feature doc when it should live in the use-case doc's *Composition specifics → Behaviors added* section. Apply - the same "earns its place" rubric `/spf-document-feature` uses: + the same "earns its place" rubric `/document-spf-feature` uses: substantial independent implementation footprint, independent priority/timeline, or a primitive other engine consumers would draw on. Failing all three → behavior stays in the use-case doc; no separate @@ -146,7 +146,7 @@ actual use. - **Composition-variant logic in always-on behaviors** — applies symmetrically to use-case work; cross-ref the existing entry in - `/spf-document-feature`'s catalog. When a use case wants to bias an + `/document-spf-feature`'s catalog. When a use case wants to bias an always-on behavior's runtime, the answer is a per-variant alternative implementation (composed in place of the default) or alternative default configuration — not a runtime conditional branch in the always-on body. @@ -223,19 +223,19 @@ Common shapes: **Counter-routes when criteria fail:** -- Fails (1) → middle pattern or cluster-E policy → `/spf-document-feature`. +- Fails (1) → middle pattern or cluster-E policy → `/document-spf-feature`. - Fails (2) or (4) → composition-variant *phase row* inside an existing feature doc (composition-variant pattern from [`../conventions/behaviors.md` § Inverse: behaviors that operate uniformly across tracks](../conventions/behaviors.md#inverse-behaviors-that-operate-uniformly-across-tracks)), not standalone. - Fails (3) → either the features aren't documented yet (write them - first via `/spf-document-feature`) or this isn't actually a use-case + first via `/document-spf-feature`) or this isn't actually a use-case composition. **Weak-criterion surface check.** When the rubric fires only *weakly* on one or more criteria, surface alternative framings proactively in the -Step 1 report — same discipline as `/spf-document-feature`'s +Step 1 report — same discipline as `/document-spf-feature`'s weak-criterion check. The user gets to see the judgment call rather than having to surface it themselves. @@ -248,13 +248,13 @@ having to surface it themselves. 5. Rubric criteria firing strongly / weakly / failing. 6. **Recommended framing** — new standalone use-case doc / extend `` / extend ``'s - composition-variant phase row / route to `/spf-document-feature` — + composition-variant phase row / route to `/document-spf-feature` — with rubric reasoning. Always have a recommendation; don't hedge. 7. Ambiguities still unresolved (going into Step 2's discussion). This is the load-bearing step. Getting it wrong invalidates everything -downstream — same failure shape as `/spf-document-feature`'s Step 1 -misdiagnosis and `/refactor-behavior`'s Step 1 purpose-articulation. +downstream — same failure shape as `/document-spf-feature`'s Step 1 +misdiagnosis and `/change-spf-behavior`'s Step 1 purpose-articulation. ### Step 2 — Discuss to resolve ambiguities @@ -599,16 +599,16 @@ ambiguities are genuinely resolved by Step 1's gathering. ## When this is the wrong skill - **You want to document an engine capability (Case-1)** → - `/spf-document-feature`. Capability docs answer "what can the engine + `/document-spf-feature`. Capability docs answer "what can the engine do?"; use-case docs answer "how is the engine composed for this delivery scenario?" - **Your candidate's implementation shape is middle-pattern, not - composition** → `/spf-document-feature`. Per `clusters.md`: most + composition** → `/document-spf-feature`. Per `clusters.md`: most candidates fail this check. -- **You want to refactor an existing behavior** → `/refactor-behavior`. -- **You want to split or merge behaviors** → `/refactor-behavior`'s - Step 3 / Step 6a may route you to `/split-behavior` or - `/merge-behaviors`. +- **You want to refactor an existing behavior** → `/change-spf-behavior`. +- **You want to split or merge behaviors** → `/change-spf-behavior`'s + Step 3 / Step 6a may route you to `/change-spf-behavior` or + `/change-spf-behavior`. - **You want to write an architectural design doc** → `design` skill. Architectural concerns live in `internal/design/spf/` directly, not under `use-cases/`. diff --git a/.agents/skills/implement-accessible-ui/SKILL.md b/.agents/skills/implement-accessible-ui/SKILL.md new file mode 100644 index 00000000..da035dbc --- /dev/null +++ b/.agents/skills/implement-accessible-ui/SKILL.md @@ -0,0 +1,30 @@ +--- +name: implement-accessible-ui +description: Implement accessible Video.js UI. Use for semantics, ARIA, keyboard interaction, focus, screen readers, captions, or live regions. +--- + +# Accessibility + +Start from the rendered semantics and interaction contract. Prefer native HTML; add ARIA only where native semantics cannot express the control. + +## Workflow + +1. Read the component, its rendered output, interaction tests, and adjacent accessible controls. +2. Define expected name, role, value/state, keyboard behavior, focus behavior, and announcements. +3. Load only the applicable reference: + - Keyboard: `references/keyboard.md` + - Focus: `references/focus.md` + - Roles and states: `references/aria.md` + - React-specific implementation: `references/react.md` + - Media-player behavior: `references/media.md` + - Suspected smell: `references/anti-patterns.md` +4. Implement the smallest semantic change and add behavior-focused tests. +5. Verify keyboard and focus behavior in a browser when interaction changed. + +Never remove visible focus without a replacement, use positive `tabindex`, convey meaning only with color, or use a changing label where a state attribute is the correct contract. + +## Example + +Input: “Make the custom volume slider keyboard accessible.” + +Output: Correct native or ARIA semantics, keyboard and focus behavior, announcements, focused tests, and browser verification. diff --git a/.claude/skills/aria/references/anti-patterns.md b/.agents/skills/implement-accessible-ui/references/anti-patterns.md similarity index 98% rename from .claude/skills/aria/references/anti-patterns.md rename to .agents/skills/implement-accessible-ui/references/anti-patterns.md index 8c80ec1d..a462acce 100644 --- a/.claude/skills/aria/references/anti-patterns.md +++ b/.agents/skills/implement-accessible-ui/references/anti-patterns.md @@ -265,4 +265,3 @@ Auto-playing audio disrupts screen reader users. Mute by default or require user - [focus.md](focus.md) — correct focus patterns - [keyboard.md](keyboard.md) — keyboard navigation - [aria.md](aria.md) — roles, states, properties -- [checklist.md](../review/checklist.md) — comprehensive review checklist diff --git a/.claude/skills/aria/references/aria.md b/.agents/skills/implement-accessible-ui/references/aria.md similarity index 100% rename from .claude/skills/aria/references/aria.md rename to .agents/skills/implement-accessible-ui/references/aria.md diff --git a/.claude/skills/aria/references/focus.md b/.agents/skills/implement-accessible-ui/references/focus.md similarity index 100% rename from .claude/skills/aria/references/focus.md rename to .agents/skills/implement-accessible-ui/references/focus.md diff --git a/.claude/skills/aria/references/keyboard.md b/.agents/skills/implement-accessible-ui/references/keyboard.md similarity index 100% rename from .claude/skills/aria/references/keyboard.md rename to .agents/skills/implement-accessible-ui/references/keyboard.md diff --git a/.claude/skills/aria/references/media.md b/.agents/skills/implement-accessible-ui/references/media.md similarity index 100% rename from .claude/skills/aria/references/media.md rename to .agents/skills/implement-accessible-ui/references/media.md diff --git a/.claude/skills/aria/references/react.md b/.agents/skills/implement-accessible-ui/references/react.md similarity index 98% rename from .claude/skills/aria/references/react.md rename to .agents/skills/implement-accessible-ui/references/react.md index 59419244..977ccc40 100644 --- a/.claude/skills/aria/references/react.md +++ b/.agents/skills/implement-accessible-ui/references/react.md @@ -471,9 +471,3 @@ test('announces state changes', () => { expect(liveRegion).toHaveTextContent('Enabled'); }); ``` - ---- - -## See Also - -- [component/react.md](../../component/references/react.md) — React component architecture (context, controlled state, render props) diff --git a/.agents/skills/implement-spf-feature/SKILL.md b/.agents/skills/implement-spf-feature/SKILL.md new file mode 100644 index 00000000..96f4ba86 --- /dev/null +++ b/.agents/skills/implement-spf-feature/SKILL.md @@ -0,0 +1,25 @@ +--- +name: implement-spf-feature +description: Implement an SPF feature from its registry entry. Use when an engine capability spans behaviors and requires tests plus status updates. +--- + +# Implement an SPF feature + +Read `references/workflow.md` completely before acting; it contains the detailed disambiguation, planning, implementation, testing, and registry-update workflow. + +Treat the feature entry as planning input. Reconcile it with code, tests, conventions, and requested scope before editing. + +1. Read the entry, related features, implementation, tests, and applicable SPF conventions. +2. List contradictions, open decisions, and out-of-scope capability. +3. Define the smallest end-to-end increment with observable value and split it into testable state, behavior, composition, adapter, and documentation chunks. +4. For each chunk, add or update a failing test, implement the minimum at the correct ownership layer, and run the narrow target. +5. Run cumulative SPF tests, build, and size checks proportional to impact. +6. Collapse the feature record to final decisions, remaining scope, and current source pointers. + +Report implemented and deferred scope, test evidence, record updates, and remaining risks. Do not claim completion for a prerequisite or partial increment. + +## Example + +Input: “Implement the smallest useful increment of timeline discontinuity support.” + +Output: The smallest end-to-end capability, test evidence, proportional checks, updated feature status, and explicit deferred scope. diff --git a/.claude/skills/spf-implement-feature/SKILL.md b/.agents/skills/implement-spf-feature/references/workflow.md similarity index 95% rename from .claude/skills/spf-implement-feature/SKILL.md rename to .agents/skills/implement-spf-feature/references/workflow.md index a7b271ca..e485cb47 100644 --- a/.claude/skills/spf-implement-feature/SKILL.md +++ b/.agents/skills/implement-spf-feature/references/workflow.md @@ -1,14 +1,14 @@ --- -name: spf-implement-feature +name: implement-spf-feature description: >- Implement a feature documented in the SPF feature registry. Consumes a feature doc at internal/design/spf/features/.md and produces the engine-side code: new behaviors, updates to existing behaviors, media-layer / network-layer primitives, and tests. The implementation analog of - /spf-document-feature (which produces the doc; this consumes it). Walks + /document-spf-feature (which produces the doc; this consumes it). Walks through resolving the doc's open questions before coding, maps phases to discrete chunks, applies the SPF conventions catalog, routes to downstream - skills (/spf-create-behavior, /spf-update-behavior, /refactor-behavior) per + skills (/create-spf-behavior, /change-spf-behavior, /change-spf-behavior) per chunk shape, and updates the feature doc's Status / Implementation surface / Verification sections as code lands. Triggers: "implement feature", "implement SPF feature", "build feature", "code feature", "scope feature @@ -35,7 +35,7 @@ once the feature, composition target, fold-ins, and intent are named. Step 9 ## Usage ``` -/spf-implement-feature [] +/implement-spf-feature [] ``` The arg is the feature doc's filename (without extension), e.g. @@ -103,12 +103,12 @@ Secondary: Downstream skills routed-to: -- `.claude/skills/spf-create-behavior/SKILL.md` — new behaviors. -- `.claude/skills/spf-update-behavior/SKILL.md` — existing behaviors whose +- `.agents/skills/create-spf-behavior/SKILL.md` — new behaviors. +- `.agents/skills/change-spf-behavior/references/update.md` — existing behaviors whose purpose is changing. -- `.claude/skills/refactor-behavior/SKILL.md` — existing behaviors whose +- `.agents/skills/change-spf-behavior/references/refactor.md` — existing behaviors whose purpose is preserved but implementation improves. -- `.claude/skills/split-behavior/SKILL.md`, `.claude/skills/merge-behaviors/SKILL.md` +- `.agents/skills/change-spf-behavior/references/split.md`, `.agents/skills/change-spf-behavior/references/merge.md` — structural changes. - *(future)* media-layer / network-layer skills for `packages/spf/src/media/` and `packages/spf/src/network/` changes. @@ -378,7 +378,7 @@ boundary: - **Source-shape correctness or engine capability?** If yes → feature (Case-1), stay here. - **Delivery-mode choice / variant assembly?** If yes → this is a - **use-case composition** (Case-2) → route to `/spf-implement-use-case`. + **use-case composition** (Case-2) → route to `/implement-spf-use-case`. - **Runtime policy tuning without composition change?** If yes → still a feature (cluster-E policy), but the implementation shape is config/middle-pattern, not composition; stay here. @@ -389,12 +389,12 @@ boundary: **1c. Route the request appropriately.** - **Stays here** — confirmed feature, doc exists. Proceed to 1d. -- **No doc exists for the candidate** → route to `/spf-document-feature` +- **No doc exists for the candidate** → route to `/document-spf-feature` to produce the doc; return here once doc lands. - **It's actually a use case, doc exists** → route to - `/spf-implement-use-case`. + `/implement-spf-use-case`. - **It's actually a use case, no doc exists** → route to - `/spf-document-use-case` first, then `/spf-implement-use-case`. + `/document-spf-use-case` first, then `/implement-spf-use-case`. - **Ambiguous between options** → surface to user; do not pick silently. **1d. Confirm composition target.** (Once routing is confirmed.) @@ -704,10 +704,10 @@ After Step 2's fold-in assessment delivers the agreed scope: rows + fold-in subsets the implementation will produce. - **Confirm composition mechanism per chunk** — (i) subtractive (no new code), (ii) config-driven (existing behavior gains a knob), - (iii) new behavior (route to `/spf-create-behavior`), (iv) + (iii) new behavior (route to `/create-spf-behavior`), (iv) behavior update with purpose change (route to - `/spf-update-behavior`), (v) behavior refactor with preserved - purpose (route to `/refactor-behavior`), (vi) media-layer / + `/change-spf-behavior`), (v) behavior refactor with preserved + purpose (route to `/change-spf-behavior`), (vi) media-layer / network-layer change (handle inline or defer per the downstream-skill-missing branch). - **Resolve open questions the implementation needs.** Only resolve @@ -732,8 +732,8 @@ Per the agreed scope, decompose into discrete chunks. Each chunk is: | Chunk | Mechanism | Downstream skill | Test target | |---|---|---|---| -| Add bandwidthState audio sampling | Update existing | `/spf-update-behavior` (setupAudioBufferActors) | `setup-buffer-actors.test.ts` audio sampling assertion | -| Create switchAudioQuality behavior | New behavior | `/spf-create-behavior` | `switch-audio-quality.test.ts` (new) | +| Add bandwidthState audio sampling | Update existing | `/change-spf-behavior` (setupAudioBufferActors) | `setup-buffer-actors.test.ts` audio sampling assertion | +| Create switchAudioQuality behavior | New behavior | `/create-spf-behavior` | `switch-audio-quality.test.ts` (new) | | Wire switchAudioQuality into composition | Composition | None | `engine.test.ts` composition assertion | ### Step 5 — Apply cross-cutting concern checks @@ -754,7 +754,7 @@ are present: pattern; uniform chunks compose against the aggregating resource. - **Composition-variant logic** — variant-specific chunks go in variant factories, not the default factory. -- **MSE codec-change implications** — per `/spf-document-feature`'s +- **MSE codec-change implications** — per `/document-spf-feature`'s catalog, if the chunk touches buffer behavior and codecs change, surface the `changeType()` vs `flushBuffer` question explicitly. @@ -784,10 +784,10 @@ Iterate per chunk: 2. **Branch by mechanism:** - **Subtractive / composition wiring** — handle inline. - **Config-driven** — handle inline. - - **New behavior** — route to `/spf-create-behavior` for this chunk. - - **Behavior update (purpose changing)** — route to `/spf-update-behavior`. - - **Behavior refactor (purpose preserved)** — route to `/refactor-behavior`. - - **Structural (split/merge)** — route via `/refactor-behavior`'s + - **New behavior** — route to `/create-spf-behavior` for this chunk. + - **Behavior update (purpose changing)** — route to `/change-spf-behavior`. + - **Behavior refactor (purpose preserved)** — route to `/change-spf-behavior`. + - **Structural (split/merge)** — route via `/change-spf-behavior`'s decomposition check. - **Media-layer / network-layer** — handle inline for now; future skills will own these. @@ -924,7 +924,7 @@ After user confirmation, proceed to Step 7 per-chunk loop. Surface Steps ## Why this order -Same shape as `/spf-document-feature` and `/spf-document-use-case`. Steps +Same shape as `/document-spf-feature` and `/document-spf-use-case`. Steps 1–3 force framing before mechanical work. Step 4 commits to a concrete chunk list; Step 5 runs cross-cutting checks while context is fresh; Step 6 commits to TDD targets. Step 7 produces the artifact; Step 8 audits. @@ -966,13 +966,13 @@ design decisions. ## When this is the wrong skill - **You want to document a feature (not yet implemented)** → - `/spf-document-feature`. + `/document-spf-feature`. - **You want to implement a use-case composition (not a single feature)** → - `/spf-implement-use-case`. That skill consumes the use-case doc and + `/implement-spf-use-case`. That skill consumes the use-case doc and routes per-constituent-feature back into this skill. - **You want to refactor an existing behavior without feature scope** → - `/refactor-behavior`. -- **You want to split or merge behaviors** → `/refactor-behavior`'s + `/change-spf-behavior`. +- **You want to split or merge behaviors** → `/change-spf-behavior`'s decomposition check. - **You want to write an architectural design doc** → `design` skill. - **You want to write an RFC** → `rfc` skill. @@ -984,11 +984,11 @@ use (most likely during Step 7 per-chunk implementation or Step 8 audit): 1. Add an entry to the *Failure-mode catalog* section above with the risk pattern and a worked-example citation. -2. If the failure-mode is downstream-skill-shaped (a recurring need - surfaces in `spf-create-behavior` / `spf-update-behavior`), the - downstream skill's catalog grows too. +2. If the failure-mode is downstream-workflow-shaped (a recurring need + surfaces while creating or changing a behavior), that workflow's + catalog grows too. 3. If the failure-mode is doc-shape-shaped (the feature doc template / - conventions don't capture something), `/spf-document-feature`'s + conventions don't capture something), `/document-spf-feature`'s catalog or the conventions docs grow. This skill is **new**; the catalog is expected to grow significantly as diff --git a/.agents/skills/implement-spf-use-case/SKILL.md b/.agents/skills/implement-spf-use-case/SKILL.md new file mode 100644 index 00000000..241002de --- /dev/null +++ b/.agents/skills/implement-spf-use-case/SKILL.md @@ -0,0 +1,25 @@ +--- +name: implement-spf-use-case +description: Implement an SPF use-case composition. Use when a delivery scenario combines feature variants and requires composition, tests, and status updates. +--- + +# Implement an SPF use case + +Read `references/workflow.md` completely before acting; it contains the detailed composition, prerequisite, implementation, testing, and cascade workflow. + +Treat the use-case entry as planning input. Read it, its constituent features, current compositions/adapters, tests, and applicable conventions. + +1. Verify each required feature is implemented for the requested scope; expose missing prerequisites instead of hiding them in wiring. +2. Reconcile documented assumptions with code and identify material open decisions. +3. Define the smallest observable composition increment and split it into independently testable prerequisite, behavior, composition, adapter, and documentation chunks. +4. Implement each chunk test-first at the correct feature, behavior, composition, or adapter boundary. +5. Run cumulative SPF tests, build, and size checks proportional to impact. +6. Collapse the use-case record to composition decisions, remaining scope, source pointers, and directly affected relationships. + +Report implemented and deferred scope, evidence, record updates, and remaining risks. Do not claim the use case complete while constituents remain partial. + +## Example + +Input: “Implement the smallest playable low-latency live composition.” + +Output: A verified minimal composition, exposed prerequisites, cumulative checks, updated use-case evidence, and explicit deferred work. diff --git a/.claude/skills/spf-implement-use-case/SKILL.md b/.agents/skills/implement-spf-use-case/references/workflow.md similarity index 92% rename from .claude/skills/spf-implement-use-case/SKILL.md rename to .agents/skills/implement-spf-use-case/references/workflow.md index 1a458aa2..90a86860 100644 --- a/.claude/skills/spf-implement-use-case/SKILL.md +++ b/.agents/skills/implement-spf-use-case/references/workflow.md @@ -1,17 +1,17 @@ --- -name: spf-implement-use-case +name: implement-spf-use-case description: >- Implement a use-case composition documented in the SPF use-case-composition registry. Consumes a use-case doc at internal/design/spf/use-cases/.md and produces the engine-side code: a variant engine factory, a parallel adapter, composition wiring, use-case-specific behaviors (if any), and - tests. The implementation analog of /spf-document-use-case (which produces + tests. The implementation analog of /document-spf-use-case (which produces the doc; this consumes it). Walks through disambiguation + routing (verify the request is actually a use case, check constituent-feature implementation status), resolves the doc's open questions with the user, maps phases to - chunks, routes to downstream skills (/spf-implement-feature for - unimplemented constituents, /spf-create-behavior, /spf-update-behavior, - /refactor-behavior), and updates both the use-case doc and constituent + chunks, routes to downstream skills (/implement-spf-feature for + unimplemented constituents, /create-spf-behavior, /change-spf-behavior, + /change-spf-behavior), and updates both the use-case doc and constituent feature docs as code lands. Treats the use-case doc as a starting point for planning, not a hardened specification. Triggers: "implement use case", "implement SPF use case", "implement use-case composition", "build use @@ -38,13 +38,13 @@ The canonical failure modes without this discipline are: top of feature capabilities that don't actually exist in code yet. Steps 1–2 are the load-bearing setup. Step 1 (disambiguation + routing) is -the most novel discipline — Steps 2–9 mostly parallel `/spf-implement-feature` +the most novel discipline — Steps 2–9 mostly parallel `/implement-spf-feature` with use-case-specific content. ## Usage ``` -/spf-implement-use-case [] +/implement-spf-use-case [] ``` The arg is optional. The skill can be invoked with a use-case-doc name (e.g., @@ -79,18 +79,18 @@ Secondary: Downstream skills routed-to: -- `.claude/skills/spf-implement-feature/SKILL.md` — **new**: when a +- `.agents/skills/implement-spf-feature/SKILL.md` — **new**: when a constituent feature is not yet implemented and the use case needs it. -- `.claude/skills/spf-create-behavior/SKILL.md` — use-case-specific +- `.agents/skills/create-spf-behavior/SKILL.md` — use-case-specific behaviors that don't promote to features (per use-cases/README cross-link discipline). -- `.claude/skills/spf-update-behavior/SKILL.md` — existing behaviors whose +- `.agents/skills/change-spf-behavior/references/update.md` — existing behaviors whose purpose changes for the variant. -- `.claude/skills/refactor-behavior/SKILL.md` — existing behaviors with +- `.agents/skills/change-spf-behavior/references/refactor.md` — existing behaviors with preserved purpose, improved implementation. -- `.claude/skills/spf-document-use-case/SKILL.md` — invoked when Step 1 +- `.agents/skills/document-spf-use-case/SKILL.md` — invoked when Step 1 routing concludes the candidate isn't yet documented as a use case. -- `.claude/skills/spf-document-feature/SKILL.md` — invoked when Step 1 +- `.agents/skills/document-spf-feature/SKILL.md` — invoked when Step 1 routing concludes the candidate is a feature, not a use case (and isn't yet documented as such). - *(future)* media-layer / network-layer skills. @@ -137,7 +137,7 @@ Discipline: disambiguation must resolve before gathering sources or planning. Proceeding-with-assumption is the canonical failure shape. Worked examples: invocation "implement resolution capping" — routes to - `/spf-implement-feature` on `rendition-selection-caps` (cluster-E + `/implement-spf-feature` on `rendition-selection-caps` (cluster-E policy), not here. Invocation "implement audio-only" maps to the `audio-only-mode-override` use case (which absorbed what was previously framed as a separate `audio-only-composition` feature); @@ -190,13 +190,13 @@ Discipline: 9. **Composing variant-specific behaviors into default factory** — variant-specific behaviors go in the variant factory, not the default `createSimpleHlsEngine`. Same failure mode as - `/spf-implement-feature`'s catalog, but more pointed here: the + `/implement-spf-feature`'s catalog, but more pointed here: the *whole point* of a use-case implementation is the variant assembly, so misrouting at the factory level is the canonical first-pass bug. 10. **Multi-writer slot mishandling** — adding a writer to a slot another behavior writes requires multi-writer characterization per - `conventions/signals.md`. Same as `/spf-implement-feature`. + `conventions/signals.md`. Same as `/implement-spf-feature`. 11. **Conventions catalog under-application** — SPF conventions (behaviors, signals, reactors, actors, config) all apply during @@ -223,7 +223,7 @@ Discipline: 15. **Downstream skill missing — silent inline implementation** — when a chunk hits a downstream-skill gap (especially: - `/spf-implement-feature` for an unimplemented constituent), the + `/implement-spf-feature` for an unimplemented constituent), the failure mode is to silently apply discipline ad-hoc. Step 6 explicitly surfaces this: branch on (i) defer chunk pending downstream skill, (ii) implement the constituent feature first via @@ -269,28 +269,28 @@ Apply the use-cases/README discriminator + 4-criterion rubric: - **Composition mechanisms?** If everything works as runtime config on always-on behaviors (no behaviors subtracted/added/swapped/defaulted at composition time) → it's a **cluster-E policy feature** → route - to `/spf-implement-feature` on the relevant feature doc. + to `/implement-spf-feature` on the relevant feature doc. - **Delivery scenario?** If the concern is source-shape correctness (engine handles a kind of source) rather than delivery-mode choice (compose differently for a consumer scenario) → it's a **Case-1 - feature** → route to `/spf-implement-feature` on the relevant + feature** → route to `/implement-spf-feature` on the relevant feature doc. - **Constituent features?** If the candidate has no real composition assembly — it's a single capability the engine gains — → it's - likely a **feature**, route to `/spf-implement-feature`. + likely a **feature**, route to `/implement-spf-feature`. - **Customer/consumer scenario?** If the request is "tune the engine differently for X" without a delivery scenario → likely **cluster-E - policy** → `/spf-implement-feature`. + policy** → `/implement-spf-feature`. **1c. Route the request appropriately.** - **Stays here** — confirmed use case, doc exists. Proceed to 1d. -- **No doc exists for the candidate** → route to `/spf-document-use-case` +- **No doc exists for the candidate** → route to `/document-spf-use-case` to produce the doc; return here once doc lands. - **It's actually a feature, no doc exists** → route to - `/spf-document-feature` first, then `/spf-implement-feature`. + `/document-spf-feature` first, then `/implement-spf-feature`. - **It's actually a feature, doc exists** → route directly to - `/spf-implement-feature`. + `/implement-spf-feature`. - **Ambiguous between options** → surface to user; do not pick silently. **1d. Gather sources.** (Once routing is confirmed and we're staying here.) @@ -306,7 +306,7 @@ Apply the use-cases/README discriminator + 4-criterion rubric: exists and defer the rest. - **Documented but unimplemented** — branches in Step 2 (defer use case / implement constituent first / bundle into this pass). - - **Not documented** — route to `/spf-document-feature` first. + - **Not documented** — route to `/document-spf-feature` first. - Direct Case-1 sibling feature doc (if applicable) — check whether shared engine factory work is in scope. - Related use cases (the doc's *Related use cases* section) — note @@ -345,7 +345,7 @@ After Step 1's report: - **Confirm constituent-feature readiness strategy.** Per Step 1's categorization, decide for each unimplemented constituent: - **Defer the use case** until constituent lands separately. - - **Implement the constituent first** via `/spf-implement-feature` + - **Implement the constituent first** via `/implement-spf-feature` in a separate pass (this skill pauses, downstream skill runs, this skill resumes). - **Bundle constituent implementation** into this use-case pass @@ -425,11 +425,11 @@ typical for use-case implementations: - **Adapter creation** (new) — parallels `SimpleHlsMediaElement` / `SimpleHlsMediaMixin`; uses `shareSignals` unchanged. - **Constituent feature implementation chunks** (if bundling per Step - 2) — route to `/spf-implement-feature`. + 2) — route to `/implement-spf-feature`. - **Use-case-specific behavior creation** (if any) — route to - `/spf-create-behavior`. + `/create-spf-behavior`. - **Existing behavior updates** (if any) — route to - `/spf-update-behavior` or `/refactor-behavior`. + `/change-spf-behavior` or `/change-spf-behavior`. - **Engine-level test scaffolding** — engine integration tests for the variant; per-behavior tests for any new use-case-specific behaviors. @@ -459,7 +459,7 @@ typical for use-case implementations: **Output of this step.** A chunk list with mechanism + downstream-skill routing per chunk + which layer it lands in. Same table shape as -`/spf-implement-feature`, with a *Layer* column added when opt-in +`/implement-spf-feature`, with a *Layer* column added when opt-in extensions are in scope. ### Step 4 — Apply cross-cutting concern checks @@ -518,14 +518,14 @@ Iterate per chunk: factory shape parallels `createSimpleHlsEngine`). - **Adapter creation** — handle inline (typically; the adapter shape parallels `SimpleHlsMediaElement` + `SimpleHlsMediaMixin`). - - **New use-case-specific behavior** → route to `/spf-create-behavior`. + - **New use-case-specific behavior** → route to `/create-spf-behavior`. - **Behavior update (purpose changing)** → route to - `/spf-update-behavior`. + `/change-spf-behavior`. - **Behavior refactor (purpose preserved)** → route to - `/refactor-behavior`. + `/change-spf-behavior`. - **Unimplemented constituent feature** → route to - `/spf-implement-feature` (per Step 2's readiness strategy). - - **Structural (split/merge)** → route via `/refactor-behavior`. + `/implement-spf-feature` (per Step 2's readiness strategy). + - **Structural (split/merge)** → route via `/change-spf-behavior`. - **Media-layer / network-layer** — handle inline for now; future skills will own these. 3. **Run the test passing.** @@ -535,7 +535,7 @@ Iterate per chunk: propose to user, get confirmation, update doc. **Downstream skill missing — explicit handling.** Same as -`/spf-implement-feature`: defer / build downstream skill inline / +`/implement-spf-feature`: defer / build downstream skill inline / apply ad-hoc with extract-later flag. User makes the call. ### Step 7 — Final-shape audit (per chunk + cumulative) @@ -649,7 +649,7 @@ overall depth). pre-implementation `"audio-only-mode-override (coarse)"`. - If a constituent feature was partially implemented as part of this pass (per Step 2's bundling strategy), the feature doc gets its - own update too — same shape as if `/spf-implement-feature` had run. + own update too — same shape as if `/implement-spf-feature` had run. **Doc revisions are explicit.** Per the doc-as-starting-point principle, every revision is proposed to the user before applying. @@ -698,7 +698,7 @@ Steps 7–9 outputs after implementation. ## Why this order Step 1 (disambiguation + routing) is the novel discipline compared to -`/spf-implement-feature`. Implementation work on a use case can route +`/implement-spf-feature`. Implementation work on a use case can route to a feature implementation, a doc creation, or stay here — getting that routing right at the start prevents wrong-skill work. @@ -707,11 +707,11 @@ cases compose features, and the readiness state of constituent features determines whether the use-case implementation can proceed straightforwardly or needs to bundle constituent work. -Steps 3–7 mostly parallel `/spf-implement-feature`'s chunk-decomposition +Steps 3–7 mostly parallel `/implement-spf-feature`'s chunk-decomposition + TDD + audit shape, with use-case-specific details. Step 8 (doc update + constituent cascade) is heavier than -`/spf-implement-feature`'s Step 8 because of the bidirectional +`/implement-spf-feature`'s Step 8 because of the bidirectional cross-link discipline: both the use-case doc and the constituent feature docs need updates. @@ -726,16 +726,16 @@ isn't; building a factory twice; silent doc drift). ## When this is the wrong skill -- **You want to implement a feature** → `/spf-implement-feature`. Use +- **You want to implement a feature** → `/implement-spf-feature`. Use cases compose features; if your invocation is really about a single capability the engine gains, the feature implementation skill is the right tool. - **You want to document a use case (not yet documented)** → - `/spf-document-use-case`. Implementation requires a starting-point + `/document-spf-use-case`. Implementation requires a starting-point doc. - **You want to refactor an existing behavior without feature/use-case - scope** → `/refactor-behavior`. -- **You want to split or merge behaviors** → `/refactor-behavior`'s + scope** → `/change-spf-behavior`. +- **You want to split or merge behaviors** → `/change-spf-behavior`'s decomposition check. - **You want to write an architectural design doc** → `design` skill. - **You want to write an RFC** → `rfc` skill. diff --git a/.agents/skills/investigate-issue/SKILL.md b/.agents/skills/investigate-issue/SKILL.md new file mode 100644 index 00000000..6351e3cf --- /dev/null +++ b/.agents/skills/investigate-issue/SKILL.md @@ -0,0 +1,24 @@ +--- +name: investigate-issue +description: Investigate GitHub issues without changing code. Use when asked to analyze, diagnose, scope, or produce an evidence-backed implementation plan. +--- + +# GitHub issue analysis + +Do not implement the fix. + +1. Fetch every referenced issue, comments, labels, and linked PRs or discussions. +2. Restate the observable problem, constraints, and acceptance criteria; identify missing or conflicting information. +3. Inspect the relevant code, tests, history, and nearest `AGENTS.md` before forming a root-cause hypothesis. +4. Apply domain-specific constraints only when the affected code or acceptance criteria make them material. +5. Distinguish verified cause, likely inference, and open question. +6. Produce a sequenced plan with files/surfaces, behavior changes, tests, compatibility risks, and documentation impact. +7. Link the source issues and relevant repository evidence. + +When multiple issues overlap, explain the dependency or shared root and propose an order that keeps each change reviewable. + +## Example + +Input: “Investigate why the player stalls after switching live sources.” + +Output: A verified problem statement, evidence-ranked root-cause analysis, open questions, and a sequenced implementation plan without code changes. diff --git a/.agents/skills/maintain-agent-docs/SKILL.md b/.agents/skills/maintain-agent-docs/SKILL.md new file mode 100644 index 00000000..8cbc9982 --- /dev/null +++ b/.agents/skills/maintain-agent-docs/SKILL.md @@ -0,0 +1,35 @@ +--- +name: maintain-agent-docs +description: Maintain repository agent guidance. Use for AGENTS.md, CLAUDE.md compatibility, skill drift, duplicated instructions, or context budgets. +--- + +# Agent documentation + +Keep agent guidance as a routing and correction layer over project sources, not a second documentation system. + +## Placement + +- Put durable, broadly applicable repository facts in the nearest `AGENTS.md`. +- Keep each `CLAUDE.md` as `@AGENTS.md` unless Claude-only behavior is genuinely required. +- Put an explicit, repeatable vertical workflow in a skill. +- Keep checked-in skills as direct children of `.agents/skills/`; treat `.claude/skills/` and `.opencode/skills/` as generated directory aliases. +- Put conditional detail in a directly linked skill reference. +- Put rules that can be mechanically checked in code, tests, lint, hooks, or `build/scripts/check-workspace.mjs`. +- Put architecture rationale in `internal/design/`, `internal/decisions/`, or `rfc/`. + +## Process + +1. Identify the repeated failure or new fact and the executable source that proves it. +2. Search existing agent docs and skills for overlap or contradiction. +3. Prefer updating a source pointer or validator over copying code, schemas, commands, or examples. +4. Remove obsolete guidance in the same change. +5. Keep skill frontmatter portable: `name` and a precise `description` only. +6. Run `pnpm check:workspace` and review the reported context budgets. + +Do not add generic software advice the model already knows. Add a rule only when an agent would plausibly get this repository wrong without it. + +## Example + +Input: “Remove duplicated setup instructions from agent guidance.” + +Output: Smaller routing docs that point to executable sources, corrected aliases, passing validators, and an updated context-budget report. diff --git a/.agents/skills/migrate-css-to-tailwind/SKILL.md b/.agents/skills/migrate-css-to-tailwind/SKILL.md new file mode 100644 index 00000000..45423ccf --- /dev/null +++ b/.agents/skills/migrate-css-to-tailwind/SKILL.md @@ -0,0 +1,21 @@ +--- +name: migrate-css-to-tailwind +description: Migrate site styles to Tailwind v4. Use for CSS, modules, SCSS, styled components, theme tokens, semantic variants, or arbitrary values. +--- + +# CSS to Tailwind + +1. Read the source CSS and affected markup together. +2. Inspect `site/src/styles/globals.css` and nearby components for existing tokens, utilities, and variants. +3. Preserve cascade, specificity, responsive states, interaction states, and runtime-driven selectors; do not translate declarations in isolation. +4. Load `references/migration.md` for the migration mapping and site-specific Tailwind conventions. +5. Prefer theme-backed tokens and semantic utilities. For site code, use inline styles for non-token one-offs and CSS custom-property bridges when variants are required; do not introduce arbitrary-value classes. +6. Compare rendered behavior across affected states and breakpoints. + +Keep the change scoped to migration; report unrelated styling problems separately. + +## Example + +Input: “Migrate the site header module to Tailwind v4.” + +Output: Equivalent utilities and tokens with preserved cascade, responsive and interaction states, plus rendered parity checks. diff --git a/.claude/skills/css-to-tailwind/references/migration.md b/.agents/skills/migrate-css-to-tailwind/references/migration.md similarity index 74% rename from .claude/skills/css-to-tailwind/references/migration.md rename to .agents/skills/migrate-css-to-tailwind/references/migration.md index e5ee5a88..f1271e79 100644 --- a/.claude/skills/css-to-tailwind/references/migration.md +++ b/.agents/skills/migrate-css-to-tailwind/references/migration.md @@ -15,7 +15,7 @@ In this repo, skin work often means keeping **`packages/skins/*/css/`** and the | Concern | Prefer | | ------- | ------ | | New design tokens | Add to **`@theme { }`** (skin shared sheet or app entry CSS) so utilities like `rounded-*`, `gap-*`, and custom namespaces stay consistent | -| One-off escapes | Arbitrary utilities that reuse **`theme()`**, e.g. **`bg-[theme(--surface)]`**, whenever the backing variable already exists in **`@theme`** | +| Site one-off values | Inline styles; use an inline CSS custom property with Tailwind custom-property syntax when a variant is required | | Repeated arbitrary variants / long class strings | **`@utility`** blocks in CSS | | Repeated selector shapes | **`@custom-variant`** (see **`site`** / **`apps/sandbox`** for examples) | | Keyframes tied to animations | Registered in CSS (`@keyframes`) and wired through **`@theme`** (see comments in skins `input-feedback` tailwind components) | @@ -36,14 +36,13 @@ Legacy **`tailwind.config.js`** theme spreads are not the primary path here—ex - `border-radius: 8px` → prefer `rounded-lg` (or a theme radius key) if equivalent/acceptable - `font-size`, `spacing`, `colors`, `shadow`, `z-index`, `radius` should map to **`@theme` or default v4 scales** when acceptable -### 3. When arbitrary values are allowed +### 3. Handle site one-offs without arbitrary-value classes -Only when: +For site code: -- The value is truly one-off, -- No existing token matches closely, -- The value is required for pixel-perfect migration, **or** -- The CSS property has no Tailwind utility +- Use an inline style when a non-token value is truly one-off. +- For responsive, dark-mode, or other variants, define an inline custom property and consume it with syntax such as `md:min-h-(--md-min-h)`. +- Add a theme token or semantic utility when the value repeats. ### 4. Avoid arbitrary values for common scale values @@ -61,9 +60,9 @@ Good: - `rounded-lg` - `text-sm` -### 5. Prefer theme-backed utilities (not raw `var()` in class strings) +### 5. Prefer theme-backed utilities -Avoid lots of: +Avoid: - `text-[var(--color-text)]` - `bg-[var(--color-surface)]` @@ -71,18 +70,13 @@ Avoid lots of: Prefer: - Semantic utilities that map to **`@theme`** variables (`text-fg`, `bg-surface`, `border-border`, etc.) -- Or **`bg-[theme(--surface)]`**-style arbitrary values only until a dedicated utility exists (replace with **`@utility`** once repeated) +- An inline custom-property bridge for a one-off site value that needs a variant (Adapt names to the project's **`@theme`** variable names; add tokens to CSS when missing.) -### 6. Use arbitrary variants/properties sparingly +### 6. Prefer named variants and utilities -Allowed examples: - -- `[container-type:inline-size]` -- Arbitrary positions that must reference a **`@theme`** custom property: **`bg-[theme(--color-fg)]`**-style values (use the actual **`--*`** names from your **`@theme`** block—see Tailwind v4 **`theme()`** documentation) - -If repeated, recommend **`@utility`** or extending **`@theme`** instead of copying the same arbitrary class everywhere. +Use existing responsive, state, data, ARIA, and custom variants. Add **`@utility`**, **`@custom-variant`**, or an **`@theme`** token when a pattern repeats instead of copying bracket syntax. ### 7. Preserve responsive, state, and media behavior @@ -97,6 +91,6 @@ If repeated, recommend **`@utility`** or extending **`@theme`** instead of copyi Include: - Converted utilities (what replaced which declarations) -- Arbitrary values used and why each is justified +- Inline one-offs or custom-property bridges used and why - Values that should become theme tokens later - Any CSS that should remain CSS (-keyframes, `@property`, unsupported selectors, etc.) diff --git a/.agents/skills/review-accessibility/SKILL.md b/.agents/skills/review-accessibility/SKILL.md new file mode 100644 index 00000000..5ce6c509 --- /dev/null +++ b/.agents/skills/review-accessibility/SKILL.md @@ -0,0 +1,21 @@ +--- +name: review-accessibility +description: Review Video.js UI accessibility without editing code. Use for semantics, names, ARIA, keyboard, focus, captions, contrast, motion, or WCAG. +--- + +# Accessibility review + +Inspect rendered semantics, interaction code, tests, and adjacent controls. + +1. Define the expected name, role, value or state, keyboard behavior, focus behavior, and announcements. +2. Use `references/checklist.md` selectively for the affected control type; do not load the whole checklist for a narrow change. +3. Verify behavior in a browser when the review covers interaction or focus. +4. Report only observable barriers or standards-backed risks. + +For each finding, identify the affected user, element or state, impact, evidence, and smallest viable fix. Distinguish blockers from enhancements and avoid speculative conformance claims. + +## Example + +Input: “Review the captions menu for keyboard and screen-reader barriers.” + +Output: Standards-backed findings with the affected user, observable impact, evidence, severity, and smallest viable fix. diff --git a/.claude/skills/aria/review/checklist.md b/.agents/skills/review-accessibility/references/checklist.md similarity index 100% rename from .claude/skills/aria/review/checklist.md rename to .agents/skills/review-accessibility/references/checklist.md diff --git a/.agents/skills/review-api/SKILL.md b/.agents/skills/review-api/SKILL.md new file mode 100644 index 00000000..0c0a158d --- /dev/null +++ b/.agents/skills/review-api/SKILL.md @@ -0,0 +1,21 @@ +--- +name: review-api +description: Review Video.js TypeScript APIs without editing code. Use for inference, state, extensibility, composition, packaging, or architecture audits. +--- + +# API review + +Read the implementation, public exports, callers, types, tests, and relevant design records. + +1. Define the changed or proposed public surface and compatibility constraints. +2. Check runtime behavior, inference, naming, defaults, composition, escape hatches, lifecycle, packaging, and adjacent API consistency. +3. Use `references/checklist.md` for a broad audit; load only the sections relevant to the surface. +4. Tie every finding to a concrete caller, failure mode, or maintenance cost. + +Report findings by severity with the location, user impact, evidence, and smallest viable improvement. Separate correctness or compatibility problems from preferences. Do not score the API or invent issues to fill a template. + +## Example + +Input: “Review this new player factory API.” + +Output: Prioritized findings tied to concrete callers, inference failures, compatibility risks, or maintenance costs. diff --git a/.claude/skills/api/review/checklist.md b/.agents/skills/review-api/references/checklist.md similarity index 73% rename from .claude/skills/api/review/checklist.md rename to .agents/skills/review-api/references/checklist.md index 99bbc9ad..917f04da 100644 --- a/.claude/skills/api/review/checklist.md +++ b/.agents/skills/review-api/references/checklist.md @@ -4,8 +4,6 @@ Single-agent checklist for fast API reviews without forking. ## Types -See `references/typescript.md` for patterns, `references/anti-patterns.md` for anti-patterns. - - [ ] Inference-first (minimal explicit generics)? - [ ] Helper types exported (`ExtractState`, etc.)? - [ ] Type guards for discriminated unions? @@ -16,8 +14,6 @@ See `references/typescript.md` for patterns, `references/anti-patterns.md` for a ## API Surface -See `references/principles.md` for principles, `references/anti-patterns.md` for anti-patterns. - - [ ] Config objects for 3+ parameters? - [ ] No boolean traps? - [ ] No function overloads? @@ -29,8 +25,6 @@ See `references/principles.md` for principles, `references/anti-patterns.md` for ## Extensibility -See `references/extensibility.md` for patterns. - - [ ] Extension through composition, not registration? - [ ] Middleware ordering explicit (onion model)? - [ ] Builder chains return new typed objects? @@ -40,8 +34,6 @@ See `references/extensibility.md` for patterns. ## Progressive Disclosure -See `references/principles.md` for patterns. - - [ ] Zero-config default works? - [ ] Complexity grows with use case? - [ ] Escape hatches compose (don't replace defaults)? @@ -50,17 +42,8 @@ See `references/principles.md` for patterns. ## Packaging -See `references/anti-patterns.md` Packaging section. - - [ ] ESM-first? - [ ] `sideEffects: false`? - [ ] Shallow subpaths (`pkg/react` not `pkg/react/hooks/store`)? - [ ] Peer deps correct (not bundled)? - [ ] Tree-shakeable exports? - -## Related Skills - -For UI component-specific reviews, also check: - -- `component` skill — compound components, polymorphism, styling patterns -- `aria` skill — keyboard, focus, ARIA attributes diff --git a/.agents/skills/review-branch/SKILL.md b/.agents/skills/review-branch/SKILL.md new file mode 100644 index 00000000..325dcade --- /dev/null +++ b/.agents/skills/review-branch/SKILL.md @@ -0,0 +1,24 @@ +--- +name: review-branch +description: Review the current branch without editing code. Use for PR-style findings, regressions, merge-base comparison, or pre-merge risk assessment. +--- + +# Branch review + +Review the branch as a whole; do not implement fixes. + +1. Determine the intended base and inspect the merge-base diff, commit history, and working tree separately. +2. Fetch a linked issue or PR when supplied and derive the acceptance boundary. +3. Read changed code in context, including callers, tests, exports, and relevant configuration. +4. Check whether the change is a good citizen of the whole codebase and documentation: follow established ownership, naming, architecture, public contracts, and nearby patterns rather than judging the diff in isolation. +5. Apply only the domain constraints and review criteria that the changed surface requires. +6. Prioritize correctness, regressions, security, accessibility, public compatibility, and missing tests. Avoid style findings already enforced by tools. +7. Validate suspected findings with a targeted command or source trace when practical. + +Lead with findings ordered by severity. For each, include the file/line, concrete failure mode, evidence, and smallest correction. Then summarize the change and residual test gaps. Say explicitly when no actionable findings remain. + +## Example + +Input: “Review this branch against main.” + +Output: Severity-ordered actionable findings with locations and evidence, followed by a concise change summary and residual test gaps. diff --git a/.agents/skills/review-docs/SKILL.md b/.agents/skills/review-docs/SKILL.md new file mode 100644 index 00000000..e8b1f81c --- /dev/null +++ b/.agents/skills/review-docs/SKILL.md @@ -0,0 +1,21 @@ +--- +name: review-docs +description: Review Video.js documentation without editing it. Use for accuracy, reader outcomes, structure, voice, examples, MDX, READMEs, or docs diffs. +--- + +# Documentation review + +Treat implementation, types, tests, schemas, and generated output as factual sources. + +1. Identify the audience, intended outcome, artifact type, and owning source files. +2. For site guides and concepts, read `site/src/content/docs/how-to/write-guides.mdx`. For generated reference pages, read `site/src/content/docs/reference/write-references.mdx`. Use neighboring docs for other artifact types. +3. Verify claims and examples against current code before reviewing voice, structure, examples, MDX conventions, and reader outcomes against the owning guide. +4. Render affected MDX and run examples where practical. + +Report broken or misleading content first. For each finding, give the location, reader impact, evidence, and concise fix. Keep stylistic preferences separate from correctness issues. + +## Example + +Input: “Review the new autoplay guide.” + +Output: Accuracy and reader-outcome findings first, followed by clearly separated structure or voice suggestions. diff --git a/.agents/skills/review-tailwind-migration/SKILL.md b/.agents/skills/review-tailwind-migration/SKILL.md new file mode 100644 index 00000000..b326a651 --- /dev/null +++ b/.agents/skills/review-tailwind-migration/SKILL.md @@ -0,0 +1,21 @@ +--- +name: review-tailwind-migration +description: Review Tailwind v4 migrations without editing code. Use for CSS parity, cascade, responsive states, tokens, variants, or arbitrary values. +--- + +# CSS-to-Tailwind review + +Read the original CSS, affected markup, `site/src/styles/globals.css`, and nearby token or utility usage together. + +1. Compare layout, cascade, specificity, breakpoints, pseudos, data or ARIA selectors, transitions, motion, and runtime-driven states. +2. Use `references/checklist.md` for a broad pass; skip sections outside the change. +3. Inspect the rendered result across affected states and breakpoints. +4. Separate behavior or parity regressions from maintainability suggestions. + +For each finding, cite the source rule and target class, explain the user-visible impact, and suggest the smallest correction. For site code, require inline styles for non-token one-offs and custom-property bridges when a variant is needed. + +## Example + +Input: “Review the header’s CSS-module-to-Tailwind migration.” + +Output: Parity findings tied to source rules and target classes, separated from optional maintainability suggestions. diff --git a/.claude/skills/css-to-tailwind/review/checklist.md b/.agents/skills/review-tailwind-migration/references/checklist.md similarity index 77% rename from .claude/skills/css-to-tailwind/review/checklist.md rename to .agents/skills/review-tailwind-migration/references/checklist.md index 8e114325..e6dead1a 100644 --- a/.claude/skills/css-to-tailwind/review/checklist.md +++ b/.agents/skills/review-tailwind-migration/references/checklist.md @@ -1,6 +1,6 @@ # CSS → Tailwind review checklist -Single-pass checklist against [references/migration.md](../references/migration.md). +Single-pass checklist for comparing a CSS-to-Tailwind change with its source. ## Utilities and readability @@ -12,10 +12,11 @@ Single-pass checklist against [references/migration.md](../references/migration. - [ ] Colors, spacing, radii, fonts, shadows, and z-index prefer **`@theme`** / default v4 scales or semantic utilities over raw arbitrary values where a close match exists - [ ] Repeated `*- [var(...)]` patterns are flagged for **`@theme`** entries and generated utilities (`theme(...)`) instead of one-off brackets -## Arbitrary values +## Non-token values -- [ ] No arbitrary spacing/radius/type for values that fit the v4 scale or an existing **`@theme`** key (`mt-[16px]` → scale / theme token) -- [ ] Remaining arbitrary values are justified (one-off, pixel-perfect requirement, no utility/property gap) and noted in report; repeated patterns flagged for **`@utility`** +- [ ] Site code does not introduce arbitrary-value classes; one-offs use inline styles +- [ ] A non-token value that needs a variant uses an inline CSS custom-property bridge +- [ ] Repeated values are flagged for **`@theme`** or **`@utility`** ## Variants and selectors diff --git a/.agents/skills/review-ui-component/SKILL.md b/.agents/skills/review-ui-component/SKILL.md new file mode 100644 index 00000000..ad6d2acf --- /dev/null +++ b/.agents/skills/review-ui-component/SKILL.md @@ -0,0 +1,21 @@ +--- +name: review-ui-component +description: Review Video.js component architecture without editing code. Use for contracts, state ownership, parity, compound APIs, props, styling, or animation. +--- + +# Component review + +Read the core contract, both platform adapters, tests, rendered semantics, and adjacent components. + +1. Check layer ownership, observable behavior, state control, platform parity, props, styling contracts, cleanup, and test coverage. +2. Use `references/checklist.md` to choose the general and Video.js-specific checklists; load only those that apply. +3. Compare proposed abstractions with a nearby component before recommending a new pattern. +4. Tie findings to a caller, rendered behavior, or maintenance failure mode. + +Report actionable findings by severity. Separate regressions and contract gaps from optional design preferences. + +## Example + +Input: “Review the HTML and React menu implementations for parity.” + +Output: Prioritized findings tied to contract drift, rendered behavior, state ownership, accessibility, or missing tests. diff --git a/.claude/skills/component/review/checklist.md b/.agents/skills/review-ui-component/references/checklist.md similarity index 54% rename from .claude/skills/component/review/checklist.md rename to .agents/skills/review-ui-component/references/checklist.md index 88ca5496..299ae6ef 100644 --- a/.claude/skills/component/review/checklist.md +++ b/.agents/skills/review-ui-component/references/checklist.md @@ -12,19 +12,8 @@ Checklists for reviewing UI components against architecture patterns and convent ## Quick Selection -**Building a new Video.js component?** -→ Use [general.md](checklists/general.md) + [videojs.md](checklists/videojs.md) - **Reviewing component architecture?** → Use [general.md](checklists/general.md) **Classifying issues found?** → Use [severity.md](checklists/severity.md) - -## See Also - -- [Anti-Patterns](../references/anti-patterns.md) — Full anti-pattern reference with examples -- [Props](../references/props.md) — Prop naming conventions -- [Styling](../references/styling.md) — Data attributes and CSS variables -- [Video.js Architecture](../references/videojs.md) — Three-layer component architecture -- [Accessibility Checklist](../../aria/review/checklist.md) — Full a11y checklist diff --git a/.claude/skills/component/review/checklists/general.md b/.agents/skills/review-ui-component/references/checklists/general.md similarity index 98% rename from .claude/skills/component/review/checklists/general.md rename to .agents/skills/review-ui-component/references/checklists/general.md index 577b89b1..af3a8972 100644 --- a/.claude/skills/component/review/checklists/general.md +++ b/.agents/skills/review-ui-component/references/checklists/general.md @@ -178,8 +178,6 @@ onOpenChange?: (open: boolean, details: ChangeDetails) => void; ## Accessibility -For full accessibility review, load the `aria` skill. - Quick checks: - [ ] All interactive elements have accessible names diff --git a/.claude/skills/component/review/checklists/severity.md b/.agents/skills/review-ui-component/references/checklists/severity.md similarity index 100% rename from .claude/skills/component/review/checklists/severity.md rename to .agents/skills/review-ui-component/references/checklists/severity.md diff --git a/.claude/skills/component/review/checklists/videojs.md b/.agents/skills/review-ui-component/references/checklists/videojs.md similarity index 94% rename from .claude/skills/component/review/checklists/videojs.md rename to .agents/skills/review-ui-component/references/checklists/videojs.md index bbb3f9b7..ff853eea 100644 --- a/.claude/skills/component/review/checklists/videojs.md +++ b/.agents/skills/review-ui-component/references/checklists/videojs.md @@ -1,6 +1,6 @@ # Video.js Component Checklist -Video.js-specific patterns. See [videojs.md](../../references/videojs.md) for architecture details. +Video.js-specific checks for core classes and platform adapters. --- diff --git a/.agents/skills/write-api-reference/SKILL.md b/.agents/skills/write-api-reference/SKILL.md new file mode 100644 index 00000000..78814d31 --- /dev/null +++ b/.agents/skills/write-api-reference/SKILL.md @@ -0,0 +1,44 @@ +--- +name: write-api-reference +description: Write generated Video.js API reference pages. Use for components, hooks, controllers, factories, builder compatibility, demos, or extracted JSDoc. +--- + +# API reference + +Treat TypeScript source and the builder E2E test as the specification. Generated JSON is diagnostic output, not an editable source. + +## Workflow + +1. Identify the export and read its implementation, public export path, tests, and any matching design record. +2. Read `site/scripts/api-docs-builder/src/tests/e2e.test.ts` to confirm the builder contract involved in the change. +3. Load only the needed reference: + - Component discovery or naming failure: `references/builder-conventions.md` + - Hook, controller, mixin, factory, or other utility: `references/util-conventions.md` + - New MDX reference page: `references/mdx-structure.md` + - New interactive example: `references/demo-patterns.md` +4. Run `pnpm -F site api-docs` and inspect the corresponding file under `site/src/content/generated-*-reference/`. +5. Fix missing metadata at the TypeScript/JSDoc source or in the builder. Do not hand-edit generated JSON. +6. Create or update `site/src/content/docs/reference/.mdx` and the matching `site/src/docs.config.ts` entry when a page is required. +7. Verify the page for every supported framework/style combination it targets. + +## Component pages + +- Derive props, state, data attributes, parts, tag names, and behavior from source. +- Add prose only for non-obvious behavior, styling contracts, accessibility, or platform constraints. +- Include basic HTML and React demos when both platforms expose the component; follow existing neighboring demos when the reference guide leaves room for judgment. + +## Utility pages + +- Confirm the export is reachable from a builder-scanned public entry point. +- Add `@public` only when the export intentionally belongs in reference docs and naming-based discovery does not include it. +- Verify generated overloads, parameters, and return data before authoring prose. + +## Validation + +Run the builder E2E test for builder changes, `pnpm -F site api-docs`, and the narrowest site check that renders the affected page. + +## Example + +Input: “Add the Menu component API reference.” + +Output: Source JSDoc and builder-compatible metadata, verified generated output, MDX examples, navigation config, and site checks. diff --git a/.claude/skills/api-reference/references/builder-conventions.md b/.agents/skills/write-api-reference/references/builder-conventions.md similarity index 100% rename from .claude/skills/api-reference/references/builder-conventions.md rename to .agents/skills/write-api-reference/references/builder-conventions.md diff --git a/.claude/skills/api-reference/references/demo-patterns.md b/.agents/skills/write-api-reference/references/demo-patterns.md similarity index 100% rename from .claude/skills/api-reference/references/demo-patterns.md rename to .agents/skills/write-api-reference/references/demo-patterns.md diff --git a/.claude/skills/api-reference/references/mdx-structure.md b/.agents/skills/write-api-reference/references/mdx-structure.md similarity index 100% rename from .claude/skills/api-reference/references/mdx-structure.md rename to .agents/skills/write-api-reference/references/mdx-structure.md diff --git a/.claude/skills/api-reference/references/util-conventions.md b/.agents/skills/write-api-reference/references/util-conventions.md similarity index 98% rename from .claude/skills/api-reference/references/util-conventions.md rename to .agents/skills/write-api-reference/references/util-conventions.md index 8eb108a0..3c33f76c 100644 --- a/.claude/skills/api-reference/references/util-conventions.md +++ b/.agents/skills/write-api-reference/references/util-conventions.md @@ -38,7 +38,7 @@ No code changes needed in the builder itself — convention over configuration. ## JSDoc Conventions -The builder extracts JSDoc from source exports to populate reference pages. These rules override the root CLAUDE.md "Minimal JSDoc" guidelines for API reference exports. +The builder extracts JSDoc from source exports to populate reference pages. These rules are the detailed form of the API-reference exception in the root `AGENTS.md`. ### Summary description (required) diff --git a/.agents/skills/write-design-doc/SKILL.md b/.agents/skills/write-design-doc/SKILL.md new file mode 100644 index 00000000..3b43a1ae --- /dev/null +++ b/.agents/skills/write-design-doc/SKILL.md @@ -0,0 +1,27 @@ +--- +name: write-design-doc +description: Write or update a Video.js design or decision record. Use for owned architecture, feature designs, component specifications, or durable rationale. +--- + +# Design records + +Read `internal/design/README.md` and `internal/decisions/README.md`; they define current placement, status, and format. + +## Workflow + +1. Decide whether the work is an architecture/feature design, a single tactical decision, an RFC, or only an implementation plan. +2. Read the relevant code, tests, existing records, and history. Treat code as current behavior and records as rationale. +3. State the problem and constraints before the chosen design. +4. Record the decision, alternatives actually considered, tradeoffs, and consequences. Use code only where it clarifies a contract. +5. Link the implementation surface and related records. +6. Remove speculative detail that the code will express better or that has not been decided. + +Use a template in `templates/` only when it matches the artifact. Load `references/structure.md` for a complex document, `references/components.md` for UI component design, or `references/features.md` for multi-part features. + +Keep the document useful after implementation: preserve why, constraints, and rejected alternatives; let code and tests own mechanics. + +## Example + +Input: “Record why source selection belongs in the core player.” + +Output: A durable design or decision record covering context, constraints, choice, credible alternatives, consequences, and implementation links. diff --git a/.claude/skills/design/references/components.md b/.agents/skills/write-design-doc/references/components.md similarity index 94% rename from .claude/skills/design/references/components.md rename to .agents/skills/write-design-doc/references/components.md index 714476e2..2183f111 100644 --- a/.claude/skills/design/references/components.md +++ b/.agents/skills/write-design-doc/references/components.md @@ -64,7 +64,7 @@ Reference [WAI-ARIA Authoring Practices](https://www.w3.org/WAI/ARIA/apg/) for t 2. **Minimal examples** — Only show what's different, use `{/* ... */}` for the rest 3. **Data attributes** — Essential for styling component states 4. **No separate parts file** — API surface covers everything; highlight noteworthy parts inline -5. **No architecture file** — Implementation details go in `.claude/plans/` +5. **No implementation log** — Temporary mechanics go in `.agents/plans/` ## Prior Art @@ -84,9 +84,3 @@ These aren't API references — look at them for edge cases, feature requirement - **[Vidstack](https://www.vidstack.io/)** — Feature requirements, what they got right, signals approach - **[Video.js v8](https://videojs.com/)** — Legacy context, what worked and what didn't, migration considerations - **[Plyr](https://plyr.io/)** — Minimal implementations, what a simple player needs - -## Related Skills - -- `aria` skill — Accessibility implementation details -- `component` skill — Component architecture patterns -- `api` skill — API design principles diff --git a/.claude/skills/design/references/features.md b/.agents/skills/write-design-doc/references/features.md similarity index 92% rename from .claude/skills/design/references/features.md rename to .agents/skills/write-design-doc/references/features.md index 7fc1541e..e3d708d6 100644 --- a/.claude/skills/design/references/features.md +++ b/.agents/skills/write-design-doc/references/features.md @@ -25,7 +25,7 @@ Guidance for writing feature design docs. Think of these as **proto-user-facing - `index.md` — Problem, API surface, state requirements, behavior - `decisions.md` — Only when decisions are raised and debated -No `architecture.md` — implementation details go in `.claude/plans/`. No `examples.md` — keep examples in the main doc, minimal. +No separate implementation log — temporary mechanics go in `.agents/plans/`. Keep minimal contract examples in the main record. ## State & Store @@ -54,7 +54,3 @@ Research prior art before drafting: - **Player libraries** (Media Chrome, Vidstack, Video.js v8, Plyr) — Not for API patterns, but for edge cases, feature requirements, platform quirks, and context about the problem space. How did they solve this? What edge cases did they hit? See existing Design Docs in `internal/design/` for examples of good prior art research (e.g., `ui/controls-design.md`, `ui/playback-rate-button.md`). - -## Related Skills - -- `api` skill — API design principles diff --git a/.claude/skills/design/references/structure.md b/.agents/skills/write-design-doc/references/structure.md similarity index 92% rename from .claude/skills/design/references/structure.md rename to .agents/skills/write-design-doc/references/structure.md index 0cb6e9b1..134d81d1 100644 --- a/.claude/skills/design/references/structure.md +++ b/.agents/skills/write-design-doc/references/structure.md @@ -41,7 +41,7 @@ internal/design/feature-name/ **`decisions.md`** is optional. Only create it when real debates happen — alternatives are weighed, trade-offs are discussed. Don't scaffold it upfront. Let the code and implementation speak for themselves. -**What about architecture?** Internal structure, data flow, and implementation details belong in `.claude/plans/` as implementation plans, not design docs. Design docs focus on the "what" and "why" of the API surface, not the "how" of internals. +**What about implementation mechanics?** Temporary file-by-file steps belong in `.agents/plans/`. Stable internal structure and data flow may belong in a design record when they explain constraints or rationale. **Example:** A compound UI component, a multi-part feature with store integration. diff --git a/.claude/skills/design/templates/component-basic.md b/.agents/skills/write-design-doc/templates/component-basic.md similarity index 100% rename from .claude/skills/design/templates/component-basic.md rename to .agents/skills/write-design-doc/templates/component-basic.md diff --git a/.claude/skills/design/templates/component-compound.md b/.agents/skills/write-design-doc/templates/component-compound.md similarity index 100% rename from .claude/skills/design/templates/component-compound.md rename to .agents/skills/write-design-doc/templates/component-compound.md diff --git a/.claude/skills/design/templates/decision.md b/.agents/skills/write-design-doc/templates/decision.md similarity index 100% rename from .claude/skills/design/templates/decision.md rename to .agents/skills/write-design-doc/templates/decision.md diff --git a/.claude/skills/design/templates/feature-multi.md b/.agents/skills/write-design-doc/templates/feature-multi.md similarity index 96% rename from .claude/skills/design/templates/feature-multi.md rename to .agents/skills/write-design-doc/templates/feature-multi.md index bb0d9c1a..5a5de3f5 100644 --- a/.claude/skills/design/templates/feature-multi.md +++ b/.agents/skills/write-design-doc/templates/feature-multi.md @@ -8,7 +8,7 @@ internal/design/feature-name/ `decisions.md` is **optional**. Don't scaffold it upfront — add it when real trade-offs are discussed and alternatives are weighed. Let the code and implementation speak for themselves. -Architecture and implementation details belong in `.claude/plans/`, not here. +Temporary implementation mechanics belong in `.agents/plans/`, not here. --- diff --git a/.claude/skills/design/templates/feature-single.md b/.agents/skills/write-design-doc/templates/feature-single.md similarity index 100% rename from .claude/skills/design/templates/feature-single.md rename to .agents/skills/write-design-doc/templates/feature-single.md diff --git a/.agents/skills/write-docs/SKILL.md b/.agents/skills/write-docs/SKILL.md new file mode 100644 index 00000000..d1457a76 --- /dev/null +++ b/.agents/skills/write-docs/SKILL.md @@ -0,0 +1,39 @@ +--- +name: write-docs +description: Write Video.js guides, concepts, READMEs, and JSDoc. Use for site prose, examples, inline API documentation, or package documentation. +--- + +# Documentation + +Treat implementation, types, tests, and content schemas as factual sources. Read `site/src/content/docs/how-to/write-guides.mdx` before authoring a site guide. + +## Choose the artifact + +- Task-oriented outcome: how-to guide +- Mental model or rationale: concept page +- Package install and entry points: package README +- Non-obvious public contract at the symbol: JSDoc +- Generated component/utility surface: update the builder-owned reference source rather than duplicating it in prose + +## Workflow + +1. Identify the audience, question, and owning source files. +2. Read neighboring docs for current voice and MDX patterns. +3. Load only what applies: + - Voice and structure: `references/writing-style.md` + - SEO-sensitive site content: `references/seo.md` + - State/tooling concepts: `references/state-tooling.md` + - Component-library comparisons: `references/component-libraries.md` + - Code or error examples: the matching file in `patterns/` + - New artifact scaffold: the matching file in `templates/` +4. Lead with the user outcome or concept. Use complete, verified examples and explain only non-obvious parts. +5. Add the sidebar entry and framework/style restrictions for new site pages. +6. Run examples or relevant tests where practical and render affected MDX for every supported variant. + +Do not duplicate signatures TypeScript already expresses. API-builder exports are the exception when its tests require structured JSDoc fields. + +## Example + +Input: “Write a guide for configuring captions.” + +Output: A task-oriented page with verified examples, the correct sidebar metadata, and rendered MDX validation. diff --git a/.claude/skills/docs/patterns/code-examples.md b/.agents/skills/write-docs/patterns/code-examples.md similarity index 98% rename from .claude/skills/docs/patterns/code-examples.md rename to .agents/skills/write-docs/patterns/code-examples.md index 6c0c0597..2ca7eafd 100644 --- a/.claude/skills/docs/patterns/code-examples.md +++ b/.agents/skills/write-docs/patterns/code-examples.md @@ -282,7 +282,7 @@ pnpm add @videojs/store ## Demo Files (Site Pages) -Live demos in reference pages use the `` component with `?raw` imports for source code display. See the `api-reference` skill for full patterns. +Live demos in reference pages use the `` component with `?raw` imports for source code display. Follow the neighboring generated-reference demo patterns. ```mdx import BasicUsageDemo from "@/components/docs/demos/play-button/react/css/BasicUsage"; diff --git a/.claude/skills/docs/patterns/error-docs.md b/.agents/skills/write-docs/patterns/error-docs.md similarity index 100% rename from .claude/skills/docs/patterns/error-docs.md rename to .agents/skills/write-docs/patterns/error-docs.md diff --git a/.claude/skills/docs/references/component-libraries.md b/.agents/skills/write-docs/references/component-libraries.md similarity index 98% rename from .claude/skills/docs/references/component-libraries.md rename to .agents/skills/write-docs/references/component-libraries.md index 72ebb538..21e62a6a 100644 --- a/.claude/skills/docs/references/component-libraries.md +++ b/.agents/skills/write-docs/references/component-libraries.md @@ -245,9 +245,3 @@ Show real-world compositions: ## Applicable to Video.js The patterns above are drawn from many libraries. Not all apply to Video.js reference pages. Where existing patterns contradict the patterns outlined here, follow the existing patterns. - ---- - -## See Also - -- [Component Patterns](../../component/SKILL.md) — building headless components diff --git a/.claude/skills/docs/references/gold-standard.md b/.agents/skills/write-docs/references/gold-standard.md similarity index 100% rename from .claude/skills/docs/references/gold-standard.md rename to .agents/skills/write-docs/references/gold-standard.md diff --git a/.claude/skills/docs/references/seo.md b/.agents/skills/write-docs/references/seo.md similarity index 100% rename from .claude/skills/docs/references/seo.md rename to .agents/skills/write-docs/references/seo.md diff --git a/.claude/skills/docs/references/state-tooling.md b/.agents/skills/write-docs/references/state-tooling.md similarity index 98% rename from .claude/skills/docs/references/state-tooling.md rename to .agents/skills/write-docs/references/state-tooling.md index b033e715..90f3136c 100644 --- a/.claude/skills/docs/references/state-tooling.md +++ b/.agents/skills/write-docs/references/state-tooling.md @@ -351,9 +351,3 @@ console.log('Playing'); player.emit('custom:event', { data: 'value' }); ``` - ---- - -## See Also - -- [State Patterns](../../api/references/state.md) — state management patterns diff --git a/.claude/skills/docs/references/writing-style.md b/.agents/skills/write-docs/references/writing-style.md similarity index 100% rename from .claude/skills/docs/references/writing-style.md rename to .agents/skills/write-docs/references/writing-style.md diff --git a/.claude/skills/docs/templates/concept.md b/.agents/skills/write-docs/templates/concept.md similarity index 100% rename from .claude/skills/docs/templates/concept.md rename to .agents/skills/write-docs/templates/concept.md diff --git a/.claude/skills/docs/templates/how-to.md b/.agents/skills/write-docs/templates/how-to.md similarity index 100% rename from .claude/skills/docs/templates/how-to.md rename to .agents/skills/write-docs/templates/how-to.md diff --git a/.claude/skills/docs/templates/readme.md b/.agents/skills/write-docs/templates/readme.md similarity index 100% rename from .claude/skills/docs/templates/readme.md rename to .agents/skills/write-docs/templates/readme.md diff --git a/.agents/skills/write-rfc/SKILL.md b/.agents/skills/write-rfc/SKILL.md new file mode 100644 index 00000000..a8adcc5b --- /dev/null +++ b/.agents/skills/write-rfc/SKILL.md @@ -0,0 +1,26 @@ +--- +name: write-rfc +description: Draft or revise a Video.js RFC. Use for public APIs, product direction, user-facing DX, shared architecture, or hard-to-reverse proposals needing buy-in. +--- + +# RFC + +Read `rfc/README.md` for the current lifecycle and format. Use an internal design or decision record when the author owns the choice and no wider agreement is needed. + +## Workflow + +1. Verify that an RFC is warranted; skip it for local implementation details, ordinary bugs, or already-approved work. +2. Read current code, existing RFCs/design records, related issues, and relevant user evidence. +3. Define the problem, affected users, constraints, goals, and explicit non-goals. +4. Present the proposed direction at the level needed for agreement, not a line-by-line implementation plan. +5. Compare credible alternatives and make costs, compatibility, migration, and unresolved questions explicit. +6. Define how success will be evaluated and what happens after acceptance. +7. Keep status `draft` until the repository's approval process changes it. + +Make public API and DX tradeoffs explicit when central. Keep implementation detail in a later design record or plan. + +## Example + +Input: “Draft an RFC for a new plugin extension model.” + +Output: A draft proposal with user evidence, goals, non-goals, compatibility and migration costs, credible alternatives, open questions, and success measures. diff --git a/.claude/plans/README.md b/.claude/plans/README.md deleted file mode 100644 index 901aba26..00000000 --- a/.claude/plans/README.md +++ /dev/null @@ -1,60 +0,0 @@ -# Implementation Plans - -Working notes and implementation details for AI agents and developers. - -## Purpose - -This directory contains: - -- Step-by-step implementation plans -- Working notes during development -- AI-agent context for executing RFCs -- Completed work logs with key decisions - -## What Belongs Here - -- Implementation details and code snippets -- Debugging notes and discoveries -- Task breakdowns for complex features -- Post-implementation summaries - -## What Doesn't Belong Here - -Design proposals and architectural decisions belong in `/rfc`. This directory is for **how** to implement, not **what** or **why**. - -## Compaction Rule - -Before merging a PR, compact completed plans: - -1. Remove verbose implementation details -2. Point to PR and commits for specifics -3. Keep key decisions and important notes -4. Update status to COMPLETED - -Example compacted plan: - -```markdown -# Feature Name - -**Status:** COMPLETED -**PR:** [#123](https://github.com/videojs/v10/pull/123) - -## Summary - -Brief description of what was implemented. - -## Key Decisions - -- Decision 1: Rationale -- Decision 2: Rationale - -## Notes - -Any gotchas or important context for future reference. -``` - -## See Also - -- [Design Docs](/internal/design/README.md) — Decisions you own -- [RFCs](/rfc/README.md) — Proposals needing buy-in -- [CLAUDE.md](/CLAUDE.md#design-documents) — How these relate diff --git a/.claude/plans/menus.md b/.claude/plans/menus.md deleted file mode 100644 index bac68128..00000000 --- a/.claude/plans/menus.md +++ /dev/null @@ -1,116 +0,0 @@ -# Menus Implementation - -Design doc: `internal/design/ui/menus.md` -Branch: `feat/menu-ui-component` (PR #1078) - -## Overview - -Three PRs along the dependency chain: - -``` -PR 1 (Core + DOM) → PR 2 (UI flat) → PR 3 (Submenus) -``` - ---- - -## PR 1 — Core + DOM layer - -**Status:** DONE — `feat/menu-core-dom` (PR #1503) - -### Files - -**New:** -- `packages/core/src/core/ui/menu/menu-core.ts` -- `packages/core/src/core/ui/menu/menu-data-attrs.ts` -- `packages/core/src/core/ui/menu/menu-item-data-attrs.ts` -- `packages/core/src/core/ui/menu/menu-css-vars.ts` -- `packages/core/src/core/ui/menu/tests/menu-core.test.ts` -- `packages/core/src/dom/ui/menu/create-menu.ts` -- `packages/core/src/dom/ui/menu/tests/create-menu.test.ts` -- `packages/core/src/dom/ui/menu/tests/create-menu-helpers.ts` - -**Modified:** -- `packages/core/src/core/index.ts` — add menu exports -- `packages/core/src/dom/index.ts` — add menu exports -- `packages/core/src/core/ui/transition.ts` — extract `TransitionDataAttrs` -- `packages/core/src/core/ui/popover/popover-data-attrs.ts` — spread `TransitionDataAttrs` -- `packages/core/src/core/ui/tooltip/tooltip-data-attrs.ts` — spread `TransitionDataAttrs` -- `packages/core/src/core/ui/alert-dialog/alert-dialog-data-attrs.ts` — spread `TransitionDataAttrs` - -### Key decisions -- `MenuCore` follows `PopoverCore` pattern: `setProps` + `setInput(TransitionState)` + `getState()` -- `isSubmenu` prop on `MenuCore` — suppresses `popover="manual"` in `getContentAttrs` and disables positioning props for nested menus -- `MenuItemDataAttrs` is not constrained by `StateAttrMap` since items have their own state, not `MenuState` -- `data-direction` belongs in DOM layer alongside `NavigationState`, not in core constants -- `createMenu()` composes `createPopover()` internally; items stored as ordered array (registration order matches DOM order for standard React list rendering) -- `destroy()` cancels the open RAF and typeahead timer before delegating to `popover.destroy()` -- Open RAF guards against `status === 'ending'` to prevent highlight firing during a rapid open/close - ---- - -## PR 2 — UI layer: flat menu (React + HTML) - -**Status:** OPEN — `feat/menu-react-html` (PR #1504) - -### React files (`packages/react/src/ui/menu/`) -- `context.tsx`, `index.parts.ts`, `index.ts` -- `menu-root.tsx`, `menu-trigger.tsx`, `menu-content.tsx` -- `menu-item.tsx`, `menu-label.tsx`, `menu-separator.tsx`, `menu-group.tsx` -- `menu-radio-group.tsx`, `menu-radio-item.tsx`, `menu-checkbox-item.tsx`, `menu-item-indicator.tsx` - -### HTML files (`packages/html/src/ui/menu/`) -- `menu-element.ts`, `menu-item-element.ts`, `menu-label-element.ts`, `menu-separator-element.ts` -- `menu-group-element.ts`, `menu-radio-group-element.ts`, `menu-radio-item-element.ts` -- `menu-checkbox-item-element.ts`, `menu-item-indicator-element.ts` - -**Modified:** -- `packages/react/src/ui/index.ts` — add Menu export -- `packages/html/src/define/ui/menu.ts` — registration barrel -- `packages/html/src/ui/index.ts` — add menu exports - -### Scope -- Fully functional flat single-level menu with items, radio groups, checkboxes, labels, separators -- No nested Root / Back / submenu navigation — that comes in PR 4 - ---- - -## PR 3 — Submenu navigation - -**Status:** OPEN — `feat/menu-sub` (PR pending) - -### Files - -**New DOM:** -- `packages/core/src/dom/ui/menu/create-menu-view-transition.ts` -- `packages/core/src/dom/ui/menu/menu-viewport-transition.ts` - -**New React:** -- `packages/react/src/ui/menu/menu-back.tsx` -- `packages/react/src/ui/menu/menu-view.tsx` - -**New HTML:** -- `packages/html/src/ui/menu/menu-back-element.ts` -- `packages/html/src/ui/menu/menu-view-element.ts` - -**Modified:** -- `packages/core/src/dom/ui/menu/create-menu.ts` — add `push`/`pop` to `MenuApi`, `NavigationState`, wire transition -- `packages/react/src/ui/menu/menu-root.tsx` — nested Root detects parent context → submenu mode -- `packages/react/src/ui/menu/menu-content.tsx` — `data-submenu`, `data-direction`, slide transition wiring -- `packages/react/src/ui/menu/index.parts.ts` — export `Menu.Back` and `Menu.View` -- `packages/html/src/ui/menu/menu-element.ts` — nested `` + `commandfor` support -- `packages/html/src/ui/menu/menu-item-element.ts` — `commandfor` attribute handling -- `packages/core/src/dom/index.ts` — add submenu and viewport transition exports -- `packages/html/src/define/ui/menu.ts` — register `` and `` - -**Status:** OPEN — branched off `feat/menu-react-html` - -### Scope -- `NavigationState`: stack of `{ menuId, triggerId }`, direction, exitingMenuId, transitioning -- `createMenuViewTransition()`: generic menu view double-RAF lifecycle, data attribute hooks, `getAnimations()` settle -- `menu-viewport-transition.ts`: shared root/child view measurement and `--media-menu-width/height` sizing -- `Menu.View` / ``: optional root view wrapper for in-place view navigation; receives `data-menu-root-view` while root `Content` / `` acts as the shared viewport -- Traditional flyout submenus are out of scope for this PR and should not require `Menu.View` / `` when added later -- Nested `Menu.Root` detection via parent `MenuContext` → `isSubmenu: true` prop, Trigger registers as parent item -- `Menu.Back` / ``: pops stack, focus restoration to trigger -- Auto-back on `RadioItem` selection in submenu -- RTL: direction-agnostic JS, CSS handles `translateX` flip via `[dir="rtl"]` diff --git a/.claude/plans/player-api-implementation.md b/.claude/plans/player-api-implementation.md deleted file mode 100644 index 610adb5d..00000000 --- a/.claude/plans/player-api-implementation.md +++ /dev/null @@ -1,1461 +0,0 @@ -# Player API Implementation Plan - -Implementation plan for the Player API, aligned with Store v2 architecture. - -Parent issue: [#320](https://github.com/videojs/v10/issues/320) - -## Phases - -| # | Issue | Title | SP | Status | -|---|-------|-------|-----|--------| -| 1 | [#365](https://github.com/videojs/v10/issues/365) | Store v2 Foundation | 5 | ✅ | -| 2 | [#366](https://github.com/videojs/v10/issues/366) | PlayerTarget & Features | 3 | ✅ | -| 3 | [#367](https://github.com/videojs/v10/issues/367) | React Player API | 3 | ✅ | -| 4 | [#368](https://github.com/videojs/v10/issues/368) | HTML Player API | 5 | ✅ | -| 5 | [#369](https://github.com/videojs/v10/issues/369) | Cleanup & Documentation | 2 | 🔄 Partial (store/html cleanup done in #368) | - -**Total: 18 SP** - -## Overview - -Transform the current store-based architecture into a feature-based player API with: - -- **Single store** with `PlayerTarget = { media: Media, container: MediaContainer | null }` -- **Selector-based subscriptions** via `useStore(store, selector)` and `usePlayer(selector)` -- `createFeatureSelector(feature)` for type-safe feature state selection -- Pre-built selectors exported from `@videojs/core/dom` (`selectPlayback`, `selectVolume`, etc.) -- `createFeatureSelector(feature)` for type-safe feature state selection -- **Feature availability** via `FeatureAvailability` type (`'available' | 'unavailable' | 'unsupported'`) -- **Base player context** in `@videojs/react` and `@videojs/html` for UI primitives -- **`createPlayer()`** factory wraps base context with typed hooks - -## Architecture - -``` - createPlayer({ features }) - │ - creates Store - │ - ▼ - ┌───────────────────────┐ - │ PlayerContext │ - │ (base, untyped) │ - │ │ - │ store: AnyStore │ - │ media: Media | null │ - │ setMedia: ... │ - └───────────────────────┘ - │ - UI primitives use base - usePlayer(selector) - useMedia() - -PlayerTarget = { - media: Media, // extends HTMLMediaElement - container: MediaContainer | null // extends HTMLElement -} - -Store = Simplify<{ - state: State, // state snapshot for selectors - attach(target: Target): () => void, - subscribe(callback: () => void): () => void, - destroy(): void -} & State> // direct state access via intersection - -PlayerContextValue = { - store: AnyStore, // base store type - media: Media | null, - setMedia: ... -} -``` - ---- - -## Store Package Changes - -### Removed from `@videojs/store/react` - -| Export | Reason | -|--------|--------| -| `createStore` | Replaced by `createPlayer` in `@videojs/react` | -| `useStoreContext` | Replaced by `usePlayerContext` in `@videojs/react` | -| `StoreContextProvider` | Replaced by player context | -| `CreateStoreConfig` | No longer needed | -| `CreateStoreResult` | No longer needed | -| `ProviderProps` | No longer needed | - -### Kept in `@videojs/store/react` - -| Export | Purpose | -|--------|---------| -| `useStore` | Hook to subscribe to store state | -| `useSelector` (new) | Selector-based subscription with shallowEqual | - -### Removed from `@videojs/store/html` - -| Export | Reason | -|--------|--------| -| `createStore` | Replaced by `createPlayer` in `@videojs/html` | -| `createProviderMixin` | Player-specific version in `@videojs/html` | -| `createContainerMixin` | Player-specific version in `@videojs/html` | -| `createStoreMixin` | Player-specific version in `@videojs/html` | -| `CreateStoreConfig` | No longer needed | -| `CreateStoreResult` | No longer needed | -| `CreateStoreHost` | No longer needed | -| `contextKey` | No longer needed | -| `StoreConsumer` | Player-specific | -| `StoreProvider` | Player-specific | - -### Kept in `@videojs/store/html` - -| Export | Purpose | -|--------|---------| -| `SubscriptionController` | Generic subscription management | -| `StoreController` | Subscribes to store state | -| `StoreAccessor` | Resolves store from direct instance or context | -| `StoreSource` | Type: store instance or context | -| `StoreAccessorHost` | Type: host requirements | -| `SubscriptionControllerHost` | Type: host requirements | - ---- - -## Phase 1: Store Enhancements - -### PR 1: feat(store): Store type refactor - -Refactor Store type to use `Target` and `State` generics with intersection (`& State`) for direct access plus `state` property for selectors. Use `Simplify<>` for flattened IntelliSense. Simplify type helpers (remove `InferStoreFeatures`). Pass `store` to feature `attach()`. - -**Files:** - -``` -packages/store/src/core/store.ts -packages/store/src/core/types.ts (new or update) -packages/store/src/core/tests/store.test.ts -``` - -**Store type (intersection with state snapshot):** - -```ts -import { Simplify } from '@videojs/utils/types'; - -/** - * Store type with direct state access via intersection. - * - Target: type for attach() parameter (default: unknown) - * - State: state merged onto store AND available via state property (default: Record) - */ -type Store> = Simplify<{ - state: State; - attach(target: Target): () => void; - subscribe(callback: () => void): () => void; - destroy(): void; -} & State>; - -/** Loose store type for contexts and base primitives. */ -type AnyStore = Store; - -/** Infer target type from store. */ -type InferStoreTarget = S extends Store ? T : never; - -/** Infer state type from store. */ -type InferStoreState = S extends Store ? State : never; - -// Note: No InferStoreFeatures - features are not stored in the type. -// State is inferred from features at createStore() call site only. - -// Base store (uses defaults) - for features and primitives -// Store> -// Access: store.paused → unknown (intersection), store.state.paused → unknown (snapshot) - -// Typed store from createStore -// Store -// Access: store.paused → boolean (intersection), store.state.paused → boolean (snapshot) -// Selectors always use store.state for consistency -``` - -**createStore infers from features:** - -```ts -function createStore( - config: { features: Features } -): Store, UnionFeatureState>; - -// Usage -const store = createStore({ features: [playbackFeature, volumeFeature] }); -// Type: Store -store.state.paused; // Access via state snapshot - -// Features are NOT stored in Store type - only Target and State -// This simplifies type helpers and avoids complex inference chains -``` - -**Feature attach receives store:** - -```ts -// Update feature attach signature -attach({ target, signal, set, store }: { - target: Target; - signal: AbortSignal; - set: (partial: Partial) => void; - store: Store; // Base store for cross-feature access -}) { - // One-time read from state snapshot - const playback = selectPlayback(store.state); - - // Reactive subscription - const unsubscribe = store.subscribe(() => { - const playback = selectPlayback(store.state); - // ... - }); - signal.addEventListener('abort', unsubscribe); -} - ---- - -### PR 2: feat(store): shallowEqual utility - -**Files:** - -``` -packages/store/src/core/shallow-equal.ts (new) -packages/store/src/core/index.ts -packages/store/src/core/tests/shallow-equal.test.ts (new) -``` - -**Implementation:** - -```ts -// packages/store/src/core/shallow-equal.ts -export function shallowEqual(a: T, b: T): boolean { - if (Object.is(a, b)) return true; - - if (typeof a !== 'object' || a === null || typeof b !== 'object' || b === null) { - return false; - } - - const keysA = Object.keys(a); - const keysB = Object.keys(b); - - if (keysA.length !== keysB.length) return false; - - for (const key of keysA) { - if (!Object.hasOwn(b, key) || !Object.is((a as any)[key], (b as any)[key])) { - return false; - } - } - - return true; -} -``` - ---- - -### PR 3: feat(store/react): useSelector hook - -**Files:** - -``` -packages/store/src/react/hooks/use-selector.ts (new) -packages/store/src/react/hooks/index.ts -packages/store/src/react/hooks/tests/use-selector.test.tsx (new) -``` - -**Implementation:** - -```ts -// packages/store/src/react/hooks/use-selector.ts -import { useRef } from 'react'; -import { useSyncExternalStore } from 'react'; -import { shallowEqual } from '../../core/shallow-equal'; - -export function useSelector( - subscribe: (cb: () => void) => () => void, - getSnapshot: () => S, - selector: (state: S) => R, - isEqual: (a: R, b: R) => boolean = shallowEqual -): R { - const cache = useRef(undefined); - - const getSelectedSnapshot = () => { - const next = selector(getSnapshot()); - if (cache.current !== undefined && isEqual(cache.current, next)) { - return cache.current; - } - cache.current = next; - return next; - }; - - return useSyncExternalStore(subscribe, getSelectedSnapshot, getSelectedSnapshot); -} -``` - ---- - -### PR 4: feat(store/react): useStore selector overload - -**Files:** - -``` -packages/store/src/react/hooks/use-store.ts -packages/store/src/react/hooks/tests/use-store.test.tsx -``` - -**Implementation:** - -```ts -// packages/store/src/react/hooks/use-store.ts -import type { AnyStore, InferStoreState } from '../../core/types'; -import { shallowEqual } from '../../core/shallow-equal'; -import { useSelector } from './use-selector'; - -export function useStore(store: S): InferStoreState; -export function useStore( - store: S, - selector: (state: Record) => R -): R; -export function useStore( - store: S, - selector?: (state: Record) => R -): InferStoreState | R { - const sub = (cb: () => void) => store.subscribe(cb); - const snap = () => store.state; // State snapshot - - if (selector) { - return useSelector(sub, snap, selector, shallowEqual); - } - - // No selector: return full state, re-render on any change - return useSelector(sub, snap, (s) => s, () => false) as InferStoreState; -} -``` - ---- - -### PR 5: feat(store): createFeatureSelector - -**Files:** - -``` -packages/store/src/core/feature-selector.ts (new) -packages/store/src/core/index.ts -packages/store/src/core/tests/feature-selector.test.ts (new) -``` - -**Implementation:** - -```ts -// packages/store/src/core/feature-selector.ts -import type { AnyFeature, InferFeatureState, StateFactoryContext } from './feature'; -import { StoreError } from './errors'; - -const stateContext: StateFactoryContext = { - task: () => { throw new StoreError('NO_TARGET'); }, - target: () => { throw new StoreError('NO_TARGET'); }, -}; - -export function createFeatureSelector( - feature: F -): (state: Record) => InferFeatureState | undefined { - const initialState = feature.state(stateContext); - const keys = Object.keys(initialState); - - return (state) => { - if (!(keys[0] in state)) return undefined; - - const result: Record = {}; - - for (const key of keys) { - result[key] = state[key]; - } - - return result as InferFeatureState; - }; -} -``` - -**Usage:** - -```ts -const selectPlayback = createFeatureSelector(playbackFeature); - -function PlayButton() { - // usePlayer passes store.state to selector internally - const playback = usePlayer(selectPlayback); - // Type: { paused: boolean; ended: boolean; ... } | undefined - - if (!playback) return null; // Feature not configured - - playback.paused; // ✓ TypeScript knows -} -``` - ---- - -## Phase 2: Core Types - -### PR 6: feat(core/dom): PlayerTarget and FeatureAvailability types - -**Files:** - -``` -packages/core/src/dom/types.ts (new) -packages/core/src/dom/index.ts -``` - -**Implementation:** - -```ts -// packages/core/src/dom/types.ts - -/** Media element contract. */ -export interface Media extends HTMLMediaElement {} - -/** Container element contract. */ -export interface MediaContainer extends HTMLElement {} - -/** Composite target for player features. */ -export interface PlayerTarget { - media: Media; - container: MediaContainer | null; -} - -/** Feature capability availability. */ -export type FeatureAvailability = 'available' | 'unavailable' | 'unsupported'; -``` - ---- - -### PR 7: refactor(core/dom): update features to use PlayerTarget - -Update all features from `HTMLMediaElement` to `PlayerTarget`. - -**Files:** - -``` -packages/core/src/dom/store/features/playback.ts -packages/core/src/dom/store/features/volume.ts -packages/core/src/dom/store/features/time.ts -packages/core/src/dom/store/features/source.ts -packages/core/src/dom/store/features/buffer.ts -packages/core/src/dom/store/features/tests/*.test.ts -``` - -**Example change:** - -```ts -// Before -const playbackFeature = defineFeature()({ - state: ({ task }) => ({ ... }), - attach({ target, signal, set }) { - listen(target, 'play', sync, { signal }); - }, -}); - -// After -const playbackFeature = defineFeature()({ - state: ({ task }) => ({ ... }), - attach({ target, signal, set }) { - listen(target.media, 'play', sync, { signal }); - }, -}); -``` - -**Volume feature with availability:** - -```ts -// packages/core/src/dom/store/features/volume.ts -import type { FeatureAvailability, PlayerTarget } from '../../types'; - -/** Check if volume can be programmatically set (fails on iOS Safari). */ -function canSetVolume(media: HTMLMediaElement): FeatureAvailability { - try { - const original = media.volume; - media.volume = 0.5; - const canSet = media.volume === 0.5; - media.volume = original; - return canSet ? 'available' : 'unsupported'; - } catch { - return 'unsupported'; - } -} - -const volumeFeature = defineFeature()(({ task }) => ({ - volume: 1, - muted: false, - volumeAvailability: 'unsupported' as FeatureAvailability, // Safe default - setVolume: task('setVolume', (volume: number, { target }) => { - target.media.volume = volume; - }), - setMuted: task('setMuted', (muted: boolean, { target }) => { - target.media.muted = muted; - }), -}), { - attach({ target, signal, set }) { - const { media } = target; - - // Check platform capability - set({ volumeAvailability: canSetVolume(media) }); - - const sync = () => set({ volume: media.volume, muted: media.muted }); - listen(media, 'volumechange', sync, { signal }); - sync(); - }, -}); -``` - ---- - -### PR 8: feat(core/dom): feature bundles - -**Files:** - -``` -packages/core/src/dom/store/features/bundles.ts (new) -packages/core/src/dom/store/features/index.ts -``` - -**Implementation:** - -```ts -// packages/core/src/dom/store/features/bundles.ts -import { playbackFeature } from './playback'; -import { volumeFeature } from './volume'; -import { timeFeature } from './time'; -import { sourceFeature } from './source'; -import { bufferFeature } from './buffer'; - -/** Base video player features. */ -export const video = [ - playbackFeature, - volumeFeature, - timeFeature, - sourceFeature, - bufferFeature, -] as const; - -/** Base audio player features. */ -export const audio = [ - playbackFeature, - volumeFeature, - timeFeature, - sourceFeature, - bufferFeature, -] as const; -``` - ---- - -### PR 9: feat(core/dom): feature selectors - -Pre-built selectors for standard features, exported from `@videojs/core/dom`. - -**Files:** - -``` -packages/core/src/dom/store/selectors.ts (new) -packages/core/src/dom/index.ts -``` - -**Implementation:** - -```ts -// packages/core/src/dom/store/selectors.ts -import { createFeatureSelector } from '@videojs/store'; -import { playbackFeature } from './features/playback'; -import { volumeFeature } from './features/volume'; -import { timeFeature } from './features/time'; -import { sourceFeature } from './features/source'; -import { bufferFeature } from './features/buffer'; - -export const selectPlayback = createFeatureSelector(playbackFeature); -export const selectVolume = createFeatureSelector(volumeFeature); -export const selectTime = createFeatureSelector(timeFeature); -export const selectSource = createFeatureSelector(sourceFeature); -export const selectBuffer = createFeatureSelector(bufferFeature); -``` - -**Usage:** - -```tsx -import { selectPlayback, selectVolume } from '@videojs/core/dom'; - -function PlayButton() { - const playback = usePlayer(selectPlayback); - if (!playback) return null; - - return ( - - ); -} - -function VolumeSlider() { - const volume = usePlayer(selectVolume); - if (!volume) return null; - - // Hide on platforms that don't support volume control - if (volume.volumeAvailability === 'unsupported') return null; - - return ( - volume.setVolume(Number(e.target.value))} - disabled={volume.volumeAvailability !== 'available'} - /> - ); -} -``` - ---- - -## Phase 3: React Player API - -### PR 10: feat(react): base player context - -Base context for UI primitives — untyped, shared across all createPlayer instances. - -> **Resolved:** Selectors define their own loose input type `(state: Record) => R`. The `as any` cast inside `useStore()` is localized — external API is fully typed. Selector returns `FeatureState | undefined`, handling "feature not configured" case. - -**Files:** - -``` -packages/react/src/player/context.tsx (new) -packages/react/src/index.ts -``` - -**Implementation:** - -```tsx -// packages/react/src/player/context.tsx -import type { AnyStore } from '@videojs/store'; -import type { Media, MediaContainer } from '@videojs/core/dom'; -import type { Dispatch, ReactNode, RefObject, SetStateAction } from 'react'; -import { createContext, useContext, useEffect, useRef } from 'react'; -import { useStore } from '@videojs/store/react'; - -/** Player context value. Store is AnyStore for base primitives. */ -export interface PlayerContextValue { - store: AnyStore; - media: Media | null; - setMedia: Dispatch>; -} - -/** Base context for player primitives. */ -const PlayerContext = createContext(null); - -/** Internal provider for player context. */ -export function PlayerContextProvider({ - value, - children, -}: { - value: PlayerContextValue; - children: ReactNode; -}): ReactNode { - return {children}; -} - -/** Access player context (internal, for primitives). */ -export function usePlayerContext(): PlayerContextValue { - const ctx = useContext(PlayerContext); - if (!ctx) throw new Error('NO_STORE'); - return ctx; -} - -/** Container component - attaches media to store when registered. */ -export function Container({ - children, - as: Element = 'div', - className, - ref: refProp, -}: ContainerProps): ReactNode { - const { store, media } = usePlayerContext(); - const internalRef = useRef(null); - const containerRef = refProp ?? internalRef; - - useEffect(() => { - if (media) { - return store.attach({ media, container: containerRef.current }); - } - }, [media, store, containerRef]); - - return ( - - {children} - - ); -} - -export interface ContainerProps { - children: ReactNode; - as?: keyof JSX.IntrinsicElements; - className?: string; - ref?: RefObject; -} - -/** Access player state. Optionally pass selector for derived state. */ -export function usePlayer(): Record; -export function usePlayer(selector: (state: Record) => R): R; -export function usePlayer(selector?: (state: Record) => R): Record | R { - const { store } = usePlayerContext(); - // useStore passes store.state to selector internally - return useStore(store, selector as any); -} - -/** Access current media element (may be null if not registered). */ -export function useMedia(): Media | null { - const { media } = usePlayerContext(); - return media; -} - -/** Access the media attach setter for connecting a media element to the player. */ -export function useMediaAttach(): Dispatch> | undefined { - const ctx = useContext(PlayerContext); - return ctx?.setMedia; -} -``` - ---- - -### PR 11: feat(react): createPlayer factory and update Video - -Factory that creates typed provider and hooks. Update existing Video component to use new context. - -**Files:** - -``` -packages/react/src/player/create-player.tsx (new) -packages/react/src/media/video.tsx (update - use useMediaAttach) -packages/react/src/index.ts -packages/react/src/player/tests/create-player.test.tsx (new) -``` - -**Implementation:** - -```tsx -// packages/react/src/player/create-player.tsx -import type { AnyFeature, Store, UnionFeatureState } from '@videojs/store'; -import { createStore } from '@videojs/store'; -import { useStore } from '@videojs/store/react'; -import type { Media, MediaContainer, PlayerTarget } from '@videojs/core/dom'; -import type { FC, ReactNode, RefObject } from 'react'; -import { useContext, useEffect, useRef, useState } from 'react'; -import { Container, PlayerContextProvider, usePlayerContext } from './context'; - -export interface CreatePlayerConfig { - features: Features; - displayName?: string; -} - -export interface CreatePlayerResult { - Provider: FC; - Container: typeof Container; // Re-exported from context - usePlayer: UsePlayerHook; -} - -export interface ProviderProps { - children: ReactNode; -} - -type UsePlayerHook = { - (): UnionFeatureState; - (selector: (state: Record) => R): R; -}; - -export function createPlayer[]>( - config: CreatePlayerConfig -): CreatePlayerResult { - type State = UnionFeatureState; - - function Provider({ children }: ProviderProps): ReactNode { - const [store] = useState(() => createStore({ features: config.features })); - const [media, setMedia] = useState(null); - - useEffect(() => () => store.destroy(), [store]); - - return ( - - {children} - - ); - } - - if (config.displayName) { - Provider.displayName = `${config.displayName}.Provider`; - } - - function usePlayer(): State; - function usePlayer(selector: (state: Record) => R): R; - function usePlayer(selector?: (state: Record) => R): State | R { - const { store } = usePlayerContext(); - // useStore passes store.state to selector internally - return useStore(store, selector as any); - } - - return { - Provider, - Container, // Re-exported from context module - usePlayer: usePlayer as UsePlayerHook, - }; -} -``` - -**Update existing Video component to use player context:** - -```tsx -// packages/react/src/media/video.tsx -'use client'; - -import type { Ref, VideoHTMLAttributes } from 'react'; -import { useCallback } from 'react'; -import { useComposedRefs } from '../utils/use-composed-refs'; -import { useMediaAttach } from '../player/context'; - -export interface VideoProps extends VideoHTMLAttributes { - ref?: Ref | React.RefObject; -} - -export function Video({ children, ref: refProp, ...props }: VideoProps): React.JSX.Element { - const setMedia = useMediaAttach(); - - const attachRef = useCallback( - (el: HTMLVideoElement): (() => void) | void => { - if (!el || !setMedia) return; - setMedia(el); - return () => setMedia(null); - }, - [setMedia] - ); - - const ref = useComposedRefs(refProp, attachRef); - - return ( - - ); -} - -export namespace Video { - export type Props = VideoProps; -} -``` - ---- - -## Phase 4: HTML Player API - -### PR 12: feat(html): directory scaffold and MediaElement - -Set up package structure with subpath exports and placeholder files. - -**Directory structure:** - -``` -packages/html/src/ -├── index.ts -├── ui/ -│ ├── media-element.ts -│ └── video-skin.ts (placeholder) -├── skin/ -│ └── modern.ts (placeholder) -├── feature/ -│ └── video.ts (placeholder) -└── player/ - ├── context.ts - ├── player-controller.ts - └── video.ts (placeholder) -``` - -**Files:** - -``` -packages/html/src/ui/media-element.ts (new) -packages/html/src/ui/video-skin.ts (new, placeholder) -packages/html/src/skin/modern.ts (new, placeholder) -packages/html/src/feature/video.ts (new, placeholder) -packages/html/src/player/video.ts (new, placeholder) -packages/html/package.json (update exports, add @lit/context dependency) -packages/html/tsdown.config.ts (new) -``` - -**MediaElement:** - -```ts -// packages/html/src/ui/media-element.ts -import { ReactiveElement } from '@lit/reactive-element'; - -/** Base class for media UI primitives. */ -export class MediaElement extends ReactiveElement { - // Base class marker for media UI primitives -} -``` - -**Package exports and dependencies:** - -```json -{ - "dependencies": { - "@lit/context": "^1.1.0", - "@lit/reactive-element": "^2.1.2", - "@videojs/core": "workspace:*", - "@videojs/store": "workspace:*", - "@videojs/utils": "workspace:*" - }, - "exports": { - ".": { "types": "./dist/index.d.ts", "default": "./dist/index.js" }, - "./ui/*": { "types": "./dist/ui/*.d.ts", "default": "./dist/ui/*.js" }, - "./skin/*": { "types": "./dist/skin/*.d.ts", "default": "./dist/skin/*.js" }, - "./feature/*": { "types": "./dist/feature/*.d.ts", "default": "./dist/feature/*.js" }, - "./player/*": { "types": "./dist/player/*.d.ts", "default": "./dist/player/*.js" } - } -} -``` - -**tsdown.config.ts:** - -```ts -import { defineConfig } from 'tsdown'; -import { glob } from 'glob'; - -export default defineConfig({ - entry: glob.sync('src/**/*.ts', { ignore: ['**/*.test.ts', '**/*.d.ts'] }), - format: 'esm', - dts: true, - clean: true, -}); -``` - ---- - -### PR 13: feat(html): base player context and controller - -Base context and controller for UI primitives. - -**Files:** - -``` -packages/html/src/player/context.ts (new) -packages/html/src/player/player-controller.ts (new) -``` - -**Implementation:** - -```ts -// packages/html/src/player/context.ts -import { createContext, ContextConsumer } from '@lit/context'; -import type { ReactiveElement } from '@lit/reactive-element'; -import type { AnyStore } from '@videojs/store'; -import type { Constructor } from '@videojs/utils/types'; -import type { Media, MediaContainer, PlayerTarget } from '@videojs/core/dom'; - -/** Player context value. Store is AnyStore for base primitives. */ -export interface PlayerContextValue { - store: AnyStore; - media: Media | null; -} - -/** Base context for player primitives. */ -export const playerContext = createContext(Symbol('@videojs/player')); - -/** Mixin that observes for media elements and attaches to store. */ -export function ContainerMixin>(BaseClass: Base) { - return class extends BaseClass { - #media: Media | null = null; - #detach: (() => void) | null = null; - #observer: MutationObserver | null = null; - #consumer = new ContextConsumer(this, { - context: playerContext, - subscribe: true, - }); - - override connectedCallback() { - super.connectedCallback(); - this.#observeMedia(); - } - - override disconnectedCallback() { - super.disconnectedCallback(); - this.#observer?.disconnect(); - this.#detach?.(); - } - - #observeMedia() { - this.#observer = new MutationObserver(() => this.#checkForMedia()); - this.#observer.observe(this, { childList: true, subtree: true }); - this.#checkForMedia(); - } - - #checkForMedia() { - const media = this.querySelector('video, audio') as Media | null; - if (media !== this.#media) { - this.#detach?.(); - this.#media = media; - this.#attachMedia(); - } - } - - #attachMedia() { - const ctx = this.#consumer.value; - if (ctx && this.#media) { - this.#detach = ctx.store.attach({ - media: this.#media, - container: this as unknown as MediaContainer, - }); - } - } - }; -} -``` - -```ts -// packages/html/src/player/player-controller.ts -import type { ReactiveController, ReactiveControllerHost } from '@lit/reactive-element'; -import type { AnyStore, InferStoreState } from '@videojs/store'; -import { shallowEqual } from '@videojs/store'; -import { StoreAccessor, type StoreSource } from '@videojs/store/html'; - -export class PlayerController> - implements ReactiveController -{ - #host: ReactiveControllerHost; - #accessor: StoreAccessor; - #selector?: (state: Record) => R; - #cached?: R; - #unsubscribe?: () => void; - - // Overload: with selector - subscribes, R from selector - constructor( - host: ReactiveControllerHost, - source: StoreSource, - selector: (state: Record) => R - ); - // Overload: without selector - no subscription, R is full state - constructor( - host: ReactiveControllerHost, - source: StoreSource - ); - constructor( - host: ReactiveControllerHost, - source: StoreSource, - selector?: (state: Record) => R - ) { - this.#host = host; - this.#accessor = new StoreAccessor(host, source); - this.#selector = selector; - host.addController(this); - } - - get store(): S { - return this.#accessor.store; - } - - get value(): R { - if (this.#selector) { - // With selector: return cached selected value - this.#cached ??= this.#selector(this.store.state); - return this.#cached; - } - // Without selector: return current full state snapshot - return this.store.state as R; - } - - hostConnected(): void { - // Only subscribe if selector provided - if (this.#selector) { - const selector = this.#selector; - this.#cached = selector(this.store.state); - - this.#unsubscribe = this.store.subscribe(() => { - const next = selector(this.store.state); - if (!shallowEqual(this.#cached, next)) { - this.#cached = next; - this.#host.requestUpdate(); - } - }); - } - } - - hostDisconnected(): void { - this.#unsubscribe?.(); - this.#unsubscribe = undefined; - } -} -``` - -**Usage:** - -```ts -import { selectPlayback } from '@videojs/core/dom'; -import { playerContext } from './context'; - -class MediaPlayButton extends MediaElement { - // With selector: subscribes, .value is PlaybackState | undefined - #ctrl = new PlayerController(this, playerContext, selectPlayback); - - render() { - const playback = this.#ctrl.value; - if (!playback) return nothing; - - return html``; - } -} - -class SomeOtherElement extends LitElement { - // Without selector: no subscription, .value is full state - #ctrl = new PlayerController(this, typedStore); - - someMethod() { - // Get current state (snapshot, not reactive) - const state = this.#ctrl.value; - } -} -``` - ---- - -### PR 14: feat(html): createPlayer factory - -Factory that creates typed mixins and controller. - -**Files:** - -``` -packages/html/src/player/create-player.ts (new) -packages/html/src/player/tests/create-player.test.ts (new) -``` - -**Implementation:** - -```ts -// packages/html/src/player/create-player.ts -import type { AnyFeature, Store, UnionFeatureState } from '@videojs/store'; -import { createStore } from '@videojs/store'; -import { ContextProvider } from '@lit/context'; -import type { ReactiveElement } from '@lit/reactive-element'; -import type { Constructor } from '@videojs/utils/types'; -import type { Media, MediaContainer, PlayerTarget } from '@videojs/core/dom'; -import { playerContext, ContainerMixin, type PlayerContextValue } from './context'; -import { PlayerController } from './player-controller'; - -export interface CreatePlayerConfig { - features: Features; -} - -export interface CreatePlayerResult { - context: typeof playerContext; - create: () => Store>; - PlayerController: typeof PlayerController; - ProviderMixin: >(base: Base) => Base; - ContainerMixin: typeof ContainerMixin; -} - -export function createPlayer[]>( - config: CreatePlayerConfig -): CreatePlayerResult { - type StoreType = Store>; - - const create = () => createStore({ features: config.features }) as StoreType; - - function ProviderMixin>(BaseClass: Base) { - return class extends BaseClass { - #store: StoreType = create(); - #media: Media | null = null; - #provider = new ContextProvider(this, { - context: playerContext, - initialValue: { store: this.#store as any, media: null }, - }); - - get store(): StoreType { - return this.#store; - } - - get media(): Media | null { - return this.#media; - } - - set media(value: Media | null) { - this.#media = value; - this.#provider.setValue({ store: this.#store as any, media: value }); - } - - override disconnectedCallback() { - super.disconnectedCallback(); - this.#store.destroy(); - } - }; - } - - return { - context: playerContext, - create, - PlayerController, - ProviderMixin, - ContainerMixin, - }; -} -``` - ---- - -## Phase 5: Cleanup - -### PR 15: refactor(store): remove createStore from lit - -Remove deprecated exports, update package indexes. - -**Note:** React cleanup (`createStore`, `useStoreContext`, etc.) was done in Phase 3 (PR 11). - -**Files:** - -``` -packages/store/src/lit/create-store.ts (delete) -packages/store/src/lit/mixins/provider-mixin.ts (delete) -packages/store/src/lit/mixins/container-mixin.ts (delete) -packages/store/src/lit/mixins/store-mixin.ts (delete) -packages/store/src/lit/mixins/index.ts (delete) -packages/store/src/lit/types.ts (delete) -packages/store/src/lit/index.ts (update) -packages/store/src/lit/tests/create-store.test.ts (delete) -``` - -**Updated exports:** - -```ts -// packages/store/src/react/index.ts -export { useStore } from './hooks/use-store'; -export { useSelector } from './hooks/use-selector'; -``` - -```ts -// packages/store/src/lit/index.ts -export { StoreController, SubscriptionController } from './controllers'; -export type { StoreSource, StoreAccessorHost } from './store-accessor'; -export { StoreAccessor } from './store-accessor'; -``` - ---- - -## Phase 6: Documentation - -### PR 16: docs(rfc): update for revised architecture - -Update RFC to reflect: - -- Single store with `PlayerTarget` -- Base player context pattern -- Selector-based subscriptions with pre-built selectors (`selectPlayback`, etc.) -- `createFeatureSelector` for type-safe feature access -- `FeatureAvailability` type for platform capability detection -- Removal of `createStore` from platform bindings -- Remove `FeatureKey` pattern (not needed yet) - -**Files:** - -``` -rfc/player-api/*.md -``` - ---- - -## Design Decisions - -### Why Store Uses Intersection AND State Property - -The `Store` type merges state via intersection AND exposes `state` property: - -```ts -type Store = Simplify<{ - state: State; // State snapshot for selectors - attach(target: Target): () => void; - subscribe(callback: () => void): () => void; - destroy(): void; -} & State>; // Direct access via intersection -``` - -**Why both intersection AND state property:** - -- **Direct access** — `store.paused` works for quick reads (intersection) -- **Selectors use state** — `selector(store.state)` for consistent snapshot reference -- **Type inference** — `Simplify<>` flattens the type for better IntelliSense -- **Framework-agnostic** — All frameworks pass `store.state` to selectors - -**Selector pattern:** - -```ts -// Selectors take state (Record), not store -const selectPlayback = (state: Record) => ({ ... }); - -// Direct access for simple reads -if (store.paused) { ... } - -// Selectors always use store.state -usePlayer(selectPlayback); // React hook passes store.state -controller.value; // Lit controller reads store.state -``` - -### Why Base Player Context - -UI primitives (PlayButton, VolumeSlider, etc.) need store access without knowing which features are configured. The base context provides: - -- `usePlayer()` — returns current state snapshot (untyped `Record`) -- `usePlayer(selector)` — returns selected state via selector -- `useMedia()` — returns current media element -- `useMediaAttach()` — for Video/Audio primitives to attach - -`createPlayer()` wraps this base with typed hooks for app code. - -Selectors define their own loose input type `(state: Record) => R`, so the `as any` cast inside `useStore()` is localized while the external API remains fully typed. - -### Why Selector-Based Over Feature-Scoped Hooks - -- Simpler mental model — one hook, one pattern -- No conditional hook calls -- `createFeatureSelector(feature)` provides type-safe feature selection -- Pre-built selectors exported from `@videojs/core/dom` (`selectPlayback`, `selectVolume`, etc.) -- Returns `T | undefined` to handle "feature not configured" case -- Matches industry patterns (Zustand, Redux Toolkit) - -### Why Feature Availability - -Platform capabilities vary (e.g., iOS Safari can't programmatically set volume). Instead of runtime errors or silent failures: - -- Features expose `*Availability` state (`'available' | 'unavailable' | 'unsupported'`) -- Safe default is `'unsupported'` — updated in `attach()` after capability check -- State doesn't contain DOM — helper functions like `canSetVolume()` return availability -- UI primitives can hide/disable based on availability - -### Why Selector Logic in Framework Bindings (Not Base) - -Selector-based change detection (shallowEqual) lives in React hooks and Lit controllers, not in `store.subscribe()`: - -- **Framework-agnostic base** — `store.subscribe(callback)` stays simple, fires on any state change -- **Svelte/Vue have their own solutions** — `derived()` and `computed()` handle this idiomatically -- **React/Lit need it** — no built-in derived state, so hooks/controllers handle shallowEqual -- **Direct state access available** — `selector(store.state)` for one-time reads without subscription - -**Pattern:** - -```ts -// Base store - simple callback, fires on any change -store.subscribe(() => { - const playback = selectPlayback(store.state); - // ... -}); - -// React/Lit - shallowEqual comparison on selector result -useStore(store, selectPlayback); // Only re-renders when playback state changes -``` - -### PlayerController Design (Lit) - -`PlayerController` exposes both `.store` and `.value`: - -- **`.store`** — direct store access for `attach()`, subscriptions, etc. -- **`.value`** — selected state (with selector) or full state snapshot (without) -- **With selector** — subscribes, calls `selector(store.state)` with shallowEqual comparison -- **Without selector** — no subscription, `.value` returns `store.state` snapshot -- **Overloads** — TypeScript knows return type based on whether selector provided - -```ts -// With selector - subscribes, .value is selected state -#playback = new PlayerController(this, playerContext, selectPlayback); -this.#playback.value; // PlaybackState | undefined - -// Without selector - no subscription, .value is full state snapshot -#ctrl = new PlayerController(this, playerContext); -this.#ctrl.value; // Current state snapshot (not reactive) -``` - -### Why Remove createStore from Platform Bindings - -- Player API is the primary use case -- `createStore` in store/react was a thin wrapper -- Base player context provides what primitives need -- Reduces API surface and confusion - ---- - -## Future Work - -Items identified during planning but deferred from initial implementation. - -### Side-Effect Registration System - -The `/ui/*`, `/video/*`, `/audio/*`, `/background/*`, `/feature/*` exports provide scaffold for future registration patterns: - -```ts -// User imports trigger registration -import '@videojs/html/ui/play-button'; -import '@videojs/html/video/skin'; -import '@videojs/html/feature/quality-selection'; -``` - -**Deferred:** Requires design decisions on: -- Global registry vs player-scoped registration -- How features/UI/skins discover each other -- Lazy loading and code splitting strategy - -### Feature Keys (`FeatureKey`) - -Typed symbols for feature identity: - -```ts -const playbackKey: FeatureKey = Symbol.for('@videojs/playback'); -store.get(playbackKey); // Typed access without importing feature -``` - -**Deferred:** -- `createFeatureSelector` provides equivalent type-safe access -- Bundle size benefit is marginal for most use cases -- Adds API surface without clear advantage yet - -### Additional Feature Availability Checks - -Expand `FeatureAvailability` pattern to other features: - -```ts -// Fullscreen availability (iframe restrictions, browser support) -fullscreen.fullscreenAvailability - -// Picture-in-Picture availability -pip.pipAvailability - -// AirPlay/Cast availability -cast.castAvailability -``` - -**Deferred:** Add as features are implemented. - -### Streaming/Ads/Live Feature Bundles - -Additional feature bundles beyond base `video`/`audio`: - -```ts -features.streaming = [qualitySelection, audioTracks, textTracks]; -features.ads = [adMarkers, adSkip, adCountdown]; -features.live = [liveIndicator, seekToLive, dvr]; -``` - -**Deferred:** Core features need stabilization first. - -### Server-Side Rendering Support - -SSR-safe patterns for React: -- Hydration-safe store initialization -- Server state serialization -- `useId()` for deterministic IDs - -**Deferred:** Requires real-world SSR testing scenarios. - -### DevTools Integration - -Browser extension for debugging: -- Store state inspector -- Feature configuration viewer -- Request/task timeline -- Performance profiling - -**Deferred:** Nice-to-have, not blocking core functionality. diff --git a/.claude/plans/slider.md b/.claude/plans/slider.md deleted file mode 100644 index 29df41d5..00000000 --- a/.claude/plans/slider.md +++ /dev/null @@ -1,1747 +0,0 @@ -# Slider Component Implementation - -**Status:** PR 3 COMPLETE — PR 4 (HTML) ready to start -**Design Doc:** `internal/design/ui/slider/` -**Issues:** #275 (time slider), #267 (volume slider), #269 (seek slider) - -## Branch Strategy - -``` -main - ├─ feat/store-snapshot (PR 0 — merges to main first) - └─ feat/slider-core (PR 1, rebased after PR 0 merges) - └─ feat/slider-dom (PR 2) - ├─ feat/slider-react (PR 3) - │ └─ feat/slider-preview-react (PR 5) - └─ feat/slider-html (PR 4) - └─ feat/slider-preview-html (PR 6) -``` - -PR 0 merges to `main` independently — it's a general-purpose store primitive, not slider-specific. -PRs 3 and 4 are siblings off `feat/slider-dom`. React and HTML have no dependency on each other. -Preview PRs branch off their respective UI PRs because they need the slider context definitions. - ---- - -## PR 0: Store Snapshot Primitives - -**Branch:** `feat/store-snapshot` -**Base:** `main` -**Package:** `@videojs/store` - -Adds `useSnapshot` — a React hook for subscribing to `State` containers from `createState()`. -This bridges the store's raw reactive state to React rendering. Required because `useStore` -only accepts `AnyStore` (with `.state`, `.attach()`, `.destroy()`), not `State` (with -`.current` and `.subscribe()`). HTML elements don't need a controller equivalent — they -subscribe directly via `state.subscribe(() => this.requestUpdate(), { signal })`. - -### 0.1 `useSnapshot` — React Hook - -**File:** `packages/store/src/react/hooks/use-snapshot.ts` - -Subscribes to a `State` container. Without selector, returns the full snapshot. With -selector, returns derived value with `shallowEqual` comparison to avoid unnecessary re-renders. - -```ts -import type { State } from '../../core/state'; -import { type Comparator, type Selector, useSelector } from './use-selector'; - -/** Subscribe to a State container's current value. */ -export function useSnapshot(state: State): T; - -export function useSnapshot( - state: State, - selector: Selector, - isEqual?: Comparator -): R; - -export function useSnapshot( - state: State, - selector?: Selector, - isEqual?: Comparator -): T | R { - return useSelector( - (cb) => state.subscribe(cb), - () => state.current, - selector ?? ((s: T) => s as unknown as R), - isEqual - ); -} -``` - -**Overload semantics:** -- `useSnapshot(state)` — returns `T`, re-renders on any shallow change to the state object. -- `useSnapshot(state, selector)` — returns `R`, re-renders only when `selector(state.current)` changes (per `shallowEqual`). -- Optional third arg `isEqual` for custom comparator. - -### 0.2 Barrel Exports - -**`packages/store/src/react/hooks/index.ts`** — add: -```ts -export { useSnapshot } from './use-snapshot'; -``` - -**`packages/store/src/react/index.ts`** — add (this barrel imports directly from individual -hook files, not from `hooks/index.ts`): -```ts -export { useSnapshot } from './hooks/use-snapshot'; -``` - -### 0.3 Tests - -**File:** `packages/store/src/react/hooks/tests/use-snapshot.test.tsx` - -- Without selector: returns full state, re-renders on patch -- With selector: returns selected value, only re-renders when selected value changes -- Custom comparator -- Does not re-render when patched values are identical (Object.is) -- Works with microtask batching (multiple patches → one render) - -### 0.4 Verify - -```bash -pnpm -F @videojs/store test src/react/hooks/tests/use-snapshot -pnpm -F @videojs/store build -pnpm typecheck -pnpm lint:fix:file -``` - ---- - -## PR 1: Core Layer - -**Branch:** `feat/slider-core` -**Base:** `main` (after PR 0 merges) -**Packages:** `@videojs/utils`, `@videojs/core` - -### 1.1 Fix `formatTimeAsPhrase(0)` in `@videojs/utils` - -**File:** `packages/utils/src/time/format.ts` - -The function returns `""` when `seconds = 0` because all three units (h, m, s) fail the `value > 0` check, producing an empty `parts` array. When `seconds = 0`, `aria-valuetext` for a time slider at video start would be `"of 10 minutes"` instead of `"0 seconds of 10 minutes"`. - -**Fix:** When `positiveSeconds === 0`, return `"0 seconds"` directly. Or modify the seconds entry in the map to pass through when all parts would be empty. - -**Test:** Add `it('formats zero seconds', () => expect(formatTimeAsPhrase(0)).toBe('0 seconds'))` to `packages/utils/src/time/tests/format.test.ts`. - -### 1.2 `rafThrottle` — rAF-Based Throttle Utility - -**File:** `packages/utils/src/dom/raf-throttle.ts` - -Throttles a function to fire at most once per animation frame. Used by the slider to -throttle seek events during scrubbing — naturally adapts to device refresh rate and avoids -intermediate seeks that would never be visually rendered. - -```ts -export interface RafThrottled { - (...args: Args): void; - cancel(): void; -} - -export function rafThrottle( - fn: (...args: Args) => void -): RafThrottled { - let rafId: number | null = null; - let latestArgs: Args; - - const throttled = (...args: Args): void => { - latestArgs = args; - if (rafId !== null) return; - rafId = requestAnimationFrame(() => { - rafId = null; - fn(...latestArgs); - }); - }; - - throttled.cancel = (): void => { - if (rafId !== null) { - cancelAnimationFrame(rafId); - rafId = null; - } - }; - - return throttled; -} -``` - -**Barrel:** Add to `packages/utils/src/dom/index.ts`: -```ts -export { rafThrottle, type RafThrottled } from './raf-throttle'; -``` - -**Test:** `packages/utils/src/dom/tests/raf-throttle.test.ts` -- Calls function on next animation frame -- Collapses multiple calls into one (latest args win) -- `.cancel()` prevents pending call -- New calls work after cancel - -### 1.3 `isRTL` — RTL Direction Detection - -**File:** `packages/utils/src/dom/direction.ts` - -Detects right-to-left text direction for an element. Checks `element.closest('[dir]')` first -for fast attribute lookup, then falls back to `getComputedStyle` which respects inherited -direction from stylesheets. - -```ts -export function isRTL(element: Element): boolean { - const dir = element.closest('[dir]')?.getAttribute('dir'); - if (dir) return dir.toLowerCase() === 'rtl'; - return getComputedStyle(element).direction === 'rtl'; -} -``` - -**Barrel:** Add to `packages/utils/src/dom/index.ts`: -```ts -export { isRTL } from './direction'; -``` - -**Test:** `packages/utils/src/dom/tests/direction.test.ts` -- Returns `false` for default LTR element -- Returns `true` when ancestor has `dir="rtl"` -- Returns `true` when element has `dir="rtl"` - -### 1.4 `SliderCore` — Generic Slider State - -**File:** `packages/core/src/core/ui/slider/slider-core.ts` - -Stateless class that computes derived slider state from interaction + value inputs. Follows existing core class patterns (`PlayButtonCore`, `TimeCore`). - -```ts -export interface SliderProps { - min?: number | undefined; - max?: number | undefined; - step?: number | undefined; - largeStep?: number | undefined; - orientation?: 'horizontal' | 'vertical' | undefined; - disabled?: boolean | undefined; - thumbAlignment?: 'center' | 'edge' | undefined; -} - -export interface SliderInteraction { - pointerPercent: number; - dragPercent: number; - dragging: boolean; - pointing: boolean; - focused: boolean; -} - -export interface SliderState { - value: number; - fillPercent: number; - pointerPercent: number; - dragging: boolean; - pointing: boolean; - interactive: boolean; // dragging || pointing || focused - orientation: 'horizontal' | 'vertical'; - disabled: boolean; - thumbAlignment: 'center' | 'edge'; -} - -``` - -**Class shape:** - -```ts -export class SliderCore { - static readonly defaultProps: NonNullableObject; - // defaultProps: min=0, max=100, step=1, largeStep=10, - // orientation='horizontal', disabled=false, thumbAlignment='center' - - #props = { ...SliderCore.defaultProps }; - - constructor(props?: SliderProps); - setProps(props: SliderProps): void; - - getState(interaction: SliderInteraction, value: number): SliderState; - // Computes fillPercent = percentFromValue(value), pointerPercent from interaction, - // interactive = dragging || pointing || focused. All percentages 0-100. - - getAttrs(state: SliderState); - // Returns inlined ARIA attrs object (no explicit return type — TS infers). - // Returns { role, tabIndex, autocomplete, aria-valuemin, aria-valuemax, - // aria-valuenow, aria-orientation, aria-disabled }. - // Generic — no label or valuetext. - - valueFromPercent(percent: number): number; - // Converts 0-100 percent to a value in [min, max], snapped to step. - // Uses roundValueToStep. - - percentFromValue(value: number): number; - // Converts a value to 0-100 percent relative to [min, max]. - - adjustPercentForAlignment(rawPercent: number, thumbSize: number, trackSize: number): number; - // For 'edge' thumb alignment: adjusts percent so thumb stays within track bounds. - // center mode: identity (return rawPercent). -} - -export namespace SliderCore { - export type Props = SliderProps; - export type State = SliderState; - export type Interaction = SliderInteraction; -} -``` - -**Utility functions** in `@videojs/utils/number` (new subpath): - -```ts -export function clamp(value: number, min: number, max: number): number; -// Clamp a value between min and max (inclusive). - -export function roundToStep(value: number, step: number, min: number): number; -// Snap a value to the nearest step, offset from min. -// Derives decimal precision from step's string representation to avoid floating-point drift. -// Integer steps skip toFixed entirely. -``` - -Requires adding `./number` export to `packages/utils/package.json` and `number` entry to `packages/utils/tsdown.config.ts`. - -### 1.5 `TimeSliderCore` — Time-Domain Slider - -**File:** `packages/core/src/core/ui/slider/time-slider-core.ts` - -Extends `SliderCore`. Accepts `MediaTimeState & MediaBufferState` (canonical types from `@videojs/core`) + `SliderInteraction`, returns time-specific state. - -```ts -export interface TimeSliderState extends SliderState, Pick { - bufferPercent: number; -} -``` - -Uses `Pick<>` to select specific fields from canonical media state. No custom `TimeMediaState` wrapper — accepts the full `MediaTimeState & MediaBufferState` and computes `bufferedEnd` internally from `media.buffered` ranges. - -**Class shape:** - -```ts -export interface TimeSliderProps extends SliderProps { - label?: string | undefined; -} - -export class TimeSliderCore extends SliderCore { - static override readonly defaultProps: NonNullableObject; - // Inherits slider defaults. label='Seek'. - - getTimeState(media: MediaTimeState & MediaBufferState, interaction: SliderInteraction): TimeSliderState; - // - min=0, max=duration (overrides generic min/max on each call). - // - Value swap: dragging ? valueFromPercent(dragPercent) : currentTime. - // - Computes bufferedEnd from media.buffered ranges internally. - // - bufferPercent = (bufferedEnd / duration) * 100 || 0. - // - Delegates to super.getState() for base slider state. - - override getAttrs(state: TimeSliderState); - // Returns inlined object (no explicit return type). - // Adds aria-label (default 'Seek') and aria-valuetext - // e.g., "5 minutes, 30 seconds of 10 minutes" - // Spreads super.getAttrs(state) for base ARIA. -} - -export namespace TimeSliderCore { - export type Props = TimeSliderProps; - export type State = TimeSliderState; -} -``` - -### 1.6 `VolumeSliderCore` — Volume-Domain Slider - -**File:** `packages/core/src/core/ui/slider/volume-slider-core.ts` - -Extends `SliderCore`. Accepts `MediaVolumeState` (canonical type from `@videojs/core`) + `SliderInteraction`, returns volume-specific state. - -```ts -export interface VolumeSliderState extends SliderState, Pick {} -``` - -Uses `Pick<>` to select specific fields from canonical media state. No custom `VolumeMediaState` wrapper. - -**Class shape:** - -```ts -export interface VolumeSliderProps extends SliderProps { - label?: string | undefined; -} - -export class VolumeSliderCore extends SliderCore { - static override readonly defaultProps: NonNullableObject; - // min=0, max=100, step=1, largeStep=10, label='Volume'. - - getVolumeState(media: MediaVolumeState, interaction: SliderInteraction): VolumeSliderState; - // - Value: always volume * 100 (actual volume as percent, regardless of muted). - // - Value swap: dragging ? valueFromPercent(dragPercent) : volume * 100. - // - fillPercent: muted ? 0 : base.fillPercent. Visual silence when muted. - // - Delegates to super.getState() for base slider state. - - override getAttrs(state: VolumeSliderState); - // Returns inlined object (no explicit return type). - // Adds aria-label (default 'Volume') and aria-valuetext - // e.g., "75 percent, muted" - // Spreads super.getAttrs(state) for base ARIA. -} - -export namespace VolumeSliderCore { - export type Props = VolumeSliderProps; - export type State = VolumeSliderState; -} -``` - -### 1.7 Constants - -**File:** `packages/core/src/core/ui/slider/slider-data-attrs.ts` - -```ts -import type { StateAttrMap } from '../types'; -import type { SliderState } from './slider-core'; - -export const SliderDataAttrs = { - dragging: 'data-dragging', - pointing: 'data-pointing', - interactive: 'data-interactive', - orientation: 'data-orientation', - disabled: 'data-disabled', -} as const satisfies StateAttrMap; -``` - -**File:** `packages/core/src/core/ui/slider/time-slider-data-attrs.ts` - -```ts -import type { StateAttrMap } from '../types'; -import type { TimeSliderState } from './time-slider-core'; - -export const TimeSliderDataAttrs = { - ...SliderDataAttrs, - seeking: 'data-seeking', -} as const satisfies StateAttrMap; -``` - -**File:** `packages/core/src/core/ui/slider/slider-css-vars.ts` - -```ts -export const SliderCSSVars = { - fill: '--media-slider-fill', - pointer: '--media-slider-pointer', - buffer: '--media-slider-buffer', -} as const; -``` - -### 1.8 Barrel Export - -No `index.ts` barrel — existing UI components don't use one. Add individual exports -to `packages/core/src/core/index.ts` (matching the pattern for play-button, time, etc.): - -```ts -export * from './ui/slider/slider-core'; -export * from './ui/slider/time-slider-core'; -export * from './ui/slider/volume-slider-core'; -export * from './ui/slider/slider-data-attrs'; -export * from './ui/slider/time-slider-data-attrs'; -export * from './ui/slider/slider-css-vars'; -``` - -### 1.9 Tests - -**File:** `packages/core/src/core/ui/slider/tests/slider-core.test.ts` - -- `clamp`: within range, clamp to min, clamp to max, min equals max, negative ranges -- `roundToStep`: nearest step, min offset, decimal steps, value equals min -- `SliderCore.getState`: basic value to percent, interaction passthrough, interactive derivation (including focused) -- `SliderCore.getAttrs`: ARIA output -- `SliderCore.valueFromPercent`: min/max bounds, step snapping -- `SliderCore.percentFromValue`: inverse of valueFromPercent -- `SliderCore.adjustPercentForAlignment`: center (identity) and edge modes -- Default props - -**File:** `packages/core/src/core/ui/slider/tests/time-slider-core.test.ts` - -- `getTimeState`: value swap on drag vs non-drag, buffer percent from buffered ranges, duration as max -- `getAttrs`: aria-label, aria-valuetext formatting -- Zero duration edge case -- Seeking state passthrough - -**File:** `packages/core/src/core/ui/slider/tests/volume-slider-core.test.ts` - -- `getVolumeState`: value always actual volume, fillPercent=0 when muted, value swap on drag -- `getAttrs`: aria-label, aria-valuetext with and without muted -- Volume 0-1 to 0-100 percent mapping - -### 1.10 Verify - -```bash -pnpm -F @videojs/utils test src/time/tests/format.test.ts -pnpm -F @videojs/utils test src/dom/tests/raf-throttle.test.ts -pnpm -F @videojs/utils test src/dom/tests/direction.test.ts -pnpm -F @videojs/utils build -pnpm -F @videojs/core test src/core/ui/slider -pnpm -F @videojs/core build -pnpm typecheck -pnpm lint:fix:file -``` - ---- - -## PR 2: DOM Layer — COMPLETE - -**Branch:** `slider-dom` — [PR #613](https://github.com/videojs/v10/pull/613) -**Base:** `main` (after PR 1 merges) -**Package:** `@videojs/core` (dom subpath) - -### What was built - -- `createSlider()` factory with pointer drag (threshold-based), keyboard stepping, focus management, and AbortController cleanup -- `getPercentFromPointerEvent()` extracted as a public DOM utility in `packages/core/src/dom/utils/pointer.ts` -- `getSliderCSSVars()` and `getTimeSliderCSSVars()` CSS variable formatters -- `UIPointerEvent` / `UIKeyboardEvent` event interfaces extended with required properties -- Shared test infrastructure: `createMockVideo()`, `createTimeRanges()`, `createSliderState()`, `createTimeSliderState()` -- jsdom `PointerEvent` polyfill in test setup -- Refactored 8 feature test files to use shared helpers -- 149 tests passing, lint clean, build clean - -### Deviations from plan - -1. **`getPercentFromPointerEvent` extracted as public utility** — Plan had it as module-private. Extracted to `packages/core/src/dom/utils/pointer.ts` and exported from the `@videojs/core/dom` barrel so the React `useSlider` hook can reuse it without duplication. - -2. **Widened event param type** — `getPercentFromPointerEvent` accepts `{ clientX: number; clientY: number }` instead of `UIPointerEvent`. This allows both `UIPointerEvent` (from public props) and native `PointerEvent` (from document listeners) without type casts. - -3. **Document listeners use native `PointerEvent` typing** — Internal handlers `onDocumentPointerMove` and `onDocumentPointerUp` accept native `PointerEvent` directly, eliminating `as unknown as EventListener` casts. - -4. **`onValueChange` guarded behind drag threshold** — Plan had `onValueChange` firing on every `pointermove`. Changed so pre-threshold moves only update `pointerPercent` for hover preview — `onValueChange` fires only once drag threshold is reached. Prevents premature seeking. - -5. **`getThumbElement` option added** — Not in original plan. Allows `createSlider` to focus the thumb on pointerdown for keyboard follow-up and screen reader tracking. - -6. **`UIKeyboardEvent` extended** — Added `shiftKey` (for Shift+Arrow large step) and `metaKey` (for meta key guard on numeric 0-9). - -7. **AbortController renamed** — `ac` → `abort` per user preference. - -8. **Shared test helpers** — Plan didn't specify these. Created `packages/core/src/dom/tests/test-helpers.ts` with importable factories and `setup.ts` with global jsdom `PointerEvent` polyfill, wired into vitest config. - -9. **Document listener cleanup simplified** — `onDocumentPointerCancel` inlined (just calls `endDrag`), `onDocumentTouchMove` inlined as arrow function. - -### Files created - -- `packages/core/src/dom/ui/slider.ts` -- `packages/core/src/dom/ui/slider-css-vars.ts` -- `packages/core/src/dom/utils/pointer.ts` -- `packages/core/src/dom/tests/setup.ts` -- `packages/core/src/dom/tests/test-helpers.ts` -- `packages/core/src/dom/ui/tests/slider.test.ts` (46 tests) -- `packages/core/src/dom/ui/tests/slider-css-vars.test.ts` (7 tests) - -### Files modified - -- `packages/core/src/dom/ui/event.ts` — added `UIPointerEvent`, `shiftKey`/`metaKey` to `UIKeyboardEvent` -- `packages/core/src/dom/index.ts` — barrel exports for event, slider, slider-css-vars -- `packages/core/src/dom/utils/index.ts` — added `getPercentFromPointerEvent` export -- `packages/core/vitest.config.ts` — added `setupFiles` for dom project -- 8 feature test files — refactored to use shared `createMockVideo`/`createTimeRanges` - ---- - -## PR 3: React Components — COMPLETE - -**Branch:** `feat/slider-react` -**Base:** `feat/slider-dom` -**Package:** `@videojs/react` - -### What was built - -- `useSlider` generic hook wrapping `createSlider()` + `useSnapshot` for React lifecycle -- `SliderProvider` / `useSliderContext` — first component-level React context in the codebase (children need interaction state from `createSlider()`, not player store state) -- 6 generic parts: `SliderRoot`, `SliderTrack`, `SliderFill`, `SliderBuffer`, `SliderThumb`, `SliderValue` -- `TimeSliderRoot` and `VolumeSliderRoot` domain roots connecting to player store via `usePlayer` -- Barrel files with compound namespace pattern (`Slider.*`, `TimeSlider.*`, `VolumeSlider.*`) -- `useLatestRef` utility for stable callback refs -- `SliderCore.props` getter added to core package -- Shared test helpers (`createMockStore`, `createPlayerWrapper`) in `packages/react/src/testing/mocks.tsx` -- Skin presets updated with real `TimeSlider` compound components -- 38 new tests (22 slider + 8 time-slider + 8 volume-slider), all 139 React tests passing -- Sandbox page with standalone slider, vertical, disabled, thumbAlignment demo, TimeSlider with video, VolumeSlider horizontal + vertical - -### Deviations from plan - -1. **`useSlider` is generic over state type** — Plan showed `UseSliderResult` returning `SliderState`. Implementation uses `` generic so domain roots get proper `TimeSliderState` / `VolumeSliderState` typing. -2. **`thumbRef` threaded through context** — Not in plan. Required so `createSlider` can programmatically focus the thumb on pointerdown. -3. **Domain roots don't compose on `SliderRoot`** — Plan implied composition. Each domain root has different state sources, cores, CSS var functions, and hardwired callbacks, making composition impractical. -4. **Domain barrel re-exports simplified** — Plan showed explicit re-exports of each part. Implementation uses `export * from '../slider/index.parts'` then overrides `Root` only. -5. **Context uses `.Provider` pattern** — React 19's direct `` isn't supported by the rolldown bundler. Uses ``. -6. **`getAttrs` in context** — Not in plan. Justified for `SliderThumb` to get domain-specific ARIA attrs without knowing which core it's using. -7. **Shared test helpers extracted** — Plan didn't specify. Created `packages/react/src/testing/mocks.tsx` with `vi.hoisted()` pattern for mock data. - -### Files created - -- `packages/react/src/utils/use-latest-ref.ts` -- `packages/react/src/ui/slider/slider-context.tsx` -- `packages/react/src/ui/hooks/use-slider.ts` -- `packages/react/src/ui/slider/slider-root.tsx` -- `packages/react/src/ui/slider/slider-track.tsx` -- `packages/react/src/ui/slider/slider-fill.tsx` -- `packages/react/src/ui/slider/slider-buffer.tsx` -- `packages/react/src/ui/slider/slider-thumb.tsx` -- `packages/react/src/ui/slider/slider-value.tsx` -- `packages/react/src/ui/slider/index.ts` + `index.parts.ts` -- `packages/react/src/ui/time-slider/time-slider-root.tsx` -- `packages/react/src/ui/time-slider/index.ts` + `index.parts.ts` -- `packages/react/src/ui/volume-slider/volume-slider-root.tsx` -- `packages/react/src/ui/volume-slider/index.ts` + `index.parts.ts` -- `packages/react/src/testing/mocks.tsx` -- `packages/react/src/ui/slider/tests/slider.test.tsx` (22 tests) -- `packages/react/src/ui/time-slider/tests/time-slider.test.tsx` (8 tests) -- `packages/react/src/ui/volume-slider/tests/volume-slider.test.tsx` (8 tests) - -### Files modified - -- `packages/core/src/core/ui/slider/slider-core.ts` — added `get props()` getter -- `packages/react/src/index.ts` — added Slider/TimeSlider/VolumeSlider/useSlider exports -- `packages/react/src/presets/video/skin.tsx` — replaced placeholder with TimeSlider compound -- `packages/react/src/presets/video/minimal-skin.tsx` — same replacement -- `packages/react/src/player/tests/context.test.tsx` — updated to use shared `createMockStore` -- `packages/sandbox/vite.config.ts` — added `react-slider` input -- `packages/sandbox/src/index.html` — added nav link - -### Original plan (for reference) - -### 3.1 Slider Context - -**File:** `packages/react/src/ui/slider/slider-context.ts` - -Internal context carrying shared slider state for child parts. The root element applies -ARIA attrs directly via `core.getAttrs(state)` and provides interaction data to children. -No `thumbAttrs` in context — `getAttrs()` returns an inferred type (no named interface). - -```ts -interface SliderContextValue { - state: SliderState; - thumbProps: SliderThumbProps; - stateAttrMap: StateAttrMap; - getAttrs: (state: SliderState) => object; // core.getAttrs bound to core instance - formatValue?: (value: number, type: 'current' | 'pointer') => string; -} -``` - -The `stateAttrMap` ensures generic parts only generate data attributes for the correct keys -(e.g., `data-dragging`, `data-disabled`) — not for every state property like `value` or -`fillPercent`. Domain roots provide their specific map: `SliderDataAttrs` for generic/volume, -`TimeSliderDataAttrs` for time (adds `data-seeking`). - -**Note:** This is the first UI component in the codebase to create its own React/Lit context. -Existing components (Time.Value, etc.) read from the player store directly. The slider needs -component-level context because child parts need shared interaction state from `createSlider()`, -not from the player store. This is a justified new pattern. - -### 3.2 `useSlider` Hook - -**File:** `packages/react/src/ui/hooks/use-slider.ts` - -Encapsulates `createSlider()` lifecycle for React. Uses `useSnapshot` from `@videojs/store/react` to subscribe to `State`. - -```ts -interface UseSliderOptions { - core: SliderCore; - computeState: (interaction: SliderInteraction) => SliderState; - onValueChange?: (percent: number) => void; - onValueCommit?: (percent: number) => void; - onDragStart?: () => void; - onDragEnd?: () => void; - disabled?: boolean; - orientation?: 'horizontal' | 'vertical'; - isRTL?: boolean; -} - -interface UseSliderResult { - state: SliderState; - rootProps: SliderRootProps; - thumbProps: SliderThumbProps; - cssVars: Record; -} - -function useSlider(options: UseSliderOptions): UseSliderResult; -``` - -**Implementation:** -- `useState(() => createSlider(...))` for lazy init. -- `useSnapshot(slider.interaction)` to subscribe to `State` (from PR 0). -- Compute `state = options.computeState(interaction)`. -- Compute `cssVars = getSliderCSSVars(state)`. -- On unmount: call `slider.destroy()`. -- Memoize `SliderOptions` callbacks with refs to avoid recreating the slider. - -### 3.3 Generic `Slider.Root` - -**File:** `packages/react/src/ui/slider/slider-root.tsx` - -The generic root creates its own `SliderCore`, calls `useSlider`, provides `SliderContext`, renders a `
` with CSS vars and pointer handlers. - -All React component files start with `'use client';` directive. Parameter is always named -`componentProps`. Destructure `render, className, style` first, then core props, then `...elementProps`. - -```tsx -'use client'; - -export interface SliderRootProps extends UIComponentProps<'div', SliderCore.State>, SliderCore.Props { - value?: number; - defaultValue?: number; - onValueChange?: (value: number) => void; - onValueCommit?: (value: number) => void; - onDragStart?: () => void; - onDragEnd?: () => void; -} - -export const Root = forwardRef(function SliderRoot( - componentProps: SliderRootProps, - forwardedRef: ForwardedRef -) { - const { render, className, style, min, max, step, largeStep, - orientation, disabled, thumbAlignment, value, defaultValue, - onValueChange, onValueCommit, onDragStart, onDragEnd, - ...elementProps } = componentProps; - - // 1. Lazy-init SliderCore, call setProps. - const [core] = useState(() => new SliderCore()); - core.setProps({ min, max, step, largeStep, orientation, disabled, thumbAlignment }); - - // 2. Call useSlider with generic computeState. - // 3. For controlled: value comes from props. For uncontrolled: track internal state. - // 4. Provide SliderContext. - // 5. CSS vars passed as { style: cssVarsObj } in props array: - return renderElement('div', { render, className, style }, { - state, - stateAttrMap: SliderDataAttrs, - ref: [forwardedRef], - props: [core.getAttrs(state), { style: cssVars }, elementProps], - }); -}); - -export namespace Root { - export type Props = SliderRootProps; - export type State = SliderCore.State; -} -``` - -### 3.4 Generic Parts - -All generic parts consume `SliderContext` for data attributes. Each is a thin `forwardRef` wrapper around `renderElement`. - -**`Slider.Track`** — `packages/react/src/ui/slider/slider-track.tsx` -- Renders `
` with data attributes from context state. -- No special behavior. - -**`Slider.Fill`** — `packages/react/src/ui/slider/slider-fill.tsx` -- Renders `
` with data attributes. -- Sizing is pure CSS via `var(--media-slider-fill)`. - -**`Slider.Buffer`** — `packages/react/src/ui/slider/slider-buffer.tsx` -- Renders `
` with data attributes. -- Sizing is pure CSS via `var(--media-slider-buffer)`. - -**`Slider.Thumb`** — `packages/react/src/ui/slider/slider-thumb.tsx` -- Renders `
` with `role="slider"`, ARIA attrs from context `thumbAttrs`. -- Event handlers from context `thumbProps`. -- `tabIndex={0}`, `autocomplete="off"`. -- Data attributes from context state. - -**`Slider.Value`** — `packages/react/src/ui/slider/slider-value.tsx` -- Renders `` with `aria-live="off"`. -- Props: `type?: 'current' | 'pointer'`. -- Reads formatted value from context `formatValue(value, type)`. -- Falls back to raw number if no formatter. - -### 3.5 Domain Roots - -**`TimeSlider.Root`** — `packages/react/src/ui/time-slider/time-slider-root.tsx` - -- `'use client';` directive at line 1. -- Connects to media store: `usePlayer(selectTime)`, `usePlayer(selectBuffer)`. -- **Feature guard:** `if (!time) { if (__DEV__) logMissingFeature('TimeSlider', 'time'); return null; }` - Same for buffer (optional — buffer can be absent without breaking, just no buffer bar). -- Computes `bufferedEnd` from `MediaBufferState.buffered` (last range end, or 0). -- Lazy-inits `TimeSliderCore`, calls `setProps`. -- Calls `useSlider` with `computeState = (interaction) => core.getTimeState(interaction, mediaState)`. -- `onValueChange`: visual update (CSS vars refresh). -- `onValueCommit`: calls `mediaState.seek(core.valueFromPercent(percent))`, throttled via `seekThrottle` prop (default 100ms, trailing edge). -- `onDragStart`/`onDragEnd`: user callbacks. -- Provides `SliderContext` with time formatting: `formatValue = (value, type) => formatTime(value)`. -- Uses `TimeSliderDataAttrs` for state-to-data-attr mapping. -- Uses `getTimeSliderCSSVars` for CSS vars (includes buffer). -- Props: `label`, `seekThrottle`, `disabled`, `thumbAlignment`, `step` (default 0.1 in seconds), `largeStep` (default 10 seconds), `render`, `onDragStart`, `onDragEnd`. -- renderElement CSS vars: `props: [core.getAttrs(state), { style: cssVars }, elementProps]`. -- `export namespace Root { export type Props = TimeSliderRootProps; export type State = TimeSliderCore.State; }` - -**`VolumeSlider.Root`** — `packages/react/src/ui/volume-slider/volume-slider-root.tsx` - -- `'use client';` directive at line 1. -- Connects to media store: `usePlayer(selectVolume)`. -- **Feature guard:** `if (!volume) { if (__DEV__) logMissingFeature('VolumeSlider', 'volume'); return null; }` -- Lazy-inits `VolumeSliderCore`, calls `setProps`. -- Calls `useSlider` with `computeState = (interaction) => core.getVolumeState(interaction, mediaState)`. -- `onValueChange`: calls `mediaState.setVolume(core.valueFromPercent(percent) / 100)` immediately. Volume changes are cheap and instant — no throttle. -- `onValueCommit`: same as `onValueChange` (no separate commit behavior for volume). -- Provides `SliderContext` with percentage formatting: `formatValue = (value) => Math.round(value) + '%'`. -- Uses `SliderDataAttrs` for state-to-data-attr mapping. -- Uses `getSliderCSSVars` for CSS vars (no buffer). -- Props: `label`, `orientation`, `disabled`, `thumbAlignment`, `step` (default 1%), `largeStep` (default 10%), `render`, `onDragStart`, `onDragEnd`. -- renderElement CSS vars: `props: [core.getAttrs(state), { style: cssVars }, elementProps]`. -- `export namespace Root { export type Props = VolumeSliderRootProps; export type State = VolumeSliderCore.State; }` - -### 3.6 Barrel Files - -**`packages/react/src/ui/slider/index.ts`:** - -```ts -export * as Slider from './index.parts'; -``` - -**`packages/react/src/ui/slider/index.parts.ts`:** - -```ts -export { Root, type SliderRootProps as RootProps } from './slider-root'; -export { Track, type SliderTrackProps as TrackProps } from './slider-track'; -export { Fill, type SliderFillProps as FillProps } from './slider-fill'; -export { Buffer, type SliderBufferProps as BufferProps } from './slider-buffer'; -export { Thumb, type SliderThumbProps as ThumbProps } from './slider-thumb'; -export { Value, type SliderValueProps as ValueProps } from './slider-value'; -``` - -**`packages/react/src/ui/time-slider/index.ts`:** - -```ts -export * as TimeSlider from './index.parts'; -``` - -**`packages/react/src/ui/time-slider/index.parts.ts`:** - -```ts -export { Root, type TimeSliderRootProps as RootProps } from './time-slider-root'; -export { Track, Fill, Buffer, Thumb, Value } from '../slider/index.parts'; -export type { SliderTrackProps as TrackProps, SliderFillProps as FillProps, - SliderBufferProps as BufferProps, SliderThumbProps as ThumbProps, - SliderValueProps as ValueProps } from '../slider/index.parts'; -``` - -**`packages/react/src/ui/volume-slider/index.ts`:** - -```ts -export * as VolumeSlider from './index.parts'; -``` - -**`packages/react/src/ui/volume-slider/index.parts.ts`:** - -```ts -export { Root, type VolumeSliderRootProps as RootProps } from './volume-slider-root'; -// Buffer is re-exported for API consistency but renders nothing useful -// for volume sliders — VolumeSlider.Root does not set --media-slider-buffer. -export { Track, Fill, Buffer, Thumb, Value } from '../slider/index.parts'; -export type { SliderTrackProps as TrackProps, SliderFillProps as FillProps, - SliderBufferProps as BufferProps, SliderThumbProps as ThumbProps, - SliderValueProps as ValueProps } from '../slider/index.parts'; -``` - -Add to `packages/react/src/index.ts`: - -```ts -export { Slider } from './ui/slider'; -export { TimeSlider } from './ui/time-slider'; -export { VolumeSlider } from './ui/volume-slider'; -``` - -### 3.7 Sandbox Demo (gitignored) - -Update `packages/sandbox/src/react/main.tsx` (or create a new route) with a working slider demo: - -```tsx - - - - -``` - -### 3.8 Tests - -**File:** `packages/react/src/ui/slider/tests/slider.test.tsx` - -- Generic `Slider.Root` renders with correct data attributes and CSS vars. -- `Slider.Thumb` has correct ARIA attributes. -- `Slider.Value` renders formatted output. -- Context propagation to children. -- `onValueChange` / `onValueCommit` callbacks fire. - -**File:** `packages/react/src/ui/time-slider/tests/time-slider.test.tsx` - -- Renders with mocked player store. -- CSS vars include buffer. -- `data-seeking` attribute. -- Seek is called on value commit. -- Time formatting in `Value`. - -**File:** `packages/react/src/ui/volume-slider/tests/volume-slider.test.tsx` - -- Renders with mocked player store. -- Fill is 0 when muted. -- `setVolume` called on value change. -- `aria-valuetext` includes muted state. - -### 3.9 Verify - -```bash -pnpm -F @videojs/react test src/ui/slider -pnpm -F @videojs/react test src/ui/time-slider -pnpm -F @videojs/react test src/ui/volume-slider -pnpm -F @videojs/react build -pnpm typecheck -pnpm lint:fix:file -``` - ---- - -## PR 4: HTML Custom Elements - -**Branch:** `feat/slider-html` -**Base:** `feat/slider-dom` -**Package:** `@videojs/html` - -### 4.1 Event Types - -**File:** `packages/html/src/ui/slider/slider-events.ts` - -```ts -export interface SliderValueEventDetail { - value: number; -} - -export interface SliderEventMap { - 'value-change': CustomEvent; - 'value-commit': CustomEvent; - 'drag-start': CustomEvent; - 'drag-end': CustomEvent; -} - -export interface DomainSliderEventMap { - 'drag-start': CustomEvent; - 'drag-end': CustomEvent; -} -``` - -### 4.2 `SliderElement` — Generic Root - -**File:** `packages/html/src/ui/slider/slider-element.ts` - -Extends `MediaElement`. The generic slider root handles pointer events, CSS variables, data attributes, and custom DOM events. Can be used standalone (without domain sliders) for custom slider use cases. - -```ts -import type { PropertyDeclarationMap } from '@videojs/element'; -import { ContextProvider } from '@videojs/element/context'; - -export class SliderElement extends MediaElement { - static readonly tagName = 'media-slider'; - - static override properties = { - value: { type: Number }, - min: { type: Number }, - max: { type: Number }, - step: { type: Number }, - largeStep: { type: Number, attribute: 'large-step' }, - orientation: { type: String }, - disabled: { type: Boolean }, - thumbAlignment: { type: String, attribute: 'thumb-alignment' }, - } satisfies PropertyDeclarationMap; - - // Property defaults from core - value = 0; - min = SliderCore.defaultProps.min; - max = SliderCore.defaultProps.max; - step = SliderCore.defaultProps.step; - largeStep = SliderCore.defaultProps.largeStep; - orientation = SliderCore.defaultProps.orientation; - disabled = SliderCore.defaultProps.disabled; - thumbAlignment = SliderCore.defaultProps.thumbAlignment; - - readonly #core = new SliderCore(); - #slider: SliderHandle | null = null; - #disconnect: AbortController | null = null; - - override connectedCallback(): void { - super.connectedCallback(); - // 1. Create AbortController - // 2. Create slider via createSlider() with options: - // - getElement: () => this - // - getOrientation, isRTL (via isRTL from @videojs/utils/dom), isDisabled - // - getPercent, getStepPercent, getLargeStepPercent - // - onValueChange: dispatch 'value-change' event - // - onValueCommit: dispatch 'value-commit' event - // - onDragStart: dispatch 'drag-start' event - // - onDragEnd: dispatch 'drag-end' event - // 3. Apply rootProps to this element via applyElementProps(this, rootProps, signal) - // 4. Subscribe to interaction state for re-renders: - // slider.interaction.subscribe(() => this.requestUpdate(), - // { signal: this.#disconnect.signal }) - // 5. Set touch-action: none, user-select: none, contain: layout style - // 6. Provide sliderContext via ContextProvider - } - - override disconnectedCallback(): void { - super.disconnectedCallback(); - // 1. slider.destroy() - // 2. abort controller (also cleans up interaction subscription) - } - - protected override willUpdate(changed: PropertyValues): void { - super.willUpdate(changed); - this.#core.setProps(this); - } - - protected override update(changed: PropertyValues): void { - super.update(changed); - // Recompute state from core + interaction - // Apply CSS vars via this.style.setProperty(key, value) — NOT applyElementProps - // Apply data attrs via applyStateDataAttrs(this, state, SliderDataAttrs) - // Apply ARIA via applyElementProps(this, this.#core.getAttrs(state)) - // Update sliderContext provider value - } -} -``` - -### 4.3 Structural Part Elements - -All child elements consume `sliderContext` via `ContextConsumer` and apply data attributes -from the context's `stateAttrMap` in their `update()` method. This ensures data attributes -like `data-dragging`, `data-pointing`, `data-interactive`, `data-orientation`, `data-disabled` -propagate to every child. - -**`SliderTrackElement`** — `packages/html/src/ui/slider/slider-track-element.ts` -```ts -export class SliderTrackElement extends MediaElement { - static readonly tagName = 'media-slider-track'; - - readonly #ctx = new ContextConsumer(this, { - context: sliderContext, - subscribe: true, - }); - - protected override update(): void { - super.update(); - const ctx = this.#ctx.value; - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); - } -} -``` - -**`SliderFillElement`** — `packages/html/src/ui/slider/slider-fill-element.ts` -```ts -export class SliderFillElement extends MediaElement { - static readonly tagName = 'media-slider-fill'; - // Same ContextConsumer + applyStateDataAttrs pattern as Track -} -``` - -**`SliderBufferElement`** — `packages/html/src/ui/slider/slider-buffer-element.ts` -```ts -export class SliderBufferElement extends MediaElement { - static readonly tagName = 'media-slider-buffer'; - // Same ContextConsumer + applyStateDataAttrs pattern as Track -} -``` - -### 4.4 `SliderThumbElement` - -**File:** `packages/html/src/ui/slider/slider-thumb-element.ts` - -```ts -export class SliderThumbElement extends MediaElement { - static readonly tagName = 'media-slider-thumb'; - - readonly #ctx = new ContextConsumer(this, { - context: sliderContext, - subscribe: true, - }); - - override connectedCallback(): void { - super.connectedCallback(); - // Set role="slider", tabindex="0", autocomplete="off" as initial attributes - } - - protected override update(): void { - super.update(); - const ctx = this.#ctx.value; - if (!ctx) return; - - // Apply ARIA from context thumbAttrs - applyElementProps(this, ctx.thumbAttrs); - // Apply keyboard/focus handlers from context thumbProps - // (needs signal management — apply once in connectedCallback or re-apply) - applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); - } -} -``` - -**Note:** `thumbProps` (keyboard/focus handlers) should be applied once in `connectedCallback` -with a signal, not re-applied every update. The root element can apply them directly to the -thumb child, or the thumb can read them from context on first connect. - -### 4.5 `SliderValueElement` - -**File:** `packages/html/src/ui/slider/slider-value-element.ts` - -```ts -export class SliderValueElement extends MediaElement { - static readonly tagName = 'media-slider-value'; - - static override properties = { - type: { type: String }, // 'current' | 'pointer' - }; - - type: 'current' | 'pointer' = 'current'; - - readonly #ctx = new ContextConsumer(this, { - context: sliderContext, - subscribe: true, - }); - - override connectedCallback(): void { - super.connectedCallback(); - this.setAttribute('aria-live', 'off'); - } - - protected override update(): void { - super.update(); - const ctx = this.#ctx.value; - if (!ctx) return; - - // Get value based on type - const value = this.type === 'pointer' - ? ctx.state.pointerPercent - : ctx.state.value; - - // Format and set text content - this.textContent = ctx.formatValue - ? ctx.formatValue(value, this.type) - : String(Math.round(value)); - - applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); - } -} -``` - -### 4.6 Context Communication (HTML) - -The HTML slider needs a way for the root element to communicate state to child elements. Options: - -- **Context** (`@videojs/element/context`): The `SliderElement` provides a context that children consume via `ContextConsumer`. This is consistent with how `PlayerController` uses `playerContext`. -- Create a `sliderContext` with `createContext()`. -- Root provides it, structural children consume it. - -```ts -// packages/html/src/ui/slider/slider-context.ts -import { createContext } from '@videojs/element/context'; -import type { StateAttrMap } from '@videojs/core'; - -export interface SliderContextValue { - state: SliderState; - thumbProps: SliderThumbProps; - stateAttrMap: StateAttrMap; - formatValue?: (value: number, type: 'current' | 'pointer') => string; -} - -const SLIDER_CONTEXT_KEY = Symbol('@videojs/slider'); -export const sliderContext = createContext(SLIDER_CONTEXT_KEY); -``` - -Root provides via `ContextProvider`. Children consume via `ContextConsumer` and apply -data-attrs/ARIA in their `update()` method. The `stateAttrMap` ensures only the correct -state keys become data attributes (not `value`, `fillPercent`, etc.). - -### 4.7 Domain Elements - -**`TimeSliderElement`** — `packages/html/src/ui/time-slider/time-slider-element.ts` - -```ts -import type { PropertyDeclarationMap } from '@videojs/element'; -import { ContextProvider } from '@videojs/element/context'; - -export class TimeSliderElement extends MediaElement { - static readonly tagName = 'media-time-slider'; - - static override properties = { - label: { type: String }, - seekThrottle: { type: Number, attribute: 'seek-throttle' }, - disabled: { type: Boolean }, - thumbAlignment: { type: String, attribute: 'thumb-alignment' }, - } satisfies PropertyDeclarationMap; - - label = TimeSliderCore.defaultProps.label; - seekThrottle = 100; - disabled = TimeSliderCore.defaultProps.disabled; - thumbAlignment = TimeSliderCore.defaultProps.thumbAlignment; - - readonly #core = new TimeSliderCore(); - readonly #timeState = new PlayerController(this, playerContext, selectTime); - readonly #bufferState = new PlayerController(this, playerContext, selectBuffer); - #slider: SliderHandle | null = null; - #disconnect: AbortController | null = null; - - override connectedCallback(): void { - super.connectedCallback(); - // 1. Create AbortController - // 2. Create slider via createSlider({ getElement: () => this, ... }) - // 3. min=0, max=duration always - // 4. onValueChange: visual update only (triggers re-render) - // 5. onValueCommit: timeState.seek(core.valueFromPercent(percent)), throttled - // 6. Apply rootProps via applyElementProps(this, rootProps, signal) - // 7. Subscribe: slider.interaction.subscribe(() => this.requestUpdate(), { signal }) - // 8. Set contain, touch-action, user-select - // 9. Provide sliderContext via ContextProvider - // 10. DEV: logMissingFeature if !this.#timeState.value - } - - protected override willUpdate(changed: PropertyValues): void { - super.willUpdate(changed); - this.#core.setProps(this); - } - - protected override update(changed: PropertyValues): void { - super.update(changed); - const media = this.#timeState.value; - if (!media) return; - // Compute TimeSliderState from core + interaction + media state - // Apply CSS vars via this.style.setProperty() using getTimeSliderCSSVars() - // Apply data attrs via applyStateDataAttrs(this, state, TimeSliderDataAttrs) - // Apply ARIA via applyElementProps(this, this.#core.getAttrs(state)) - // Update sliderContext provider value - } -} -``` - -**`VolumeSliderElement`** — `packages/html/src/ui/volume-slider/volume-slider-element.ts` - -```ts -import type { PropertyDeclarationMap } from '@videojs/element'; -import { ContextProvider } from '@videojs/element/context'; - -export class VolumeSliderElement extends MediaElement { - static readonly tagName = 'media-volume-slider'; - - static override properties = { - label: { type: String }, - orientation: { type: String }, - disabled: { type: Boolean }, - thumbAlignment: { type: String, attribute: 'thumb-alignment' }, - } satisfies PropertyDeclarationMap; - - label = VolumeSliderCore.defaultProps.label; - orientation = VolumeSliderCore.defaultProps.orientation; - disabled = VolumeSliderCore.defaultProps.disabled; - thumbAlignment = VolumeSliderCore.defaultProps.thumbAlignment; - - readonly #core = new VolumeSliderCore(); - readonly #volumeState = new PlayerController(this, playerContext, selectVolume); - #slider: SliderHandle | null = null; - #disconnect: AbortController | null = null; - - override connectedCallback(): void { - super.connectedCallback(); - // 1. Create AbortController - // 2. Create slider via createSlider({ getElement: () => this, ... }) - // 3. onValueChange: volumeState.setVolume(percent / 100) - // 4. Apply rootProps via applyElementProps(this, rootProps, signal) - // 5. Subscribe: slider.interaction.subscribe(() => this.requestUpdate(), { signal }) - // 6. Provide sliderContext via ContextProvider - // 7. DEV: logMissingFeature if !this.#volumeState.value - } - - protected override willUpdate(changed: PropertyValues): void { - super.willUpdate(changed); - this.#core.setProps(this); - } - - protected override update(changed: PropertyValues): void { - super.update(changed); - const media = this.#volumeState.value; - if (!media) return; - // Compute VolumeSliderState from core + interaction + volume state - // Apply CSS vars via this.style.setProperty() using getSliderCSSVars() - // Apply data attrs via applyStateDataAttrs(this, state, SliderDataAttrs) - // Apply ARIA via applyElementProps(this, this.#core.getAttrs(state)) - // Update sliderContext provider value - } -} -``` - -### 4.8 Registration Files - -Shared child elements are extracted into their own define modules. Both `time-slider` and -`volume-slider` import these as side-effect imports. The ES module singleton guarantee ensures -each executes only once — no `defineCustomElement` guard needed. - -**Shared define modules:** - -**`packages/html/src/define/ui/slider-track.ts`:** -```ts -import { SliderTrackElement } from '../../ui/slider/slider-track-element'; - -customElements.define(SliderTrackElement.tagName, SliderTrackElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderTrackElement.tagName]: SliderTrackElement; - } -} -``` - -**`packages/html/src/define/ui/slider-fill.ts`:** -```ts -import { SliderFillElement } from '../../ui/slider/slider-fill-element'; - -customElements.define(SliderFillElement.tagName, SliderFillElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderFillElement.tagName]: SliderFillElement; - } -} -``` - -**`packages/html/src/define/ui/slider-buffer.ts`:** -```ts -import { SliderBufferElement } from '../../ui/slider/slider-buffer-element'; - -customElements.define(SliderBufferElement.tagName, SliderBufferElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderBufferElement.tagName]: SliderBufferElement; - } -} -``` - -**`packages/html/src/define/ui/slider-thumb.ts`:** -```ts -import { SliderThumbElement } from '../../ui/slider/slider-thumb-element'; - -customElements.define(SliderThumbElement.tagName, SliderThumbElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderThumbElement.tagName]: SliderThumbElement; - } -} -``` - -**`packages/html/src/define/ui/slider-value.ts`:** -```ts -import { SliderValueElement } from '../../ui/slider/slider-value-element'; - -customElements.define(SliderValueElement.tagName, SliderValueElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderValueElement.tagName]: SliderValueElement; - } -} -``` - -**Domain define modules (import shared as side effects):** - -**`packages/html/src/define/ui/time-slider.ts`:** -```ts -import { TimeSliderElement } from '../../ui/time-slider/time-slider-element'; - -// Side-effect imports — each module executes once (ES module singleton) -import './slider-track'; -import './slider-fill'; -import './slider-buffer'; -import './slider-thumb'; -import './slider-value'; - -customElements.define(TimeSliderElement.tagName, TimeSliderElement); - -declare global { - interface HTMLElementTagNameMap { - [TimeSliderElement.tagName]: TimeSliderElement; - } -} -``` - -**`packages/html/src/define/ui/volume-slider.ts`:** -```ts -import { VolumeSliderElement } from '../../ui/volume-slider/volume-slider-element'; - -// Side-effect imports — shared modules already executed if time-slider was imported first -import './slider-track'; -import './slider-fill'; -import './slider-thumb'; -import './slider-value'; - -customElements.define(VolumeSliderElement.tagName, VolumeSliderElement); - -declare global { - interface HTMLElementTagNameMap { - [VolumeSliderElement.tagName]: VolumeSliderElement; - } -} -``` - -**Package.json exports** — No changes needed. The existing wildcard glob in `packages/html/package.json` -already covers any new files added to `src/define/ui/`: -```json -"./ui/*": { "types": "...", "development": "...", "default": "..." } -``` -New define files (`time-slider.ts`, `volume-slider.ts`, `slider-track.ts`, etc.) are automatically -exposed as `@videojs/html/ui/time-slider`, `@videojs/html/ui/slider-track`, etc. - -### 4.9 Barrel Export - -Add to `packages/html/src/index.ts`: -```ts -export { SliderElement } from './ui/slider/slider-element'; -export { SliderTrackElement } from './ui/slider/slider-track-element'; -export { SliderFillElement } from './ui/slider/slider-fill-element'; -export { SliderBufferElement } from './ui/slider/slider-buffer-element'; -export { SliderThumbElement } from './ui/slider/slider-thumb-element'; -export { SliderValueElement } from './ui/slider/slider-value-element'; -export { TimeSliderElement } from './ui/time-slider/time-slider-element'; -export { VolumeSliderElement } from './ui/volume-slider/volume-slider-element'; -``` - -### 4.10 Sandbox Demo (gitignored) - -Update `packages/sandbox/src/html/main.ts` with: - -```html - - - - - - - - - - - - - - -``` - -### 4.11 Tests - -**File:** `packages/html/src/ui/slider/tests/slider-element.test.ts` -- Element registers with correct tag name -- Reactive properties reflect to attributes -- CSS vars set on host -- Data attributes propagate -- Custom events fire: `value-change`, `value-commit`, `drag-start`, `drag-end` -- Thumb child gets ARIA - -**File:** `packages/html/src/ui/time-slider/tests/time-slider-element.test.ts` -- Connects to store, renders time state -- Buffer CSS var present -- `data-seeking` attribute -- Seek called on commit - -**File:** `packages/html/src/ui/volume-slider/tests/volume-slider-element.test.ts` -- Volume changes on value change -- Muted fill is 0 -- Vertical orientation support - -### 4.12 Verify - -```bash -pnpm -F @videojs/html test src/ui/slider -pnpm -F @videojs/html test src/ui/time-slider -pnpm -F @videojs/html test src/ui/volume-slider -pnpm -F @videojs/html build -pnpm typecheck -pnpm lint:fix:file -``` - ---- - -## PR 5: React Preview Component - -**Branch:** `feat/slider-preview-react` -**Base:** `feat/slider-react` -**Package:** `@videojs/react` - -### Positioning Strategy (CSS-Only) - -No Floating UI dependency. Slider preview uses pure CSS positioning with JS-computed clamping, -matching Vidstack's proven approach: - -- Absolute positioning within the slider root -- Horizontal position: `left: min(max(0px, calc(var(--media-slider-pointer) - halfWidth)), calc(100% - fullWidth))` -- Vertical offset: `bottom: calc(100% + var(--media-slider-preview-offset, 8px))` -- `ResizeObserver` on the preview element recalculates clamping when content changes size -- Supports `noClamp` prop for previews that intentionally extend beyond slider bounds -- The `--media-slider-pointer` CSS variable is already set by the slider root (from PR 2) - -### 5.1 `Slider.Preview` - -**File:** `packages/react/src/ui/slider/slider-preview.tsx` - -```tsx -export interface SliderPreviewProps extends UIComponentProps<'div', SliderState> { - noClamp?: boolean; -} - -export const Preview = forwardRef(function SliderPreview( - props: SliderPreviewProps, - ref: ForwardedRef -) { - const context = useContext(SliderContext); - const previewRef = useRef(null); - const [width, setWidth] = useState(0); - - // ResizeObserver tracks preview width for clamping - useEffect(() => { - const el = previewRef.current; - if (!el) return; - const ro = new ResizeObserver(([entry]) => setWidth(entry.contentRect.width)); - ro.observe(el); - return () => ro.disconnect(); - }, []); - - // Compute clamped left position - const halfWidth = width / 2; - const style = props.noClamp - ? { left: `calc(var(--media-slider-pointer) - ${halfWidth}px)` } - : { left: `min(max(0px, calc(var(--media-slider-pointer) - ${halfWidth}px)), calc(100% - ${width}px))` }; - - // Render
with position: absolute, clamped style, data attributes from context. -}); -``` - -Re-export from `Slider`, `TimeSlider`, and `VolumeSlider` namespaces. - -### 5.2 Tests - -- Preview renders within slider context -- Data attributes propagate -- Clamping style applied by default -- `noClamp` disables clamping - -### 5.3 Verify - -```bash -pnpm -F @videojs/react test src/ui/slider/tests/slider-preview -pnpm -F @videojs/react build -pnpm typecheck -``` - ---- - -## PR 6: HTML Preview Element - -**Branch:** `feat/slider-preview-html` -**Base:** `feat/slider-html` -**Package:** `@videojs/html` - -### 6.1 `SliderPreviewElement` - -**File:** `packages/html/src/ui/slider/slider-preview-element.ts` - -```ts -export class SliderPreviewElement extends MediaElement { - static readonly tagName = 'media-slider-preview'; - - static override properties = { - noClamp: { type: Boolean, attribute: 'no-clamp' }, - }; - - noClamp = false; - - readonly #ctx = new ContextConsumer(this, { - context: sliderContext, - subscribe: true, - }); - - #resizeObserver: ResizeObserver | null = null; - #width = 0; - - override connectedCallback(): void { - super.connectedCallback(); - // Set position: absolute, width: max-content - this.#resizeObserver = new ResizeObserver(([entry]) => { - this.#width = entry.contentRect.width; - this.#updatePosition(); - }); - this.#resizeObserver.observe(this); - } - - override disconnectedCallback(): void { - super.disconnectedCallback(); - this.#resizeObserver?.disconnect(); - this.#resizeObserver = null; - } - - #updatePosition(): void { - const halfWidth = this.#width / 2; - if (this.noClamp) { - this.style.left = `calc(var(--media-slider-pointer) - ${halfWidth}px)`; - } else { - this.style.left = `min(max(0px, calc(var(--media-slider-pointer) - ${halfWidth}px)), calc(100% - ${this.#width}px))`; - } - } - - protected override update(): void { - super.update(); - const ctx = this.#ctx.value; - if (ctx) applyStateDataAttrs(this, ctx.state, ctx.stateAttrMap); - this.#updatePosition(); - } -} -``` - -### 6.2 Registration - -**`packages/html/src/define/ui/slider-preview.ts`:** - -```ts -import { SliderPreviewElement } from '../../ui/slider/slider-preview-element'; -customElements.define(SliderPreviewElement.tagName, SliderPreviewElement); - -declare global { - interface HTMLElementTagNameMap { - [SliderPreviewElement.tagName]: SliderPreviewElement; - } -} -``` - -No changes needed to `packages/html/package.json` — the existing `"./ui/*"` wildcard -covers `@videojs/html/ui/slider-preview` automatically. - -### 6.3 Tests & Verify - -```bash -pnpm -F @videojs/html test src/ui/slider/tests/slider-preview -pnpm -F @videojs/html build -pnpm typecheck -``` - ---- - -## File Structure Summary - -``` -packages/store/src/react/hooks/ - use-snapshot.ts # useSnapshot hook (PR 0) - tests/use-snapshot.test.ts - -packages/utils/src/time/ - format.ts # Fix formatTimeAsPhrase(0) - tests/format.test.ts # Add zero-seconds test - -packages/utils/src/dom/ - raf-throttle.ts # rafThrottle utility (PR 1) - direction.ts # isRTL utility (PR 1) - tests/ - raf-throttle.test.ts - direction.test.ts - -packages/core/src/core/ui/slider/ - slider-core.ts # SliderCore class + types + utilities - time-slider-core.ts # TimeSliderCore class + types - volume-slider-core.ts # VolumeSliderCore class + types - slider-data-attrs.ts # SliderDataAttrs, TimeSliderDataAttrs - slider-css-vars.ts # SliderCSSVars constant - tests/ - slider-core.test.ts - time-slider-core.test.ts - volume-slider-core.test.ts - -packages/core/src/dom/ui/ - event.ts # Add UIPointerEvent - slider.ts # createSlider() factory - slider-css-vars.ts # getSliderCSSVars, getTimeSliderCSSVars - tests/ - slider.test.ts - slider-css-vars.test.ts - -packages/react/src/ui/ - hooks/use-slider.ts # useSlider hook - slider/ - index.ts # export * as Slider - index.parts.ts # Root, Track, Fill, Buffer, Thumb, Value - slider-context.ts # React context - slider-root.tsx - slider-track.tsx - slider-fill.tsx - slider-buffer.tsx - slider-thumb.tsx - slider-preview.tsx # PR 5 - slider-value.tsx - tests/ - slider.test.tsx - time-slider/ - index.ts # export * as TimeSlider - index.parts.ts # Root + re-exports - time-slider-root.tsx - tests/ - time-slider.test.tsx - volume-slider/ - index.ts # export * as VolumeSlider - index.parts.ts # Root + re-exports - volume-slider-root.tsx - tests/ - volume-slider.test.tsx - -packages/html/src/ui/ - slider/ - slider-element.ts # - slider-track-element.ts # - slider-fill-element.ts # - slider-buffer-element.ts # - slider-thumb-element.ts # - slider-value-element.ts # - slider-preview-element.ts # (PR 6) - slider-events.ts # Event type interfaces - slider-context.ts # Lit context for child communication - tests/ - slider-element.test.ts - time-slider/ - time-slider-element.ts # - tests/ - time-slider-element.test.ts - volume-slider/ - volume-slider-element.ts # - tests/ - volume-slider-element.test.ts - -packages/html/src/define/ui/ - slider-track.ts # Shared: - slider-fill.ts # Shared: - slider-buffer.ts # Shared: - slider-thumb.ts # Shared: - slider-value.ts # Shared: - time-slider.ts # Domain: + imports shared - volume-slider.ts # Domain: + imports shared - slider-preview.ts # Registration: preview (PR 6) -``` - -## Resolved Questions - -1. **Idempotent custom element registration** — Extract shared child elements (``, `-fill`, `-buffer`, `-thumb`, `-value`) into their own `define/ui/slider-track.ts`, `define/ui/slider-fill.ts`, etc. Both `time-slider` and `volume-slider` define files import the shared ones as side-effect imports. ES module singleton guarantee ensures each executes only once. No `defineCustomElement` guard utility needed — the shared module extraction is sufficient. - -2. **Seek throttle implementation** — No throttle/debounce exists in `@videojs/utils`. Add `rafThrottle` to `@videojs/utils/dom` — a reusable rAF-based throttle that automatically adapts to device refresh rate. Added to PR 1 as a new utility. API: `const throttled = rafThrottle(fn)` returns a throttled function with a `.cancel()` method. - -3. **RTL detection** — No RTL handling exists anywhere in v10. Add `isRTL(element: Element): boolean` to `@videojs/utils/dom` using `getComputedStyle(element).direction === 'rtl'`. In `createSlider`, the `isRTL()` callback reads this once on `pointerdown` and caches for the drag session. Keyboard arrows flip in RTL per WAI-ARIA slider spec. Added to PR 1 as a new utility. - -4. **Slider Preview positioning** — CSS-only. No Floating UI needed. Vidstack confirms: they use absolute positioning + `min(max(...), ...)` CSS clamping for slider preview, reserving `@floating-ui/dom` only for tooltips/menus. A `ResizeObserver` recalculates clamping when preview content changes size. PRs 5/6 use this approach. - -## Resolved During Audit - -- **`ElementProps` extension** — No. `SliderThumbAttrs` is a standalone type with slider-specific ARIA attrs. It bypasses `ElementProps` and is applied directly via `applyElementProps` to the thumb element. -- **CSS custom properties** — Set via `element.style.setProperty()` in the element's `update()` method, not through `applyElementProps` (which only handles attributes and event listeners). -- **HTML interaction subscription** — Direct `state.subscribe(() => this.requestUpdate(), { signal })` instead of `SnapshotController`. Simpler, uses existing `AbortController` cleanup. -- **Preview branch bases** — Preview PRs branch off their respective UI PRs (React/HTML), not off DOM, because they need the slider context definitions. -- **`StateAttrMap` import** — From `'../types'` (in `packages/core/src/core/ui/types.ts`), not from a nonexistent `element.ts`. -- **Core barrel** — Individual file exports in `packages/core/src/core/index.ts`, no `index.ts` barrel in the slider directory (matching existing convention). diff --git a/.claude/plans/spf-text-track-switching-refactor.md b/.claude/plans/spf-text-track-switching-refactor.md deleted file mode 100644 index ddd2b04f..00000000 --- a/.claude/plans/spf-text-track-switching-refactor.md +++ /dev/null @@ -1,97 +0,0 @@ -# SPF text-track selection → track-switching refactor - -Working reference for migrating text-track selection onto the `track-switching` -rule chain so it gains **constraints** (failed-CDN pruning) and **CDN priority**, -and converting the bidirectional `syncTextTracks` write path to a **user-intent -signal** so `selectedTextTrackId` becomes single-writer. - -Branch: `feat/spf-text-tracks-switching`. Shipped feature docs -(`internal/design/spf/features/subtitles.md`, -`internal/design/spf/text-track-architecture.md`) describe current reality and -get updated at phase 5, not before. - -## Problem - -- `selectTextTrack` (entry-once, opt-in default pick) and `syncTextTracks` (DOM - `change` → write) both write `selectedTextTrackId` — `subtitles.md` flags this - multi-writer slot as "intentionally orthogonal." It's the smell to remove. -- Text selection never runs the rule chain, so it ignores `excludeFailedCdns` - and `preferActiveCdn`. A failed CDN or CDN priority has no effect on captions. - -## Agreed design - -**Intent signal** — `userTextTrackSelection: Partial | 'off' | undefined` - -| Value | Meaning | Resolution | -|---|---|---| -| `undefined` | auto — no user preference | `preferredSubtitleLanguage` config → `DEFAULT=YES + AUTOSELECT=YES` → none | -| `Partial` (language-based) | explicit on | `filterByUserSelection` narrows; `preferActiveCdn` picks surviving-CDN copy; head | -| `'off'` | explicit none | terminal short-circuits to no-selection; sticky through re-eval | - -- **Not cleared on source unload** — mirrors `userVideoTrackSelection` / - `userAudioTrackSelection` (embedder-owned, persistent). Stickiness (sticky - language + sticky off across sources) falls out for free; no clear-on-unload - behavior to add. Config-driven opt-out is a later addition. -- DOM bridge writes a **language-based** partial (mirrors the audio sibling), so - a pick re-resolves per-source. Known limitation: two same-language tracks - differing by forced/characteristics resolve by language alone; the terminal - tie-breaks deterministically. Enrich the partial later if it bites. - -**Ownership** — `selectedTextTrackId` becomes the single-writer **output** of -`switchTextTrack`, cleared per-source by the helper's existing exit cleanup. - -**Text chain** (`setupTrackSwitching` variant): -- constraints: `[excludeFailedCdns]` — no `excludeUnplayableTracks` (`canPlayTrack` - is MSE-based, wrong probe for text; text playability is SPF-parser support). -- rules: `[filterByUserSelection, preferActiveCdn]` -- `resolveSelection: pickResolvedTextTrack` — the one text-specific terminal that - understands `'off'` / auto / explicit + opt-in policy. `filterByUserSelection` - and `preferActiveCdn` stay shared and untouched (the picker, not the filter, - understands `'off'`). - -**Framework touch-point** — `setupTrackSwitching` gains an optional -`resolveSelection(candidates, deps) => string | undefined`, defaulting to the -chain head. Video/audio don't supply it → unchanged. `SelectionKey` / -`UserSelectionKey` each gain one literal. - -**Consumer-facing change** — programmatic selection moves from writing -`selectedTextTrackId` to writing `userTextTrackSelection` (via `shareSignals` / -`onSignalsReady`); `selectedTextTrackId` becomes read-only output. - -## Open items (iterate during implementation) - -- **Echo guard** — once the resolver can override the user (pick `es`, es only on - a failed CDN → resolved `undefined` → mode-mirror disables → `change` event), - the DOM bridge must not write that correction back as "user turned off." - Prototype: track the modes SPF itself sets, write back only genuine user - deltas; fall back to extending the settling window if not worth the state. -- **Same-language ambiguity** — see DOM-bridge note above. -- **Empty candidate set** — keep current behavior: `if (!tracks.length) return` - leaves the prior pick (no caption flicker when every text CDN is cooled down; - recovers on cooldown). `'off'`/auto-declines go through `resolveSelection` - (non-empty set → undefined), distinct from the empty-set case. - -## Phases (TDD per phase, one commit each, check in at each boundary) - -1. **No-selection seam in `setupTrackSwitching`.** Add `resolveSelection` config + - `selectChainHead` default; thread into the effect. Export `setupTrackSwitching` - for tests (module-only, like `applyRules`). Tests: undefined resolution clears - the slot; non-default picker is threaded the chain candidates; video/audio - default path unchanged. -2. **`switchTextTrack` variant + `pickResolvedTextTrack`.** Port `pickTextTrack` - policy into the terminal; handle off/auto/explicit; add `selectedTextTrackId` - to `SelectionKey`, `userTextTrackSelection` to `UserSelectionKey`. Tests: - auto-default, explicit language, off-stays-off across re-eval, failed-CDN - re-resolution, CDN-priority copy selection. -3. **Refactor `syncTextTracks`.** Change-bridge writes `userTextTrackSelection` - (language partial / `'off'`) instead of `selectedTextTrackId`; mode-mirror - still reads `selectedTextTrackId`; echo guard. Tests: DOM pick → intent → - resolved → mode round-trip without echo; resolver override corrects DOM - without write-back. -4. **Remove `selectTextTrack` text path + engine rewire.** Compose - `switchTextTrack` after `deriveCdnPriority` / `setupFailoverMonitor`, before - `resolveTextTrack`; expose `userTextTrackSelection` via `shareSignals`. - Full-engine test. -5. **Docs.** Update `subtitles.md` (state slots → single-writer; implementation - surface) and `text-track-architecture.md` (bidirectional section → intent - model + echo guard). diff --git a/.claude/plans/spf/behaviors-and-media-cleanup-assessment.md b/.claude/plans/spf/behaviors-and-media-cleanup-assessment.md deleted file mode 100644 index a16a11e8..00000000 --- a/.claude/plans/spf/behaviors-and-media-cleanup-assessment.md +++ /dev/null @@ -1,701 +0,0 @@ ---- -status: in-progress -date: 2026-05-07 -branch: fix/spf-behaviors-and-media-cleanup ---- - -# SPF behaviors + media + network — assessment - -> Point-in-time mapping of in-scope code (`packages/spf/src/playback/behaviors/`, `media/`, `network/`) against the [evaluation axes](../../../internal/design/spf/evaluation-axes.md) and the [conventions docs](../../../internal/design/spf/conventions/). **Pure observation** — recommendations land in the cleanup backlog ([`behaviors-and-media-cleanup.md`](behaviors-and-media-cleanup.md)), not here. -> -> Ephemeral by design. The *axes* and *conventions* are durable; the file:line *citations* decay as the code changes. When a citation goes stale, prefer re-running the assessment over patching individual lines. - -## Method - -- Read each file in scope. -- Anchor observations to axis letters (**A** Reusability, **B** Robustness, **C** Patternability, **D** Simplicity, **E** Size) and to specific conventions in [`behaviors.md`](../../../internal/design/spf/conventions/behaviors.md) and [`signals.md`](../../../internal/design/spf/conventions/signals.md). -- `file:line` citations for every claim. -- Skip files with nothing notable. -- Defer recommendations — they live in the backlog tagged with their axes/conventions, not here. - -## Status - -| Section | Files | Status | -| ------- | ----- | ------ | -| Behaviors (`packages/spf/src/playback/behaviors/`) | 17 | complete | -| Media (`packages/spf/src/media/`) | 17 | complete | -| Network (`packages/spf/src/network/`) | 2 | complete | - -## Already-tracked follow-ups (excluded from new findings) - -These are known follow-ups documented elsewhere; the assessment notes them when encountered but does not re-flag: - -- Code-reuse compromise across `select-tracks`, `resolve-track`, `load-segments` ([`discrete-signals-and-behavior-objects.md`](discrete-signals-and-behavior-objects.md) → "Code-reuse compromise"). -- `selectedVideoTrackId` decomposition (`abrDisabled` cleanup) — `quality-switching.ts` TODO. -- Writer-audit lint rule. -- `setup-mediasource.ts:63` "Reactor with internal finite state" comment — likely predates `createMachineReactor`. - -## Behaviors - -### Per-file findings - -#### `packages/spf/src/playback/behaviors/calculate-presentation-duration.ts` - -**Shape**: simple (single `effect`, no primitives reached for) - -**Axis findings**: -- A: `getDurationFromResolvedTracks` hardcodes "video first, audio fallback" (calculate-presentation-duration.ts:46-62) — audio-only on the pressure list does not force a rewrite (it falls through), but multi-language audio + audio ABR don't change which audio-track's duration is read; assumption is "all selected tracks of a type agree on duration." -- C: Helper functions `canCalculateDuration` / `shouldCalculateDuration` / `getDurationFromResolvedTracks` are pure, exported, named with `can*`/`should*` prefixes (calculate-presentation-duration.ts:17-63) — same pattern as `update-duration.ts` and `end-of-stream.ts` but with no convention doc coverage. -- D: `shouldCalculateDuration` re-derives the same `getSelectedTrack` lookup that `getDurationFromResolvedTracks` then redoes (calculate-presentation-duration.ts:35-39, 47-58). - -**Convention findings**: -- Wide-shape state typing: `PresentationDurationState` is exported as a wide interface and re-used in helper signatures (calculate-presentation-duration.ts:8-12, 17, 26, 45) — the setup itself does narrow with `Pick`-like inline shape (calculate-presentation-duration.ts:71-75), but the helpers don't, going against the "narrow with `Pick`" convention (behaviors.md "Narrow the slice with `Pick`"). -- `presentation` slot typed `Signal<...>` (write); `selectedVideoTrackId` and `selectedAudioTrackId` typed `ReadonlySignal<...>` (calculate-presentation-duration.ts:72-74) — read/write intent annotation is correct. - -**Code sniffs**: -- fix-in-place: the same `getSelectedTrack` lookup is performed in two helpers per call (calculate-presentation-duration.ts:47-58). - -#### `packages/spf/src/playback/behaviors/load-text-track-cues.ts` - -**Shape**: primitive-augmented (factory: `createMachineReactor`) - -**Axis findings**: -- A: Hardcoded "first switching set, type=text" assumption (load-text-track-cues.ts:60) — multi-language audio/text via multiple switching sets isn't supported. -- B: `untrack(() => context.segmentLoaderActor.get())` followed by non-null assertion (load-text-track-cues.ts:144-145) — comment claims `deriveState` guarantees presence in this state; correctness depends on the monitor running before the per-state effect when an actor is unset, which is implicit ordering. -- C: `deriveState` + `monitor: () => derivedStateSignal.get()` pattern matches `resolve-presentation.ts` and `track-playback-initiated.ts` (load-text-track-cues.ts:75-90, 125, 131) — emergent convention not in docs. -- D: `derivedStateSignal`, `currentTimeSignal`, `selectedTrackSignal` all created up-front (load-text-track-cues.ts:125-127) — `selectedTrackSignal` is only read in one place (load-text-track-cues.ts:143), `currentTimeSignal` only in one place (load-text-track-cues.ts:142). - -**Convention findings**: -- All state slots typed `ReadonlySignal<...>` (load-text-track-cues.ts:114-123) — zero writers; matches "consume-only" usage but no `initialState` seed declaration is visible at this layer (would be in the engine assembly). -- Per-type specialization not applied — text-track cue loading is type-bound (only text), where the doc says "co-locate per-type in one module" (behaviors.md "Per-type specialization") — this is a single-purpose module so the rule doesn't apply, but the body's coupling to `'text'` (load-text-track-cues.ts:60) is implicit. - -#### `packages/spf/src/playback/behaviors/quality-switching.ts` - -**Shape**: simple (single `effect`) - -**Axis findings**: -- A: TODO at quality-switching.ts:16-20 explicitly names the `abrDisabled` decomposition pressure point — known follow-up. -- A: `safetyMargin`, `minUpgradeInterval`, `initialBandwidth` are configurable (quality-switching.ts:28-49) — accommodates per-asset tuning without surgery; flat `videoTracks` only (quality-switching.ts:65-67) hardcodes "first switching set" — multi-bitrate audio (audio ABR) on the pressure list would need a peer factory or audio path. -- B: `lastUpgradeTime` + `firstMeaningfulFire` are closure-captured flags (quality-switching.ts:109-110) carrying state across `effect` re-runs — re-creating the behavior resets them. -- C: Closure flags acting as mini state machine (quality-switching.ts:109-110, 123-124, 138-139) — behaviors.md "fight the shape" sniff for body manually serializing work via flags. -- D: The body has two interleaved concerns (gating ABR via `abrDisabled` and the upgrade-interval gate) wrapped in one `effect` (quality-switching.ts:112-143). -- E: `selectQuality(videoTracks as any, ...)` cast (quality-switching.ts:127) — defensive compatibility cast. - -**Convention findings**: -- Documented multi-writer slot: `selectedVideoTrackId: Signal<...>` at quality-switching.ts:96 paired with the same slot also written by `setupTrackResolution` consumers — the "intent + default" pattern; TODO at quality-switching.ts:16-20 calls out the planned decomposition (signals.md multi-writer, also called out as known follow-up). -- `abrDisabled === true` boolean equality (quality-switching.ts:116) bypasses the closure rather than transitioning out — quality-switching.ts:22 confirms it's a "blunt instrument." - -**Code sniffs**: -- fight-the-shape: hand-rolled FSM via closure flags (quality-switching.ts:109-110, 123-124, 138-139) — `createMachineReactor` is the documented target for this shape. - -#### `packages/spf/src/playback/behaviors/resolve-presentation.ts` - -**Shape**: primitive-augmented (factory: `createMachineReactor`) - -**Axis findings**: -- B: `entry` returns the `AbortController` (resolve-presentation.ts:114) — relies on framework abort-on-state-exit; cleanup contract is correct. -- B: Catch arm checks `error.name === 'AbortError'` and rethrows otherwise (resolve-presentation.ts:109-112) — silent "throw" inside a then-chain has no observable error surface (no error slot, no rejection handler). -- C: Single `monitor: () => derivedStateSignal.get()` driven by exported `deriveState` (resolve-presentation.ts:60-65, 88, 92) — the recurring convention shape. - -**Convention findings**: -- `presentation: Signal<...>`, others `ReadonlySignal<...>` (resolve-presentation.ts:83-86) — clean single-writer. -- `shouldResolve` and `canResolve` are exported as standalone helpers (resolve-presentation.ts:30-47) — same exported-helpers pattern as `calculate-presentation-duration.ts` and `end-of-stream.ts`. -- `canResolve` returns a TS predicate `value is ...` (resolve-presentation.ts:43-47) — matches CLAUDE.md type-guard convention. - -#### `packages/spf/src/playback/behaviors/resolve-track.ts` - -**Shape**: primitive-augmented (primitive: `Task` + `ConcurrentRunner`); per-type specialization via three exports - -**Axis findings**: -- A: `setupTrackResolution` is parameterized over `selectedKey` and a finder (resolve-track.ts:86-93) — accommodates new track types without rewrite. -- B: Stale-snapshot problem and resolution documented inline (resolve-track.ts:115-123) — explicit fix for concurrent-task-write race. -- C: Inline TODO comments at resolve-track.ts:94-96 and resolve-track.ts:108-109 propose factoring the runner and Task creation into `createTaskRunner` / `createResolveTrackTask` (known follow-up). -- D: Ad-hoc `runner` lifecycle (resolve-track.ts:97, 130-133) — one-off shape; not a known primitive yet. - -**Convention findings**: -- Per-type specialization with a typed helper: matches behaviors.md "code-reuse compromise" mention and `select-tracks.ts` shape (resolve-track.ts:86-134, 145-178). -- `presentation: Signal<...>` writer; `selected*TrackId` readers (resolve-track.ts:82-84) — correct. -- `setupTrackResolution` returns `() => { runner.abortAll(); cleanup(); }` (resolve-track.ts:130-133) — uses two-step cleanup composed manually rather than `AbortController` (CLAUDE.md "Cleanup Pattern" suggests `AbortController` for multiple cleanups). - -**Code sniffs**: -- no-good-fit-yet: `ConcurrentRunner` + per-task ID is hand-rolled scheduling (resolve-track.ts:97, 107-127); the inline TODOs (resolve-track.ts:94-96, 108-109) match behaviors.md "hold the sniff visibly." - -#### `packages/spf/src/playback/behaviors/select-tracks.ts` - -**Shape**: simple (one `effect` per export); per-type specialization with three exports - -**Axis findings**: -- A: `pickFirstTrackId` for video/audio (select-tracks.ts:19-21) — known POC; comment at select-tracks.ts:14-18 says it'll be replaced by `pickVideoTrack` / `pickAudioTrack` once selection algorithm matures (multi-language audio, BCP-47). -- C: `setupTrackSelection` shares orchestration with `resolve-track.ts:setupTrackResolution` — same shape (helper bound at module load by selectedKey + variant fn) (select-tracks.ts:39-50). -- D: `selectTextTrack` config flows through as `Omit` and re-injects `type: 'text'` (select-tracks.ts:106-110) — a minor type-shape mismatch handled at the call site. - -**Convention findings**: -- `presentation: ReadonlySignal<...>`; `selected*TrackId: Signal<...>` (select-tracks.ts:36-37) — writer typing matches the multi-writer with quality-switching pattern documented in signals.md. -- Per-type specialization via a typed helper: matches behaviors.md `select-tracks` referenced as the canonical example. - -#### `packages/spf/src/playback/behaviors/sync-preload-attribute.ts` - -**Shape**: simple (single `effect`) - -**Axis findings**: -- C: Cited as the canonical "simple" example in behaviors.md:21-24 and signals.md:21-29 — the file matches the doc. - -**Convention findings**: -- `Pick` to narrow (sync-preload-attribute.ts:20) — matches behaviors.md "Narrow the slice with `Pick`". -- `state: { preload: Signal<...> }`, `context: { mediaElement: ReadonlySignal<...> }` (sync-preload-attribute.ts:36-37) — single read, single write; canonical. -- `mediaElement` is wrapped in `computed(() => context.mediaElement.get())` (sync-preload-attribute.ts:39) — a tracked alias of an already-tracked signal; no observable behavior difference. - -**Code sniffs**: -- fix-in-place: `computed` wrap of `context.mediaElement.get()` (sync-preload-attribute.ts:39) adds a layer with no observable benefit. - -#### `packages/spf/src/playback/behaviors/dom/end-of-stream.ts` - -**Shape**: primitive-augmented (helpers + ad-hoc `effect`-inside-Promise; no factory) - -**Axis findings**: -- A: `videoBuffer`/`audioBuffer` hardcoded throughout — `getMaxBufferedEnd` iterates `[videoBuffer, audioBuffer]` (end-of-stream.ts:209), audio-only/video-only handled but multi-language audio (multiple audio buffers) would force a rewrite. -- B: `hasEnded` flag with re-entry window note (end-of-stream.ts:292, 297-308) — comments document the exact race; flag is set "before awaiting" to close the window. Async task fires from inside an `effect` (end-of-stream.ts:309). -- B: `waitForSourceBuffersReady` creates `effect()` inside `new Promise()` with manual `resolved` flag and `queueMicrotask` cleanup (end-of-stream.ts:170-200) — comment explains why the cleanup is microtask-deferred. -- C: Same `can*`/`should*`/`is*` exported-helper layout as `update-duration.ts` and `calculate-presentation-duration.ts` (end-of-stream.ts:78-163). -- D: `shouldEndStream` is ~60 lines reading 6 context slots and 4 state slots (end-of-stream.ts:110-163); behavior body is 50+ lines. -- E: Defensive non-null assertions throughout (end-of-stream.ts:229, 238, 247, 250). - -**Convention findings**: -- All slots `ReadonlySignal<...>` (end-of-stream.ts:269-283) — zero direct writes; the only write side-effect is `mediaSource.endOfStream()` and `mediaSource.duration =` outside the signal graph (end-of-stream.ts:247, 250). -- `state.mediaSourceReadyState` reactive mirror documented as the dependency anchor (end-of-stream.ts:286-289) — relates obliquely to signals.md "Maintaining a parallel state slot for an Actor's snapshot" (the actor is not formalized; readyState is a DOM property mirrored to keep `computed` re-evaluating). - -**Code sniffs**: -- augment-with-a-primitive: ~50-line `effect` body with internal flag (end-of-stream.ts:292-310); comment at end-of-stream.ts:73-92 ("right long-term condition") implies a future re-shape. -- fight-the-shape: `effect()` instantiated inside `new Promise()` with a `resolved` boolean and `queueMicrotask` cleanup (end-of-stream.ts:188-196) — a bespoke once-when-condition-met pattern, no convention coverage. -- no-good-fit-yet: `waitForSourceBuffersReady` (end-of-stream.ts:170-200) — "wait for actor to leave updating state" reads as a primitive shape that recurs (also seen in `update-duration.ts:79-91` waiting via `addEventListener('updateend')` instead of actor-aware). - -#### `packages/spf/src/playback/behaviors/dom/load-segments.ts` - -**Shape**: primitive-augmented (Actor: `createSegmentLoaderActor`; multiple effects + computeds + local signals) - -**Axis findings**: -- A: `MediaTrackType` is `'video' | 'audio'` (load-segments.ts:18 imports from `setup-sourcebuffer.ts`); audio ABR is split into a `type === 'video'` branch for tracked-fetch (load-segments.ts:284); audio-only is handled. -- B: Local `throughput` signal kept separate from `state.bandwidthState` and bridged via `onSample` (load-segments.ts:270-278, 64-66) — duplicate state by design, with the comment explaining why (load-segments.ts:280-283). -- B: `prevInputs` closure-state used in equality check (load-segments.ts:346-354) — replaces a prior combineLatest pattern; only updated when a message is sent. -- C: Helper-and-factory style (`setupSegmentLoading` plus per-type `loadVideoSegments`/`loadAudioSegments` exports) matches `select-tracks.ts`/`resolve-track.ts` (load-segments.ts:260-265, 400-426). -- D: 165 lines of body in `setupSegmentLoading` (load-segments.ts:260-379), three nested computeds, two effects, two local signals (`throughput`, `segmentLoader`), and a captured `currentLoader`. -- E: Two `@ts-expect-error` calls (load-segments.ts:359, 364) — type system is not capturing the actor message contract. - -**Convention findings**: -- `bandwidthState` is `Signal<...>` only in `loadVideoSegments` (load-segments.ts:411), `ReadonlySignal<...>` in `loadAudioSegments` and the shared `SegmentLoadingStateMap` (load-segments.ts:245, 424) — per-export read/write divergence handled via `Omit` type intersection (load-segments.ts:410-412); legitimate per-spec, but not documented as a pattern. -- `selectedTextTrackId` declared in `SegmentLoadingStateMap` (load-segments.ts:250, 389) but never read in the body — leftover key. -- `signal(undefined)` (load-segments.ts:287) and `signal(...)` (load-segments.ts:270) — local signals not part of the composition's signal map; relates to signals.md "Maintaining a parallel state slot for an Actor's snapshot" risk for `segmentLoader` (the loader's snapshot is itself a signal but is not exposed directly). - -**Code sniffs**: -- fight-the-shape: `prevInputs !== undefined && loadingInputsEq(prevInputs, inputs)` (load-segments.ts:349) is a hand-rolled "skip-if-equivalent" gate; the equality function `loadingInputsEq` is a 40-line condition hierarchy (load-segments.ts:177-203) inline-encoded as a comparator. -- augment-with-a-primitive: actor-lifecycle `effect` with `currentLoader` captured ref (load-segments.ts:295-310) — "creates a resource ... that has its own lifecycle and needs disposing" (behaviors.md "augment with a primitive"). -- no-good-fit-yet: the local `throughput` Signal bridged via callback (load-segments.ts:270, 64-66, 414) is a one-off pattern for "private state mirrored back to shared state" not covered by current primitives. - -#### `packages/spf/src/playback/behaviors/dom/setup-mediasource.ts` - -**Shape**: primitive-augmented (nested `effect`s; no factory) - -**Axis findings**: -- B: `abortController.abort()` from outer cleanup propagates to `onMediaSourceReadyStateChange` only via `signal: abortSignal` (setup-mediasource.ts:50, 74, 91) — outer effect `return cleanup` sequence is documented but inner effect's cleanup runs from the outer's return (setup-mediasource.ts:79-87). -- C: NOTE comment at setup-mediasource.ts:63 explicitly identifies the fight-the-shape sniff and predates `createMachineReactor` (called out as known follow-up; behaviors.md:130 references this exact line). -- D: `canSetupSignal`, `shouldSetupSignal`, `mediaElementSrcSignal`, `mediaSourceSignal` — four computeds feeding two nested effects (setup-mediasource.ts:53-67). - -**Convention findings**: -- `state.mediaSourceReadyState: Signal<...>`; others `ReadonlySignal<...>` (setup-mediasource.ts:42-48) — read/write intent OK. -- `context.mediaSource: Signal<...>` writer (setup-mediasource.ts:48) — the behavior owns the resource and writes the slot exposing it. - -**Code sniffs**: -- fight-the-shape: nested `effect()` inside `effect()` (setup-mediasource.ts:66, 79) gated by flag-shaped computeds (`canSetupSignal`/`shouldSetupSignal`) — exactly the pattern called out in behaviors.md "fight the shape" + "Worked example: where setup-mediasource.ts falls today." - -#### `packages/spf/src/playback/behaviors/dom/setup-sourcebuffer.ts` - -**Shape**: simple (single `effect`, plus computeds; no factory) - -**Axis findings**: -- A: Track types are derived from the presentation rather than hardcoded (setup-sourcebuffer.ts:91-97) — audio-only and video-only handled. `MediaTrackType = 'video' | 'audio'` (setup-sourcebuffer.ts:14) — adding more bufferable types would force a rewrite. -- B: All buffers created synchronously between `addSourceBuffer` calls (setup-sourcebuffer.ts:128-136) — comment explains the Firefox bug being avoided (setup-sourcebuffer.ts:60-65). State commit is per-iteration, not batched. -- C: `canSetupSignal` / `shouldSetupSignal` (setup-sourcebuffer.ts:100-117) — same flag-shaped FSM pattern as `setup-mediasource.ts:57, 61` and `quality-switching.ts:109-110`. -- D: Computeds re-snapshot full state inside their bodies (setup-sourcebuffer.ts:103-107, 121-126) — the same `s` shape is built twice. - -**Convention findings**: -- Multiple writes to `context.videoBuffer` / `audioBuffer` / `videoBufferActor` / `audioBufferActor` (setup-sourcebuffer.ts:84-87, 134-135) — single-writer per behavior; `setupSourceBuffers` is the canonical owner of these slots. - -**Code sniffs**: -- fight-the-shape: `if (!canSetupSignal.get() || !shouldSetupSignal.get()) return;` (setup-sourcebuffer.ts:120) — same flag-shaped FSM-in-an-effect as `setup-mediasource.ts`. - -#### `packages/spf/src/playback/behaviors/dom/setup-text-track-actors.ts` - -**Shape**: simple (single `effect`; resource-owning) - -**Axis findings**: -- A: Cue parser injected via `config.resolveTextTrackSegment` (setup-text-track-actors.ts:33-35, 76) — extensible to new cue formats without rewrite. -- B: Cleanup destroys actors and clears the slots (setup-text-track-actors.ts:80-85) — actor lifecycle bound to behavior teardown; matches behaviors.md "Behaviors that own an Actor without disposing it in cleanup. Always a leak." -- C: Subscribes to a `computed` projection of `mediaElement` rather than the full context signal (setup-text-track-actors.ts:69-72) — avoids self-trigger from writing other context slots; comment explains (setup-text-track-actors.ts:46-49). - -**Convention findings**: -- `mediaElement: ReadonlySignal<...>`, `textTracksActor: Signal<...>`, `segmentLoaderActor: Signal<...>` (setup-text-track-actors.ts:62-66) — single owner of those two context slots. -- `stateKeys: []` (setup-text-track-actors.ts:90) — context-only behavior; no `Pick` from a state interface is needed. - -#### `packages/spf/src/playback/behaviors/dom/sync-text-tracks.ts` - -**Shape**: primitive-augmented (factory: `createMachineReactor`) - -**Axis findings**: -- B: Settling-window guard via `setTimeout(0)` (sync-text-tracks.ts:143-145) and a `change`-listener that re-applies modes if it fires inside the window (sync-text-tracks.ts:147-156) — bespoke timing-based defense against browser auto-selection. -- B: `effects` cleanup uses `clearTimeout(syncTimeout ?? undefined)` and `unlisten()` (sync-text-tracks.ts:173-176) — manual ordering; `entry`'s cleanup runs separately (sync-text-tracks.ts:127-132). -- C: `computed` with a hand-rolled `equals` for arrays (sync-text-tracks.ts:95-109) — TODO at sync-text-tracks.ts:96 suggests "make generic and abstract away for `Array | undefined`" — recurring shape, not covered by signals.md. -- D: Two-state machine (`'preconditions-unmet'` and `'set-up'`) (sync-text-tracks.ts:113-181) — entry creates DOM, exit cleans up; minimal but the `effects` body has both mode-sync and the change listener. - -**Convention findings**: -- `selectedTextTrackId: Signal<...>` (sync-text-tracks.ts:90) — the behavior writes it (DOM-driven write back to model); paired with `selectTextTrack`/`syncTextTracks` writes — multi-writer not in the documented "intent + default" / "pipeline" / "two-way DOM sync" list (signals.md:46-49) but matches "two-way DOM sync." -- `entry`'s body is "automatically untracked" — comment notes it (sync-text-tracks.ts:122-123); `effects` uses explicit `untrack(() => mediaElementSignal.get())` (sync-text-tracks.ts:139, 154, 166). - -**Code sniffs**: -- fix-in-place: `clearTimeout(syncTimeout ?? undefined)` (sync-text-tracks.ts:174) — `?? undefined` is a no-op since `syncTimeout` is already `... | undefined`. - -#### `packages/spf/src/playback/behaviors/dom/track-current-time.ts` - -**Shape**: simple (single `effect`) - -**Axis findings**: -- B: `lastMediaElement` and `removeListeners` are closure-captured imperative bookkeeping (track-current-time.ts:52-53, 60-62) — manually tracking element identity to avoid double-binding. -- D: Cleanup composed manually as `() => { removeListeners?.(); cleanupEffect(); }` (track-current-time.ts:80-83) — listeners are attached outside the effect and torn down manually rather than via the effect's cleanup return. - -**Convention findings**: -- `state.currentTime: Signal<...>`, `context.mediaElement: ReadonlySignal<...>` (track-current-time.ts:49-50) — clean. - -**Code sniffs**: -- fight-the-shape: `effect()` doing imperative `addEventListener` plumbing for events that drive a signal (track-current-time.ts:55-78) — behaviors.md "fight the shape" lists this exact pattern. - -#### `packages/spf/src/playback/behaviors/dom/track-playback-initiated.ts` - -**Shape**: primitive-augmented (factory: `createMachineReactor`) - -**Axis findings**: -- C: Same `deriveState` + `monitor: () => derivedStateSignal.get()` pattern as `resolve-presentation.ts` and `load-text-track-cues.ts` (track-playback-initiated.ts:39-46, 73, 79). -- D: Three states (track-playback-initiated.ts:80-110) for "watch element + URL while playback active" — `effects` reads two signals to track them and returns a single cleanup. - -**Convention findings**: -- `state.playbackInitiated: Signal<...>`; `state.presentation: ReadonlySignal<...>`; `context.mediaElement: ReadonlySignal<...>` (track-playback-initiated.ts:67-71) — clean. -- Two writes to `playbackInitiated` (track-playback-initiated.ts:88, 90, 108) — entry sets to `!el.paused`; exit cleanup sets to `false`. Single behavior owns the slot. - -#### `packages/spf/src/playback/behaviors/dom/track-playback-rate.ts` - -**Shape**: simple (single `effect`) - -**Axis findings**: -- (nothing notable beyond conformance) - -**Convention findings**: -- `state.playbackRate: Signal<...>`, `context.mediaElement: ReadonlySignal<...>` (track-playback-rate.ts:33-34) — clean. -- `canTrackPlaybackRate` computed (track-playback-rate.ts:37) — single-line predicate read once. - -**Code sniffs**: -- fix-in-place: `canTrackPlaybackRate` computed wraps a one-line truthiness check used once (track-playback-rate.ts:36-37, 40) — adds a layer with no observable benefit. -- fight-the-shape: imperative `addEventListener` for an event that drives a signal (track-playback-rate.ts:42-47) — same pattern as `track-current-time.ts:72-73` and `sync-text-tracks.ts:171`. - -#### `packages/spf/src/playback/behaviors/dom/update-duration.ts` - -**Shape**: simple (single `effect` with async-fork; no factory) - -**Axis findings**: -- B: `running` and `destroyed` flags (update-duration.ts:110-111, 117, 120, 128, 142, 147) protect against re-entry and post-destroy writes — comment explains the race (update-duration.ts:122-128). -- B: `Number.isNaN(mediaSource.duration)` as a "has not been set yet" gate (update-duration.ts:67-71) — comment explicitly warns that re-syncing races with `loadSegmentsTask`. -- C: `can*`/`should*` exported helpers (update-duration.ts:22-71) — same pattern as `end-of-stream.ts` and `calculate-presentation-duration.ts`. -- D: Manual `running`/`destroyed` flags + `.finally(() => running = false)` (update-duration.ts:117, 120, 142) — matches behaviors.md "Body manually serializes work via flags... `createTransitionActor` — Actor message handling already serializes." - -**Convention findings**: -- All slots `ReadonlySignal<...>` (update-duration.ts:101-108) — zero writes; effect mutates `mediaSource.duration` directly (update-duration.ts:139), bypassing the signal graph (the slot `mediaSourceReadyState` is mirrored to keep the dependency tracked). - -**Code sniffs**: -- fight-the-shape: `running` flag as a job lock (update-duration.ts:111, 117, 120, 142) — behaviors.md sniff "Body manually serializes work via flags (`isLoading`, `pending`, `inFlight`)" → `createTransitionActor`. -- no-good-fit-yet: `waitForSourceBuffersReady` (update-duration.ts:79-91) using `addEventListener('updateend')` — same shape as `end-of-stream.ts:170-200` but using DOM events vs actor snapshot, **inconsistent across files solving the same problem**. - -### Cross-cutting - -#### Recurring patterns - -- **`deriveState` + `monitor: () => derivedStateSignal.get()`** as the canonical machine-reactor wiring (resolve-presentation.ts:60-65, 88; load-text-track-cues.ts:75-90, 131; track-playback-initiated.ts:39-46, 73, 79; sync-text-tracks.ts:111-115). Convention not in `behaviors.md`. -- **Exported `can*` / `should*` / `is*` predicate helpers** taking `(state, context)` or `(state)` (calculate-presentation-duration.ts:17-40; end-of-stream.ts:78-163; update-duration.ts:22-71; resolve-presentation.ts:30-47). Convention not in `behaviors.md`. -- **Per-type specialization with a typed `setupX` helper** at module scope (resolve-track.ts:86-134, 145-178; select-tracks.ts:39-50, 64-112; load-segments.ts:260-379, 400-426). Documented in behaviors.md as the chosen compromise; the specific shape (helper + key + variant fn + per-export `defineBehavior`) is the same across all three but not codified beyond the conventions doc's narrative. -- **Hand-rolled flag-shaped FSM in an `effect` body**: `canSetup`/`shouldSetup` computeds gating an effect (setup-mediasource.ts:57, 61, 66; setup-sourcebuffer.ts:100, 115, 119); job-lock flags (update-duration.ts:111, 117, 120; end-of-stream.ts:292-308; quality-switching.ts:109-110, 138). All match the "fight-the-shape" sniffs in behaviors.md. -- **Imperative `addEventListener` plumbing for events that drive signals**: `listen(...)` inside an effect body, with manual cleanup (track-current-time.ts:72-77; track-playback-rate.ts:42-47; sync-text-tracks.ts:171; track-playback-initiated.ts:89-91). Behaviors.md "fight the shape" sniff. -- **`computed` that wraps a single `signal.get()`** with no other logic (sync-preload-attribute.ts:39; setup-mediasource.ts:53, 60; track-playback-rate.ts:36; setup-text-track-actors.ts:69; track-playback-initiated.ts:74-75; sync-text-tracks.ts:94, 110). No semantic effect; pattern is uniform but unmotivated by signals.md. -- **`snapshot(state)`/`snapshot(context)` then operating on plain objects in helper functions** (calculate-presentation-duration.ts:78; quality-switching.ts:113; end-of-stream.ts:290, 296; track-playback-initiated.ts:73; load-text-track-cues.ts:125; resolve-presentation.ts:88; update-duration.ts:114-115). The pure-functions-take-snapshot pattern is consistent. -- **`waitFor*` helpers returning `Promise` driven by either DOM events or actor snapshots** (end-of-stream.ts:170-200 via `effect()` over actor snapshots; update-duration.ts:79-91 via `addEventListener('updateend')`). Solve the same problem two different ways. - -#### Conventions doc gaps - -- **`deriveState`/`monitor` shape** for `createMachineReactor` is not documented in `behaviors.md` despite being the recurring pattern in 4 of 5 reactor-using behaviors. The conventions doc names `createMachineReactor` as the target but does not show the canonical "exported `deriveState` + `monitor` reading a single computed" structure. -- **Exported `can*`/`should*`/`is*` predicate helpers** colocated with behaviors are widespread (4 files) but not classified in behaviors.md "Helpers and behavior factories" — the doc covers helpers operating on already-resolved values inside a body, but not exported predicate helpers used by external tests/composition. -- **`computed` with a custom `equals` function** for arrays (sync-text-tracks.ts:95-109, with an inline TODO suggesting a reusable abstraction) — signals.md "Follow-ups" mentions "computed best practices" but no current guidance. -- **Local-signal-as-private-state inside a behavior body**, bridged back to engine state via a callback (load-segments.ts:270-278, 287-309) — a private parallel signal not in the composition's signal map; signals.md anti-patterns mention parallel-state-for-actor-snapshot but not this private-mirror-then-callback bridge. -- **Manual cleanup composition** vs. `AbortController` is uneven: CLAUDE.md/behaviors.md prescribes `AbortController` for multiple cleanups, but most multi-cleanup behaviors compose `() => { x(); y(); }` manually (resolve-track.ts:130-133; track-current-time.ts:80-83; load-segments.ts:374-378; setup-mediasource.ts:90-93; update-duration.ts:146-149). Only setup-mediasource.ts uses `AbortController` and it's for a single signal-based cleanup. -- **Multi-writer slot not on the documented list**: `selectedTextTrackId` is written by `selectTextTrack` (select-tracks.ts:48), `syncTextTracks` (sync-text-tracks.ts:131, 168). This is a "two-way DOM sync" + "intent + default" hybrid — fits signals.md:46-49 generously but isn't called out in the writer-audit examples. -- **Async-task-inside-`effect` with re-entry guard flag** (end-of-stream.ts:292-310; update-duration.ts:110-145) — neither matches `Task + Runner` (no scheduling) nor an Actor (no message inbox); the shape is recurrent enough that "no good fit yet" applies, but behaviors.md doesn't list "single-shot async work gated by a derived condition" as a known unfit shape. - -#### Cross-file inconsistencies - -- **"Wait for SourceBuffers to be idle" implemented twice with different mechanisms**: `end-of-stream.ts:170-200` uses `effect()` over the actor's `snapshot.value !== 'updating'`; `update-duration.ts:79-91` uses `addEventListener('updateend')` on the raw `SourceBuffer.updating` boolean. Same problem, two abstractions (signal-based vs DOM-event-based). -- **Cleanup composition** — `AbortController` used in `setup-mediasource.ts:50, 91`; manual `() => { a(); b(); }` in `resolve-track.ts:130-133`, `track-current-time.ts:80-83`, `load-segments.ts:374-378`, `update-duration.ts:146-149`. Behaviors.md prescribes `AbortController` for multi-cleanup; behaviors don't follow it consistently. -- **`computed`-wrap of a single `.get()`** is applied unevenly: `sync-preload-attribute.ts:39` wraps `context.mediaElement` but `track-current-time.ts:56` reads it directly. No discernible rule. -- **Per-type specialization helper signature** is consistent in `select-tracks.ts` and `resolve-track.ts` (helper takes `state`, `selectedKey`, variant fn) but `load-segments.ts:260-265` uses a slightly different shape (helper takes `state`, `context`, `type`, `onThroughputSample`) — track type rather than key, plus an extra callback. The "shared via a typed helper" pattern is recognizable but not a consistent signature. -- **`monitor` body** is uniformly a one-liner reading a `derivedStateSignal` *except* `sync-text-tracks.ts:115`, which writes the literal mapping inline (`preconditionsMetSignal.get() ? 'set-up' : 'preconditions-unmet'`). Both work; only one matches the recurring `deriveState` shape. -- **State slot with multiple writers** is annotated `Signal<...>` in both writers (quality-switching.ts:96, select-tracks.ts:37 for `selectedVideoTrackId`/`selectedTextTrackId`; sync-text-tracks.ts:90 for `selectedTextTrackId`) — correct per signals.md, but tracking is implicit. - -## Media - -`packages/spf/src/media/` — DOM-free / signal-free except for `media/dom/` subdirs. Behaviors and signals conventions docs do not strongly apply; findings are anchored on axes and CLAUDE.md general code rules. - -### Per-file findings - -#### `packages/spf/src/media/abr/bandwidth-estimator.ts` - -**Purpose**: Dual-EWMA bandwidth estimator producing `min(fast, slow)` bandwidth estimate. - -**Axis findings**: -- A: Estimator is bandwidth-only — no rendition/track-type tagging on samples; a single shared state combines all downloads regardless of whether they're audio, video, or text segments (bandwidth-estimator.ts:23-34, bandwidth-estimator.ts:85-123). Multi-bitrate audio (audio ABR) on the pressure list shares the same estimator state with video. -- D: Pure functional shape — state-in/state-out, no class machinery (bandwidth-estimator.ts:85-123). - -**Repo-rule findings**: -- Rich JSDoc with `@param`, `@returns`, `@example` on internal (non-API-reference) helpers (bandwidth-estimator.ts:65-83, 125-141, 163-176) — exceeds the "minimal JSDoc" rule for non-public-API exports. - -**Notable assumptions**: -- Single global estimator state; no per-track-type partitioning (bandwidth-estimator.ts:23-34) — touches axis A pressure list (audio ABR). -- Constants `8000`, `1000` inline in conversion math (bandwidth-estimator.ts:110, 114). - -#### `packages/spf/src/media/abr/ewma.ts` - -**Purpose**: Pure functional EWMA primitives (alpha, weighted average, zero-factor correction). - -**Repo-rule findings**: -- `@param`/`@returns`/`@example` on every export (ewma.ts:8-23, 26-43, 49-67) — non-API-reference exports go above minimal-JSDoc. - -#### `packages/spf/src/media/abr/quality-selection.ts` - -**Purpose**: Bandwidth-driven track selector with safety-margin / resolution tiebreak. - -**Axis findings**: -- A: Hardcoded to `PartiallyResolvedVideoTrack` — signature locks selection to video tracks only (quality-selection.ts:13, 58-62). Audio ABR (pressure list) requires a different track type; no generic `Track`-shaped overload exists. -- A: Resolution tiebreak baked in (quality-selection.ts:80-86, 102-106) — irrelevant for audio quality selection (which would tiebreak on bitrate / channels). -- A: Consumer cannot cap by max resolution — Mux 1080p+ resolution cap pressure-list item is not exposed via config (quality-selection.ts:18-25). -- D: `selectQuality` does a sort followed by a linear scan + assignment to find the last matching element (quality-selection.ts:67-92). - -**Repo-rule findings**: -- Verbose JSDoc with multiple `@example` blocks (quality-selection.ts:35-58, 94-101). - -**Notable assumptions**: -- Video-only selection (quality-selection.ts:58-62) — touches axis A pressure list (multi-bitrate audio). -- 1080p cap not configurable (quality-selection.ts:18-25) — touches axis A pressure list (Mux platform). - -#### `packages/spf/src/media/buffer/back-buffer.ts` - -**Purpose**: Compute back-buffer flush point by "keep N segments behind currentTime." - -**Axis findings**: -- A: Strategy is segment-count-based; no time-based or memory-pressure-based variant (back-buffer.ts:13-19, 56-91). Mux buffer-stall recovery / quota-pressure scenarios on the pressure list have no swappable interface. -- B: `segments` parameter is documented as "should be sorted by startTime" (back-buffer.ts:42) but the function does not sort or assert — silent breakage if caller passes unsorted. -- D: Five branching returns in a single body (back-buffer.ts:61-91). - -#### `packages/spf/src/media/buffer/forward-buffer.ts` - -**Purpose**: Forward-buffer "what to load" + "what to flush ahead of position" calculators. - -**Axis findings**: -- A: `getSegmentsToLoad` keys "is this position buffered" by `segment.startTime` only (forward-buffer.ts:108-122). Comment at forward-buffer.ts:107-108 explicitly notes this won't work for quality switches; no path to extend without rewriting. -- B: `Math.min(...beyond.map(seg => seg.startTime))` (forward-buffer.ts:89) — for very large buffer arrays, spread of large maps risks call-stack issues; correctness aside, an assumed-small-array invariant is implicit. -- D: Two top-level functions with overlapping config types in same file; one (`calculateForwardFlushPoint`) has its JSDoc placed between two unrelated functions, attached to the wrong block (forward-buffer.ts:51-73 vs 92). - -**Repo-rule findings**: -- Doubled/orphaned JSDoc — the `getSegmentsToLoad` doc at forward-buffer.ts:28-50 is followed by a second JSDoc block (forward-buffer.ts:51-73) that documents `calculateForwardFlushPoint` instead, then `calculateForwardFlushPoint` is defined first (forward-buffer.ts:74) — function-doc order doesn't match. - -**Notable assumptions**: -- Single SourceBuffer model; flushing decisions don't account for differing buffered ranges per video/audio buffer (forward-buffer.ts:74-90) — touches axis A pressure list (audio-only / multi-buffer). - -#### `packages/spf/src/media/dom/mse/append-segment.ts` - -**Purpose**: Serialize SourceBuffer appends (full buffer or chunked stream), with abort-aware partial cleanup. - -**Axis findings**: -- A: No pre-append wait hook — pre-append DRM key handshake gating (pressure-list item) has no insertion point; appends happen unconditionally as soon as `updating` clears (append-segment.ts:60-69, 90-91). -- A: No `timestampOffset` / `appendWindowStart`/`End` configuration — instant clips / non-zero PTS pressure-list item not addressable here without changing signature (append-segment.ts:22). -- B: `updateend` listener at append-segment.ts:62-68 has no `error` listener — if the SourceBuffer errors while waiting, the promise never resolves (deadlock). The second phase (append-segment.ts:71-96) does add `error`, but the gating wait does not. -- B: `sourceBuffer.abort()` swallowed in nested `try/catch` (append-segment.ts:48-54) — comment explains "thrown if MS not open"; passes silently regardless of the actual cause. - -**Repo-rule findings**: -- Manual event listener add/remove (append-segment.ts:62-68, 73-89) instead of `AbortController` + `signal` pattern from CLAUDE.md "Cleanup Pattern". -- DOMException instance-of + name compare (append-segment.ts:48) instead of a guard helper. - -#### `packages/spf/src/media/dom/mse/buffer-flusher.ts` - -**Purpose**: Wait-for-`updating`, then `remove(start, end)`, await `updateend`. - -**Axis findings**: -- B: Pre-remove wait (buffer-flusher.ts:23-31) lacks `error` listener — same deadlock pattern as `appendSegment`'s wait phase. - -**Repo-rule findings**: -- Same manual listener add/remove (buffer-flusher.ts:24-31, 34-50) duplicates the wait-for-updateend logic from `append-segment.ts`. Two separate copies of "wait until SourceBuffer is not updating." -- Verbose JSDoc with `@param`/`@returns`/`@example` for non-public utility (buffer-flusher.ts:7-20). - -#### `packages/spf/src/media/dom/mse/mediasource-setup.ts` - -**Purpose**: MediaSource / ManagedMediaSource creation, attach/detach, source-buffer creation, `isTypeSupported` check, ready-state observer. - -**Axis findings**: -- A: `createSourceBuffer` requires `mimeCodec` string up-front (mediasource-setup.ts:129-139) — no path for codec capability detection feedback (HEVC, 5.1) before commit; consumer must pre-check. -- A: `attachMediaSource`'s ManagedMediaSource branch hardcodes `disableRemotePlayback = true` (mediasource-setup.ts:92) — opinionated AirPlay/remote-playback policy with no opt-out; conflicts with FairPlay/AirPlay path on pressure list. -- B: `detach` path calls `mediaElement.load()` (mediasource-setup.ts:99, 111) — synchronous element reset that races with any in-flight network/append work; teardown ordering responsibility shifts to caller. -- B: `onMediaSourceReadyStateChange` returns `void` (mediasource-setup.ts:178-188) — caller-only cleanup via abortSignal; no return-value to chain into broader cleanup. -- D: Three type casts on `mediaElement` for `disableRemotePlayback`/`srcObject` (mediasource-setup.ts:92, 95, 98). - -**Repo-rule findings**: -- Type cast `(mediaElement as HTMLMediaElement & { srcObject: ... })` (mediasource-setup.ts:95, 98) — `srcObject` is on `HTMLMediaElement` in lib.dom and these casts add no value; CLAUDE.md "No Pointless Type Casts." -- Non-null assertion on `ManagedMediaSource!` (mediasource-setup.ts:49, 87) — relies on the `.d.ts` typing it as `... | undefined`; pattern inconsistent with the `supportsManagedMediaSource()` guard immediately preceding. - -**Notable assumptions**: -- ManagedMediaSource availability checked once via `typeof` (mediasource-setup.ts:21-23); no caching or capability snapshot. - -#### `packages/spf/src/media/dom/mse/mediasource.d.ts` - -**Purpose**: Ambient types for ManagedMediaSource. (Trivial; no findings.) - -#### `packages/spf/src/media/dom/text/resolve-vtt-segment.ts` - -**Purpose**: Singleton-dummy-video-element VTT parser using browser-native ``. - -**Axis findings**: -- A: Singleton `dummyVideo` (resolve-vtt-segment.ts:9-20) — module-level state means concurrent `resolveVttSegment` calls share one element. Implicit single-resolver-at-a-time assumption; no queue. -- B: Multiple in-flight `resolveVttSegment` calls share one `