fix: align subtitle sidebar state and behavior updates

This commit is contained in:
2026-03-21 22:24:42 -07:00
parent 4c6e4b9f0b
commit 5ea064a446
12 changed files with 34 additions and 17 deletions

View File

@@ -1,11 +1,11 @@
--- ---
id: TASK-216 id: TASK-216
title: 'Address PR #28 CodeRabbit follow-ups on subtitle sidebar' title: 'Address PR #28 CodeRabbit follow-ups on subtitle sidebar'
status: In Progress status: Completed
assignee: assignee:
- '@codex' - '@codex'
created_date: '2026-03-21 00:00' created_date: '2026-03-21 00:00'
updated_date: '2026-03-22 04:05' updated_date: '2026-03-21 00:00'
labels: labels:
- pr-review - pr-review
- subtitle-sidebar - 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: 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-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.
<!-- SECTION:NOTES:END --> <!-- SECTION:NOTES:END -->
## Final Summary ## Final Summary

View File

@@ -124,7 +124,8 @@ export function buildSubtitleConfigOptionRegistry(
}, },
{ {
path: 'subtitleSidebar.layout', path: 'subtitleSidebar.layout',
kind: 'string', kind: 'enum',
enumValues: ['overlay', 'embedded'],
defaultValue: defaultConfig.subtitleSidebar.layout, defaultValue: defaultConfig.subtitleSidebar.layout,
description: 'Render the subtitle sidebar as a floating overlay or reserve space inside mpv.', description: 'Render the subtitle sidebar as a floating overlay or reserve space inside mpv.',
}, },

View File

