From b66b1ebf234f4c1298334f5d26a5b63c6c3f59af Mon Sep 17 00:00:00 2001 From: Rahim Date: Tue, 14 Jul 2026 15:48:51 -0700 Subject: [PATCH] chore(root): streamline agent guidance and skills --- .agents/plans/README.md | 10 + .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/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/merge-behaviors/SKILL.md | 216 -- .claude/skills/refactor-behavior/SKILL.md | 613 ------ .claude/skills/review-branch/SKILL.md | 154 -- .claude/skills/rfc/SKILL.md | 81 - .claude/skills/spf-create-behavior/SKILL.md | 185 -- .claude/skills/spf-document-feature/SKILL.md | 591 ------ .claude/skills/spf-document-use-case/SKILL.md | 637 ------ .claude/skills/spf-implement-feature/SKILL.md | 997 ---------- .../skills/spf-implement-use-case/SKILL.md | 753 ------- .claude/skills/spf-update-behavior/SKILL.md | 190 -- .claude/skills/split-behavior/SKILL.md | 311 --- .github/workflows/api-reference-sync.yml | 4 +- .github/workflows/changelog-prose.yml | 2 +- .gitignore | 12 +- AGENTS.md | 82 + CLAUDE.md | 564 +----- CONTRIBUTING.md | 46 +- build/scripts/check-workspace.mjs | 313 ++- build/scripts/link-aliases.mjs | 73 +- internal/decisions/README.md | 4 +- internal/decisions/spf-signal-ownership.md | 25 + internal/decisions/store-reactive-state.md | 25 + internal/design/README.md | 93 +- .../archive/feature-availability-design.md | 111 -- internal/design/archive/queue-design.md | 429 ---- .../design/{i18n.md => i18n/architecture.md} | 0 .../{media.md => media/architecture.md} | 0 .../design/spf/actor-reactor-factories.md | 2 +- internal/design/spf/architecture.md | 2 +- internal/design/spf/conventions/README.md | 4 +- internal/design/spf/conventions/actors.md | 2 +- internal/design/spf/conventions/behaviors.md | 26 +- internal/design/spf/conventions/config.md | 2 +- internal/design/spf/conventions/reactors.md | 2 +- internal/design/spf/conventions/signals.md | 4 +- internal/design/spf/decisions.md | 2 +- internal/design/spf/evaluation-axes.md | 8 +- internal/design/spf/features/audio-abr.md | 2 +- .../design/spf/features/audio-playback.md | 2 +- internal/design/spf/features/clusters.md | 12 +- .../spf/features/multi-language-audio.md | 8 +- internal/design/spf/index.md | 171 +- .../spf/multi-cdn-failover-prior-art.md | 2 +- internal/design/spf/presentation-modeling.md | 2 +- internal/design/spf/primitives.md | 2 +- internal/design/spf/signals.md | 2 +- .../design/spf/text-track-architecture.md | 2 +- internal/design/spf/track-switching-model.md | 2 +- internal/design/spf/use-cases/README.md | 22 +- .../design/spf/use-cases/background-video.md | 8 +- internal/design/ui/menus.md | 2 +- packages/spf/src/AGENTS.md | 34 + packages/spf/src/CLAUDE.md | 105 +- rfc/README.md | 6 +- rfc/player-api/index.md | 4 +- site/AGENTS.md | 71 + site/CLAUDE.md | 996 +--------- site/README.md | 23 +- site/astro.config.mjs | 1 - .../src/content/docs/how-to/build-with-ai.mdx | 2 +- site/src/content/docs/how-to/write-guides.mdx | 4 +- .../docs/reference/write-references.mdx | 4 +- 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 skills/change-spf-behavior/SKILL.md | 30 + skills/commit-pr/SKILL.md | 31 + .../commit-pr}/references/branch.md | 0 .../commit-pr}/references/commit.md | 0 .../git => skills/commit-pr}/references/pr.md | 0 .../commit-pr}/references/scope.md | 0 skills/create-issue/SKILL.md | 28 + skills/create-skill/SKILL.md | 39 + skills/create-skill/references/patterns.md | 19 + skills/create-skill/references/principles.md | 25 + skills/create-skill/references/structure.md | 29 + skills/create-spf-behavior/SKILL.md | 23 + 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 .../design-api}/references/state.md | 0 .../design-api}/references/typescript.md | 0 .../design-api}/references/voices.md | 0 skills/document-spf-feature/SKILL.md | 21 + skills/document-spf-use-case/SKILL.md | 21 + 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 - skills/implement-spf-feature/SKILL.md | 23 + skills/implement-spf-use-case/SKILL.md | 23 + skills/investigate-issue/SKILL.md | 24 + skills/maintain-agent-docs/SKILL.md | 35 + skills/migrate-css-to-tailwind/SKILL.md | 21 + .../references/migration.md | 0 skills/review-accessibility/SKILL.md | 21 + .../references}/checklist.md | 0 skills/review-api/SKILL.md | 21 + .../review-api/references}/checklist.md | 17 - skills/review-branch/SKILL.md | 23 + skills/review-docs/SKILL.md | 21 + .../review-docs/references}/checklist.md | 10 - skills/review-tailwind-migration/SKILL.md | 21 + .../references}/checklist.md | 2 +- 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 +- 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 +- 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 skills/write-docs/SKILL.md | 39 + .../write-docs}/patterns/code-examples.md | 2 +- .../write-docs}/patterns/error-docs.md | 0 .../references/component-libraries.md | 6 - .../write-docs}/references/gold-standard.md | 0 .../write-docs}/references/seo.md | 0 .../write-docs}/references/state-tooling.md | 6 - .../write-docs}/references/writing-style.md | 0 .../write-docs}/templates/concept.md | 0 .../write-docs}/templates/how-to.md | 0 .../write-docs}/templates/readme.md | 0 skills/write-rfc/SKILL.md | 26 + 190 files changed, 1560 insertions(+), 18335 deletions(-) create mode 100644 .agents/plans/README.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/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/merge-behaviors/SKILL.md delete mode 100644 .claude/skills/refactor-behavior/SKILL.md delete mode 100644 .claude/skills/review-branch/SKILL.md delete mode 100644 .claude/skills/rfc/SKILL.md delete mode 100644 .claude/skills/spf-create-behavior/SKILL.md delete mode 100644 .claude/skills/spf-document-feature/SKILL.md delete mode 100644 .claude/skills/spf-document-use-case/SKILL.md delete mode 100644 .claude/skills/spf-implement-feature/SKILL.md delete mode 100644 .claude/skills/spf-implement-use-case/SKILL.md delete mode 100644 .claude/skills/spf-update-behavior/SKILL.md delete mode 100644 .claude/skills/split-behavior/SKILL.md create mode 100644 AGENTS.md create mode 100644 internal/decisions/spf-signal-ownership.md create mode 100644 internal/decisions/store-reactive-state.md delete mode 100644 internal/design/archive/feature-availability-design.md delete mode 100644 internal/design/archive/queue-design.md rename internal/design/{i18n.md => i18n/architecture.md} (100%) rename internal/design/{media.md => media/architecture.md} (100%) create mode 100644 packages/spf/src/AGENTS.md create mode 100644 site/AGENTS.md create mode 100644 skills/build-ui-component/SKILL.md rename {.claude/skills/component => skills/build-ui-component}/references/animation.md (100%) rename {.claude/skills/component => skills/build-ui-component}/references/anti-patterns.md (98%) rename {.claude/skills/component => skills/build-ui-component}/references/collection.md (97%) rename {.claude/skills/component => skills/build-ui-component}/references/html.md (100%) rename {.claude/skills/component => skills/build-ui-component}/references/polymorphism.md (97%) rename {.claude/skills/component => skills/build-ui-component}/references/props.md (100%) rename {.claude/skills/component => skills/build-ui-component}/references/react.md (98%) rename {.claude/skills/component => skills/build-ui-component}/references/styling.md (100%) rename {.claude/skills/component => skills/build-ui-component}/references/videojs-element.md (100%) rename {.claude/skills/component => skills/build-ui-component}/references/videojs.md (100%) create mode 100644 skills/change-spf-behavior/SKILL.md create mode 100644 skills/commit-pr/SKILL.md rename {.claude/skills/git => skills/commit-pr}/references/branch.md (100%) rename {.claude/skills/git => skills/commit-pr}/references/commit.md (100%) rename {.claude/skills/git => skills/commit-pr}/references/pr.md (100%) rename {.claude/skills/git => skills/commit-pr}/references/scope.md (100%) create mode 100644 skills/create-issue/SKILL.md create mode 100644 skills/create-skill/SKILL.md create mode 100644 skills/create-skill/references/patterns.md create mode 100644 skills/create-skill/references/principles.md create mode 100644 skills/create-skill/references/structure.md create mode 100644 skills/create-spf-behavior/SKILL.md create mode 100644 skills/design-api/SKILL.md rename {.claude/skills/api => skills/design-api}/references/anti-patterns.md (98%) rename {.claude/skills/api => skills/design-api}/references/extensibility.md (100%) rename {.claude/skills/api => skills/design-api}/references/libraries.md (100%) rename {.claude/skills/api => skills/design-api}/references/principles.md (100%) rename {.claude/skills/api => skills/design-api}/references/state.md (100%) rename {.claude/skills/api => skills/design-api}/references/typescript.md (100%) rename {.claude/skills/api => skills/design-api}/references/voices.md (100%) create mode 100644 skills/document-spf-feature/SKILL.md create mode 100644 skills/document-spf-use-case/SKILL.md create mode 100644 skills/implement-accessible-ui/SKILL.md rename {.claude/skills/aria => skills/implement-accessible-ui}/references/anti-patterns.md (98%) rename {.claude/skills/aria => skills/implement-accessible-ui}/references/aria.md (100%) rename {.claude/skills/aria => skills/implement-accessible-ui}/references/focus.md (100%) rename {.claude/skills/aria => skills/implement-accessible-ui}/references/keyboard.md (100%) rename {.claude/skills/aria => skills/implement-accessible-ui}/references/media.md (100%) rename {.claude/skills/aria => skills/implement-accessible-ui}/references/react.md (98%) create mode 100644 skills/implement-spf-feature/SKILL.md create mode 100644 skills/implement-spf-use-case/SKILL.md create mode 100644 skills/investigate-issue/SKILL.md create mode 100644 skills/maintain-agent-docs/SKILL.md create mode 100644 skills/migrate-css-to-tailwind/SKILL.md rename {.claude/skills/css-to-tailwind => skills/migrate-css-to-tailwind}/references/migration.md (100%) create mode 100644 skills/review-accessibility/SKILL.md rename {.claude/skills/aria/review => skills/review-accessibility/references}/checklist.md (100%) create mode 100644 skills/review-api/SKILL.md rename {.claude/skills/api/review => skills/review-api/references}/checklist.md (73%) create mode 100644 skills/review-branch/SKILL.md create mode 100644 skills/review-docs/SKILL.md rename {.claude/skills/docs/review => skills/review-docs/references}/checklist.md (86%) create mode 100644 skills/review-tailwind-migration/SKILL.md rename {.claude/skills/css-to-tailwind/review => skills/review-tailwind-migration/references}/checklist.md (95%) create mode 100644 skills/review-ui-component/SKILL.md rename {.claude/skills/component/review => skills/review-ui-component/references}/checklist.md (54%) rename {.claude/skills/component/review => skills/review-ui-component/references}/checklists/general.md (98%) rename {.claude/skills/component/review => skills/review-ui-component/references}/checklists/severity.md (100%) rename {.claude/skills/component/review => skills/review-ui-component/references}/checklists/videojs.md (94%) create mode 100644 skills/write-api-reference/SKILL.md rename {.claude/skills/api-reference => skills/write-api-reference}/references/builder-conventions.md (100%) rename {.claude/skills/api-reference => skills/write-api-reference}/references/demo-patterns.md (100%) rename {.claude/skills/api-reference => skills/write-api-reference}/references/mdx-structure.md (100%) rename {.claude/skills/api-reference => skills/write-api-reference}/references/util-conventions.md (98%) create mode 100644 skills/write-design-doc/SKILL.md rename {.claude/skills/design => skills/write-design-doc}/references/components.md (94%) rename {.claude/skills/design => skills/write-design-doc}/references/features.md (92%) rename {.claude/skills/design => skills/write-design-doc}/references/structure.md (92%) rename {.claude/skills/design => skills/write-design-doc}/templates/component-basic.md (100%) rename {.claude/skills/design => skills/write-design-doc}/templates/component-compound.md (100%) rename {.claude/skills/design => skills/write-design-doc}/templates/decision.md (100%) rename {.claude/skills/design => skills/write-design-doc}/templates/feature-multi.md (96%) rename {.claude/skills/design => skills/write-design-doc}/templates/feature-single.md (100%) create mode 100644 skills/write-docs/SKILL.md rename {.claude/skills/docs => skills/write-docs}/patterns/code-examples.md (98%) rename {.claude/skills/docs => skills/write-docs}/patterns/error-docs.md (100%) rename {.claude/skills/docs => skills/write-docs}/references/component-libraries.md (98%) rename {.claude/skills/docs => skills/write-docs}/references/gold-standard.md (100%) rename {.claude/skills/docs => skills/write-docs}/references/seo.md (100%) rename {.claude/skills/docs => skills/write-docs}/references/state-tooling.md (98%) rename {.claude/skills/docs => skills/write-docs}/references/writing-style.md (100%) rename {.claude/skills/docs => skills/write-docs}/templates/concept.md (100%) rename {.claude/skills/docs => skills/write-docs}/templates/how-to.md (100%) rename {.claude/skills/docs => skills/write-docs}/templates/readme.md (100%) create mode 100644 skills/write-rfc/SKILL.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/.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 `