From c8e42b3973fbb82d237546e335172100f207a5ae Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 30 Mar 2026 18:43:41 -0700 Subject: [PATCH] refactor: split playlist browser wiring --- ...l-for-sibling-video-files-and-mpv-queue.md | 8 +- src/main.ts | 23 +-- src/main/runtime/playlist-browser-ipc.ts | 46 ++++++ .../runtime/playlist-browser-runtime.test.ts | 107 +++++++++++-- src/main/runtime/playlist-browser-runtime.ts | 43 ++++-- src/renderer/handlers/keyboard.test.ts | 19 +++ src/renderer/handlers/keyboard.ts | 11 +- .../modals/playlist-browser-renderer.ts | 144 ++++++++++++++++++ src/renderer/modals/playlist-browser.test.ts | 82 ++++++++++ src/renderer/modals/playlist-browser.ts | 142 +++-------------- 10 files changed, 455 insertions(+), 170 deletions(-) create mode 100644 src/main/runtime/playlist-browser-ipc.ts create mode 100644 src/renderer/modals/playlist-browser-renderer.ts diff --git a/backlog/tasks/task-255 - Add-overlay-playlist-browser-modal-for-sibling-video-files-and-mpv-queue.md b/backlog/tasks/task-255 - Add-overlay-playlist-browser-modal-for-sibling-video-files-and-mpv-queue.md index fd761307..2e005a41 100644 --- a/backlog/tasks/task-255 - Add-overlay-playlist-browser-modal-for-sibling-video-files-and-mpv-queue.md +++ b/backlog/tasks/task-255 - Add-overlay-playlist-browser-modal-for-sibling-video-files-and-mpv-queue.md @@ -5,7 +5,7 @@ status: In Progress assignee: - codex created_date: '2026-03-30 05:46' -updated_date: '2026-03-30 09:22' +updated_date: '2026-03-31 01:42' labels: - feature - overlay @@ -64,4 +64,10 @@ Repo gate blockers outside this feature: `bun run test:fast` hits existing Bun ` 2026-03-30: Pulled unresolved CodeRabbit review threads for PR #37. Actionable set is three items: unreadable-file stat error handling in playlist-browser runtime, stale playlist-browser DOM after failed refresh/close, and missing modal-ownership guard before opening the playlist-browser overlay. Proceeding test-first for each. 2026-03-30: Addressed current CodeRabbit follow-up findings for PR #37. Fixed playlist-browser unreadable-file stat handling, stale playlist-browser DOM reset on refresh failure/close, modal-ownership guard before opening the playlist-browser overlay, async rejection surfacing for PLAYLIST_BROWSER_OPEN IPC commands, overlay bootstrap before playlist-browser open dispatch, texthooker option normalization in the mpv plugin, and superseded local subtitle-rearm suppression. Added targeted regressions plus new playlist-browser-open helper coverage. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts src/main/runtime/playlist-browser-open.test.ts src/core/services/ipc-command.test.ts src/renderer/modals/playlist-browser.test.ts`, `lua scripts/test-plugin-start-gate.lua`, `bun run typecheck`, `bun run build`. + +Addressed CodeRabbit follow-ups on the playlist browser PR: clamped stale playingIndex values, failed mutation paths when MPV rejects send(), added temp-dir cleanup in runtime tests, and blocked action-button dblclick bubbling in the renderer. Verification: `bun run typecheck`, `bun run build`, `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts`. + +Additional follow-up: moved playlist-browser keydown handling ahead of keyboard-driven lookup controls so KeyH/ArrowLeft/ArrowRight and related chords are routed to the modal first. Verification refreshed with `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`, `bun run typecheck`, and `bun run build`. + +Split playlist-browser UI row rendering into `src/renderer/modals/playlist-browser-renderer.ts` and left `src/renderer/modals/playlist-browser.ts` as the controller/wiring layer. Moved playlist-browser IPC/runtime wiring into `src/main/runtime/playlist-browser-ipc.ts` and collapsed the `src/main.ts` registration block to use that helper. Verification after refactor: `bun run typecheck`, `bun run build`, `bun test src/main/runtime/playlist-browser-runtime.test.ts src/renderer/modals/playlist-browser.test.ts src/renderer/handlers/keyboard.test.ts`. diff --git a/src/main.ts b/src/main.ts index c347fec2..a1645ded 100644 --- a/src/main.ts +++ b/src/main.ts @@ -428,13 +428,7 @@ import { handleCliCommandRuntimeServiceWithContext } from './main/cli-runtime'; import { createOverlayModalRuntimeService } from './main/overlay-runtime'; import { createOverlayModalInputState } from './main/runtime/overlay-modal-input-state'; import { openYoutubeTrackPicker } from './main/runtime/youtube-picker-open'; -import { - appendPlaylistBrowserFileRuntime, - getPlaylistBrowserSnapshotRuntime, - movePlaylistBrowserIndexRuntime, - playPlaylistBrowserIndexRuntime, - removePlaylistBrowserIndexRuntime, -} from './main/runtime/playlist-browser-runtime'; +import { createPlaylistBrowserIpcRuntime } from './main/runtime/playlist-browser-ipc'; import { createOverlayShortcutsRuntimeService } from './main/overlay-shortcuts-runtime'; import { createFrequencyDictionaryRuntimeService, @@ -4134,9 +4128,7 @@ const shiftSubtitleDelayToAdjacentCueHandler = createShiftSubtitleDelayToAdjacen showMpvOsd: (text) => showMpvOsd(text), }); -const playlistBrowserRuntimeDeps = { - getMpvClient: () => appState.mpvClient, -}; +const { playlistBrowserMainDeps } = createPlaylistBrowserIpcRuntime(() => appState.mpvClient); const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({ mpvCommandMainDeps: { @@ -4320,16 +4312,7 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({ getAnilistQueueStatus: () => anilistStateRuntime.getQueueStatusSnapshot(), retryAnilistQueueNow: () => processNextAnilistRetryUpdate(), appendClipboardVideoToQueue: () => appendClipboardVideoToQueue(), - getPlaylistBrowserSnapshot: () => - getPlaylistBrowserSnapshotRuntime(playlistBrowserRuntimeDeps), - appendPlaylistBrowserFile: (filePath) => - appendPlaylistBrowserFileRuntime(playlistBrowserRuntimeDeps, filePath), - playPlaylistBrowserIndex: (index) => - playPlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index), - removePlaylistBrowserIndex: (index) => - removePlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index), - movePlaylistBrowserIndex: (index, direction) => - movePlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index, direction), + ...playlistBrowserMainDeps, getImmersionTracker: () => appState.immersionTracker, }, ankiJimakuDeps: createAnkiJimakuIpcRuntimeServiceDeps({ diff --git a/src/main/runtime/playlist-browser-ipc.ts b/src/main/runtime/playlist-browser-ipc.ts new file mode 100644 index 00000000..3dbecf43 --- /dev/null +++ b/src/main/runtime/playlist-browser-ipc.ts @@ -0,0 +1,46 @@ +import type { RegisterIpcRuntimeServicesParams } from '../ipc-runtime'; +import { + appendPlaylistBrowserFileRuntime, + getPlaylistBrowserSnapshotRuntime, + movePlaylistBrowserIndexRuntime, + playPlaylistBrowserIndexRuntime, + removePlaylistBrowserIndexRuntime, + type PlaylistBrowserRuntimeDeps, +} from './playlist-browser-runtime'; + +type PlaylistBrowserMainDeps = Pick< + RegisterIpcRuntimeServicesParams['mainDeps'], + | 'getPlaylistBrowserSnapshot' + | 'appendPlaylistBrowserFile' + | 'playPlaylistBrowserIndex' + | 'removePlaylistBrowserIndex' + | 'movePlaylistBrowserIndex' +>; + +export type PlaylistBrowserIpcRuntime = { + playlistBrowserRuntimeDeps: PlaylistBrowserRuntimeDeps; + playlistBrowserMainDeps: PlaylistBrowserMainDeps; +}; + +export function createPlaylistBrowserIpcRuntime( + getMpvClient: PlaylistBrowserRuntimeDeps['getMpvClient'], +): PlaylistBrowserIpcRuntime { + const playlistBrowserRuntimeDeps: PlaylistBrowserRuntimeDeps = { + getMpvClient, + }; + + return { + playlistBrowserRuntimeDeps, + playlistBrowserMainDeps: { + getPlaylistBrowserSnapshot: () => getPlaylistBrowserSnapshotRuntime(playlistBrowserRuntimeDeps), + appendPlaylistBrowserFile: (filePath: string) => + appendPlaylistBrowserFileRuntime(playlistBrowserRuntimeDeps, filePath), + playPlaylistBrowserIndex: (index: number) => + playPlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index), + removePlaylistBrowserIndex: (index: number) => + removePlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index), + movePlaylistBrowserIndex: (index: number, direction: 1 | -1) => + movePlaylistBrowserIndexRuntime(playlistBrowserRuntimeDeps, index, direction), + }, + }; +} diff --git a/src/main/runtime/playlist-browser-runtime.test.ts b/src/main/runtime/playlist-browser-runtime.test.ts index 970ad6ea..6c0e2433 100644 --- a/src/main/runtime/playlist-browser-runtime.test.ts +++ b/src/main/runtime/playlist-browser-runtime.test.ts @@ -2,7 +2,7 @@ import assert from 'node:assert/strict'; import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; -import test from 'node:test'; +import test, { type TestContext } from 'node:test'; import type { PlaylistBrowserQueueItem } from '../../types'; import { @@ -21,8 +21,12 @@ type FakePlaylistEntry = { id?: number; }; -function createTempVideoDir(): string { - return fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-playlist-browser-')); +function createTempVideoDir(t: TestContext): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-playlist-browser-')); + t.after(() => { + fs.rmSync(dir, { recursive: true, force: true }); + }); + return dir; } function createFakeMpvClient(options: { @@ -121,8 +125,8 @@ function createFakeMpvClient(options: { }; } -test('getPlaylistBrowserSnapshotRuntime lists sibling videos in best-effort episode order', async () => { - const dir = createTempVideoDir(); +test('getPlaylistBrowserSnapshotRuntime lists sibling videos in best-effort episode order', async (t) => { + const dir = createTempVideoDir(t); const episode2 = path.join(dir, 'Show - S01E02.mkv'); const episode1 = path.join(dir, 'Show - S01E01.mkv'); const special = path.join(dir, 'Show - Special.mp4'); @@ -169,6 +173,35 @@ test('getPlaylistBrowserSnapshotRuntime lists sibling videos in best-effort epis ); }); +test('getPlaylistBrowserSnapshotRuntime clamps stale playing index to the playlist bounds', async (t) => { + const dir = createTempVideoDir(t); + const episode1 = path.join(dir, 'Show - S01E01.mkv'); + const episode2 = path.join(dir, 'Show - S01E02.mkv'); + fs.writeFileSync(episode1, ''); + fs.writeFileSync(episode2, ''); + + const mpvClient = createFakeMpvClient({ + currentVideoPath: episode1, + playlist: [ + { filename: episode1, current: true, playing: true, title: 'Episode 1' }, + { filename: episode2, title: 'Episode 2' }, + ], + }); + const requestProperty = mpvClient.requestProperty.bind(mpvClient); + mpvClient.requestProperty = async (name: string): Promise => { + if (name === 'playlist-playing-pos') { + return 99; + } + return requestProperty(name); + }; + + const snapshot = await getPlaylistBrowserSnapshotRuntime({ + getMpvClient: () => mpvClient, + }); + + assert.equal(snapshot.playingIndex, 1); +}); + test('getPlaylistBrowserSnapshotRuntime degrades directory pane for remote media', async () => { const mpvClient = createFakeMpvClient({ currentVideoPath: 'https://example.com/video.m3u8', @@ -185,8 +218,8 @@ test('getPlaylistBrowserSnapshotRuntime degrades directory pane for remote media assert.equal(snapshot.playlistItems.length, 1); }); -test('playlist-browser mutation runtimes mutate queue and return refreshed snapshots', async () => { - const dir = createTempVideoDir(); +test('playlist-browser mutation runtimes mutate queue and return refreshed snapshots', async (t) => { + const dir = createTempVideoDir(t); const episode1 = path.join(dir, 'Show - S01E01.mkv'); const episode2 = path.join(dir, 'Show - S01E02.mkv'); const episode3 = path.join(dir, 'Show - S01E03.mkv'); @@ -249,8 +282,52 @@ test('playlist-browser mutation runtimes mutate queue and return refreshed snaps ); }); -test('appendPlaylistBrowserFileRuntime returns an error result when statSync throws', async () => { - const dir = createTempVideoDir(); +test('playlist-browser mutation runtimes report MPV send rejection', async (t) => { + const dir = createTempVideoDir(t); + const episode1 = path.join(dir, 'Show - S01E01.mkv'); + const episode2 = path.join(dir, 'Show - S01E02.mkv'); + const episode3 = path.join(dir, 'Show - S01E03.mkv'); + fs.writeFileSync(episode1, ''); + fs.writeFileSync(episode2, ''); + fs.writeFileSync(episode3, ''); + + const mpvClient = createFakeMpvClient({ + currentVideoPath: episode1, + playlist: [ + { filename: episode1, current: true, title: 'Episode 1' }, + { filename: episode2, title: 'Episode 2' }, + { filename: episode3, title: 'Episode 3' }, + ], + }); + const scheduled: Array<{ callback: () => void; delayMs: number }> = []; + mpvClient.send = () => false; + const deps = { + getMpvClient: () => mpvClient, + schedule: (callback: () => void, delayMs: number) => { + scheduled.push({ callback, delayMs }); + }, + }; + + const appendResult = await appendPlaylistBrowserFileRuntime(deps, episode3); + assert.equal(appendResult.ok, false); + assert.equal(appendResult.snapshot, undefined); + + const playResult = await playPlaylistBrowserIndexRuntime(deps, 1); + assert.equal(playResult.ok, false); + assert.equal(playResult.snapshot, undefined); + assert.deepEqual(scheduled, []); + + const removeResult = await removePlaylistBrowserIndexRuntime(deps, 1); + assert.equal(removeResult.ok, false); + assert.equal(removeResult.snapshot, undefined); + + const moveResult = await movePlaylistBrowserIndexRuntime(deps, 1, 1); + assert.equal(moveResult.ok, false); + assert.equal(moveResult.snapshot, undefined); +}); + +test('appendPlaylistBrowserFileRuntime returns an error result when statSync throws', async (t) => { + const dir = createTempVideoDir(t); const episode1 = path.join(dir, 'Show - S01E01.mkv'); fs.writeFileSync(episode1, ''); @@ -284,8 +361,8 @@ test('appendPlaylistBrowserFileRuntime returns an error result when statSync thr } }); -test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', async () => { - const dir = createTempVideoDir(); +test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', async (t) => { + const dir = createTempVideoDir(t); const episode1 = path.join(dir, 'Show - S01E01.mkv'); const episode2 = path.join(dir, 'Show - S01E02.mkv'); fs.writeFileSync(episode1, ''); @@ -316,8 +393,8 @@ test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', as }); }); -test('getPlaylistBrowserSnapshotRuntime normalizes playlist labels from title then filename', async () => { - const dir = createTempVideoDir(); +test('getPlaylistBrowserSnapshotRuntime normalizes playlist labels from title then filename', async (t) => { + const dir = createTempVideoDir(t); const episode1 = path.join(dir, 'Show - S01E01.mkv'); fs.writeFileSync(episode1, ''); @@ -360,8 +437,8 @@ test('playPlaylistBrowserIndexRuntime skips local subtitle reset for remote play assert.equal(scheduled.length, 0); }); -test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm callbacks', async () => { - const dir = createTempVideoDir(); +test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm callbacks', async (t) => { + const dir = createTempVideoDir(t); const episode1 = path.join(dir, 'Show - S01E01.mkv'); const episode2 = path.join(dir, 'Show - S01E02.mkv'); const episode3 = path.join(dir, 'Show - S01E03.mkv'); diff --git a/src/main/runtime/playlist-browser-runtime.ts b/src/main/runtime/playlist-browser-runtime.ts index 5d99a389..2a9324a2 100644 --- a/src/main/runtime/playlist-browser-runtime.ts +++ b/src/main/runtime/playlist-browser-runtime.ts @@ -148,12 +148,33 @@ function ensureConnectedClient( return client; } +function buildRejectedCommandResult(): PlaylistBrowserMutationResult { + return { + ok: false, + message: 'Could not send command to MPV.', + }; +} + async function getPlaylistItemsFromClient( client: MpvPlaylistBrowserClientLike | null, ): Promise { return normalizePlaylistItems(await readProperty(client, 'playlist')); } +function resolvePlayingIndex( + playlistItems: PlaylistBrowserQueueItem[], + playingPosValue: unknown, +): number | null { + if (playlistItems.length === 0) { + return null; + } + if (typeof playingPosValue === 'number' && Number.isInteger(playingPosValue)) { + return Math.min(Math.max(playingPosValue, 0), playlistItems.length - 1); + } + const playingIndex = playlistItems.findIndex((item) => item.current || item.playing); + return playingIndex >= 0 ? playingIndex : null; +} + export async function getPlaylistBrowserSnapshotRuntime( deps: PlaylistBrowserRuntimeDeps, ): Promise { @@ -163,15 +184,11 @@ export async function getPlaylistBrowserSnapshotRuntime( getPlaylistItemsFromClient(client), readProperty(client, 'playlist-playing-pos'), ]); - const playingIndex = - typeof playingPosValue === 'number' && Number.isInteger(playingPosValue) - ? playingPosValue - : playlistItems.findIndex((item) => item.current || item.playing); return { ...resolveDirectorySnapshot(currentFilePath), playlistItems, - playingIndex: playingIndex >= 0 ? playingIndex : null, + playingIndex: resolvePlayingIndex(playlistItems, playingPosValue), currentFilePath, }; } @@ -270,7 +287,9 @@ export async function appendPlaylistBrowserFileRuntime( }; } - client.send({ command: ['loadfile', resolvedPath, 'append'] }); + if (!client.send({ command: ['loadfile', resolvedPath, 'append'] })) { + return buildRejectedCommandResult(); + } return buildMutationResult(`Queued ${path.basename(resolvedPath)}`, deps); } @@ -287,7 +306,9 @@ export async function playPlaylistBrowserIndexRuntime( if (isLocalPlaylistItem(targetItem)) { prepareLocalSubtitleAutoload(result.client); } - result.client.send({ command: ['playlist-play-index', index] }); + if (!result.client.send({ command: ['playlist-play-index', index] })) { + return buildRejectedCommandResult(); + } if (isLocalPlaylistItem(targetItem)) { scheduleLocalSubtitleSelectionRearm(deps, result.client, path.resolve(targetItem.path)); } @@ -303,7 +324,9 @@ export async function removePlaylistBrowserIndexRuntime( return result; } - result.client.send({ command: ['playlist-remove', index] }); + if (!result.client.send({ command: ['playlist-remove', index] })) { + return buildRejectedCommandResult(); + } return buildMutationResult(`Removed playlist item ${index + 1}`, deps); } @@ -331,6 +354,8 @@ export async function movePlaylistBrowserIndexRuntime( }; } - result.client.send({ command: ['playlist-move', index, targetIndex] }); + if (!result.client.send({ command: ['playlist-move', index, targetIndex] })) { + return buildRejectedCommandResult(); + } return buildMutationResult(`Moved playlist item ${index + 1}`, deps); } diff --git a/src/renderer/handlers/keyboard.test.ts b/src/renderer/handlers/keyboard.test.ts index 2825bc85..d81f1cbe 100644 --- a/src/renderer/handlers/keyboard.test.ts +++ b/src/renderer/handlers/keyboard.test.ts @@ -653,6 +653,25 @@ test('keyboard mode: playlist browser modal handles arrow keys before yomitan po } }); +test('keyboard mode: playlist browser modal handles h before lookup controls', async () => { + const { ctx, testGlobals, handlers, playlistBrowserKeydownCount } = + createKeyboardHandlerHarness(); + + try { + await handlers.setupMpvInputForwarding(); + handlers.handleKeyboardModeToggleRequested(); + ctx.state.playlistBrowserModalOpen = true; + ctx.state.keyboardSelectedWordIndex = 2; + + testGlobals.dispatchKeydown({ key: 'h', code: 'KeyH' }); + + assert.equal(playlistBrowserKeydownCount(), 1); + assert.equal(ctx.state.keyboardSelectedWordIndex, 2); + } finally { + testGlobals.restore(); + } +}); + test('keyboard mode: configured stats toggle works even while popup is open', async () => { const { handlers, testGlobals } = createKeyboardHandlerHarness(); diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index 80313a70..a98bcea9 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -816,6 +816,12 @@ export function createKeyboardHandlers( return; } + if (ctx.state.playlistBrowserModalOpen) { + if (options.handlePlaylistBrowserKeydown(e)) { + return; + } + } + if (handleKeyboardDrivenModeLookupControls(e)) { e.preventDefault(); return; @@ -842,11 +848,6 @@ export function createKeyboardHandlers( return; } } - if (ctx.state.playlistBrowserModalOpen) { - if (options.handlePlaylistBrowserKeydown(e)) { - return; - } - } if (ctx.state.controllerSelectModalOpen) { options.handleControllerSelectKeydown(e); return; diff --git a/src/renderer/modals/playlist-browser-renderer.ts b/src/renderer/modals/playlist-browser-renderer.ts new file mode 100644 index 00000000..d3e9d24d --- /dev/null +++ b/src/renderer/modals/playlist-browser-renderer.ts @@ -0,0 +1,144 @@ +import type { + PlaylistBrowserDirectoryItem, + PlaylistBrowserQueueItem, +} from '../../types'; +import type { RendererContext } from '../context'; + +type PlaylistBrowserRowRenderActions = { + appendDirectoryItem: (filePath: string) => void; + movePlaylistItem: (index: number, direction: 1 | -1) => void; + playPlaylistItem: (index: number) => void; + removePlaylistItem: (index: number) => void; + render: () => void; +}; + +function createActionButton(label: string, onClick: () => void): HTMLButtonElement { + const button = document.createElement('button'); + button.type = 'button'; + button.textContent = label; + button.className = 'playlist-browser-action'; + button.addEventListener('click', (event) => { + event.stopPropagation(); + onClick(); + }); + button.addEventListener('dblclick', (event) => { + event.preventDefault?.(); + event.stopPropagation(); + }); + return button; +} + +export function renderPlaylistBrowserDirectoryRow( + ctx: RendererContext, + item: PlaylistBrowserDirectoryItem, + index: number, + actions: PlaylistBrowserRowRenderActions, +): HTMLElement { + const row = document.createElement('li'); + row.className = 'playlist-browser-row'; + if (item.isCurrentFile) row.classList.add('current'); + if ( + ctx.state.playlistBrowserActivePane === 'directory' && + ctx.state.playlistBrowserSelectedDirectoryIndex === index + ) { + row.classList.add('active'); + } + + const main = document.createElement('div'); + main.className = 'playlist-browser-row-main'; + const label = document.createElement('div'); + label.className = 'playlist-browser-row-label'; + label.textContent = item.basename; + const meta = document.createElement('div'); + meta.className = 'playlist-browser-row-meta'; + meta.textContent = item.isCurrentFile + ? item.episodeLabel + ? `${item.episodeLabel} · Current file` + : 'Current file' + : item.episodeLabel ?? 'Video file'; + main.append(label, meta); + + const trailing = document.createElement('div'); + trailing.className = 'playlist-browser-row-trailing'; + if (item.episodeLabel) { + const badge = document.createElement('div'); + badge.className = 'playlist-browser-chip'; + badge.textContent = item.episodeLabel; + trailing.appendChild(badge); + } + trailing.appendChild( + createActionButton('Add', () => { + void actions.appendDirectoryItem(item.path); + }), + ); + + row.append(main, trailing); + row.addEventListener('click', () => { + ctx.state.playlistBrowserActivePane = 'directory'; + ctx.state.playlistBrowserSelectedDirectoryIndex = index; + actions.render(); + }); + row.addEventListener('dblclick', () => { + ctx.state.playlistBrowserSelectedDirectoryIndex = index; + void actions.appendDirectoryItem(item.path); + }); + return row; +} + +export function renderPlaylistBrowserPlaylistRow( + ctx: RendererContext, + item: PlaylistBrowserQueueItem, + index: number, + actions: PlaylistBrowserRowRenderActions, +): HTMLElement { + const row = document.createElement('li'); + row.className = 'playlist-browser-row'; + if (item.current || item.playing) row.classList.add('current'); + if ( + ctx.state.playlistBrowserActivePane === 'playlist' && + ctx.state.playlistBrowserSelectedPlaylistIndex === index + ) { + row.classList.add('active'); + } + + const main = document.createElement('div'); + main.className = 'playlist-browser-row-main'; + const label = document.createElement('div'); + label.className = 'playlist-browser-row-label'; + label.textContent = `${index + 1}. ${item.displayLabel}`; + const meta = document.createElement('div'); + meta.className = 'playlist-browser-row-meta'; + meta.textContent = item.current || item.playing ? 'Playing now' : 'Queued'; + const submeta = document.createElement('div'); + submeta.className = 'playlist-browser-row-submeta'; + submeta.textContent = item.filename; + main.append(label, meta, submeta); + + const trailing = document.createElement('div'); + trailing.className = 'playlist-browser-row-actions'; + trailing.append( + createActionButton('Play', () => { + void actions.playPlaylistItem(item.index); + }), + createActionButton('Up', () => { + void actions.movePlaylistItem(item.index, -1); + }), + createActionButton('Down', () => { + void actions.movePlaylistItem(item.index, 1); + }), + createActionButton('Remove', () => { + void actions.removePlaylistItem(item.index); + }), + ); + row.append(main, trailing); + row.addEventListener('click', () => { + ctx.state.playlistBrowserActivePane = 'playlist'; + ctx.state.playlistBrowserSelectedPlaylistIndex = index; + actions.render(); + }); + row.addEventListener('dblclick', () => { + ctx.state.playlistBrowserSelectedPlaylistIndex = index; + void actions.playPlaylistItem(item.index); + }); + return row; +} diff --git a/src/renderer/modals/playlist-browser.test.ts b/src/renderer/modals/playlist-browser.test.ts index 432c6b39..7941559f 100644 --- a/src/renderer/modals/playlist-browser.test.ts +++ b/src/renderer/modals/playlist-browser.test.ts @@ -216,6 +216,88 @@ test('playlist browser modal opens with playlist-focused current item selection' } }); +test('playlist browser modal action buttons stop double-click propagation', async () => { + const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; + const previousWindow = globals.window; + const previousDocument = globals.document; + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + getPlaylistBrowserSnapshot: async () => createSnapshot(), + notifyOverlayModalOpened: () => {}, + notifyOverlayModalClosed: () => {}, + focusMainWindow: async () => {}, + setIgnoreMouseEvents: () => {}, + appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }), + playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }), + removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }), + movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }), + } as unknown as ElectronAPI, + focus: () => {}, + }, + }); + Object.defineProperty(globalThis, 'document', { + configurable: true, + value: { + createElement: () => createPlaylistRow(), + }, + }); + + try { + const state = createRendererState(); + const directoryList = createListStub(); + const playlistList = createListStub(); + const ctx = { + state, + platform: { + shouldToggleMouseIgnore: false, + }, + dom: { + overlay: { + classList: createClassList(), + focus: () => {}, + }, + playlistBrowserModal: createFakeElement(), + playlistBrowserTitle: createFakeElement(), + playlistBrowserStatus: createFakeElement(), + playlistBrowserDirectoryList: directoryList, + playlistBrowserPlaylistList: playlistList, + playlistBrowserClose: createFakeElement(), + }, + }; + + const modal = createPlaylistBrowserModal(ctx as never, { + modalStateReader: { isAnyModalOpen: () => false }, + syncSettingsModalSubtitleSuppression: () => {}, + }); + + await modal.openPlaylistBrowserModal(); + + const row = directoryList.children[0] as ReturnType | undefined; + const trailing = row?.children?.[1] as ReturnType | undefined; + const button = + trailing?.children?.at(-1) as + | { listeners?: Map void>> } + | undefined; + const dblclickHandler = button?.listeners?.get('dblclick')?.[0]; + + assert.equal(typeof dblclickHandler, 'function'); + let stopped = false; + dblclickHandler?.({ + stopPropagation: () => { + stopped = true; + }, + }); + + assert.equal(stopped, true); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); + } +}); + test('playlist browser modal keydown routes append, remove, reorder, tab switch, and play', async () => { const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; const previousWindow = globals.window; diff --git a/src/renderer/modals/playlist-browser.ts b/src/renderer/modals/playlist-browser.ts index b8392619..6c002541 100644 --- a/src/renderer/modals/playlist-browser.ts +++ b/src/renderer/modals/playlist-browser.ts @@ -1,28 +1,19 @@ import type { - PlaylistBrowserDirectoryItem, PlaylistBrowserMutationResult, PlaylistBrowserQueueItem, PlaylistBrowserSnapshot, } from '../../types'; import type { ModalStateReader, RendererContext } from '../context'; +import { + renderPlaylistBrowserDirectoryRow, + renderPlaylistBrowserPlaylistRow, +} from './playlist-browser-renderer.js'; function clampIndex(index: number, length: number): number { if (length <= 0) return 0; return Math.min(Math.max(index, 0), length - 1); } -function createActionButton(label: string, onClick: () => void): HTMLButtonElement { - const button = document.createElement('button'); - button.type = 'button'; - button.textContent = label; - button.className = 'playlist-browser-action'; - button.addEventListener('click', (event) => { - event.stopPropagation(); - onClick(); - }); - return button; -} - function buildDefaultStatus(snapshot: PlaylistBrowserSnapshot): string { const directoryCount = snapshot.directoryItems.length; const playlistCount = snapshot.playlistItems.length; @@ -75,111 +66,6 @@ export function createPlaylistBrowserModal( ); } - function renderDirectoryRow(item: PlaylistBrowserDirectoryItem, index: number): HTMLElement { - const row = document.createElement('li'); - row.className = 'playlist-browser-row'; - if (item.isCurrentFile) row.classList.add('current'); - if ( - ctx.state.playlistBrowserActivePane === 'directory' && - ctx.state.playlistBrowserSelectedDirectoryIndex === index - ) { - row.classList.add('active'); - } - - const main = document.createElement('div'); - main.className = 'playlist-browser-row-main'; - const label = document.createElement('div'); - label.className = 'playlist-browser-row-label'; - label.textContent = item.basename; - const meta = document.createElement('div'); - meta.className = 'playlist-browser-row-meta'; - meta.textContent = item.isCurrentFile - ? item.episodeLabel - ? `${item.episodeLabel} · Current file` - : 'Current file' - : item.episodeLabel ?? 'Video file'; - main.append(label, meta); - - const trailing = document.createElement('div'); - trailing.className = 'playlist-browser-row-trailing'; - if (item.episodeLabel) { - const badge = document.createElement('div'); - badge.className = 'playlist-browser-chip'; - badge.textContent = item.episodeLabel; - trailing.appendChild(badge); - } - trailing.appendChild( - createActionButton('Add', () => { - void appendDirectoryItem(item.path); - }), - ); - - row.append(main, trailing); - row.addEventListener('click', () => { - ctx.state.playlistBrowserActivePane = 'directory'; - ctx.state.playlistBrowserSelectedDirectoryIndex = index; - render(); - }); - row.addEventListener('dblclick', () => { - ctx.state.playlistBrowserSelectedDirectoryIndex = index; - void appendDirectoryItem(item.path); - }); - return row; - } - - function renderPlaylistRow(item: PlaylistBrowserQueueItem, index: number): HTMLElement { - const row = document.createElement('li'); - row.className = 'playlist-browser-row'; - if (item.current || item.playing) row.classList.add('current'); - if ( - ctx.state.playlistBrowserActivePane === 'playlist' && - ctx.state.playlistBrowserSelectedPlaylistIndex === index - ) { - row.classList.add('active'); - } - - const main = document.createElement('div'); - main.className = 'playlist-browser-row-main'; - const label = document.createElement('div'); - label.className = 'playlist-browser-row-label'; - label.textContent = `${index + 1}. ${item.displayLabel}`; - const meta = document.createElement('div'); - meta.className = 'playlist-browser-row-meta'; - meta.textContent = item.current || item.playing ? 'Playing now' : 'Queued'; - const submeta = document.createElement('div'); - submeta.className = 'playlist-browser-row-submeta'; - submeta.textContent = item.filename; - main.append(label, meta, submeta); - - const trailing = document.createElement('div'); - trailing.className = 'playlist-browser-row-actions'; - trailing.append( - createActionButton('Play', () => { - void playPlaylistItem(item.index); - }), - createActionButton('Up', () => { - void movePlaylistItem(item.index, -1); - }), - createActionButton('Down', () => { - void movePlaylistItem(item.index, 1); - }), - createActionButton('Remove', () => { - void removePlaylistItem(item.index); - }), - ); - row.append(main, trailing); - row.addEventListener('click', () => { - ctx.state.playlistBrowserActivePane = 'playlist'; - ctx.state.playlistBrowserSelectedPlaylistIndex = index; - render(); - }); - row.addEventListener('dblclick', () => { - ctx.state.playlistBrowserSelectedPlaylistIndex = index; - void playPlaylistItem(item.index); - }); - return row; - } - function render(): void { const snapshot = getSnapshot(); if (!snapshot) { @@ -192,10 +78,26 @@ export function createPlaylistBrowserModal( ctx.dom.playlistBrowserStatus.textContent = ctx.state.playlistBrowserStatus || buildDefaultStatus(snapshot); ctx.dom.playlistBrowserDirectoryList.replaceChildren( - ...snapshot.directoryItems.map((item, index) => renderDirectoryRow(item, index)), + ...snapshot.directoryItems.map((item, index) => + renderPlaylistBrowserDirectoryRow(ctx, item, index, { + appendDirectoryItem, + movePlaylistItem, + playPlaylistItem, + removePlaylistItem, + render, + }), + ), ); ctx.dom.playlistBrowserPlaylistList.replaceChildren( - ...snapshot.playlistItems.map((item, index) => renderPlaylistRow(item, index)), + ...snapshot.playlistItems.map((item, index) => + renderPlaylistBrowserPlaylistRow(ctx, item, index, { + appendDirectoryItem, + movePlaylistItem, + playPlaylistItem, + removePlaylistItem, + render, + }), + ), ); }