diff --git a/backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md b/backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md index 1d6f265..867dce0 100644 --- a/backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md +++ b/backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md @@ -1,11 +1,11 @@ --- id: TASK-216 title: 'Address PR #28 CodeRabbit follow-ups on subtitle sidebar' -status: In Progress +status: Completed assignee: - '@codex' created_date: '2026-03-21 00:00' -updated_date: '2026-03-22 04:05' +updated_date: '2026-03-21 00:00' labels: - pr-review - subtitle-sidebar @@ -64,6 +64,8 @@ Verification: 2026-03-22: Addressed the live hover-state and startup mouse-ignore follow-ups from the latest CodeRabbit pass. `handleMouseLeave()` now clears `isOverSubtitle` and drops `secondary-sub-hover-active` when leaving the secondary subtitle container toward the primary container, and renderer startup now calls `syncOverlayMouseIgnoreState(ctx)` instead of forcing `setIgnoreMouseEvents(true, { forward: true })`. The sidebar IPC hover catch and CSS spacing comments were already satisfied in the current tree. 2026-03-22: Regenerated `bun.lock` from a clean install so the `electron-builder-squirrel-windows` override now resolves at `26.8.2` in the lockfile alongside `app-builder-lib@26.8.2`. + +2026-03-21: Finished the remaining cleanup pass from the latest review. `subtitleSidebar.layout` now uses enum validation, `SubtitleCue` is re-exported from `src/types.ts` as the single public type path, and the subtitle sidebar resize listener now has unload cleanup wired through the renderer. ## Final Summary diff --git a/src/config/definitions/options-subtitle.ts b/src/config/definitions/options-subtitle.ts index 98db3a4..9b2d294 100644 --- a/src/config/definitions/options-subtitle.ts +++ b/src/config/definitions/options-subtitle.ts @@ -124,7 +124,8 @@ export function buildSubtitleConfigOptionRegistry( }, { path: 'subtitleSidebar.layout', - kind: 'string', + kind: 'enum', + enumValues: ['overlay', 'embedded'], defaultValue: defaultConfig.subtitleSidebar.layout, description: 'Render the subtitle sidebar as a floating overlay or reserve space inside mpv.', }, diff --git a/src/core/services/subtitle-cue-parser.test.ts b/src/core/services/subtitle-cue-parser.test.ts index 6a656b7..5c59e62 100644 --- a/src/core/services/subtitle-cue-parser.test.ts +++ b/src/core/services/subtitle-cue-parser.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { parseSrtCues, parseAssCues, parseSubtitleCues } from './subtitle-cue-parser'; -import type { SubtitleCue } from './subtitle-cue-parser'; +import type { SubtitleCue } from '../../types'; test('parseSrtCues parses basic SRT content', () => { const content = [ diff --git a/src/core/services/subtitle-prefetch.test.ts b/src/core/services/subtitle-prefetch.test.ts index 57f7df3..d053056 100644 --- a/src/core/services/subtitle-prefetch.test.ts +++ b/src/core/services/subtitle-prefetch.test.ts @@ -1,8 +1,8 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { computePriorityWindow, createSubtitlePrefetchService } from './subtitle-prefetch'; -import type { SubtitleCue } from './subtitle-cue-parser'; import type { SubtitleData } from '../../types'; +import type { SubtitleCue } from '../../types'; function makeCues(count: number, startOffset = 0): SubtitleCue[] { return Array.from({ length: count }, (_, i) => ({ diff --git a/src/core/services/subtitle-prefetch.ts b/src/core/services/subtitle-prefetch.ts index eb0eb9a..9641b35 100644 --- a/src/core/services/subtitle-prefetch.ts +++ b/src/core/services/subtitle-prefetch.ts @@ -1,5 +1,5 @@ -import type { SubtitleCue } from './subtitle-cue-parser'; import type { SubtitleData } from '../../types'; +import type { SubtitleCue } from '../../types'; export interface SubtitlePrefetchServiceDeps { cues: SubtitleCue[]; diff --git a/src/main/runtime/subtitle-prefetch-init.test.ts b/src/main/runtime/subtitle-prefetch-init.test.ts index 8314393..162e85e 100644 --- a/src/main/runtime/subtitle-prefetch-init.test.ts +++ b/src/main/runtime/subtitle-prefetch-init.test.ts @@ -1,7 +1,7 @@ import assert from 'node:assert/strict'; import test from 'node:test'; -import type { SubtitleCue } from '../../core/services/subtitle-cue-parser'; import type { SubtitlePrefetchService } from '../../core/services/subtitle-prefetch'; +import type { SubtitleCue } from '../../types'; import { createSubtitlePrefetchInitController } from './subtitle-prefetch-init'; function createDeferred(): { diff --git a/src/main/runtime/subtitle-prefetch-init.ts b/src/main/runtime/subtitle-prefetch-init.ts index c281e94..42b717e 100644 --- a/src/main/runtime/subtitle-prefetch-init.ts +++ b/src/main/runtime/subtitle-prefetch-init.ts @@ -1,9 +1,9 @@ -import type { SubtitleCue } from '../../core/services/subtitle-cue-parser'; import type { SubtitlePrefetchService, SubtitlePrefetchServiceDeps, } from '../../core/services/subtitle-prefetch'; import type { SubtitleData } from '../../types'; +import type { SubtitleCue } from '../../types'; export interface SubtitlePrefetchInitControllerDeps { getCurrentService: () => SubtitlePrefetchService | null; diff --git a/src/main/state.ts b/src/main/state.ts index b570c5e..16372dd 100644 --- a/src/main/state.ts +++ b/src/main/state.ts @@ -5,11 +5,11 @@ import type { MpvSubtitleRenderMetrics, SecondarySubMode, SubtitleData, - SubtitleCue, SubtitlePosition, KikuFieldGroupingChoice, JlptLevel, FrequencyDictionaryLookup, + SubtitleCue, } from '../types'; import type { CliArgs } from '../cli/args'; import type { SubtitleTimingTracker } from '../subtitle-timing-tracker'; diff --git a/src/renderer/modals/subtitle-sidebar.ts b/src/renderer/modals/subtitle-sidebar.ts index 59582db..d5b4ee0 100644 --- a/src/renderer/modals/subtitle-sidebar.ts +++ b/src/renderer/modals/subtitle-sidebar.ts @@ -145,6 +145,7 @@ export function createSubtitleSidebarModal( let snapshotPollInterval: ReturnType | null = null; let lastAppliedVideoMarginRatio: number | null = null; let subtitleSidebarHoverRequestId = 0; + let disposeDomEvents: (() => void) | null = null; function restoreEmbeddedSidebarPassthrough(): void { syncOverlayMouseIgnoreState(ctx); @@ -515,6 +516,10 @@ export function createSubtitleSidebarModal( } function wireDomEvents(): void { + if (disposeDomEvents) { + return; + } + ctx.dom.subtitleSidebarClose.addEventListener('click', () => { closeSubtitleSidebarModal(); }); @@ -565,12 +570,17 @@ export function createSubtitleSidebarModal( ctx.state.isOverSubtitleSidebar = false; resumeSubtitleSidebarHoverPause(); }); - window.addEventListener('resize', () => { + const resizeHandler = () => { if (!ctx.state.subtitleSidebarModalOpen) { return; } syncEmbeddedSidebarLayout(); - }); + }; + window.addEventListener('resize', resizeHandler); + disposeDomEvents = () => { + window.removeEventListener('resize', resizeHandler); + disposeDomEvents = null; + }; } return { @@ -580,6 +590,9 @@ export function createSubtitleSidebarModal( toggleSubtitleSidebarModal, refreshSubtitleSidebarSnapshot: refreshSnapshot, wireDomEvents, + disposeDomEvents: () => { + disposeDomEvents?.(); + }, handleSubtitleUpdated, seekToCue, }; diff --git a/src/renderer/renderer.ts b/src/renderer/renderer.ts index f5c3508..ebfc07c 100644 --- a/src/renderer/renderer.ts +++ b/src/renderer/renderer.ts @@ -543,6 +543,9 @@ async function init(): Promise { controllerDebugModal.wireDomEvents(); sessionHelpModal.wireDomEvents(); subtitleSidebarModal.wireDomEvents(); + window.addEventListener('beforeunload', () => { + subtitleSidebarModal.disposeDomEvents(); + }); window.electronAPI.onRuntimeOptionsChanged((options: RuntimeOptionState[]) => { runGuarded('runtime-options:changed', () => { diff --git a/src/renderer/state.ts b/src/renderer/state.ts index 5f5f517..62dcc5e 100644 --- a/src/renderer/state.ts +++ b/src/renderer/state.ts @@ -10,8 +10,8 @@ import type { RuntimeOptionState, RuntimeOptionValue, SubtitlePosition, - SubtitleCue, SubtitleSidebarConfig, + SubtitleCue, SubsyncSourceTrack, } from '../types'; diff --git a/src/types.ts b/src/types.ts index 355f7f6..0784bb2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -16,6 +16,8 @@ * along with this program. If not, see . */ +import type { SubtitleCue } from './core/services/subtitle-cue-parser'; + export enum PartOfSpeech { noun = 'noun', verb = 'verb', @@ -364,11 +366,7 @@ export interface ResolvedTokenPos2ExclusionConfig { export type FrequencyDictionaryMode = 'single' | 'banded'; -export interface SubtitleCue { - startTime: number; - endTime: number; - text: string; -} +export type { SubtitleCue } from './core/services/subtitle-cue-parser'; export type SubtitleSidebarLayout = 'overlay' | 'embedded';