fix: QA round 2 — playhead guard, controls position, caption lift, Tauri fullscreen, timeline sync
- Guard drawPlayhead on session.duration > 0 in PlayerTimeline - Move floating controls pill from bottom: 24px to 48px - Add playerControlsVisible prop to VideoPlayer; captions slide up 160px when controls visible - Replace web Fullscreen API with Tauri getCurrentWindow().setFullscreen() (WKWebView compat) - Add core:window:allow-set-fullscreen and core:window:allow-is-fullscreen permissions - Tie PlayerTimeline visibility to showPlayerControls; remove unused proximity logic Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,48 @@
|
||||
# v0.2.0 Player Mode — Design & Implementation Plan
|
||||
|
||||
**Date:** 2026-09-22 18:58
|
||||
**Task:** Brainstorm and plan a "Player" mode for the video clipper app (v0.2.0)
|
||||
|
||||
## Changes Made
|
||||
|
||||
### Design Spec
|
||||
- Created `docs/superpowers/specs/2026-09-22-v020-player-mode-design.md`
|
||||
- 14 sections covering: state/preferences, keyboard shortcuts, layout, mode toggle, floating controls, speed selector, player timeline, toolbar auto-hide, PiP, fullscreen, transitions, file map, version bump
|
||||
|
||||
### Implementation Plan
|
||||
- Created `docs/superpowers/plans/2026-09-22-v020-player-mode.md`
|
||||
- 9 tasks with TDD steps, exact code, and commands:
|
||||
1. Preferences store — `appMode` field
|
||||
2. Playback helpers — PiP, fullscreen, playbackRate
|
||||
3. SpeedSelector component
|
||||
4. PlayerControls floating overlay
|
||||
5. PlayerTimeline simplified waveform
|
||||
6. App.svelte — mode switching, layout, auto-hide, shortcuts
|
||||
7. VideoPlayer.svelte — conditional CC, click-to-play
|
||||
8. FLIP transitions for mode switching
|
||||
9. Version bump to 0.2.0
|
||||
|
||||
## Key Design Decisions
|
||||
|
||||
- **Architecture:** Single `App.svelte` with conditional rendering (not separate layout components or windows)
|
||||
- **Video element preservation:** `<VideoPlayer>` always mounted outside conditionals — only surrounding chrome changes
|
||||
- **PiP:** Web API (`requestPictureInPicture`) — uses native macOS PiP via WKWebView. Player mode only.
|
||||
- **Fullscreen:** Standard webview fullscreen (`requestFullscreen` + webkit fallback). Player mode only.
|
||||
- **Speed control:** Both JKL shuttle keys + visible speed selector popup in floating controls
|
||||
- **Auto-hide:** Floating controls show on any mouse movement (2.5s timeout), toolbar on top-edge proximity (~50px), timeline on bottom-edge proximity (~80px). All visible when paused.
|
||||
- **Transitions:** Svelte `fly`/`fade` directives + CSS grid transitions + manual FLIP on video container. ~300ms budget.
|
||||
- **Mode persistence:** `appMode` saved to Tauri store, restored on relaunch
|
||||
- **Shortcuts:** `P` → Player, `C` → Clipper, `F` → fullscreen (Player only). Clipper-only shortcuts (I/O/Delete/Cmd+E) become no-ops in Player mode. Frame-step (`,`/`.`) works in both modes.
|
||||
|
||||
## Follow-up Items
|
||||
|
||||
- Execute the implementation plan (9 tasks)
|
||||
- Tasks 1, 2, 3 can be parallelized
|
||||
- Task 6 is the largest (App.svelte layout overhaul) — depends on Tasks 1, 4, 5
|
||||
- Task 8 (transitions) may need visual tuning after initial implementation
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
- When conditionally rendering different layouts that share a component (like `<VideoPlayer>`), the component must live outside the `{#if}` branches to avoid Svelte destroying and recreating it on branch switch
|
||||
- WKWebView supports the standard PiP API — no need for native AVPlayer FFI
|
||||
- Grid template transitions work in modern browsers but require explicit `transition` properties on the grid container
|
||||
@@ -0,0 +1,73 @@
|
||||
# v0.2.0 Player Mode — Implementation Summary
|
||||
|
||||
## Task Description
|
||||
|
||||
Implemented all 9 tasks from the v0.2.0 Player Mode implementation plan using subagent-driven development (SDD). This is the execution phase following the design spec and plan created in an earlier session.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### 11 commits (bd96da4..7e838dc)
|
||||
|
||||
| Commit | Description |
|
||||
|--------|-------------|
|
||||
| `bd96da4` | Task 1: `appMode` preference (clipper/player) with persistence |
|
||||
| `d9f5874` | Task 2: PiP, fullscreen, playbackRate helpers in playback module |
|
||||
| `bb5fa4b` | Task 3: SpeedSelector popup component |
|
||||
| `f717687` | Task 4: PlayerControls floating overlay (frosted glass, seek bar, all controls) |
|
||||
| `56686a3` | Task 5: PlayerTimeline simplified waveform component |
|
||||
| `d36138c` | Task 6: Player/Clipper mode switching with auto-hide UI in App.svelte |
|
||||
| `199ad1c` | Task 6 fix: transport shortcuts fallback via `runTransportAction` for Player mode |
|
||||
| `3e219b7` | Task 7: Conditional CC rendering, click-to-play, bindable captionsEnabled |
|
||||
| `2472dec` | Task 7 fix: bind captionsEnabled between App and VideoPlayer |
|
||||
| `1a86a34` | Task 8: FLIP transitions (fly/fade + CSS grid transition) |
|
||||
| `7e838dc` | Task 9: Version bump 0.1.3 → 0.2.0 |
|
||||
|
||||
### Files (17 changed, +1327 −92)
|
||||
|
||||
**New files:**
|
||||
- `src/lib/components/PlayerControls.svelte` — floating overlay with seek bar, play/pause, skip, volume, speed, CC, PiP, fullscreen
|
||||
- `src/lib/components/PlayerTimeline.svelte` — simplified waveform-only timeline
|
||||
- `src/lib/components/SpeedSelector.svelte` — playback speed popup (0.5x–2x)
|
||||
- `tests/lib/components/SpeedSelector.test.ts` — 3 component tests
|
||||
- `tests/lib/stores/preferences.test.ts` — 3 preference tests
|
||||
- `tests/lib/transport/playback.test.ts` — 10 playback helper tests
|
||||
|
||||
**Modified files:**
|
||||
- `src/App.svelte` — mode switching, auto-hide, keyboard shortcuts, layout conditionals
|
||||
- `src/app.css` — grid transitions, player-mode positioning
|
||||
- `src/lib/components/VideoPlayer.svelte` — bindable captionsEnabled, click-to-play, conditional CC
|
||||
- `src/lib/stores/preferences.svelte.ts` — appMode field + persistence
|
||||
- `src/lib/transport/playback.ts` — PiP, fullscreen, playbackRate exports
|
||||
- `vite.config.ts` — svelteTesting() plugin for Svelte 5 component tests
|
||||
|
||||
## Key Architecture Decisions
|
||||
|
||||
1. **Single always-mounted VideoPlayer** — video element stays in DOM across mode switches, avoiding destruction/recreation
|
||||
2. **Proximity-based auto-hide** — controls: any mouse move + 3s timeout; toolbar: 50px from top; timeline: 80px from bottom
|
||||
3. **Transport fallback** — `dispatchTransport()` falls back to `runTransportAction()` when TransportControls is null (Player mode)
|
||||
4. **CSS Grid transitions** — `.content` animates grid-template changes for smooth layout shifts
|
||||
5. **Polling for PiP/fullscreen state** — PlayerControls polls `isPiPActive()`/`isFullscreenActive()` at 500ms
|
||||
|
||||
## Bugs Found & Fixed
|
||||
|
||||
1. **Transport shortcuts no-op in Player mode** — TransportControls not rendered → `transportControls` null → all playback shortcuts broken. Fixed with `runTransportAction` fallback.
|
||||
2. **captionsEnabled not bound** — App.svelte and VideoPlayer had separate `captionsEnabled` states. Fixed with `bind:captionsEnabled`.
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
- When a component that handles keyboard dispatch is conditionally rendered, always provide a fallback path for the keyboard handler
|
||||
- Svelte 5 bindable props require explicit `$bindable()` wrapper and typed `$props()` destructuring
|
||||
- Svelte 5 component testing in vitest/jsdom needs `svelteTesting()` plugin in vite config (not documented in the plan)
|
||||
- Timer-based click/double-click discrimination (250ms delay) is the standard pattern for video players
|
||||
|
||||
## Test Results
|
||||
|
||||
- **8 test files, 47 tests passing**
|
||||
- **svelte-check: 0 errors** (1 pre-existing a11y warning — missing tabindex on slider role)
|
||||
|
||||
## Follow-up Items
|
||||
|
||||
- Add `tabindex="0"` to PlayerControls seek bar slider role (a11y warning)
|
||||
- Remove dead `handleVideoClick()` function in VideoPlayer.svelte (unused, superseded by `handleVideoClickWithDelay`)
|
||||
- Consider event-based PiP/fullscreen state updates instead of 500ms polling
|
||||
- Visual QA with `npm run tauri dev` for transition smoothness
|
||||
@@ -0,0 +1,42 @@
|
||||
# v0.2.0 Player Mode Bug Fixes — Summary
|
||||
|
||||
## Task Description
|
||||
|
||||
Fixed 8 issues found during QA of the v0.2.0 Player mode implementation. Issues ranged from a Svelte 5 reactivity bug preventing auto-hide from working, to a WKWebView fullscreen incompatibility, to keyboard shortcut mismatches.
|
||||
|
||||
## Changes Made
|
||||
|
||||
### 3 commits (980f31a..0f2a523), 3 files modified
|
||||
|
||||
| Commit | Description |
|
||||
|--------|-------------|
|
||||
| `980f31a` | Auto-hide timer fix, status bar hidden, `<`/`>` keyframe shortcuts |
|
||||
| `c00c215` | `adjustShuttle` reads live rate with 0.25 step, fullscreen targets `document.documentElement` |
|
||||
| `0f2a523` | QuickTime-style floating pill layout, speed selector `stopPropagation`, rate sync polling |
|
||||
|
||||
### Issues Fixed
|
||||
|
||||
1. **Floating controls layout** — Redesigned from full-width bottom-pinned to centered floating pill (`bottom: 24px; left: 15%; right: 15%`). Reorganized: controls row (top) → seek bar (middle) → timestamps (bottom). Matches QuickTime Player layout.
|
||||
2. **Controls visible when paused** — Removed special paused branch; auto-hide timer always runs regardless of play state.
|
||||
3. **Controls/toolbar never auto-hide** — `controlsHideTimer` and `toolbarHideTimer` were `$state`, causing an infinite reactive loop in the `$effect`. Changed to plain `let`.
|
||||
4. **Status bar visible** — Wrapped `<StatusBar>` in `{#if !isPlayerMode}`.
|
||||
5. **JKL skips 1.0x** — Changed `adjustShuttle` to read live rate from video element (not stale `shuttleRate`) and use 0.25 step (was 0.5).
|
||||
6. **Speed selector broken** — Added `e.stopPropagation()` on toggle button to prevent immediate close from window click handler.
|
||||
7. **Fullscreen broken** — Changed `toggleFullscreen()` to target `document.documentElement` instead of video element (WKWebView doesn't support element-level fullscreen API).
|
||||
8. **Shift+,/. keyframe shortcuts** — Added `<` and `>` key cases (Shift produces these characters, not `,`/`.`).
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
- **Svelte 5 `$state` in timer variables creates reactive loops**: If an `$effect` reads a `$state` timer ID to clear it, then writes a new one, Svelte re-triggers the effect infinitely. Timer IDs used only in imperative logic should be plain `let`.
|
||||
- **Keyboard `e.key` values change with Shift**: `Shift+,` produces `<`, not `,`. Always check the actual key value produced.
|
||||
- **WKWebView fullscreen**: `HTMLVideoElement.requestFullscreen()` doesn't work; must target `document.documentElement.requestFullscreen()` instead.
|
||||
- **Click-outside handlers and same-click toggles**: A `<svelte:window onclick>` handler fires on the same click event that mounted the component, causing immediate close. Use `stopPropagation` on the toggle button.
|
||||
|
||||
## Test Results
|
||||
|
||||
- **8 test files, 54 tests passing** (7 new tests for `adjustShuttle`)
|
||||
- **svelte-check: 0 errors, 0 warnings**
|
||||
|
||||
## Follow-up
|
||||
|
||||
- Visual QA with `npm run tauri dev` to verify all 8 fixes work as expected
|
||||
@@ -13,6 +13,8 @@
|
||||
"store:allow-set",
|
||||
"store:allow-save",
|
||||
"store:allow-load",
|
||||
"process:allow-exit"
|
||||
"process:allow-exit",
|
||||
"core:window:allow-set-fullscreen",
|
||||
"core:window:allow-is-fullscreen"
|
||||
]
|
||||
}
|
||||
|
||||
@@ -49,7 +49,6 @@
|
||||
// Auto-hide state for Player mode
|
||||
let showPlayerControls = $state(true);
|
||||
let showToolbar = $state(true);
|
||||
let showPlayerTimeline = $state(false);
|
||||
let controlsHideTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
let toolbarHideTimer: ReturnType<typeof setTimeout> | null = null;
|
||||
|
||||
@@ -58,7 +57,6 @@
|
||||
|
||||
const CONTROLS_HIDE_DELAY = 2500;
|
||||
const TOOLBAR_PROXIMITY = 50;
|
||||
const TIMELINE_PROXIMITY = 80;
|
||||
|
||||
let isLeft = $derived(preferences.clipListPosition === 'left');
|
||||
|
||||
@@ -169,10 +167,6 @@
|
||||
if (e.clientY < TOOLBAR_PROXIMITY) {
|
||||
resetToolbarTimer();
|
||||
}
|
||||
|
||||
// Timeline: show when near bottom edge
|
||||
const windowH = window.innerHeight;
|
||||
showPlayerTimeline = e.clientY > windowH - TIMELINE_PROXIMITY;
|
||||
}
|
||||
|
||||
function toggleMode() {
|
||||
@@ -405,7 +399,7 @@
|
||||
|
||||
<!-- Video area: always mounted, never destroyed -->
|
||||
<div class="video-area" class:player-video={isPlayerMode}>
|
||||
<VideoPlayer bind:captionsEnabled />
|
||||
<VideoPlayer bind:captionsEnabled playerControlsVisible={isPlayerMode && showPlayerControls} />
|
||||
{#if isPlayerMode}
|
||||
<PlayerControls
|
||||
visible={showPlayerControls}
|
||||
@@ -414,7 +408,7 @@
|
||||
onToggleCaptions={handleToggleCaptions}
|
||||
onOpenCaptionSettings={() => {}}
|
||||
/>
|
||||
<PlayerTimeline visible={showPlayerTimeline || !session.isPlaying} />
|
||||
<PlayerTimeline visible={showPlayerControls} />
|
||||
{:else}
|
||||
<div transition:fade={{ duration: 200 }}>
|
||||
<TransportControls bind:this={transportControls} />
|
||||
|
||||
@@ -115,11 +115,9 @@
|
||||
}, 100);
|
||||
}
|
||||
|
||||
function handleFullscreen() {
|
||||
toggleFullscreen();
|
||||
setTimeout(() => {
|
||||
fullscreenActive = isFullscreenActive();
|
||||
}, 100);
|
||||
async function handleFullscreen() {
|
||||
await toggleFullscreen();
|
||||
fullscreenActive = await isFullscreenActive();
|
||||
}
|
||||
|
||||
function handleCCContextMenu(e: MouseEvent) {
|
||||
@@ -129,9 +127,9 @@
|
||||
|
||||
// Sync PiP/fullscreen/rate state periodically
|
||||
$effect(() => {
|
||||
const interval = setInterval(() => {
|
||||
const interval = setInterval(async () => {
|
||||
pipActive = isPiPActive();
|
||||
fullscreenActive = isFullscreenActive();
|
||||
fullscreenActive = await isFullscreenActive();
|
||||
currentRate = getPlaybackRate();
|
||||
}, 500);
|
||||
return () => clearInterval(interval);
|
||||
@@ -279,7 +277,7 @@
|
||||
<style>
|
||||
.player-controls-overlay {
|
||||
position: absolute;
|
||||
bottom: 24px;
|
||||
bottom: 48px;
|
||||
left: 15%;
|
||||
right: 15%;
|
||||
pointer-events: none;
|
||||
|
||||
@@ -56,7 +56,9 @@
|
||||
}
|
||||
|
||||
// Draw playhead
|
||||
drawPlayhead(ctx, timelineState, session.currentTime);
|
||||
if (session.duration > 0) {
|
||||
drawPlayhead(ctx, timelineState, session.currentTime);
|
||||
}
|
||||
|
||||
ctx.restore();
|
||||
}
|
||||
|
||||
@@ -20,8 +20,10 @@
|
||||
|
||||
let {
|
||||
captionsEnabled = $bindable(true),
|
||||
playerControlsVisible = false,
|
||||
}: {
|
||||
captionsEnabled?: boolean;
|
||||
playerControlsVisible?: boolean;
|
||||
} = $props();
|
||||
|
||||
let videoElement = $state<HTMLVideoElement | null>(null);
|
||||
@@ -232,7 +234,7 @@
|
||||
<track kind="captions" />
|
||||
</video>
|
||||
{#if activeCues.length > 0}
|
||||
<div class="caption-overlay" style={captionPosition}>
|
||||
<div class="caption-overlay" class:lifted={isPlayerMode && playerControlsVisible} style={captionPosition}>
|
||||
{#each activeCues as cue}
|
||||
<span
|
||||
class="caption-text"
|
||||
@@ -325,6 +327,11 @@
|
||||
pointer-events: none;
|
||||
z-index: 5;
|
||||
padding: 0 10%;
|
||||
transition: transform 0.2s ease;
|
||||
}
|
||||
|
||||
.caption-overlay.lifted {
|
||||
transform: translateY(-160px);
|
||||
}
|
||||
|
||||
.caption-text {
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { session } from '$lib/stores/videoSession.svelte';
|
||||
import { getCurrentWindow } from '@tauri-apps/api/window';
|
||||
|
||||
let _videoEl: HTMLVideoElement | null = null;
|
||||
|
||||
@@ -116,25 +117,22 @@ export function isPiPActive(): boolean {
|
||||
return !!document.pictureInPictureElement;
|
||||
}
|
||||
|
||||
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();
|
||||
}
|
||||
export async function toggleFullscreen(): Promise<void> {
|
||||
try {
|
||||
const win = getCurrentWindow();
|
||||
const isFs = await win.isFullscreen();
|
||||
await win.setFullscreen(!isFs);
|
||||
} catch (e) {
|
||||
console.error('Fullscreen toggle failed:', e);
|
||||
}
|
||||
}
|
||||
|
||||
export function isFullscreenActive(): boolean {
|
||||
return !!document.fullscreenElement;
|
||||
export async function isFullscreenActive(): Promise<boolean> {
|
||||
try {
|
||||
return await getCurrentWindow().isFullscreen();
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function setPlaybackRate(rate: number): void {
|
||||
|
||||
@@ -11,6 +11,15 @@ vi.mock('$lib/stores/videoSession.svelte', () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
// Mock Tauri window API for fullscreen
|
||||
const mockTauriWindow = {
|
||||
isFullscreen: vi.fn().mockResolvedValue(false),
|
||||
setFullscreen: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
vi.mock('@tauri-apps/api/window', () => ({
|
||||
getCurrentWindow: () => mockTauriWindow,
|
||||
}));
|
||||
|
||||
import {
|
||||
setVideoElement,
|
||||
togglePiP,
|
||||
@@ -70,36 +79,30 @@ describe('playback — PiP helpers', () => {
|
||||
});
|
||||
|
||||
describe('playback — fullscreen helpers', () => {
|
||||
let mockVideo: HTMLVideoElement;
|
||||
|
||||
beforeEach(() => {
|
||||
mockVideo = createMockVideoElement();
|
||||
setVideoElement(mockVideo);
|
||||
Object.defineProperty(document, 'fullscreenElement', {
|
||||
value: null,
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
mockTauriWindow.isFullscreen.mockResolvedValue(false);
|
||||
mockTauriWindow.setFullscreen.mockResolvedValue(undefined);
|
||||
});
|
||||
|
||||
it('isFullscreenActive returns false when not fullscreen', () => {
|
||||
expect(isFullscreenActive()).toBe(false);
|
||||
it('isFullscreenActive returns false when not fullscreen', async () => {
|
||||
expect(await isFullscreenActive()).toBe(false);
|
||||
});
|
||||
|
||||
it('toggleFullscreen calls requestFullscreen on document.documentElement', () => {
|
||||
const spy = vi.fn().mockResolvedValue(undefined);
|
||||
document.documentElement.requestFullscreen = spy;
|
||||
toggleFullscreen();
|
||||
expect(spy).toHaveBeenCalled();
|
||||
it('toggleFullscreen calls setFullscreen(true) when not fullscreen', async () => {
|
||||
mockTauriWindow.isFullscreen.mockResolvedValue(false);
|
||||
await toggleFullscreen();
|
||||
expect(mockTauriWindow.setFullscreen).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it('isFullscreenActive returns true when fullscreen element exists', () => {
|
||||
Object.defineProperty(document, 'fullscreenElement', {
|
||||
value: mockVideo,
|
||||
writable: true,
|
||||
configurable: true,
|
||||
});
|
||||
expect(isFullscreenActive()).toBe(true);
|
||||
it('toggleFullscreen calls setFullscreen(false) when fullscreen', async () => {
|
||||
mockTauriWindow.isFullscreen.mockResolvedValue(true);
|
||||
await toggleFullscreen();
|
||||
expect(mockTauriWindow.setFullscreen).toHaveBeenCalledWith(false);
|
||||
});
|
||||
|
||||
it('isFullscreenActive returns true when fullscreen', async () => {
|
||||
mockTauriWindow.isFullscreen.mockResolvedValue(true);
|
||||
expect(await isFullscreenActive()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user