diff --git a/CLAUDE.md b/CLAUDE.md index 06304752..f8cc437c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -464,6 +464,17 @@ export function onEvent(...): Promise<...>; export function supportsIdleCallback(): boolean { ... } get size(): number { ... } add(cleanup: CleanupFn): void { ... } + +// Bad - comment restates the obvious +/** Media element contract. */ +export interface Media extends HTMLMediaElement {} + +/** Feature capability availability. */ +export type FeatureAvailability = 'available' | 'unavailable' | 'unsupported'; + +// Good - no comment needed +export interface Media extends HTMLMediaElement {} +export type FeatureAvailability = 'available' | 'unavailable' | 'unsupported'; ``` ## Design Documents diff --git a/packages/core/src/dom/index.ts b/packages/core/src/dom/index.ts index 4b5227a3..1591c736 100644 --- a/packages/core/src/dom/index.ts +++ b/packages/core/src/dom/index.ts @@ -1 +1,3 @@ export * from './store/features'; +export * from './store/selectors'; +export * from './types'; diff --git a/packages/core/src/dom/store/features/buffer.ts b/packages/core/src/dom/store/features/buffer.ts index 3922de61..851fe966 100644 --- a/packages/core/src/dom/store/features/buffer.ts +++ b/packages/core/src/dom/store/features/buffer.ts @@ -3,7 +3,9 @@ import type { InferFeatureState } from '@videojs/store'; import { defineFeature } from '@videojs/store'; import { listen, serializeTimeRanges } from '@videojs/utils/dom'; -export const bufferFeature = defineFeature()({ +import type { PlayerTarget } from '../../types'; + +export const bufferFeature = defineFeature()({ state: () => ({ /** Buffered time ranges as [start, end] tuples. */ buffered: [] as [number, number][], @@ -12,16 +14,18 @@ export const bufferFeature = defineFeature()({ }), attach({ target, signal, set }) { + const { media } = target; + const sync = () => set({ - buffered: serializeTimeRanges(target.buffered), - seekable: serializeTimeRanges(target.seekable), + buffered: serializeTimeRanges(media.buffered), + seekable: serializeTimeRanges(media.seekable), }); sync(); - listen(target, 'progress', sync, { signal }); - listen(target, 'emptied', sync, { signal }); + listen(media, 'progress', sync, { signal }); + listen(media, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/media.parts.ts b/packages/core/src/dom/store/features/feature.parts.ts similarity index 60% rename from packages/core/src/dom/store/features/media.parts.ts rename to packages/core/src/dom/store/features/feature.parts.ts index 25232a6c..a8d8a164 100644 --- a/packages/core/src/dom/store/features/media.parts.ts +++ b/packages/core/src/dom/store/features/feature.parts.ts @@ -4,6 +4,7 @@ import { sourceFeature } from './source'; import { timeFeature } from './time'; import { volumeFeature } from './volume'; +// Short aliases export { bufferFeature as buffer, playbackFeature as playback, @@ -12,4 +13,6 @@ export { volumeFeature as volume, }; -export const all = [bufferFeature, playbackFeature, sourceFeature, timeFeature, volumeFeature] as const; +export const video = [playbackFeature, volumeFeature, timeFeature, sourceFeature, bufferFeature] as const; + +export const audio = [playbackFeature, volumeFeature, timeFeature, sourceFeature, bufferFeature] as const; diff --git a/packages/core/src/dom/store/features/index.ts b/packages/core/src/dom/store/features/index.ts index 7dd7334d..5689a05c 100644 --- a/packages/core/src/dom/store/features/index.ts +++ b/packages/core/src/dom/store/features/index.ts @@ -1,5 +1,5 @@ export * from './buffer'; -export * as media from './media.parts'; +export * as features from './feature.parts'; export * from './playback'; export * from './source'; export * from './time'; diff --git a/packages/core/src/dom/store/features/playback.ts b/packages/core/src/dom/store/features/playback.ts index 8992d839..b315b8f1 100644 --- a/packages/core/src/dom/store/features/playback.ts +++ b/packages/core/src/dom/store/features/playback.ts @@ -3,7 +3,9 @@ import type { InferFeatureState } from '@videojs/store'; import { defineFeature } from '@videojs/store'; import { listen } from '@videojs/utils/dom'; -export const playbackFeature = defineFeature()({ +import type { PlayerTarget } from '../../types'; + +export const playbackFeature = defineFeature()({ state: ({ task }) => ({ /** Whether playback is paused. */ paused: true, @@ -20,7 +22,7 @@ export const playbackFeature = defineFeature()({ key: 'playback', mode: 'shared', async handler({ target }) { - await target.play(); + await target.media.play(); }, }); }, @@ -30,28 +32,30 @@ export const playbackFeature = defineFeature()({ return task({ key: 'playback', handler({ target }) { - target.pause(); + target.media.pause(); }, }); }, }), attach({ target, signal, set }) { + const { media } = target; + const sync = () => set({ - paused: target.paused, - ended: target.ended, - started: !target.paused || target.currentTime > 0, - waiting: target.readyState < HTMLMediaElement.HAVE_FUTURE_DATA && !target.paused, + paused: media.paused, + ended: media.ended, + started: !media.paused || media.currentTime > 0, + waiting: media.readyState < HTMLMediaElement.HAVE_FUTURE_DATA && !media.paused, }); sync(); - listen(target, 'play', sync, { signal }); - listen(target, 'pause', sync, { signal }); - listen(target, 'ended', sync, { signal }); - listen(target, 'playing', sync, { signal }); - listen(target, 'waiting', sync, { signal }); + listen(media, 'play', sync, { signal }); + listen(media, 'pause', sync, { signal }); + listen(media, 'ended', sync, { signal }); + listen(media, 'playing', sync, { signal }); + listen(media, 'waiting', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/source.ts b/packages/core/src/dom/store/features/source.ts index f7bf696b..62e95ad0 100644 --- a/packages/core/src/dom/store/features/source.ts +++ b/packages/core/src/dom/store/features/source.ts @@ -3,7 +3,9 @@ import type { InferFeatureState } from '@videojs/store'; import { CANCEL_ALL, defineFeature } from '@videojs/store'; import { listen } from '@videojs/utils/dom'; -export const sourceFeature = defineFeature()({ +import type { PlayerTarget } from '../../types'; + +export const sourceFeature = defineFeature()({ state: ({ task }) => ({ /** Current media source URL (null if none). */ source: null as string | null, @@ -16,8 +18,8 @@ export const sourceFeature = defineFeature()({ key: 'source', cancels: [CANCEL_ALL], handler({ target }) { - target.src = src; - target.load(); + target.media.src = src; + target.media.load(); return src; }, }); @@ -25,18 +27,20 @@ export const sourceFeature = defineFeature()({ }), attach({ target, signal, set }) { + const { media } = target; + const sync = () => set({ - source: target.currentSrc || target.src || null, - canPlay: target.readyState >= HTMLMediaElement.HAVE_ENOUGH_DATA, + source: media.currentSrc || media.src || null, + canPlay: media.readyState >= HTMLMediaElement.HAVE_ENOUGH_DATA, }); sync(); - listen(target, 'canplay', sync, { signal }); - listen(target, 'canplaythrough', sync, { signal }); - listen(target, 'loadstart', sync, { signal }); - listen(target, 'emptied', sync, { signal }); + listen(media, 'canplay', sync, { signal }); + listen(media, 'canplaythrough', sync, { signal }); + listen(media, 'loadstart', sync, { signal }); + listen(media, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/tests/buffer.test.ts b/packages/core/src/dom/store/features/tests/buffer.test.ts index 636041b9..45f55230 100644 --- a/packages/core/src/dom/store/features/tests/buffer.test.ts +++ b/packages/core/src/dom/store/features/tests/buffer.test.ts @@ -12,7 +12,7 @@ describe('bufferFeature', () => { }); const store = createStore({ features: [bufferFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.buffered).toEqual([[0, 60]]); expect(store.state.seekable).toEqual([[0, 120]]); @@ -28,7 +28,7 @@ describe('bufferFeature', () => { }); const store = createStore({ features: [bufferFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.buffered).toEqual([ [0, 30], @@ -43,7 +43,7 @@ describe('bufferFeature', () => { }); const store = createStore({ features: [bufferFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); // Update the mock video's buffered range Object.defineProperty(video, 'buffered', { @@ -64,7 +64,7 @@ describe('bufferFeature', () => { }); const store = createStore({ features: [bufferFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); // Update the mock video to have no buffered content Object.defineProperty(video, 'buffered', { diff --git a/packages/core/src/dom/store/features/tests/playback.test.ts b/packages/core/src/dom/store/features/tests/playback.test.ts index 6be0d1a5..54ed98c9 100644 --- a/packages/core/src/dom/store/features/tests/playback.test.ts +++ b/packages/core/src/dom/store/features/tests/playback.test.ts @@ -14,7 +14,7 @@ describe('playbackFeature', () => { }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.paused).toBe(false); expect(store.state.ended).toBe(false); @@ -29,7 +29,7 @@ describe('playbackFeature', () => { }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.waiting).toBe(true); }); @@ -41,7 +41,7 @@ describe('playbackFeature', () => { }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.started).toBe(true); }); @@ -53,7 +53,7 @@ describe('playbackFeature', () => { }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.started).toBe(true); }); @@ -62,7 +62,7 @@ describe('playbackFeature', () => { const video = createMockVideo({ paused: true }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.paused).toBe(true); @@ -77,7 +77,7 @@ describe('playbackFeature', () => { const video = createMockVideo({ paused: false }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.paused).toBe(false); @@ -92,7 +92,7 @@ describe('playbackFeature', () => { const video = createMockVideo({ ended: false }); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.ended).toBe(false); @@ -107,7 +107,7 @@ describe('playbackFeature', () => { const video = createMockVideo({}); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); store.destroy(); @@ -126,7 +126,7 @@ describe('playbackFeature', () => { video.play = vi.fn().mockResolvedValue(undefined); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); await store.play(); @@ -138,7 +138,7 @@ describe('playbackFeature', () => { video.pause = vi.fn(); const store = createStore({ features: [playbackFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); store.pause(); diff --git a/packages/core/src/dom/store/features/tests/source.test.ts b/packages/core/src/dom/store/features/tests/source.test.ts index 2f776d8e..0c55090b 100644 --- a/packages/core/src/dom/store/features/tests/source.test.ts +++ b/packages/core/src/dom/store/features/tests/source.test.ts @@ -13,7 +13,7 @@ describe('sourceFeature', () => { }); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.source).toBe('https://example.com/video.mp4'); expect(store.state.canPlay).toBe(true); @@ -26,7 +26,7 @@ describe('sourceFeature', () => { Object.defineProperty(video, 'readyState', { value: HTMLMediaElement.HAVE_NOTHING, writable: false }); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.source).toBe(null); expect(store.state.canPlay).toBe(false); @@ -39,7 +39,7 @@ describe('sourceFeature', () => { }); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.canPlay).toBe(false); @@ -60,7 +60,7 @@ describe('sourceFeature', () => { }); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.source).toBe('https://example.com/video.mp4'); @@ -82,7 +82,7 @@ describe('sourceFeature', () => { }); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.canPlay).toBe(true); @@ -107,7 +107,7 @@ describe('sourceFeature', () => { video.load = vi.fn(); const store = createStore({ features: [sourceFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); const result = await store.loadSource('https://example.com/new.mp4'); diff --git a/packages/core/src/dom/store/features/tests/time.test.ts b/packages/core/src/dom/store/features/tests/time.test.ts index bc699f9f..3831fc02 100644 --- a/packages/core/src/dom/store/features/tests/time.test.ts +++ b/packages/core/src/dom/store/features/tests/time.test.ts @@ -12,7 +12,7 @@ describe('timeFeature', () => { }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.currentTime).toBe(30); expect(store.state.duration).toBe(120); @@ -25,7 +25,7 @@ describe('timeFeature', () => { }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.duration).toBe(0); }); @@ -34,7 +34,7 @@ describe('timeFeature', () => { const video = createMockVideo({ currentTime: 0 }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.currentTime).toBe(0); @@ -49,7 +49,7 @@ describe('timeFeature', () => { const video = createMockVideo({ duration: 0 }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.duration).toBe(0); @@ -64,7 +64,7 @@ describe('timeFeature', () => { const video = createMockVideo({ currentTime: 0 }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); // Update mock currentTime video.currentTime = 50; @@ -80,7 +80,7 @@ describe('timeFeature', () => { }); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); // Update mock to empty state video.currentTime = 0; @@ -97,7 +97,7 @@ describe('timeFeature', () => { it('sets currentTime on target and waits for seeked event', async () => { const video = createMockVideo({}); const store = createStore({ features: [timeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); const resultPromise = store.seek(45); diff --git a/packages/core/src/dom/store/features/tests/volume.test.ts b/packages/core/src/dom/store/features/tests/volume.test.ts index e8c098d8..d3a27c16 100644 --- a/packages/core/src/dom/store/features/tests/volume.test.ts +++ b/packages/core/src/dom/store/features/tests/volume.test.ts @@ -12,17 +12,26 @@ describe('volumeFeature', () => { }); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.volume).toBe(0.8); expect(store.state.muted).toBe(false); }); + it('sets volumeAvailability on attach', () => { + const video = createMockVideo({}); + const store = createStore({ features: [volumeFeature] }); + store.attach({ media: video, container: null }); + + // Should be 'available' or 'unsupported' based on browser capability + expect(['available', 'unsupported']).toContain(store.state.volumeAvailability); + }); + it('updates on volumechange event', () => { const video = createMockVideo({ volume: 1, muted: false }); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); expect(store.state.volume).toBe(1); @@ -41,7 +50,7 @@ describe('volumeFeature', () => { it('sets volume on target', async () => { const video = createMockVideo({}); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); const result = await store.changeVolume(0.7); @@ -52,7 +61,7 @@ describe('volumeFeature', () => { it('clamps volume to min 0', async () => { const video = createMockVideo({}); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); await store.changeVolume(-0.5); @@ -62,7 +71,7 @@ describe('volumeFeature', () => { it('clamps volume to max 1', async () => { const video = createMockVideo({}); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); await store.changeVolume(1.5); @@ -74,7 +83,7 @@ describe('volumeFeature', () => { it('toggles mute from false to true', async () => { const video = createMockVideo({ muted: false }); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); const result = await store.toggleMute(); @@ -85,7 +94,7 @@ describe('volumeFeature', () => { it('toggles mute from true to false', async () => { const video = createMockVideo({ muted: true }); const store = createStore({ features: [volumeFeature] }); - store.attach(video); + store.attach({ media: video, container: null }); const result = await store.toggleMute(); diff --git a/packages/core/src/dom/store/features/time.ts b/packages/core/src/dom/store/features/time.ts index 69f48101..2932db0d 100644 --- a/packages/core/src/dom/store/features/time.ts +++ b/packages/core/src/dom/store/features/time.ts @@ -3,7 +3,9 @@ import type { InferFeatureState } from '@videojs/store'; import { defineFeature } from '@videojs/store'; import { listen, onEvent } from '@videojs/utils/dom'; -export const timeFeature = defineFeature()({ +import type { PlayerTarget } from '../../types'; + +export const timeFeature = defineFeature()({ state: ({ task }) => ({ /** Current playback position in seconds. */ currentTime: 0, @@ -15,28 +17,30 @@ export const timeFeature = defineFeature()({ return task({ key: 'seek', async handler({ target, signal }) { - target.currentTime = time; - await onEvent(target, 'seeked', { signal }); - return target.currentTime; // actual position after seek + target.media.currentTime = time; + await onEvent(target.media, 'seeked', { signal }); + return target.media.currentTime; // actual position after seek }, }); }, }), attach({ target, signal, set }) { + const { media } = target; + const sync = () => set({ - currentTime: target.currentTime, - duration: target.duration || 0, + currentTime: media.currentTime, + duration: media.duration || 0, }); sync(); - listen(target, 'timeupdate', sync, { signal }); - listen(target, 'durationchange', sync, { signal }); - listen(target, 'seeked', sync, { signal }); - listen(target, 'loadedmetadata', sync, { signal }); - listen(target, 'emptied', sync, { signal }); + listen(media, 'timeupdate', sync, { signal }); + listen(media, 'durationchange', sync, { signal }); + listen(media, 'seeked', sync, { signal }); + listen(media, 'loadedmetadata', sync, { signal }); + listen(media, 'emptied', sync, { signal }); }, }); diff --git a/packages/core/src/dom/store/features/volume.ts b/packages/core/src/dom/store/features/volume.ts index 83dd0fb2..6e786f5b 100644 --- a/packages/core/src/dom/store/features/volume.ts +++ b/packages/core/src/dom/store/features/volume.ts @@ -3,20 +3,24 @@ import type { InferFeatureState } from '@videojs/store'; import { defineFeature } from '@videojs/store'; import { listen } from '@videojs/utils/dom'; -export const volumeFeature = defineFeature()({ +import type { FeatureAvailability, PlayerTarget } from '../../types'; + +export const volumeFeature = defineFeature()({ state: ({ task }) => ({ /** Volume level from 0 (silent) to 1 (max). */ volume: 1, /** Whether audio is muted. */ muted: false, + /** Whether volume can be programmatically set on this platform. */ + volumeAvailability: 'unavailable' as FeatureAvailability, /** Set volume (clamped 0-1). Returns the clamped value. */ changeVolume(volume: number) { return task({ key: 'volume', handler({ target }) { - target.volume = Math.max(0, Math.min(1, volume)); - return target.volume; + target.media.volume = Math.max(0, Math.min(1, volume)); + return target.media.volume; }, }); }, @@ -26,20 +30,35 @@ export const volumeFeature = defineFeature()({ return task({ key: 'mute', handler({ target }) { - target.muted = !target.muted; - return target.muted; + target.media.muted = !target.media.muted; + return target.media.muted; }, }); }, }), attach({ target, signal, set }) { - const sync = () => set({ volume: target.volume, muted: target.muted }); + const { media } = target; + + set({ volumeAvailability: canSetVolume() }); + + const sync = () => set({ volume: media.volume, muted: media.muted }); sync(); - listen(target, 'volumechange', sync, { signal }); + listen(media, 'volumechange', sync, { signal }); }, }); export type VolumeState = InferFeatureState; + +/** Check if volume can be programmatically set (fails on iOS Safari). */ +function canSetVolume(): FeatureAvailability { + const video = document.createElement('video'); + try { + video.volume = 0.5; + return video.volume === 0.5 ? 'available' : 'unsupported'; + } catch { + return 'unsupported'; + } +} diff --git a/packages/core/src/dom/store/selectors.ts b/packages/core/src/dom/store/selectors.ts new file mode 100644 index 00000000..864753f2 --- /dev/null +++ b/packages/core/src/dom/store/selectors.ts @@ -0,0 +1,13 @@ +import { createFeatureSelector } from '@videojs/store'; + +import { bufferFeature } from './features/buffer'; +import { playbackFeature } from './features/playback'; +import { sourceFeature } from './features/source'; +import { timeFeature } from './features/time'; +import { volumeFeature } from './features/volume'; + +export const selectBuffer = createFeatureSelector(bufferFeature); +export const selectPlayback = createFeatureSelector(playbackFeature); +export const selectSource = createFeatureSelector(sourceFeature); +export const selectTime = createFeatureSelector(timeFeature); +export const selectVolume = createFeatureSelector(volumeFeature); diff --git a/packages/core/src/dom/types.ts b/packages/core/src/dom/types.ts new file mode 100644 index 00000000..5cafb4d9 --- /dev/null +++ b/packages/core/src/dom/types.ts @@ -0,0 +1,10 @@ +export interface Media extends HTMLMediaElement {} + +export interface MediaContainer extends HTMLElement {} + +export interface PlayerTarget { + media: Media; + container: MediaContainer | null; +} + +export type FeatureAvailability = 'available' | 'unavailable' | 'unsupported';