From f901433eeaffca0b948a0c03ea0dfc998b7604ba Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 30 Mar 2026 17:50:39 -0700 Subject: [PATCH] fix: address playlist browser coderabbit feedback --- plugin/subminer/process.lua | 23 +- scripts/test-plugin-start-gate.lua | 25 ++ src/core/services/ipc-command.test.ts | 16 +- src/core/services/ipc-command.ts | 7 +- src/main.ts | 9 +- .../runtime/playlist-browser-open.test.ts | 28 +++ src/main/runtime/playlist-browser-open.ts | 23 ++ .../runtime/playlist-browser-runtime.test.ts | 84 +++++++ src/main/runtime/playlist-browser-runtime.ts | 28 ++- src/renderer/modals/playlist-browser.test.ts | 238 ++++++++++++++++++ src/renderer/modals/playlist-browser.ts | 44 ++-- 11 files changed, 494 insertions(+), 31 deletions(-) create mode 100644 src/main/runtime/playlist-browser-open.test.ts create mode 100644 src/main/runtime/playlist-browser-open.ts diff --git a/plugin/subminer/process.lua b/plugin/subminer/process.lua index 50f72cf..d4f9a72 100644 --- a/plugin/subminer/process.lua +++ b/plugin/subminer/process.lua @@ -34,6 +34,17 @@ function M.create(ctx) return options_helper.coerce_bool(raw_pause_until_ready, false) end + local function resolve_texthooker_enabled(override_value) + if override_value ~= nil then + return options_helper.coerce_bool(override_value, false) + end + local raw_texthooker_enabled = opts.texthooker_enabled + if raw_texthooker_enabled == nil then + raw_texthooker_enabled = opts["texthooker-enabled"] + end + return options_helper.coerce_bool(raw_texthooker_enabled, false) + end + local function resolve_pause_until_ready_timeout_seconds() local raw_timeout_seconds = opts.auto_start_pause_until_ready_timeout_seconds if raw_timeout_seconds == nil then @@ -192,10 +203,7 @@ function M.create(ctx) table.insert(args, "--hide-visible-overlay") end - local texthooker_enabled = overrides.texthooker_enabled - if texthooker_enabled == nil then - texthooker_enabled = opts.texthooker_enabled - end + local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled) if texthooker_enabled then table.insert(args, "--texthooker") end @@ -296,10 +304,7 @@ function M.create(ctx) return end - local texthooker_enabled = overrides.texthooker_enabled - if texthooker_enabled == nil then - texthooker_enabled = opts.texthooker_enabled - end + local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled) local socket_path = overrides.socket_path or opts.socket_path local should_pause_until_ready = ( overrides.auto_start_trigger == true @@ -498,7 +503,7 @@ function M.create(ctx) end end) - if opts.texthooker_enabled then + if resolve_texthooker_enabled(nil) then ensure_texthooker_running(function() end) end end) diff --git a/scripts/test-plugin-start-gate.lua b/scripts/test-plugin-start-gate.lua index 7b20ec2..dc4489f 100644 --- a/scripts/test-plugin-start-gate.lua +++ b/scripts/test-plugin-start-gate.lua @@ -531,6 +531,31 @@ do ) end +do + local recorded, err = run_plugin_scenario({ + process_list = "", + option_overrides = { + binary_path = binary_path, + auto_start = "yes", + auto_start_visible_overlay = "yes", + auto_start_pause_until_ready = "no", + socket_path = "/tmp/subminer-socket", + texthooker_enabled = "no", + }, + input_ipc_server = "/tmp/subminer-socket", + media_title = "Random Movie", + files = { + [binary_path] = true, + }, + }) + assert_true(recorded ~= nil, "plugin failed to load for disabled texthooker auto-start scenario: " .. tostring(err)) + fire_event(recorded, "file-loaded") + local start_call = find_start_call(recorded.async_calls) + assert_true(start_call ~= nil, "disabled texthooker auto-start should still issue --start command") + assert_true(not call_has_arg(start_call, "--texthooker"), "disabled texthooker should not include --texthooker on --start") + assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "disabled texthooker should not issue a helper texthooker command") +end + do local recorded, err = run_plugin_scenario({ process_list = "", diff --git a/src/core/services/ipc-command.test.ts b/src/core/services/ipc-command.test.ts index 2296615..4888fdf 100644 --- a/src/core/services/ipc-command.test.ts +++ b/src/core/services/ipc-command.test.ts @@ -114,14 +114,28 @@ test('handleMpvCommandFromIpc dispatches special youtube picker open command', ( assert.deepEqual(osd, []); }); -test('handleMpvCommandFromIpc dispatches special playlist browser open command', () => { +test('handleMpvCommandFromIpc dispatches special playlist browser open command', async () => { const { options, calls, sentCommands, osd } = createOptions(); handleMpvCommandFromIpc(['__playlist-browser-open'], options); + await new Promise((resolve) => setImmediate(resolve)); assert.deepEqual(calls, ['playlist-browser']); assert.deepEqual(sentCommands, []); assert.deepEqual(osd, []); }); +test('handleMpvCommandFromIpc surfaces playlist browser open rejections via mpv osd', async () => { + const { options, osd } = createOptions({ + openPlaylistBrowser: async () => { + throw new Error('overlay failed'); + }, + }); + + handleMpvCommandFromIpc(['__playlist-browser-open'], options); + await new Promise((resolve) => setImmediate(resolve)); + + assert.deepEqual(osd, ['Playlist browser failed: overlay failed']); +}); + test('handleMpvCommandFromIpc does not forward commands while disconnected', () => { const { options, sentCommands, osd } = createOptions({ isMpvConnected: () => false, diff --git a/src/core/services/ipc-command.ts b/src/core/services/ipc-command.ts index baec99e..1a9e414 100644 --- a/src/core/services/ipc-command.ts +++ b/src/core/services/ipc-command.ts @@ -100,7 +100,12 @@ export function handleMpvCommandFromIpc( } if (first === options.specialCommands.PLAYLIST_BROWSER_OPEN) { - void options.openPlaylistBrowser(); + Promise.resolve() + .then(() => options.openPlaylistBrowser()) + .catch((error) => { + const message = error instanceof Error ? error.message : String(error); + options.showMpvOsd(`Playlist browser failed: ${message}`); + }); return; } diff --git a/src/main.ts b/src/main.ts index 81101d8..c347fec 100644 --- a/src/main.ts +++ b/src/main.ts @@ -31,6 +31,7 @@ import { screen, } from 'electron'; import { applyControllerConfigUpdate } from './main/controller-config-update.js'; +import { openPlaylistBrowser as openPlaylistBrowserRuntime } from './main/runtime/playlist-browser-open'; import { createDiscordRpcClient } from './main/runtime/discord-rpc-client.js'; import { mergeAiConfig } from './ai/config'; @@ -1941,8 +1942,12 @@ function openPlaylistBrowser(): void { showMpvOsd('Playlist browser requires active playback.'); return; } - const opened = sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, { - restoreOnModalClose: 'playlist-browser', + const opened = openPlaylistBrowserRuntime({ + ensureOverlayStartupPrereqs: () => ensureOverlayStartupPrereqs(), + ensureOverlayWindowsReadyForVisibilityActions: () => + ensureOverlayWindowsReadyForVisibilityActions(), + sendToActiveOverlayWindow: (channel, payload, runtimeOptions) => + sendToActiveOverlayWindow(channel, payload, runtimeOptions), }); if (!opened) { showMpvOsd('Playlist browser overlay unavailable.'); diff --git a/src/main/runtime/playlist-browser-open.test.ts b/src/main/runtime/playlist-browser-open.test.ts new file mode 100644 index 0000000..970d10e --- /dev/null +++ b/src/main/runtime/playlist-browser-open.test.ts @@ -0,0 +1,28 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { IPC_CHANNELS } from '../../shared/ipc/contracts'; +import { openPlaylistBrowser } from './playlist-browser-open'; + +test('playlist browser open bootstraps overlay runtime before dispatching the modal event', () => { + const calls: string[] = []; + + const opened = openPlaylistBrowser({ + ensureOverlayStartupPrereqs: () => { + calls.push('prereqs'); + }, + ensureOverlayWindowsReadyForVisibilityActions: () => { + calls.push('windows'); + }, + sendToActiveOverlayWindow: (channel, payload, runtimeOptions) => { + calls.push(`send:${channel}`); + assert.equal(payload, undefined); + assert.deepEqual(runtimeOptions, { + restoreOnModalClose: 'playlist-browser', + }); + return true; + }, + }); + + assert.equal(opened, true); + assert.deepEqual(calls, ['prereqs', 'windows', `send:${IPC_CHANNELS.event.playlistBrowserOpen}`]); +}); diff --git a/src/main/runtime/playlist-browser-open.ts b/src/main/runtime/playlist-browser-open.ts new file mode 100644 index 0000000..ba4ce1f --- /dev/null +++ b/src/main/runtime/playlist-browser-open.ts @@ -0,0 +1,23 @@ +import type { OverlayHostedModal } from '../../shared/ipc/contracts'; +import { IPC_CHANNELS } from '../../shared/ipc/contracts'; + +const PLAYLIST_BROWSER_MODAL: OverlayHostedModal = 'playlist-browser'; + +export function openPlaylistBrowser(deps: { + ensureOverlayStartupPrereqs: () => void; + ensureOverlayWindowsReadyForVisibilityActions: () => void; + sendToActiveOverlayWindow: ( + channel: string, + payload?: unknown, + runtimeOptions?: { + restoreOnModalClose?: OverlayHostedModal; + preferModalWindow?: boolean; + }, + ) => boolean; +}): boolean { + deps.ensureOverlayStartupPrereqs(); + deps.ensureOverlayWindowsReadyForVisibilityActions(); + return deps.sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, { + restoreOnModalClose: PLAYLIST_BROWSER_MODAL, + }); +} diff --git a/src/main/runtime/playlist-browser-runtime.test.ts b/src/main/runtime/playlist-browser-runtime.test.ts index c218992..970ad6e 100644 --- a/src/main/runtime/playlist-browser-runtime.test.ts +++ b/src/main/runtime/playlist-browser-runtime.test.ts @@ -249,6 +249,41 @@ test('playlist-browser mutation runtimes mutate queue and return refreshed snaps ); }); +test('appendPlaylistBrowserFileRuntime returns an error result when statSync throws', async () => { + const dir = createTempVideoDir(); + const episode1 = path.join(dir, 'Show - S01E01.mkv'); + fs.writeFileSync(episode1, ''); + + const mutableFs = fs as typeof fs & { statSync: typeof fs.statSync }; + const originalStatSync = mutableFs.statSync; + mutableFs.statSync = ((targetPath: fs.PathLike) => { + if (path.resolve(String(targetPath)) === episode1) { + throw new Error('EACCES'); + } + return originalStatSync(targetPath); + }) as typeof fs.statSync; + + try { + const result = await appendPlaylistBrowserFileRuntime( + { + getMpvClient: () => + createFakeMpvClient({ + currentVideoPath: episode1, + playlist: [{ filename: episode1, current: true }], + }), + }, + episode1, + ); + + assert.deepEqual(result, { + ok: false, + message: 'Playlist browser file is not readable.', + }); + } finally { + mutableFs.statSync = originalStatSync; + } +}); + test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', async () => { const dir = createTempVideoDir(); const episode1 = path.join(dir, 'Show - S01E01.mkv'); @@ -324,3 +359,52 @@ test('playPlaylistBrowserIndexRuntime skips local subtitle reset for remote play assert.deepEqual(mpvClient.getCommands().slice(-1), [['playlist-play-index', 1]]); assert.equal(scheduled.length, 0); }); + +test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm callbacks', async () => { + const dir = createTempVideoDir(); + 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 scheduled: Array<() => void> = []; + const mpvClient = createFakeMpvClient({ + currentVideoPath: episode1, + playlist: [ + { filename: episode1, current: true, title: 'Episode 1' }, + { filename: episode2, title: 'Episode 2' }, + { filename: episode3, title: 'Episode 3' }, + ], + }); + + const deps = { + getMpvClient: () => mpvClient, + schedule: (callback: () => void) => { + scheduled.push(callback); + }, + }; + + const firstPlay = await playPlaylistBrowserIndexRuntime(deps, 1); + const secondPlay = await playPlaylistBrowserIndexRuntime(deps, 2); + + assert.equal(firstPlay.ok, true); + assert.equal(secondPlay.ok, true); + assert.equal(scheduled.length, 2); + + scheduled[0]?.(); + scheduled[1]?.(); + + assert.deepEqual( + mpvClient.getCommands().slice(-6), + [ + ['set_property', 'sub-auto', 'fuzzy'], + ['playlist-play-index', 1], + ['set_property', 'sub-auto', 'fuzzy'], + ['playlist-play-index', 2], + ['set_property', 'sid', 'auto'], + ['set_property', 'secondary-sid', 'auto'], + ], + ); +}); diff --git a/src/main/runtime/playlist-browser-runtime.ts b/src/main/runtime/playlist-browser-runtime.ts index dcc385e..5d99a38 100644 --- a/src/main/runtime/playlist-browser-runtime.ts +++ b/src/main/runtime/playlist-browser-runtime.ts @@ -30,6 +30,8 @@ export type PlaylistBrowserRuntimeDeps = { schedule?: (callback: () => void, delayMs: number) => void; }; +const pendingLocalSubtitleSelectionRearms = new WeakMap(); + function trimToNull(value: unknown): string | null { if (typeof value !== 'string') return null; const trimmed = value.trim(); @@ -219,15 +221,26 @@ function prepareLocalSubtitleAutoload(client: MpvPlaylistBrowserClientLike): voi client.send({ command: ['set_property', 'sub-auto', 'fuzzy'] }); } -function isLocalPlaylistItem(item: PlaylistBrowserQueueItem | null | undefined): item is PlaylistBrowserQueueItem { +function isLocalPlaylistItem( + item: PlaylistBrowserQueueItem | null | undefined, +): item is PlaylistBrowserQueueItem & { path: string } { return Boolean(item?.path && !isRemoteMediaPath(item.path)); } function scheduleLocalSubtitleSelectionRearm( deps: PlaylistBrowserRuntimeDeps, client: MpvPlaylistBrowserClientLike, + expectedPath: string, ): void { + const nextToken = (pendingLocalSubtitleSelectionRearms.get(client) ?? 0) + 1; + pendingLocalSubtitleSelectionRearms.set(client, nextToken); (deps.schedule ?? setTimeout)(() => { + if (pendingLocalSubtitleSelectionRearms.get(client) !== nextToken) return; + pendingLocalSubtitleSelectionRearms.delete(client); + const currentPath = trimToNull(client.currentVideoPath); + if (currentPath && path.resolve(currentPath) !== expectedPath) { + return; + } rearmLocalSubtitleSelection(client); }, 400); } @@ -241,7 +254,16 @@ export async function appendPlaylistBrowserFileRuntime( return client; } const resolvedPath = path.resolve(filePath); - if (!fs.existsSync(resolvedPath) || !fs.statSync(resolvedPath).isFile()) { + let stats: fs.Stats; + try { + stats = fs.statSync(resolvedPath); + } catch { + return { + ok: false, + message: 'Playlist browser file is not readable.', + }; + } + if (!stats.isFile()) { return { ok: false, message: 'Playlist browser file is not readable.', @@ -267,7 +289,7 @@ export async function playPlaylistBrowserIndexRuntime( } result.client.send({ command: ['playlist-play-index', index] }); if (isLocalPlaylistItem(targetItem)) { - scheduleLocalSubtitleSelectionRearm(deps, result.client); + scheduleLocalSubtitleSelectionRearm(deps, result.client, path.resolve(targetItem.path)); } return buildMutationResult(`Playing playlist item ${index + 1}`, deps); } diff --git a/src/renderer/modals/playlist-browser.test.ts b/src/renderer/modals/playlist-browser.test.ts index b7413e9..432c6b3 100644 --- a/src/renderer/modals/playlist-browser.test.ts +++ b/src/renderer/modals/playlist-browser.test.ts @@ -428,3 +428,241 @@ test('playlist browser keeps modal open when playing selected queue item fails', Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); } }); + +test('playlist browser refresh failure clears stale rendered rows and reports the error', async () => { + const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; + const previousWindow = globals.window; + const previousDocument = globals.document; + const notifications: string[] = []; + let refreshShouldFail = false; + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + getPlaylistBrowserSnapshot: async () => { + if (refreshShouldFail) { + throw new Error('snapshot failed'); + } + return createSnapshot(); + }, + notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`), + notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`), + 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 playlistBrowserTitle = createFakeElement(); + const playlistBrowserStatus = createFakeElement(); + const directoryList = createListStub(); + const playlistList = createListStub(); + const ctx = { + state, + platform: { + shouldToggleMouseIgnore: false, + }, + dom: { + overlay: { + classList: createClassList(), + focus: () => {}, + }, + playlistBrowserModal: createFakeElement(), + playlistBrowserTitle, + playlistBrowserStatus, + playlistBrowserDirectoryList: directoryList, + playlistBrowserPlaylistList: playlistList, + playlistBrowserClose: createFakeElement(), + }, + }; + + const modal = createPlaylistBrowserModal(ctx as never, { + modalStateReader: { isAnyModalOpen: () => false }, + syncSettingsModalSubtitleSuppression: () => {}, + }); + + await modal.openPlaylistBrowserModal(); + assert.equal(directoryList.children.length, 2); + assert.equal(playlistList.children.length, 2); + + refreshShouldFail = true; + await modal.refreshSnapshot(); + + assert.equal(state.playlistBrowserSnapshot, null); + assert.equal(directoryList.children.length, 0); + assert.equal(playlistList.children.length, 0); + assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser'); + assert.equal(playlistBrowserStatus.textContent, 'snapshot failed'); + assert.equal(playlistBrowserStatus.classList.contains('error'), true); + assert.deepEqual(notifications, ['open:playlist-browser']); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); + } +}); + +test('playlist browser close clears rendered snapshot ui', async () => { + const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; + const previousWindow = globals.window; + const previousDocument = globals.document; + const notifications: string[] = []; + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + getPlaylistBrowserSnapshot: async () => createSnapshot(), + notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`), + notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`), + 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 playlistBrowserTitle = createFakeElement(); + const playlistBrowserStatus = createFakeElement(); + const directoryList = createListStub(); + const playlistList = createListStub(); + const ctx = { + state, + platform: { + shouldToggleMouseIgnore: false, + }, + dom: { + overlay: { + classList: createClassList(), + focus: () => {}, + }, + playlistBrowserModal: createFakeElement(), + playlistBrowserTitle, + playlistBrowserStatus, + playlistBrowserDirectoryList: directoryList, + playlistBrowserPlaylistList: playlistList, + playlistBrowserClose: createFakeElement(), + }, + }; + + const modal = createPlaylistBrowserModal(ctx as never, { + modalStateReader: { isAnyModalOpen: () => false }, + syncSettingsModalSubtitleSuppression: () => {}, + }); + + await modal.openPlaylistBrowserModal(); + assert.equal(directoryList.children.length, 2); + assert.equal(playlistList.children.length, 2); + + modal.closePlaylistBrowserModal(); + + assert.equal(state.playlistBrowserSnapshot, null); + assert.equal(state.playlistBrowserStatus, ''); + assert.equal(directoryList.children.length, 0); + assert.equal(playlistList.children.length, 0); + assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser'); + assert.equal(playlistBrowserStatus.textContent, ''); + assert.deepEqual(notifications, ['open:playlist-browser', 'close:playlist-browser']); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); + } +}); + +test('playlist browser open is ignored while another modal is already open', async () => { + const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown }; + const previousWindow = globals.window; + const previousDocument = globals.document; + const notifications: string[] = []; + let snapshotCalls = 0; + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + getPlaylistBrowserSnapshot: async () => { + snapshotCalls += 1; + return createSnapshot(); + }, + notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`), + notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`), + 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 overlay = { + classList: createClassList(), + focus: () => {}, + }; + const ctx = { + state, + platform: { + shouldToggleMouseIgnore: false, + }, + dom: { + overlay, + playlistBrowserModal: createFakeElement(), + playlistBrowserTitle: createFakeElement(), + playlistBrowserStatus: createFakeElement(), + playlistBrowserDirectoryList: createListStub(), + playlistBrowserPlaylistList: createListStub(), + playlistBrowserClose: createFakeElement(), + }, + }; + + const modal = createPlaylistBrowserModal(ctx as never, { + modalStateReader: { isAnyModalOpen: () => true }, + syncSettingsModalSubtitleSuppression: () => {}, + }); + + await modal.openPlaylistBrowserModal(); + + assert.equal(state.playlistBrowserModalOpen, false); + assert.equal(snapshotCalls, 0); + assert.equal(overlay.classList.contains('interactive'), false); + assert.deepEqual(notifications, []); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); + } +}); diff --git a/src/renderer/modals/playlist-browser.ts b/src/renderer/modals/playlist-browser.ts index 99e6e0e..b839261 100644 --- a/src/renderer/modals/playlist-browser.ts +++ b/src/renderer/modals/playlist-browser.ts @@ -49,6 +49,18 @@ export function createPlaylistBrowserModal( return ctx.state.playlistBrowserSnapshot; } + function resetSnapshotUi(): void { + ctx.state.playlistBrowserSnapshot = null; + ctx.state.playlistBrowserStatus = ''; + ctx.state.playlistBrowserSelectedDirectoryIndex = 0; + ctx.state.playlistBrowserSelectedPlaylistIndex = 0; + ctx.dom.playlistBrowserTitle.textContent = 'Playlist Browser'; + ctx.dom.playlistBrowserDirectoryList.replaceChildren(); + ctx.dom.playlistBrowserPlaylistList.replaceChildren(); + ctx.dom.playlistBrowserStatus.textContent = ''; + ctx.dom.playlistBrowserStatus.classList.remove('error'); + } + function syncSelection(snapshot: PlaylistBrowserSnapshot): void { const directoryIndex = snapshot.directoryItems.findIndex((item) => item.isCurrentFile); const playlistIndex = @@ -194,13 +206,18 @@ export function createPlaylistBrowserModal( } async function refreshSnapshot(): Promise { - const snapshot = await window.electronAPI.getPlaylistBrowserSnapshot(); - ctx.state.playlistBrowserStatus = ''; - applySnapshot(snapshot); - setStatus( - buildDefaultStatus(snapshot), - !snapshot.directoryAvailable && snapshot.directoryStatus.length > 0, - ); + try { + const snapshot = await window.electronAPI.getPlaylistBrowserSnapshot(); + ctx.state.playlistBrowserStatus = ''; + applySnapshot(snapshot); + setStatus( + buildDefaultStatus(snapshot), + !snapshot.directoryAvailable && snapshot.directoryStatus.length > 0, + ); + } catch (error) { + resetSnapshotUi(); + setStatus(error instanceof Error ? error.message : String(error), true); + } } async function handleMutation( @@ -249,6 +266,9 @@ export function createPlaylistBrowserModal( await refreshSnapshot(); return; } + if (options.modalStateReader.isAnyModalOpen()) { + return; + } ctx.state.playlistBrowserModalOpen = true; ctx.state.playlistBrowserActivePane = 'playlist'; @@ -257,19 +277,13 @@ export function createPlaylistBrowserModal( ctx.dom.playlistBrowserModal.classList.remove('hidden'); ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'false'); window.electronAPI.notifyOverlayModalOpened('playlist-browser'); - - try { - await refreshSnapshot(); - } catch (error) { - setStatus(error instanceof Error ? error.message : String(error), true); - } + await refreshSnapshot(); } function closePlaylistBrowserModal(): void { if (!ctx.state.playlistBrowserModalOpen) return; ctx.state.playlistBrowserModalOpen = false; - ctx.state.playlistBrowserSnapshot = null; - ctx.state.playlistBrowserStatus = ''; + resetSnapshotUi(); ctx.dom.playlistBrowserModal.classList.add('hidden'); ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'true'); window.electronAPI.notifyOverlayModalClosed('playlist-browser');