diff --git a/docs/superpowers/plans/2026-09-22-v020-player-mode-bugfixes.md b/docs/superpowers/plans/2026-09-22-v020-player-mode-bugfixes.md new file mode 100644 index 0000000..aef33ac --- /dev/null +++ b/docs/superpowers/plans/2026-09-22-v020-player-mode-bugfixes.md @@ -0,0 +1,730 @@ +# v0.2.0 Player Mode Bug Fixes — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Fix 8 issues found during QA of the v0.2.0 Player mode: auto-hide broken, controls layout wrong, status bar visible, JKL skips 1x, speed selector broken, fullscreen broken, keyframe shortcuts broken. + +**Architecture:** All fixes target existing files — no new files. Three main areas: App.svelte (auto-hide + keyboard), playback.ts (shuttle + fullscreen), PlayerControls.svelte (layout redesign + speed selector). + +**Tech Stack:** Svelte 5 (runes), TypeScript, Vitest + +## Global Constraints + +- All imports at top of file (no inline imports). +- Exhaustive switch with `never` default for TypeScript unions/enums. +- Existing tests must continue to pass (`npm test`). +- Zero errors AND zero warnings from `npx svelte-check --tsconfig ./tsconfig.json`. + +--- + +### Task 1: App.svelte — Auto-Hide + Status Bar + Keyboard Fixes + +**Files:** +- Modify: `src/App.svelte` + +**Interfaces:** +- Consumes: `isPlayerMode` (existing derived), `session.isPlaying` (existing store), `dispatchTransport` (existing function) +- Produces: Working auto-hide for controls and toolbar, hidden status bar in Player mode, working `<`/`>` keyframe shortcuts + +This task fixes issues 2, 3, 4, and 8 from the spec. + +- [ ] **Step 1: Fix timer variables — remove `$state`** + +In `src/App.svelte`, find these two lines (around lines 54-55): + +```typescript + let controlsHideTimer = $state | null>(null); + let toolbarHideTimer = $state | null>(null); +``` + +Replace with plain `let` (no reactivity — these are only used in imperative timer logic): + +```typescript + let controlsHideTimer: ReturnType | null = null; + let toolbarHideTimer: ReturnType | null = null; +``` + +- [ ] **Step 2: Simplify the auto-hide `$effect`** + +Find the `$effect` block that checks `session.isPlaying` (around lines 85-96): + +```typescript + $effect(() => { + if (!isPlayerMode) return; + if (!session.isPlaying) { + // Video paused — show everything + showPlayerControls = true; + showToolbar = true; + if (controlsHideTimer) clearTimeout(controlsHideTimer); + if (toolbarHideTimer) clearTimeout(toolbarHideTimer); + } else { + // Video playing — start hide timers + resetControlsTimer(); + } + }); +``` + +Replace with: + +```typescript + $effect(() => { + if (!isPlayerMode) return; + // Read play state so this re-runs on play/pause changes — + // controls briefly show then auto-hide regardless of play state + void session.isPlaying; + resetControlsTimer(); + }); +``` + +- [ ] **Step 3: Update `resetControlsTimer` — always start timer** + +Find `resetControlsTimer` (around line 152): + +```typescript + function resetControlsTimer() { + if (controlsHideTimer) clearTimeout(controlsHideTimer); + showPlayerControls = true; + if (session.isPlaying) { + controlsHideTimer = setTimeout(() => { + showPlayerControls = false; + }, CONTROLS_HIDE_DELAY); + } + } +``` + +Replace with (remove the `if (session.isPlaying)` guard): + +```typescript + function resetControlsTimer() { + if (controlsHideTimer) clearTimeout(controlsHideTimer); + showPlayerControls = true; + controlsHideTimer = setTimeout(() => { + showPlayerControls = false; + }, CONTROLS_HIDE_DELAY); + } +``` + +- [ ] **Step 4: Update `resetToolbarTimer` — always start timer** + +Find `resetToolbarTimer` (around line 162): + +```typescript + function resetToolbarTimer() { + if (toolbarHideTimer) clearTimeout(toolbarHideTimer); + showToolbar = true; + if (session.isPlaying) { + toolbarHideTimer = setTimeout(() => { + showToolbar = false; + }, CONTROLS_HIDE_DELAY); + } + } +``` + +Replace with: + +```typescript + function resetToolbarTimer() { + if (toolbarHideTimer) clearTimeout(toolbarHideTimer); + showToolbar = true; + toolbarHideTimer = setTimeout(() => { + showToolbar = false; + }, CONTROLS_HIDE_DELAY); + } +``` + +- [ ] **Step 5: Hide status bar in Player mode** + +Find the StatusBar line (around line 463): + +```svelte + (showAboutDialog = true)} /> +``` + +Wrap it: + +```svelte + {#if !isPlayerMode} + (showAboutDialog = true)} /> + {/if} +``` + +- [ ] **Step 6: Fix keyframe shortcuts — add `<` and `>` cases** + +In `handleGlobalKeydown`, find the `,` and `.` cases (around lines 212-219): + +```typescript + case ',': + e.preventDefault(); + dispatchTransport(e.shiftKey ? 'keyframe-back' : 'frame-back'); + break; + case '.': + e.preventDefault(); + dispatchTransport(e.shiftKey ? 'keyframe-forward' : 'frame-forward'); + break; +``` + +Replace with (remove the now-unreachable shiftKey ternary, add separate `<`/`>` cases): + +```typescript + case ',': + e.preventDefault(); + dispatchTransport('frame-back'); + break; + case '.': + e.preventDefault(); + dispatchTransport('frame-forward'); + break; + case '<': + e.preventDefault(); + dispatchTransport('keyframe-back'); + break; + case '>': + e.preventDefault(); + dispatchTransport('keyframe-forward'); + break; +``` + +- [ ] **Step 7: Verify** + +Run: `npx svelte-check --tsconfig ./tsconfig.json` +Expected: 0 errors, 0 warnings + +Run: `npm test` +Expected: All tests pass + +- [ ] **Step 8: Commit** + +```bash +git add src/App.svelte +git commit -m "fix: auto-hide timers, status bar, keyframe shortcuts in Player mode" +``` + +--- + +### Task 2: Playback Helpers — adjustShuttle + Fullscreen + +**Files:** +- Modify: `src/lib/transport/playback.ts` +- Modify: `tests/lib/transport/playback.test.ts` +- Modify: `src/App.svelte` (call sites only) + +**Interfaces:** +- Consumes: `getVideo()` (internal), `document.documentElement` (DOM) +- Produces: + - `adjustShuttle(dir: 1 | -1): number` — reads rate from video element, steps by 0.25, returns new rate + - `toggleFullscreen(): void` — targets `document.documentElement` instead of video element + +- [ ] **Step 1: Write updated tests for `adjustShuttle`** + +In `tests/lib/transport/playback.test.ts`, add this import at the top (with the existing imports): + +```typescript +import { + setVideoElement, + togglePiP, + isPiPActive, + toggleFullscreen, + isFullscreenActive, + setPlaybackRate, + getPlaybackRate, + adjustShuttle, +} from '$lib/transport/playback'; +``` + +Add a new describe block at the end of the file: + +```typescript +describe('playback — adjustShuttle', () => { + let mockVideo: HTMLVideoElement; + + beforeEach(() => { + mockVideo = createMockVideoElement(); + setVideoElement(mockVideo); + mockVideo.playbackRate = 1; + mockVideo.paused = true; + Object.defineProperty(mockVideo, 'paused', { + value: true, + writable: true, + configurable: true, + }); + mockVideo.play = vi.fn().mockResolvedValue(undefined); + }); + + it('increases rate by 0.25 when dir is 1', () => { + const result = adjustShuttle(1); + expect(result).toBe(1.25); + expect(mockVideo.playbackRate).toBe(1.25); + }); + + it('decreases rate by 0.25 when dir is -1', () => { + const result = adjustShuttle(-1); + expect(result).toBe(0.75); + expect(mockVideo.playbackRate).toBe(0.75); + }); + + it('reads current rate from video element, not external state', () => { + mockVideo.playbackRate = 0.75; + const result = adjustShuttle(1); + expect(result).toBe(1); + }); + + it('clamps to minimum 0.25', () => { + mockVideo.playbackRate = 0.25; + const result = adjustShuttle(-1); + expect(result).toBe(0.25); + }); + + it('clamps to maximum 4', () => { + mockVideo.playbackRate = 4; + const result = adjustShuttle(1); + expect(result).toBe(4); + }); + + it('starts playback if paused', () => { + adjustShuttle(1); + expect(mockVideo.play).toHaveBeenCalled(); + }); + + it('returns 1 when no video element', () => { + setVideoElement(null); + expect(adjustShuttle(1)).toBe(1); + }); +}); +``` + +- [ ] **Step 2: Run tests to verify new tests fail** + +Run: `npm test` +Expected: `adjustShuttle` tests fail because the current signature is `adjustShuttle(dir, shuttleRate)` — calling with one arg will use `undefined` for `shuttleRate`. + +- [ ] **Step 3: Update `adjustShuttle` implementation** + +In `src/lib/transport/playback.ts`, find (around line 87): + +```typescript +export function adjustShuttle(dir: 1 | -1, shuttleRate: number): number { + const videoEl = getVideo(); + if (!videoEl) return shuttleRate; + + const nextRate = Math.max(0.25, Math.min(4, shuttleRate + dir * 0.5)); + videoEl.playbackRate = nextRate; + if (videoEl.paused) { + void videoEl.play(); + } + return nextRate; +} +``` + +Replace with: + +```typescript +export function adjustShuttle(dir: 1 | -1): number { + const videoEl = getVideo(); + if (!videoEl) return 1; + + const current = videoEl.playbackRate; + const nextRate = Math.max(0.25, Math.min(4, current + dir * 0.25)); + videoEl.playbackRate = nextRate; + if (videoEl.paused) { + void videoEl.play(); + } + return nextRate; +} +``` + +- [ ] **Step 4: Update App.svelte call sites** + +In `src/App.svelte`, find the J/L key handlers (around lines 266-277): + +```typescript + case 'j': + case 'J': + e.preventDefault(); + shuttleRate = adjustShuttle(-1, shuttleRate); + break; +``` + +Replace with: + +```typescript + case 'j': + case 'J': + e.preventDefault(); + shuttleRate = adjustShuttle(-1); + break; +``` + +And: + +```typescript + case 'l': + case 'L': + e.preventDefault(); + shuttleRate = adjustShuttle(1, shuttleRate); + break; +``` + +Replace with: + +```typescript + case 'l': + case 'L': + e.preventDefault(); + shuttleRate = adjustShuttle(1); + break; +``` + +- [ ] **Step 5: Update fullscreen tests** + +In `tests/lib/transport/playback.test.ts`, find the fullscreen test (around line 89): + +```typescript + it('toggleFullscreen calls requestFullscreen when not fullscreen', () => { + toggleFullscreen(); + expect(mockVideo.requestFullscreen).toHaveBeenCalled(); + }); +``` + +Replace with: + +```typescript + it('toggleFullscreen calls requestFullscreen on document.documentElement', () => { + const spy = vi.fn().mockResolvedValue(undefined); + document.documentElement.requestFullscreen = spy; + toggleFullscreen(); + expect(spy).toHaveBeenCalled(); + }); +``` + +- [ ] **Step 6: Update `toggleFullscreen` implementation** + +In `src/lib/transport/playback.ts`, find `toggleFullscreen` (around line 118): + +```typescript +export function toggleFullscreen(): void { + if (document.fullscreenElement) { + document.exitFullscreen().catch((e) => { + console.error('Exit fullscreen failed:', e); + }); + } else { + const videoEl = getVideo(); + if (!videoEl) return; + if (videoEl.requestFullscreen) { + videoEl.requestFullscreen().catch((e) => { + console.error('Fullscreen request failed:', e); + }); + } else if ((videoEl as HTMLVideoElement & { webkitEnterFullscreen?: () => void }).webkitEnterFullscreen) { + (videoEl as HTMLVideoElement & { webkitEnterFullscreen: () => void }).webkitEnterFullscreen(); + } + } +} +``` + +Replace with: + +```typescript +export function toggleFullscreen(): void { + if (document.fullscreenElement) { + document.exitFullscreen().catch((e) => { + console.error('Exit fullscreen failed:', e); + }); + } else { + const el = document.documentElement; + if (el.requestFullscreen) { + el.requestFullscreen().catch((e) => { + console.error('Fullscreen request failed:', e); + }); + } else if ((el as HTMLElement & { webkitRequestFullscreen?: () => void }).webkitRequestFullscreen) { + (el as HTMLElement & { webkitRequestFullscreen: () => void }).webkitRequestFullscreen(); + } + } +} +``` + +- [ ] **Step 7: Run tests** + +Run: `npm test` +Expected: All tests pass (existing + new adjustShuttle tests) + +Run: `npx svelte-check --tsconfig ./tsconfig.json` +Expected: 0 errors, 0 warnings + +- [ ] **Step 8: Commit** + +```bash +git add src/lib/transport/playback.ts tests/lib/transport/playback.test.ts src/App.svelte +git commit -m "fix: adjustShuttle reads live rate with 0.25 step, fullscreen targets documentElement" +``` + +--- + +### Task 3: PlayerControls — QuickTime Layout + Speed Selector + Rate Sync + +**Files:** +- Modify: `src/lib/components/PlayerControls.svelte` + +**Interfaces:** +- Consumes: `getPlaybackRate()` from `$lib/transport/playback` (already imported), `session` from videoSession store +- Produces: QuickTime-style floating pill layout, working speed selector popup, live rate sync from video element + +This task fixes issues 1 and 6 from the spec. The speed selector `stopPropagation` fix (issue 6) is included in the new markup in Step 2. + +- [ ] **Step 1: Add rate sync to the polling `$effect`** + +Find the polling effect (around line 132): + +```typescript + // Sync PiP/fullscreen state periodically + $effect(() => { + const interval = setInterval(() => { + pipActive = isPiPActive(); + fullscreenActive = isFullscreenActive(); + }, 500); + return () => clearInterval(interval); + }); +``` + +Add `currentRate` sync: + +```typescript + // Sync PiP/fullscreen/rate state periodically + $effect(() => { + const interval = setInterval(() => { + pipActive = isPiPActive(); + fullscreenActive = isFullscreenActive(); + currentRate = getPlaybackRate(); + }, 500); + return () => clearInterval(interval); + }); +``` + +- [ ] **Step 2: Reorganize markup — controls row first, seek bar second, timestamps third** + +Replace the entire content inside `
` (everything between the opening and closing tags of `.player-controls-panel`). + +The current order is: seek bar → controls row. + +New order with QuickTime layout: + +```svelte +
+ +
+ +
+ + +
+ + +
+ + + +
+ + +
+
+ + {#if showSpeedSelector} + { showSpeedSelector = false; }} + /> + {/if} +
+ + {#if hasCaptions} + + {/if} + + + + +
+
+ + +
+
+
+
+
+ {#if hoverTime !== null} +
+ {formatTime(hoverTime)} +
+ {/if} +
+ + +
+ {formatTime(session.currentTime)} + {formatTime(session.duration)} +
+
+``` + +- [ ] **Step 3: Update CSS — floating pill positioning** + +Replace the `.player-controls-overlay` CSS: + +```css + .player-controls-overlay { + position: absolute; + bottom: 0; + left: 0; + right: 0; + pointer-events: none; + opacity: 0; + transition: opacity 0.2s ease 0.1s; + z-index: 20; + } +``` + +With: + +```css + .player-controls-overlay { + position: absolute; + bottom: 24px; + left: 15%; + right: 15%; + pointer-events: none; + opacity: 0; + transition: opacity 0.2s ease 0.1s; + z-index: 20; + } +``` + +- [ ] **Step 4: Update CSS — pill border-radius** + +Replace the `.player-controls-panel` border-radius: + +```css + border-radius: 12px 12px 0 0; +``` + +With: + +```css + border-radius: 12px; +``` + +- [ ] **Step 5: Add timestamps-row CSS** + +Add this new rule in the `