@@ -1,7 +1,7 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test from 'node:test'; import test from 'node:test';
import { parseSrtCues, parseAssCues, parseSubtitleCues } from './subtitle-cue-parser'; 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', () => { test('parseSrtCues parses basic SRT content', () => {
const content = [ const content = [

View File

@@ -1,8 +1,8 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test from 'node:test'; import test from 'node:test';
import { computePriorityWindow, createSubtitlePrefetchService } from './subtitle-prefetch'; import { computePriorityWindow, createSubtitlePrefetchService } from './subtitle-prefetch';
import type { SubtitleCue } from './subtitle-cue-parser';
import type { SubtitleData } from '../../types'; import type { SubtitleData } from '../../types';
import type { SubtitleCue } from '../../types';
function makeCues(count: number, startOffset = 0): SubtitleCue[] { function makeCues(count: number, startOffset = 0): SubtitleCue[] {
return Array.from({ length: count }, (_, i) => ({ return Array.from({ length: count }, (_, i) => ({

View File

@@ -1,5 +1,5 @@
import type { SubtitleCue } from './subtitle-cue-parser';
import type { SubtitleData } from '../../types'; import type { SubtitleData } from '../../types';
import type { SubtitleCue } from '../../types';
export interface SubtitlePrefetchServiceDeps { export interface SubtitlePrefetchServiceDeps {
cues: SubtitleCue[]; cues: SubtitleCue[];

View File

@@ -1,7 +1,7 @@
import assert from 'node:assert/strict'; import assert from 'node:assert/strict';
import test from 'node:test'; import test from 'node:test';
import type { SubtitleCue } from '../../core/services/subtitle-cue-parser';
import type { SubtitlePrefetchService } from '../../core/services/subtitle-prefetch'; import type { SubtitlePrefetchService } from '../../core/services/subtitle-prefetch';
import type { SubtitleCue } from '../../types';
import { createSubtitlePrefetchInitController } from './subtitle-prefetch-init'; import { createSubtitlePrefetchInitController } from './subtitle-prefetch-init';
function createDeferred<T>(): { function createDeferred<T>(): {

View File

@@ -1,9 +1,9 @@
import type { SubtitleCue } from '../../core/services/subtitle-cue-parser';
import type { import type {
SubtitlePrefetchService, SubtitlePrefetchService,
SubtitlePrefetchServiceDeps, SubtitlePrefetchServiceDeps,
} from '../../core/services/subtitle-prefetch'; } from '../../core/services/subtitle-prefetch';
import type { SubtitleData } from '../../types'; import type { SubtitleData } from '../../types';
import type { SubtitleCue } from '../../types';
export interface SubtitlePrefetchInitControllerDeps { export interface SubtitlePrefetchInitControllerDeps {
getCurrentService: () => SubtitlePrefetchService | null; getCurrentService: () => SubtitlePrefetchService | null;

View File

@@ -5,11 +5,11 @@ import type {
MpvSubtitleRenderMetrics, MpvSubtitleRenderMetrics,
SecondarySubMode, SecondarySubMode,
SubtitleData, SubtitleData,
SubtitleCue,
SubtitlePosition, SubtitlePosition,
KikuFieldGroupingChoice, KikuFieldGroupingChoice,
JlptLevel, JlptLevel,
FrequencyDictionaryLookup, FrequencyDictionaryLookup,
SubtitleCue,
} from '../types'; } from '../types';
import type { CliArgs } from '../cli/args'; import type { CliArgs } from '../cli/args';
import type { SubtitleTimingTracker } from '../subtitle-timing-tracker'; import type { SubtitleTimingTracker } from '../subtitle-timing-tracker';

View File

@@ -145,6 +145,7 @@ export function createSubtitleSidebarModal(
let snapshotPollInterval: ReturnType<typeof setTimeout> | null = null; let snapshotPollInterval: ReturnType<typeof setTimeout> | null = null;
let lastAppliedVideoMarginRatio: number | null = null; let lastAppliedVideoMarginRatio: number | null = null;
let subtitleSidebarHoverRequestId = 0; let subtitleSidebarHoverRequestId = 0;
let disposeDomEvents: (() => void) | null = null;
function restoreEmbeddedSidebarPassthrough(): void { function restoreEmbeddedSidebarPassthrough(): void {
syncOverlayMouseIgnoreState(ctx); syncOverlayMouseIgnoreState(ctx);
@@ -515,6 +516,10 @@ export function createSubtitleSidebarModal(
} }
function wireDomEvents(): void { function wireDomEvents(): void {
if (disposeDomEvents) {
return;
}
ctx.dom.subtitleSidebarClose.addEventListener('click', () => { ctx.dom.subtitleSidebarClose.addEventListener('click', () => {
closeSubtitleSidebarModal(); closeSubtitleSidebarModal();
}); });
@@ -565,12 +570,17 @@ export function createSubtitleSidebarModal(
ctx.state.isOverSubtitleSidebar = false; ctx.state.isOverSubtitleSidebar = false;
resumeSubtitleSidebarHoverPause(); resumeSubtitleSidebarHoverPause();
}); });
window.addEventListener('resize', () => { const resizeHandler = () => {
if (!ctx.state.subtitleSidebarModalOpen) { if (!ctx.state.subtitleSidebarModalOpen) {
return; return;
} }
syncEmbeddedSidebarLayout(); syncEmbeddedSidebarLayout();
}); };
window.addEventListener('resize', resizeHandler);
disposeDomEvents = () => {
window.removeEventListener('resize', resizeHandler);
disposeDomEvents = null;
};
} }
return { return {
@@ -580,6 +590,9 @@ export function createSubtitleSidebarModal(
toggleSubtitleSidebarModal, toggleSubtitleSidebarModal,
refreshSubtitleSidebarSnapshot: refreshSnapshot, refreshSubtitleSidebarSnapshot: refreshSnapshot,
wireDomEvents, wireDomEvents,
disposeDomEvents: () => {
disposeDomEvents?.();
},
handleSubtitleUpdated, handleSubtitleUpdated,
seekToCue, seekToCue,
}; };

View File

@@ -543,6 +543,9 @@ async function init(): Promise<void> {
controllerDebugModal.wireDomEvents(); controllerDebugModal.wireDomEvents();
sessionHelpModal.wireDomEvents(); sessionHelpModal.wireDomEvents();
subtitleSidebarModal.wireDomEvents(); subtitleSidebarModal.wireDomEvents();
window.addEventListener('beforeunload', () => {
subtitleSidebarModal.disposeDomEvents();
});
window.electronAPI.onRuntimeOptionsChanged((options: RuntimeOptionState[]) => { window.electronAPI.onRuntimeOptionsChanged((options: RuntimeOptionState[]) => {
runGuarded('runtime-options:changed', () => { runGuarded('runtime-options:changed', () => {

View File

@@ -10,8 +10,8 @@ import type {
RuntimeOptionState, RuntimeOptionState,
RuntimeOptionValue, RuntimeOptionValue,
SubtitlePosition, SubtitlePosition,
SubtitleCue,
SubtitleSidebarConfig, SubtitleSidebarConfig,
SubtitleCue,
SubsyncSourceTrack, SubsyncSourceTrack,
} from '../types'; } from '../types';

View File

@@ -16,6 +16,8 @@
* along with this program. If not, see <https://www.gnu.org/licenses/>. * along with this program. If not, see <https://www.gnu.org/licenses/>.
*/ */
import type { SubtitleCue } from './core/services/subtitle-cue-parser';
export enum PartOfSpeech { export enum PartOfSpeech {
noun = 'noun', noun = 'noun',
verb = 'verb', verb = 'verb',
@@ -364,11 +366,7 @@ export interface ResolvedTokenPos2ExclusionConfig {
export type FrequencyDictionaryMode = 'single' | 'banded'; export type FrequencyDictionaryMode = 'single' | 'banded';
export interface SubtitleCue { export type { SubtitleCue } from './core/services/subtitle-cue-parser';
startTime: number;
endTime: number;
text: string;
}
export type SubtitleSidebarLayout = 'overlay' | 'embedded'; export type SubtitleSidebarLayout = 'overlay' | 'embedded';