From e84825754ba7034bb4a086c6b1cdff0d521a928d Mon Sep 17 00:00:00 2001 From: sudacode Date: Thu, 4 Jun 2026 23:35:37 -0700 Subject: [PATCH] fix(notifications): show thumbnail image in overlay mined-card notificat - Share generated notification icon between overlay and system notification paths - Add `image` field to OverlayNotificationPayload; render as IMG with has-image layout - Widen overlay stack to 420px; enlarge card padding and min-height for image variant - Show OSD message after successful anilist retry when attempt key already handled --- changes/overlay-notifications.md | 1 + docs-site/anki-integration.md | 2 + src/anki-integration.test.ts | 83 +++++++++++ src/anki-integration.ts | 85 ++++++----- src/main/runtime/anilist-post-watch.test.ts | 4 +- src/main/runtime/anilist-post-watch.ts | 5 +- src/renderer/overlay-notifications.test.ts | 147 ++++++++++++++++++++ src/renderer/overlay-notifications.ts | 25 +++- src/renderer/style.css | 23 ++- src/types/notification.ts | 1 + 10 files changed, 329 insertions(+), 47 deletions(-) diff --git a/changes/overlay-notifications.md b/changes/overlay-notifications.md index f8b4e65c..33d7acb1 100644 --- a/changes/overlay-notifications.md +++ b/changes/overlay-notifications.md @@ -7,5 +7,6 @@ breaking: true - Routed startup tokenization and subtitle annotation status through the configured notification surfaces; the bundled mpv plugin now only emits startup OSD messages for `osd` and `osd-system`. - Kept playback feedback such as subtitle visibility, subtitle track, and subtitle delay text on overlay/OSD surfaces only; desktop/system notifications are reserved for real notifications like mined cards, errors, and updates. - Reused the active primary/secondary subtitle mode overlay notification while cycling modes so rapid toggles update one card instead of stacking duplicate feedback. +- Fixed mined-card overlay notifications so `overlay` and `both` modes show the same generated card thumbnail as system notifications. - Changed `both` notification routing to mean overlay + system; users who used `both` for mpv OSD + system notifications should set `notificationType` to `osd-system` in `config.jsonc`. - Kept `osd` and `osd-system` as config-file-only legacy notification values; Settings normally offers only overlay, system, both, and none, while still showing an already configured legacy value as selected. diff --git a/docs-site/anki-integration.md b/docs-site/anki-integration.md index bbecc0cd..18f6bc32 100644 --- a/docs-site/anki-integration.md +++ b/docs-site/anki-integration.md @@ -223,6 +223,8 @@ Animated AVIF requires an AV1 encoder (`libaom-av1`, `libsvtav1`, or `librav1e`) `both` now means overlay + system notification. `osd` and `osd-system` are legacy config-file-only values; set `notificationType` to `"osd-system"` in `config.jsonc` if you previously used `both` and want to keep mpv OSD + system notifications. The Settings window shows `osd` or `osd-system` when already configured, but only offers `overlay`, `system`, `both`, and `none` as normal choices. +When media is available, mined-card overlay and system notifications include the same current-frame thumbnail. + `overwriteAudio` applies to automatic card updates and duplicate-card enrichment. Manual clipboard subtitle updates (`Ctrl/Cmd+C`, then `Ctrl/Cmd+V`) always replace generated sentence audio, while leaving the word audio field unchanged. ## AI Translation diff --git a/src/anki-integration.test.ts b/src/anki-integration.test.ts index 49798e5b..615fe5e8 100644 --- a/src/anki-integration.test.ts +++ b/src/anki-integration.test.ts @@ -3,10 +3,18 @@ import assert from 'node:assert/strict'; import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; +import { pathToFileURL } from 'url'; import { AnkiIntegration } from './anki-integration'; import { FieldGroupingMergeCollaborator } from './anki-integration/field-grouping-merge'; import { AnkiConnectConfig } from './types'; +type TestOverlayNotificationPayload = { + title: string; + body?: string; + image?: string; + variant?: string; +}; + interface IntegrationTestContext { integration: AnkiIntegration; calls: { @@ -406,6 +414,81 @@ test('AnkiIntegration marks partial update notifications as failures in OSD mode assert.deepEqual(osdMessages, ['x Updated card: taberu (image failed)']); }); +test('AnkiIntegration includes generated notification image on overlay mined-card notifications', async () => { + const desktopNotifications: Array<{ title: string; body?: string; icon?: string }> = []; + const overlayNotifications: TestOverlayNotificationPayload[] = []; + const generatedFrom: Array<{ videoPath: string; timestamp: number }> = []; + const cleanupPaths: string[] = []; + const notificationIconPath = path.join(os.tmpdir(), 'subminer-notification-icon.png'); + + const integration = new AnkiIntegration( + { + behavior: { + notificationType: 'both', + }, + }, + {} as never, + { + currentVideoPath: '/tmp/show.mkv', + currentTimePos: 123.45, + } as never, + undefined, + (title, options) => { + desktopNotifications.push({ title, body: options.body, icon: options.icon }); + }, + undefined, + undefined, + {}, + undefined, + (payload) => { + overlayNotifications.push(payload as TestOverlayNotificationPayload); + }, + ); + + ( + integration as unknown as { + mediaGenerator: { + generateNotificationIcon: (videoPath: string, timestamp: number) => Promise; + writeNotificationIconToFile: (iconBuffer: Buffer, noteId: number) => string; + scheduleNotificationIconCleanup: (filePath: string) => void; + }; + } + ).mediaGenerator = { + generateNotificationIcon: async (videoPath, timestamp) => { + generatedFrom.push({ videoPath, timestamp }); + return Buffer.from('png'); + }, + writeNotificationIconToFile: (iconBuffer, noteId) => { + assert.equal(iconBuffer.toString(), 'png'); + assert.equal(noteId, 42); + return notificationIconPath; + }, + scheduleNotificationIconCleanup: (filePath) => { + cleanupPaths.push(filePath); + }, + }; + + await ( + integration as unknown as { + showNotification: (noteId: number, label: string | number) => Promise; + } + ).showNotification(42, '食べる'); + + assert.deepEqual(generatedFrom, [{ videoPath: '/tmp/show.mkv', timestamp: 123.45 }]); + assert.equal(overlayNotifications.length, 1); + assert.equal(overlayNotifications[0]?.title, 'Anki Card Updated'); + assert.equal(overlayNotifications[0]?.body, 'Updated card: 食べる'); + assert.equal(overlayNotifications[0]?.image, pathToFileURL(notificationIconPath).toString()); + assert.deepEqual(desktopNotifications, [ + { + title: 'Anki Card Updated', + body: 'Updated card: 食べる', + icon: notificationIconPath, + }, + ]); + assert.deepEqual(cleanupPaths, [notificationIconPath]); +}); + test('AnkiIntegration routes workflow status notifications through configured surfaces', async () => { const osdMessages: string[] = []; const desktopMessages: string[] = []; diff --git a/src/anki-integration.ts b/src/anki-integration.ts index ebad7af2..025d5e90 100644 --- a/src/anki-integration.ts +++ b/src/anki-integration.ts @@ -20,6 +20,7 @@ import { AnkiConnectClient } from './anki-connect'; import { SubtitleTimingTracker } from './subtitle-timing-tracker'; import { MediaGenerator } from './media-generator'; import path from 'path'; +import { pathToFileURL } from 'url'; import { AnkiConnectConfig, KikuDuplicateCardInfo, @@ -120,6 +121,10 @@ function shouldPreferMediaTitleForMiscInfo(rawPath: string, filename: string): b ); } +function toOverlayNotificationImageSource(filePath: string): string { + return pathToFileURL(filePath).toString(); +} + export class AnkiIntegration { private client: AnkiConnectClient; private mediaGenerator: MediaGenerator; @@ -1082,56 +1087,64 @@ export class AnkiIntegration { this.clearUpdateProgress(); } - if ((type === 'overlay' || type === 'both') && this.overlayNotificationCallback) { + const shouldShowOverlayNotification = + (type === 'overlay' || type === 'both') && this.overlayNotificationCallback !== null; + const shouldShowSystemNotification = + (type === 'system' || type === 'both' || type === 'osd-system') && + this.notificationCallback !== null; + const notificationIconPath = + shouldShowOverlayNotification || shouldShowSystemNotification + ? await this.generateNotificationIconPath(noteId) + : undefined; + + if (shouldShowOverlayNotification && this.overlayNotificationCallback) { this.overlayNotificationCallback({ id: 'anki-update-progress', title: 'Anki Card Updated', body: message, + ...(notificationIconPath + ? { image: toOverlayNotificationImageSource(notificationIconPath) } + : {}), variant: errorSuffix === undefined ? 'success' : 'error', persistent: false, }); } - if ( - (type === 'system' || type === 'both' || type === 'osd-system') && - this.notificationCallback - ) { - let notificationIconPath: string | undefined; - - if (this.mpvClient && this.mpvClient.currentVideoPath) { - try { - const timestamp = this.mpvClient.currentTimePos || 0; - const notificationIconSource = await resolveMediaGenerationInputPath( - this.mpvClient, - 'video', - ); - if (!notificationIconSource) { - throw new Error('No media source available for notification icon'); - } - const iconBuffer = await this.mediaGenerator.generateNotificationIcon( - notificationIconSource, - timestamp, - ); - if (iconBuffer && iconBuffer.length > 0) { - notificationIconPath = this.mediaGenerator.writeNotificationIconToFile( - iconBuffer, - noteId, - ); - } - } catch (err) { - log.warn('Failed to generate notification icon:', (err as Error).message); - } - } - + if (shouldShowSystemNotification && this.notificationCallback) { this.notificationCallback('Anki Card Updated', { body: message, icon: notificationIconPath, }); - - if (notificationIconPath) { - this.mediaGenerator.scheduleNotificationIconCleanup(notificationIconPath); - } } + + if (notificationIconPath) { + this.mediaGenerator.scheduleNotificationIconCleanup(notificationIconPath); + } + } + + private async generateNotificationIconPath(noteId: number): Promise { + if (!this.mpvClient?.currentVideoPath) { + return undefined; + } + + try { + const timestamp = this.mpvClient.currentTimePos || 0; + const notificationIconSource = await resolveMediaGenerationInputPath(this.mpvClient, 'video'); + if (!notificationIconSource) { + throw new Error('No media source available for notification icon'); + } + const iconBuffer = await this.mediaGenerator.generateNotificationIcon( + notificationIconSource, + timestamp, + ); + if (iconBuffer && iconBuffer.length > 0) { + return this.mediaGenerator.writeNotificationIconToFile(iconBuffer, noteId); + } + } catch (err) { + log.warn('Failed to generate notification icon:', (err as Error).message); + } + + return undefined; } private showUpdateResult(message: string, success: boolean): void { diff --git a/src/main/runtime/anilist-post-watch.test.ts b/src/main/runtime/anilist-post-watch.test.ts index 47bda6bc..57ad173e 100644 --- a/src/main/runtime/anilist-post-watch.test.ts +++ b/src/main/runtime/anilist-post-watch.test.ts @@ -330,7 +330,7 @@ test('createMaybeRunAnilistPostWatchUpdateHandler skips youtube playback entirel assert.deepEqual(calls, []); }); -test('createMaybeRunAnilistPostWatchUpdateHandler does not live-update after retry already handled current attempt key', async () => { +test('createMaybeRunAnilistPostWatchUpdateHandler notifies when retry already handled current attempt key', async () => { const calls: string[] = []; const attemptedKeys = new Set(); const mediaKey = '/tmp/video.mkv'; @@ -378,5 +378,5 @@ test('createMaybeRunAnilistPostWatchUpdateHandler does not live-update after ret assert.equal(calls.includes('update'), false); assert.equal(calls.includes('enqueue'), false); assert.equal(calls.includes('mark-failure'), false); - assert.deepEqual(calls, ['inflight:true', 'process-retry', 'inflight:false']); + assert.deepEqual(calls, ['inflight:true', 'process-retry', 'osd:retry ok', 'inflight:false']); }); diff --git a/src/main/runtime/anilist-post-watch.ts b/src/main/runtime/anilist-post-watch.ts index be3b0f7d..cbd81e84 100644 --- a/src/main/runtime/anilist-post-watch.ts +++ b/src/main/runtime/anilist-post-watch.ts @@ -194,8 +194,11 @@ export function createMaybeRunAnilistPostWatchUpdateHandler(deps: { return; } - await deps.processNextAnilistRetryUpdate(); + const retryResult = await deps.processNextAnilistRetryUpdate(); if (deps.hasAttemptedUpdateKey(attemptKey)) { + if (retryResult.ok) { + deps.showMpvOsd(retryResult.message); + } return; } diff --git a/src/renderer/overlay-notifications.test.ts b/src/renderer/overlay-notifications.test.ts index 59fcd15e..be3ffe93 100644 --- a/src/renderer/overlay-notifications.test.ts +++ b/src/renderer/overlay-notifications.test.ts @@ -1,12 +1,94 @@ import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import path from 'node:path'; import test from 'node:test'; import { + createOverlayNotificationRenderer, createOverlayNotificationStore, handleOverlayNotificationEvent, overlayNotificationPositionClass, } from './overlay-notifications'; +function createClassList(initialTokens: string[] = []) { + const tokens = new Set(initialTokens); + return { + add: (...entries: string[]) => { + for (const entry of entries) tokens.add(entry); + }, + remove: (...entries: string[]) => { + for (const entry of entries) tokens.delete(entry); + }, + contains: (entry: string) => tokens.has(entry), + toggle: (entry: string, force?: boolean) => { + if (force === true) tokens.add(entry); + else if (force === false) tokens.delete(entry); + else if (tokens.has(entry)) tokens.delete(entry); + else tokens.add(entry); + }, + }; +} + +type FakeElement = { + tagName: string; + className: string; + textContent: string; + src: string; + alt: string; + type: string; + dataset: Record; + children: FakeElement[]; + classList: ReturnType; + append: (...children: FakeElement[]) => void; + replaceChildren: (...children: FakeElement[]) => void; + setAttribute: (name: string, value: string) => void; + getAttribute: (name: string) => string | null; + addEventListener: (type: string, listener: (event?: unknown) => void) => void; +}; + +function createFakeElement(tagName = 'div'): FakeElement { + const attributes = new Map(); + const element: FakeElement = { + tagName: tagName.toUpperCase(), + className: '', + textContent: '', + src: '', + alt: '', + type: '', + dataset: {}, + children: [], + classList: createClassList(), + append: (...children) => { + element.children.push(...children); + }, + replaceChildren: (...children) => { + element.children = [...children]; + }, + setAttribute: (name, value) => { + attributes.set(name, value); + }, + getAttribute: (name) => attributes.get(name) ?? null, + addEventListener: () => undefined, + }; + return element; +} + +function findChildByClass(element: FakeElement, className: string): FakeElement | null { + if (element.className.split(/\s+/).includes(className)) { + return element; + } + for (const child of element.children) { + const match = findChildByClass(child, className); + if (match) return match; + } + return null; +} + +const overlayNotificationCss = readFileSync( + path.join(process.cwd(), 'src/renderer/style.css'), + 'utf8', +); + test('overlay notification store caps transient notifications and keeps pinned jobs visible', () => { const store = createOverlayNotificationStore({ maxVisible: 3 }); @@ -63,3 +145,68 @@ test('overlay notification event handler dismisses notifications by id', () => { assert.deepEqual(calls, ['remove:overlay-loading-status']); }); + +test('overlay notification renderer shows thumbnail image from payload', () => { + const originalDocument = Object.getOwnPropertyDescriptor(globalThis, 'document'); + const stack = createFakeElement(); + + Object.defineProperty(globalThis, 'document', { + configurable: true, + writable: true, + value: { + createElement: (tagName: string) => createFakeElement(tagName), + }, + }); + + try { + const renderer = createOverlayNotificationRenderer({ + dom: { + overlayNotificationStack: stack, + }, + state: { + isOverOverlayNotification: false, + }, + } as never); + + renderer.show({ + title: 'Anki Card Updated', + body: 'Updated card: 食べる', + image: 'file:///tmp/subminer-notification-icon.png', + variant: 'success', + persistent: true, + }); + + const card = stack.children[0]; + if (!card) { + assert.fail('Expected overlay notification card.'); + } + const image = findChildByClass(card, 'overlay-notification-image'); + if (!image) { + assert.fail('Expected overlay notification image.'); + } + + assert.equal(image.tagName, 'IMG'); + assert.equal(image.src, 'file:///tmp/subminer-notification-icon.png'); + assert.equal(image.alt, ''); + } finally { + if (originalDocument) { + Object.defineProperty(globalThis, 'document', originalDocument); + } else { + delete (globalThis as { document?: unknown }).document; + } + } +}); + +test('overlay notification cards use larger display dimensions', () => { + assert.match( + overlayNotificationCss, + /\.overlay-notification-stack\s*\{[^}]*width:\s*min\(420px,\s*calc\(100vw - 32px\)\);/s, + ); + assert.match(overlayNotificationCss, /\.overlay-notification-card\s*\{[^}]*min-height:\s*76px;/s); + assert.match( + overlayNotificationCss, + /\.overlay-notification-card\.has-image\s*\{[^}]*min-height:\s*88px;/s, + ); + assert.match(overlayNotificationCss, /\.overlay-notification-image\s*\{[^}]*width:\s*56px;/s); + assert.match(overlayNotificationCss, /\.overlay-notification-image\s*\{[^}]*height:\s*56px;/s); +}); diff --git a/src/renderer/overlay-notifications.ts b/src/renderer/overlay-notifications.ts index 4d9984ef..330331bc 100644 --- a/src/renderer/overlay-notifications.ts +++ b/src/renderer/overlay-notifications.ts @@ -122,6 +122,12 @@ function normalizeVariant( return variant ?? 'info'; } +function normalizeImageSource(image: string | undefined): string | null { + if (!image) return null; + const trimmed = image.trim(); + return trimmed.length > 0 ? trimmed : null; +} + function setInteractiveState(ctx: RendererContext, value: boolean): void { ctx.state.isOverOverlayNotification = value; syncOverlayMouseIgnoreState(ctx); @@ -157,14 +163,23 @@ export function createOverlayNotificationRenderer( ctx.dom.overlayNotificationStack.classList.add(overlayNotificationPositionClass(position)); for (const entry of visible) { + const imageSource = normalizeImageSource(entry.image); const card = document.createElement('section'); - card.className = `overlay-notification-card ${normalizeVariant(entry.variant)}`; + card.className = `overlay-notification-card ${normalizeVariant(entry.variant)}${ + imageSource ? ' has-image' : '' + }`; card.dataset.notificationId = entry.id; card.setAttribute('role', 'status'); - const icon = document.createElement('span'); - icon.className = 'overlay-notification-icon'; - icon.setAttribute('aria-hidden', 'true'); + const leading = imageSource ? document.createElement('img') : document.createElement('span'); + leading.className = imageSource ? 'overlay-notification-image' : 'overlay-notification-icon'; + leading.setAttribute('aria-hidden', 'true'); + if (imageSource) { + const image = leading as HTMLImageElement; + image.src = imageSource; + image.alt = ''; + image.decoding = 'async'; + } const content = document.createElement('div'); content.className = 'overlay-notification-content'; @@ -205,7 +220,7 @@ export function createOverlayNotificationRenderer( closeButton.textContent = '×'; closeButton.addEventListener('click', () => remove(entry.id)); - card.append(icon, content, closeButton); + card.append(leading, content, closeButton); ctx.dom.overlayNotificationStack.append(card); } diff --git a/src/renderer/style.css b/src/renderer/style.css index cdda0f78..75a3d060 100644 --- a/src/renderer/style.css +++ b/src/renderer/style.css @@ -149,7 +149,7 @@ body:focus-visible, .overlay-notification-stack { position: absolute; top: 16px; - width: min(360px, calc(100vw - 32px)); + width: min(420px, calc(100vw - 32px)); display: flex; flex-direction: column; gap: 8px; @@ -184,8 +184,8 @@ body:focus-visible, grid-template-columns: 22px minmax(0, 1fr) 22px; gap: 11px; align-items: start; - min-height: 64px; - padding: 13px 14px 13px 17px; + min-height: 76px; + padding: 16px 16px 16px 20px; border-radius: 11px; border: 1px solid color-mix(in srgb, var(--overlay-notification-accent) 24%, var(--ctp-surface1)); background: linear-gradient( @@ -233,6 +233,23 @@ body:focus-visible, --overlay-notification-accent: var(--ctp-red); } +.overlay-notification-card.has-image { + grid-template-columns: 56px minmax(0, 1fr) 22px; + min-height: 88px; + padding-left: 14px; +} + +.overlay-notification-image { + width: 56px; + height: 56px; + align-self: center; + display: block; + border-radius: 7px; + border: 1px solid color-mix(in srgb, var(--overlay-notification-accent) 28%, var(--ctp-surface2)); + background: var(--ctp-crust); + object-fit: cover; +} + .overlay-notification-icon { width: 22px; height: 22px; diff --git a/src/types/notification.ts b/src/types/notification.ts index 16975573..a72cff17 100644 --- a/src/types/notification.ts +++ b/src/types/notification.ts @@ -23,6 +23,7 @@ export interface OverlayNotificationPayload { id?: string; title: string; body?: string; + image?: string; variant?: OverlayNotificationVariant; position?: OverlayNotificationPosition; persistent?: boolean;