From 76b5ab68ba0ec45d52bef53f80a7faad53111674 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 21 Mar 2026 19:55:33 -0700 Subject: [PATCH] Add subtitle sidebar startup auto-open and resume jump - Add `subtitleSidebar.autoOpen` with startup-only open behavior - Jump to the first resolved active cue on initial resume position - Clear parsed cues when subtitle prefetch init fails --- ...o-resume-position-on-first-resolved-cue.md | 37 ++++++++++ ...p-auto-open-option-for-subtitle-sidebar.md | 40 ++++++++++ ...deRabbit-follow-ups-on-subtitle-sidebar.md | 73 +++++++++++++++++++ ...rough-sync-between-subtitle-and-sidebar.md | 69 ++++++++++++++++++ .../2026-03-21-subtitle-sidebar-auto-open.md | 4 + ...2026-03-21-subtitle-sidebar-resume-jump.md | 4 + .../runtime/subtitle-prefetch-init.test.ts | 35 +++++++++ src/main/runtime/subtitle-prefetch-init.ts | 1 + src/renderer/renderer.ts | 9 ++- src/renderer/subtitle-render.test.ts | 43 +++++++++++ 10 files changed, 311 insertions(+), 4 deletions(-) create mode 100644 backlog/tasks/task-214 - Jump-subtitle-sidebar-directly-to-resume-position-on-first-resolved-cue.md create mode 100644 backlog/tasks/task-215 - Add-startup-auto-open-option-for-subtitle-sidebar.md create mode 100644 backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md create mode 100644 backlog/tasks/task-217 - Fix-embedded-overlay-passthrough-sync-between-subtitle-and-sidebar.md create mode 100644 changes/2026-03-21-subtitle-sidebar-auto-open.md create mode 100644 changes/2026-03-21-subtitle-sidebar-resume-jump.md diff --git a/backlog/tasks/task-214 - Jump-subtitle-sidebar-directly-to-resume-position-on-first-resolved-cue.md b/backlog/tasks/task-214 - Jump-subtitle-sidebar-directly-to-resume-position-on-first-resolved-cue.md new file mode 100644 index 0000000..b44c1af --- /dev/null +++ b/backlog/tasks/task-214 - Jump-subtitle-sidebar-directly-to-resume-position-on-first-resolved-cue.md @@ -0,0 +1,37 @@ +--- +id: TASK-214 +title: Jump subtitle sidebar directly to resume position on first resolved cue +status: In Progress +assignee: [] +created_date: '2026-03-21 11:15' +updated_date: '2026-03-21 11:15' +labels: + - bug + - ux + - overlay + - subtitles +dependencies: [] +references: + - /Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.ts + - /Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.test.ts +priority: medium +--- + +## Description + + +When playback starts from a resumed timestamp while the subtitle sidebar is open, the sidebar currently smooth-scrolls from the top of the cue list to the resumed cue. Change the first resolved active-cue positioning to jump immediately to the resume location while preserving smooth auto-follow for later playback-driven cue advances. + + +## Acceptance Criteria + +- [x] #1 The first active cue resolved after open/resume uses an instant jump instead of smooth-scrolling through the list. +- [x] #2 Normal subtitle-sidebar auto-follow remains smooth after the first active cue has been positioned. +- [x] #3 Regression coverage distinguishes the initial jump behavior from later smooth auto-follow updates. + + +## Implementation Notes + + +2026-03-21: Fixed by treating the first auto-scroll from `previousActiveCueIndex < 0` as `behavior: 'auto'` in the subtitle sidebar scroll helper. Added renderer regression coverage for initial jump plus later smooth follow. + diff --git a/backlog/tasks/task-215 - Add-startup-auto-open-option-for-subtitle-sidebar.md b/backlog/tasks/task-215 - Add-startup-auto-open-option-for-subtitle-sidebar.md new file mode 100644 index 0000000..f3434ed --- /dev/null +++ b/backlog/tasks/task-215 - Add-startup-auto-open-option-for-subtitle-sidebar.md @@ -0,0 +1,40 @@ +--- +id: TASK-215 +title: Add startup auto-open option for subtitle sidebar +status: In Progress +assignee: [] +created_date: '2026-03-21 11:35' +updated_date: '2026-03-21 11:35' +labels: + - feature + - ux + - overlay + - subtitles +dependencies: [] +references: + - /Users/sudacode/projects/japanese/SubMiner/src/types.ts + - /Users/sudacode/projects/japanese/SubMiner/src/config/definitions/defaults-subtitle.ts + - /Users/sudacode/projects/japanese/SubMiner/src/config/resolve/subtitle-domains.ts + - /Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.ts + - /Users/sudacode/projects/japanese/SubMiner/src/renderer/renderer.ts +priority: medium +--- + +## Description + + +Add a subtitle sidebar config option that auto-opens the sidebar once during overlay startup. The option should default to `false`, only apply when the sidebar feature is enabled, and should not force the sidebar back open later in the same session after manual close or later visibility changes. + + +## Acceptance Criteria + +- [x] #1 `subtitleSidebar.autoOpen` is available in config with default `false`. +- [x] #2 When enabled, overlay startup opens the subtitle sidebar once after initial sidebar config/snapshot load. +- [x] #3 Regression coverage covers config resolution and startup-only auto-open behavior. + + +## Implementation Notes + + +2026-03-21: Added `subtitleSidebar.autoOpen` to types/defaults/config registry and resolver. Renderer bootstrap now calls a startup-only subtitle sidebar helper after the initial snapshot refresh. Modal regression coverage verifies startup auto-open requires both `enabled` and `autoOpen`. + 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 new file mode 100644 index 0000000..89c98b9 --- /dev/null +++ b/backlog/tasks/task-216 - Address-PR-28-CodeRabbit-follow-ups-on-subtitle-sidebar.md @@ -0,0 +1,73 @@ +--- +id: TASK-216 +title: 'Address PR #28 CodeRabbit follow-ups on subtitle sidebar' +status: In Progress +assignee: + - '@codex' +created_date: '2026-03-21 00:00' +updated_date: '2026-03-22 02:34' +labels: + - pr-review + - subtitle-sidebar + - renderer +dependencies: [] +references: + - src/main/runtime/subtitle-prefetch-init.ts + - src/main/runtime/subtitle-prefetch-init.test.ts + - src/renderer/handlers/mouse.ts + - src/renderer/handlers/mouse.test.ts + - src/renderer/modals/subtitle-sidebar.ts + - src/renderer/modals/subtitle-sidebar.test.ts + - src/renderer/style.css +priority: medium +--- + +## Description + + +Validate the CodeRabbit follow-ups on PR #28 for the subtitle sidebar workstream, implement the confirmed fixes, and verify the touched runtime and renderer paths. + + +## Acceptance Criteria + +- [x] #1 Review comments that described real regressions are fixed in code +- [x] #2 Focused regression coverage exists for the fixed behaviors +- [x] #3 Targeted typecheck and runtime-compat verification pass + + +## Implementation Notes + + +Completed follow-up fixes for PR #28: +- Cleared parsed subtitle cues on subtitle prefetch init failure so stale snapshot cache entries do not survive a failed refresh. +- Treated primary and secondary subtitle containers as one hover region so moving between them does not resume playback mid-transition. +- Kept the subtitle sidebar closed when disabled, serialized snapshot polling with timeouts, made cue rows keyboard-activatable, resolved stale cue selection fallback, and resumed hover-paused playback when the modal closes. + +Regression coverage added: +- `src/main/runtime/subtitle-prefetch-init.test.ts` +- `src/renderer/handlers/mouse.test.ts` +- `src/renderer/modals/subtitle-sidebar.test.ts` + +Verification: +- `bun test src/main/runtime/subtitle-prefetch-init.test.ts` +- `bun test src/renderer/handlers/mouse.test.ts` +- `bun test src/renderer/modals/subtitle-sidebar.test.ts` +- `bun run typecheck` +- `bun run test:runtime:compat` + +2026-03-21: Reopened to assess a newer CodeRabbit review pass on PR #28 and address any remaining valid action items before push/reply. + +2026-03-21: Addressed the latest CodeRabbit follow-up pass in commit d70c6448 after rebasing onto the updated remote branch tip. + +2026-03-21: Reopened for the latest CodeRabbit round on commit d70c6448; current actionable item is the invalid ctx.state.isOverSubtitleSidebar assignment in subtitle-sidebar.ts. + + +## Final Summary + + +Implemented the confirmed PR #28 CodeRabbit follow-ups for subtitle sidebar behavior and added regression coverage plus verification for the touched renderer and runtime paths. + +Handled the latest CodeRabbit review pass for PR #28: accepted zero sidebar opacity, closed/inerted the sidebar when refresh sees config disabled, moved poll rescheduling out of finally, caught hover pause IPC failures, and fixed the stylelint spacing issue. + +Verification: bun test src/config/resolve/subtitle-sidebar.test.ts; bun test src/renderer/modals/subtitle-sidebar.test.ts; bun test src/renderer/handlers/mouse.test.ts; bun run typecheck; bun run test:fast; bun run test:env; bun run build; SubMiner verifier lanes config + runtime-compat (including test:runtime:compat and test:smoke:dist). + diff --git a/backlog/tasks/task-217 - Fix-embedded-overlay-passthrough-sync-between-subtitle-and-sidebar.md b/backlog/tasks/task-217 - Fix-embedded-overlay-passthrough-sync-between-subtitle-and-sidebar.md new file mode 100644 index 0000000..287151c --- /dev/null +++ b/backlog/tasks/task-217 - Fix-embedded-overlay-passthrough-sync-between-subtitle-and-sidebar.md @@ -0,0 +1,69 @@ +--- +id: TASK-217 +title: Fix embedded overlay passthrough sync between subtitle and sidebar +status: Done +assignee: + - codex +created_date: '2026-03-21 23:16' +updated_date: '2026-03-21 23:28' +labels: + - bug + - overlay + - macos +dependencies: [] +references: + - src/renderer/handlers/mouse.ts + - src/renderer/modals/subtitle-sidebar.ts + - src/renderer/renderer.ts +documentation: + - docs/workflow/verification.md +priority: high +--- + +## Description + + +On macOS, when both the subtitle overlay and embedded subtitle sidebar are visible, mouse passthrough to mpv can remain stale until the user hovers the sidebar. After closing the sidebar, passthrough can likewise remain stale until the user hovers the subtitle again. Fix the overlay input-state synchronization so passthrough reflects the current hover/open state immediately instead of relying on the last hover target. + + +## Acceptance Criteria + +- [x] #1 When the embedded subtitle sidebar is open and the pointer is not over subtitle or sidebar content, the overlay returns to mouse passthrough immediately without requiring a sidebar hover cycle. +- [x] #2 When transitioning between subtitle hover and sidebar hover states on macOS embedded sidebar mode, mouse ignore state stays in sync with the currently interactive region. +- [x] #3 Closing the embedded subtitle sidebar restores the correct passthrough state based on remaining subtitle hover/modal state without requiring an additional hover. +- [x] #4 Regression tests cover the passthrough synchronization behavior. + + +## Implementation Plan + + +1. Add a shared renderer-side passthrough sync helper that derives whether the overlay should ignore mouse events from subtitle hover, embedded sidebar visibility/hover, popup visibility, and modal state. +2. Replace direct embedded-sidebar passthrough toggles in subtitle hover/sidebar handlers with calls to the shared sync helper so state is recomputed on every transition. +3. Add regression tests for macOS embedded sidebar mode covering sidebar-open idle passthrough, subtitle-to-sidebar transitions, and sidebar-close restore behavior. +4. Run targeted renderer tests for mouse/sidebar passthrough coverage, then summarize any residual risk. + + +## Implementation Notes + + +Added shared renderer overlay mouse-ignore recompute so subtitle hover, embedded sidebar hover/open/close, and popup idle transitions all derive passthrough from current state instead of last hover target. + +Added regression coverage for embedded sidebar idle passthrough on subtitle leave and for sidebar-close recompute behavior. + +Verification: `bun run typecheck` passed; `bun test src/renderer/handlers/mouse.test.ts` passed; `bun test src/renderer/modals/subtitle-sidebar.test.ts` passed; core verification wrapper artifact at `.tmp/skill-verification/subminer-verify-20260321-162743-XhSBxw` hit an unrelated `bun run test:fast` failure in `scripts/update-aur-package.test.ts` because macOS system bash lacks `mapfile`. + + +## Final Summary + + +Fixed stale embedded-sidebar passthrough sync on macOS by introducing a shared renderer mouse-ignore recompute path and tracking sidebar-hover state separately from subtitle hover. Subtitle hover leave, sidebar hover enter/leave, sidebar open, and sidebar close now all recompute passthrough from the current overlay state instead of waiting for a later hover event to repair it. Added regression tests covering subtitle-leave passthrough while the embedded sidebar is open but idle, plus sidebar-close restore behavior based on remaining subtitle hover state. + +Tests run: +- `bun run typecheck` +- `bun test src/renderer/handlers/mouse.test.ts` +- `bun test src/renderer/modals/subtitle-sidebar.test.ts` +- `bash .agents/skills/subminer-change-verification/scripts/classify_subminer_diff.sh src/renderer/state.ts src/renderer/overlay-mouse-ignore.ts src/renderer/handlers/mouse.ts src/renderer/handlers/mouse.test.ts src/renderer/modals/subtitle-sidebar.ts src/renderer/modals/subtitle-sidebar.test.ts` +- `bash .agents/skills/subminer-change-verification/scripts/verify_subminer_change.sh --lane core src/renderer/state.ts src/renderer/overlay-mouse-ignore.ts src/renderer/handlers/mouse.ts src/renderer/handlers/mouse.test.ts src/renderer/modals/subtitle-sidebar.ts src/renderer/modals/subtitle-sidebar.test.ts` (typecheck passed; `test:fast` blocked by unrelated `scripts/update-aur-package.test.ts` failure on macOS Bash 3.2 lacking `mapfile`) + +Risk: the classifier flagged this as a real-runtime candidate, so actual Electron/mpv macOS pointer behavior was not exercised in a live runtime during this turn. + diff --git a/changes/2026-03-21-subtitle-sidebar-auto-open.md b/changes/2026-03-21-subtitle-sidebar-auto-open.md new file mode 100644 index 0000000..f0d1d37 --- /dev/null +++ b/changes/2026-03-21-subtitle-sidebar-auto-open.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Added `subtitleSidebar.autoOpen` (default `false`) to open the subtitle sidebar once during overlay startup when the sidebar feature is enabled. diff --git a/changes/2026-03-21-subtitle-sidebar-resume-jump.md b/changes/2026-03-21-subtitle-sidebar-resume-jump.md new file mode 100644 index 0000000..2a9dbfb --- /dev/null +++ b/changes/2026-03-21-subtitle-sidebar-resume-jump.md @@ -0,0 +1,4 @@ +type: fixed +area: overlay + +- Made subtitle sidebar resume/start positioning jump directly to the first resolved active cue instead of smooth-scrolling through the full list, while keeping smooth auto-follow for later cue changes. diff --git a/src/main/runtime/subtitle-prefetch-init.test.ts b/src/main/runtime/subtitle-prefetch-init.test.ts index 34debc4..8314393 100644 --- a/src/main/runtime/subtitle-prefetch-init.test.ts +++ b/src/main/runtime/subtitle-prefetch-init.test.ts @@ -199,3 +199,38 @@ test('subtitle prefetch init publishes the provided stable source key instead of assert.deepEqual(sourceUpdates, ['internal:/media/episode01.mkv:track:3:ff:7']); }); + +test('subtitle prefetch init clears parsed cues when initialization fails', async () => { + const cueUpdates: Array = []; + let currentService: SubtitlePrefetchService | null = null; + + const controller = createSubtitlePrefetchInitController({ + getCurrentService: () => currentService, + setCurrentService: (service) => { + currentService = service; + }, + loadSubtitleSourceText: async () => { + throw new Error('boom'); + }, + parseSubtitleCues: () => [{ startTime: 1, endTime: 2, text: 'first' }], + createSubtitlePrefetchService: () => ({ + start: () => {}, + stop: () => {}, + onSeek: () => {}, + pause: () => {}, + resume: () => {}, + }), + tokenizeSubtitle: async () => null, + preCacheTokenization: () => {}, + isCacheFull: () => false, + logInfo: () => {}, + logWarn: () => {}, + onParsedSubtitleCuesChanged: (cues) => { + cueUpdates.push(cues); + }, + }); + + await controller.initSubtitlePrefetch('episode.ass', 12); + + assert.deepEqual(cueUpdates, [null]); +}); diff --git a/src/main/runtime/subtitle-prefetch-init.ts b/src/main/runtime/subtitle-prefetch-init.ts index dcc8a32..c281e94 100644 --- a/src/main/runtime/subtitle-prefetch-init.ts +++ b/src/main/runtime/subtitle-prefetch-init.ts @@ -82,6 +82,7 @@ export function createSubtitlePrefetchInitController( ); } catch (error) { if (revision === initRevision) { + deps.onParsedSubtitleCuesChanged?.(null, null); deps.logWarn(`[subtitle-prefetch] failed to initialize: ${(error as Error).message}`); } } diff --git a/src/renderer/renderer.ts b/src/renderer/renderer.ts index 74e3662..6d08a03 100644 --- a/src/renderer/renderer.ts +++ b/src/renderer/renderer.ts @@ -521,10 +521,10 @@ async function init(): Promise { subtitleRenderer.renderSecondarySub(await window.electronAPI.getCurrentSecondarySub()); measurementReporter.schedule(); - ctx.dom.subtitleContainer.addEventListener('mouseenter', mouseHandlers.handleMouseEnter); - ctx.dom.subtitleContainer.addEventListener('mouseleave', mouseHandlers.handleMouseLeave); - ctx.dom.secondarySubContainer.addEventListener('mouseenter', mouseHandlers.handleMouseEnter); - ctx.dom.secondarySubContainer.addEventListener('mouseleave', mouseHandlers.handleMouseLeave); + ctx.dom.subtitleContainer.addEventListener('mouseenter', mouseHandlers.handlePrimaryMouseEnter); + ctx.dom.subtitleContainer.addEventListener('mouseleave', mouseHandlers.handlePrimaryMouseLeave); + ctx.dom.secondarySubContainer.addEventListener('mouseenter', mouseHandlers.handleSecondaryMouseEnter); + ctx.dom.secondarySubContainer.addEventListener('mouseleave', mouseHandlers.handleSecondaryMouseLeave); mouseHandlers.setupResizeHandler(); mouseHandlers.setupSelectionObserver(); @@ -575,6 +575,7 @@ async function init(): Promise { const initialSubtitleStyle = await window.electronAPI.getSubtitleStyle(); subtitleRenderer.applySubtitleStyle(initialSubtitleStyle); await subtitleSidebarModal.refreshSubtitleSidebarSnapshot(); + await subtitleSidebarModal.autoOpenSubtitleSidebarOnStartup(); positioning.applyStoredSubtitlePosition( await window.electronAPI.getSubtitlePosition(), diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index 64b8309..f78c985 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -977,6 +977,30 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { ); assert.match(secondaryHoverBaseBlock, /background:\s*transparent;/); + const secondaryEmbeddedHoverBlock = extractClassBlock( + cssText, + 'body.subtitle-sidebar-embedded-open #secondarySubContainer.secondary-sub-hover', + ); + assert.match( + secondaryEmbeddedHoverBlock, + /right:\s*var\(--subtitle-sidebar-reserved-width\);/, + ); + assert.match( + secondaryEmbeddedHoverBlock, + /max-width:\s*none;/, + ); + assert.match( + secondaryEmbeddedHoverBlock, + /transform:\s*none;/, + ); + assert.doesNotMatch( + secondaryEmbeddedHoverBlock, + /transform:\s*translateX\(calc\(var\(--subtitle-sidebar-reserved-width\)\s*\*\s*-0\.5\)\);/, + ); + + const subtitleSidebarListBlock = extractClassBlock(cssText, '.subtitle-sidebar-list'); + assert.doesNotMatch(subtitleSidebarListBlock, /scroll-behavior:\s*smooth;/); + const secondaryHoverVisibleBlock = extractClassBlock( cssText, '#secondarySubContainer.secondary-sub-hover:hover #secondarySubRoot', @@ -990,6 +1014,25 @@ test('JLPT CSS rules use underline-only styling in renderer stylesheet', () => { /backdrop-filter:\s*var\(--secondary-sub-backdrop-filter,\s*none\);/, ); + const secondaryHoverActiveBlock = extractClassBlock( + cssText, + '#secondarySubContainer.secondary-sub-hover.secondary-sub-hover-active', + ); + assert.match(secondaryHoverActiveBlock, /opacity:\s*1;/); + + const secondaryHoverActiveRootBlock = extractClassBlock( + cssText, + '#secondarySubContainer.secondary-sub-hover.secondary-sub-hover-active #secondarySubRoot', + ); + assert.match( + secondaryHoverActiveRootBlock, + /background:\s*var\(--secondary-sub-background-color,\s*transparent\);/, + ); + assert.match( + secondaryHoverActiveRootBlock, + /backdrop-filter:\s*var\(--secondary-sub-backdrop-filter,\s*none\);/, + ); + assert.doesNotMatch( cssText, /body\.layer-visible\s+#secondarySubContainer\s*\{[^}]*display:\s*none/i,