Files
gui-video-clipper/docs/superpowers/plans/2026-09-22-v020-player-mode-bugfixes.md

731 lines
19 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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<ReturnType<typeof setTimeout> | null>(null);
let toolbarHideTimer = $state<ReturnType<typeof setTimeout> | null>(null);
```
Replace with plain `let` (no reactivity — these are only used in imperative timer logic):
```typescript
let controlsHideTimer: ReturnType<typeof setTimeout> | null = null;
let toolbarHideTimer: ReturnType<typeof setTimeout> | 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
<StatusBar onOpenAbout={() => (showAboutDialog = true)} />
```
Wrap it:
```svelte
{#if !isPlayerMode}
<StatusBar onOpenAbout={() => (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 `<div class="player-controls-panel">` (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
<div class="player-controls-panel">
<!-- Controls row (top) -->
<div class="controls-row">
<!-- Left group: volume -->
<div class="controls-group left">
<button
class="ctrl-btn vol-btn"
onclick={handleToggleMute}
title={isMuted ? 'Unmute' : 'Mute'}
type="button"
>
{#if isMuted || volume === 0}🔇{:else if volume < 0.5}🔉{:else}🔊{/if}
</button>
<input
type="range"
class="vol-slider"
min="0"
max="1"
step="0.05"
value={isMuted ? 0 : volume}
oninput={handleVolumeChange}
/>
</div>
<!-- Center group: playback -->
<div class="controls-group center">
<button
class="ctrl-btn"
onclick={() => seekBy(-10)}
title="Skip back 10s"
type="button"
>⏪</button>
<button
class="ctrl-btn play-btn"
onclick={togglePlayPause}
title={session.isPlaying ? 'Pause' : 'Play'}
type="button"
>{session.isPlaying ? '❚❚' : '▶'}</button>
<button
class="ctrl-btn"
onclick={() => seekBy(10)}
title="Skip forward 10s"
type="button"
>⏩</button>
</div>
<!-- Right group: settings -->
<div class="controls-group right">
<div class="speed-wrapper">
<button
class="ctrl-btn"
onclick={(e) => { e.stopPropagation(); showSpeedSelector = !showSpeedSelector; }}
title="Playback speed"
type="button"
>{currentRate}×</button>
{#if showSpeedSelector}
<SpeedSelector
{currentRate}
onSelect={handleSpeedSelect}
onClose={() => { showSpeedSelector = false; }}
/>
{/if}
</div>
{#if hasCaptions}
<button
class="ctrl-btn"
class:active={captionsEnabled}
onclick={onToggleCaptions}
oncontextmenu={handleCCContextMenu}
title={captionsEnabled ? 'Hide captions' : 'Show captions'}
type="button"
>CC</button>
{/if}
<button
class="ctrl-btn"
class:active={pipActive}
onclick={handlePiP}
title="Picture-in-Picture"
type="button"
>⧉</button>
<button
class="ctrl-btn"
class:active={fullscreenActive}
onclick={handleFullscreen}
title={fullscreenActive ? 'Exit fullscreen' : 'Fullscreen'}
type="button"
>{fullscreenActive ? '⤓' : '⛶'}</button>
</div>
</div>
<!-- Seek bar (middle) -->
<div
class="seek-bar"
bind:this={seekBarEl}
onmousedown={handleSeekBarMouseDown}
onmousemove={handleSeekBarMouseMove}
onmouseup={handleSeekBarMouseUp}
onmouseleave={handleSeekBarMouseLeave}
role="slider"
tabindex="0"
aria-label="Seek"
aria-valuenow={session.currentTime}
aria-valuemin={0}
aria-valuemax={session.duration}
>
<div class="seek-track">
<div class="seek-fill" style="width: {progress * 100}%"></div>
<div class="seek-thumb" style="left: {progress * 100}%"></div>
</div>
{#if hoverTime !== null}
<div class="seek-tooltip" style="left: {hoverX}px">
{formatTime(hoverTime)}
</div>
{/if}
</div>
<!-- Timestamps (bottom) -->
<div class="timestamps-row">
<span class="time-display">{formatTime(session.currentTime)}</span>
<span class="time-display">{formatTime(session.duration)}</span>
</div>
</div>
```
- [ ] **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 `<style>` block (after the `.time-display` rule):
```css
.timestamps-row {
display: flex;
justify-content: space-between;
padding: 0 4px;
}
```
Remove the `.time-sep` rule (no longer used) and update `.time-display` to remove `white-space: nowrap` if present (it's fine to keep).
Also remove the `.controls-group.center` old time-display styles. The center group now holds playback buttons, not time. Update center group:
```css
.controls-group.center {
flex: 0 0 auto;
}
```
(This is unchanged — it already handles playback buttons correctly since they have fixed sizes.)
- [ ] **Step 6: Verify**
Run: `npx svelte-check --tsconfig ./tsconfig.json`
Expected: 0 errors, 0 warnings
Run: `npm test`
Expected: All tests pass
- [ ] **Step 7: Commit**
```bash
git add src/lib/components/PlayerControls.svelte
git commit -m "fix: QuickTime-style floating controls pill, speed selector, rate sync"
```
---
## Task Dependency Graph
```
Task 1 (App.svelte: auto-hide + status bar + keys) ──→ Task 2 (playback.ts + App.svelte call sites) ──→ Task 3 (PlayerControls layout)
```
Tasks are sequential because Tasks 1 and 2 both modify `src/App.svelte`.