From 2a4fdb74e4870406f6afdbc63b7b9e18a01f6f3a Mon Sep 17 00:00:00 2001 From: sudacode Date: Tue, 9 Jun 2026 01:59:11 -0700 Subject: [PATCH] fix(notifications): stabilize overlay startup and macOS hover passthroug - Skip re-append for cards already in stack to avoid replaying enter animation - Track enter animation end to remove `entering` class after first render - Gate mouseenter interactive state on cards with explicit actions only - Bind interactive hover only to action buttons and their close buttons - Add regression tests for passive vs action-bearing notification hover --- changes/overlay-notifications.md | 1 + ...6-06-09-macos-notification-hover-design.md | 27 ++ .../overlay-notifications-hover.test.ts | 245 ++++++++++++++++++ src/renderer/overlay-notifications.test.ts | 9 + src/renderer/overlay-notifications.ts | 53 +++- 5 files changed, 331 insertions(+), 4 deletions(-) create mode 100644 docs/plans/2026-06-09-macos-notification-hover-design.md create mode 100644 src/renderer/overlay-notifications-hover.test.ts diff --git a/changes/overlay-notifications.md b/changes/overlay-notifications.md index e8938d0a..e8159642 100644 --- a/changes/overlay-notifications.md +++ b/changes/overlay-notifications.md @@ -12,6 +12,7 @@ breaking: true - 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. - Updated repeated progress notifications such as subsync syncing in place so their spinner stays live instead of flickering on every tick. +- Stabilized overlay startup notifications so queued progress updates do not replay the card entrance animation or trigger macOS pass-through hover flicker after the loading OSD hands off to overlay notifications. - Fixed mined-card overlay notifications so `overlay` and `both` modes show generated card thumbnails in both live cards and the notification history panel. - Added Open in Anki buttons to mined-card overlay notifications and their history entries, with a direct AnkiConnect fallback when the live integration is unavailable. - Fixed those Open in Anki buttons so their fallback honors runtime AnkiConnect URL overrides and the default AnkiConnect endpoint. diff --git a/docs/plans/2026-06-09-macos-notification-hover-design.md b/docs/plans/2026-06-09-macos-notification-hover-design.md new file mode 100644 index 00000000..31303e71 --- /dev/null +++ b/docs/plans/2026-06-09-macos-notification-hover-design.md @@ -0,0 +1,27 @@ + + +# macOS Notification Hover Stability Design + +Status: approved +Date: 2026-06-09 + +## Problem + +On macOS, hovering a character dictionary build notification can make the card flicker and slide as +if it is hiding, then snap back. The likely trigger is the notification stack changing the overlay +window's mouse-passthrough state for a progress card that has no user action. + +## Chosen Approach + +Keep non-action overlay notifications visually stable and click-through on hover. Only notifications +with explicit actions should request interactive overlay input. The notification history panel keeps +its existing interactive behavior. + +This avoids a macOS mouseenter/mouseleave passthrough loop for passive progress cards while +preserving clickable notification actions. + +## Checks + +- Add a renderer regression test for passive notification hover. +- Keep action-bearing notification cards interactive. +- Run the targeted overlay notification and mouse-ignore tests. diff --git a/src/renderer/overlay-notifications-hover.test.ts b/src/renderer/overlay-notifications-hover.test.ts new file mode 100644 index 00000000..05b192a0 --- /dev/null +++ b/src/renderer/overlay-notifications-hover.test.ts @@ -0,0 +1,245 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { createOverlayNotificationRenderer } from './overlay-notifications'; + +function createClassList() { + const tokens = new Set(); + 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; + type: string; + dataset: Record; + children: FakeElement[]; + classList: ReturnType; + append: (...children: FakeElement[]) => void; + replaceChildren: (...children: FakeElement[]) => void; + remove: () => void; + setAttribute: (name: string, value: string) => void; + addEventListener: (type: string, listener: (event?: unknown) => void) => void; + dispatchEventType: (type: string, event?: unknown) => void; +}; + +function createFakeElement(tagName = 'div'): FakeElement { + const listeners = new Map void>>(); + const element: FakeElement = { + tagName: tagName.toUpperCase(), + className: '', + textContent: '', + type: '', + dataset: {}, + children: [], + classList: createClassList(), + append: (...children) => { + for (const child of children) { + const existingIndex = element.children.indexOf(child); + if (existingIndex >= 0) { + element.children.splice(existingIndex, 1); + } + element.children.push(child); + } + }, + replaceChildren: (...children) => { + element.children = [...children]; + }, + setAttribute: () => undefined, + remove: () => undefined, + addEventListener: (type, listener) => { + listeners.set(type, [...(listeners.get(type) ?? []), listener]); + }, + dispatchEventType: (type, event) => { + for (const listener of listeners.get(type) ?? []) listener(event); + }, + }; + 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; +} + +function createHoverContext(stack: FakeElement, ignoreCalls: Array<{ ignore: boolean }>) { + return { + dom: { + overlay: { classList: createClassList() }, + overlayNotificationStack: stack, + }, + platform: { + shouldToggleMouseIgnore: true, + }, + state: { + isOverSubtitle: false, + isOverSubtitleSidebar: false, + isOverOverlayNotification: false, + isOverNotificationHistory: false, + yomitanPopupVisible: false, + controllerSelectModalOpen: false, + controllerDebugModalOpen: false, + jimakuModalOpen: false, + youtubePickerModalOpen: false, + kikuModalOpen: false, + runtimeOptionsModalOpen: false, + subsyncModalOpen: false, + sessionHelpModalOpen: false, + }, + }; +} + +function installDomGlobals(ignoreCalls: Array<{ ignore: boolean }>): () => void { + const originalDocument = Object.getOwnPropertyDescriptor(globalThis, 'document'); + const originalWindow = Object.getOwnPropertyDescriptor(globalThis, 'window'); + + Object.defineProperty(globalThis, 'document', { + configurable: true, + writable: true, + value: { + createElement: (tagName: string) => createFakeElement(tagName), + querySelectorAll: () => [], + }, + }); + Object.defineProperty(globalThis, 'window', { + configurable: true, + writable: true, + value: { + clearTimeout: () => undefined, + setTimeout: () => 1, + electronAPI: { + setIgnoreMouseEvents: (ignore: boolean) => { + ignoreCalls.push({ ignore }); + }, + }, + }, + }); + + return () => { + if (originalDocument) { + Object.defineProperty(globalThis, 'document', originalDocument); + } else { + delete (globalThis as { document?: unknown }).document; + } + if (originalWindow) { + Object.defineProperty(globalThis, 'window', originalWindow); + } else { + delete (globalThis as { window?: unknown }).window; + } + }; +} + +test('passive overlay notification hover stays click-through on macOS passthrough overlays', () => { + const stack = createFakeElement(); + const ignoreCalls: Array<{ ignore: boolean }> = []; + const ctx = createHoverContext(stack, ignoreCalls); + const restore = installDomGlobals(ignoreCalls); + + try { + const renderer = createOverlayNotificationRenderer(ctx as never); + renderer.show({ + id: 'character-dictionary-auto-sync', + title: 'Character dictionary', + body: 'Building character dictionary...', + variant: 'progress', + persistent: true, + }); + + stack.dispatchEventType('mouseenter'); + + assert.equal(ctx.state.isOverOverlayNotification, false); + assert.deepEqual(ignoreCalls, []); + + const card = stack.children[0]; + const close = card ? findChildByClass(card, 'overlay-notification-close') : null; + if (!close) { + assert.fail('Expected overlay notification close button.'); + } + + close.dispatchEventType('mouseenter'); + assert.equal(ctx.state.isOverOverlayNotification, false); + assert.deepEqual(ignoreCalls, []); + } finally { + restore(); + } +}); + +test('overlay notification controls become interactive on hover', () => { + const stack = createFakeElement(); + const ignoreCalls: Array<{ ignore: boolean }> = []; + const ctx = createHoverContext(stack, ignoreCalls); + const restore = installDomGlobals(ignoreCalls); + + try { + const renderer = createOverlayNotificationRenderer(ctx as never); + renderer.show({ + id: 'mined-card', + title: 'Card created', + body: 'Added sentence card', + actions: [{ id: 'open-anki-card', label: 'Open in Anki', noteId: 42 }], + persistent: true, + }); + + const card = stack.children[0]; + const action = card ? findChildByClass(card, 'overlay-notification-action') : null; + if (!action) { + assert.fail('Expected overlay notification action.'); + } + + action.dispatchEventType('mouseenter'); + assert.equal(ctx.state.isOverOverlayNotification, true); + assert.deepEqual(ignoreCalls, [{ ignore: false }]); + + action.dispatchEventType('mouseleave'); + assert.equal(ctx.state.isOverOverlayNotification, false); + assert.deepEqual(ignoreCalls, [{ ignore: false }, { ignore: true }]); + } finally { + restore(); + } +}); + +test('action overlay notification stack hover keeps card controls interactive', () => { + const stack = createFakeElement(); + const ignoreCalls: Array<{ ignore: boolean }> = []; + const ctx = createHoverContext(stack, ignoreCalls); + const restore = installDomGlobals(ignoreCalls); + + try { + const renderer = createOverlayNotificationRenderer(ctx as never); + renderer.show({ + id: 'anki-card-updated', + title: 'Anki Card Updated', + body: 'Updated card: 食べる', + persistent: true, + actions: [{ id: 'open-anki-card', label: 'Open in Anki', noteId: 42 }], + }); + + stack.dispatchEventType('mouseenter'); + + assert.equal(ctx.state.isOverOverlayNotification, true); + assert.deepEqual(ignoreCalls, [{ ignore: false }]); + } finally { + restore(); + } +}); diff --git a/src/renderer/overlay-notifications.test.ts b/src/renderer/overlay-notifications.test.ts index f08602d5..c2ac00ee 100644 --- a/src/renderer/overlay-notifications.test.ts +++ b/src/renderer/overlay-notifications.test.ts @@ -39,6 +39,7 @@ type FakeElement = { dataset: Record; children: FakeElement[]; classList: ReturnType; + appendCalls: number; replaceChildrenCalls: number; append: (...children: FakeElement[]) => void; replaceChildren: (...children: FakeElement[]) => void; @@ -62,8 +63,10 @@ function createFakeElement(tagName = 'div'): FakeElement { dataset: {}, children: [], classList: createClassList(), + appendCalls: 0, replaceChildrenCalls: 0, append: (...children) => { + element.appendCalls += 1; for (const child of children) { const existingIndex = element.children.indexOf(child); if (existingIndex >= 0) { @@ -339,6 +342,8 @@ test('overlay notification renderer updates same-id progress without replacing t if (!card) { assert.fail('Expected overlay notification card.'); } + assert.equal(stack.appendCalls, 1); + assert.equal(card.classList.contains('entering'), true); const spinner = findChildByClass(card, 'overlay-notification-icon'); if (!spinner) { assert.fail('Expected overlay notification spinner.'); @@ -355,12 +360,16 @@ test('overlay notification renderer updates same-id progress without replacing t assert.equal(stack.children.length, 1); assert.equal(stack.children[0], card); + assert.equal(stack.appendCalls, 1); assert.equal(card.replaceChildrenCalls, cardReplacements); assert.equal(findChildByClass(card, 'overlay-notification-icon'), spinner); assert.equal( findChildByClass(card, 'overlay-notification-body')?.textContent, 'Subsync: syncing /', ); + + card.dispatchEventType('animationend', { animationName: 'overlay-notification-enter-right' }); + assert.equal(card.classList.contains('entering'), false); } finally { if (originalDocument) { Object.defineProperty(globalThis, 'document', originalDocument); diff --git a/src/renderer/overlay-notifications.ts b/src/renderer/overlay-notifications.ts index 43bce2f9..9d347e27 100644 --- a/src/renderer/overlay-notifications.ts +++ b/src/renderer/overlay-notifications.ts @@ -164,6 +164,10 @@ function isNotificationCardCloseButton(element: Element | undefined): boolean { return hasElementClass(element, 'overlay-notification-close'); } +function hasExplicitNotificationActions(entry: OverlayNotificationEntry): boolean { + return (entry.actions?.length ?? 0) > 0; +} + export function createOverlayNotificationRenderer( ctx: RendererContext, options: { onChanged?: () => void; onShow?: (entry: OverlayNotificationEntry) => void } = {}, @@ -217,6 +221,39 @@ export function createOverlayNotificationRenderer( ); } + function markEnterComplete(card: HTMLElement): void { + card.classList.remove('entering'); + } + + function watchEnterAnimation(card: HTMLElement): void { + if (typeof window === 'undefined') { + return; + } + const fallback = window.setTimeout(() => markEnterComplete(card), 320); + card.addEventListener( + 'animationend', + (event) => { + if ((event as AnimationEvent).animationName?.startsWith('overlay-notification-enter')) { + window.clearTimeout(fallback); + markEnterComplete(card); + } + }, + { once: true }, + ); + } + + function appendCardIfNeeded(card: HTMLElement): void { + if (Array.prototype.includes.call(ctx.dom.overlayNotificationStack.children, card)) { + return; + } + ctx.dom.overlayNotificationStack.append(card); + } + + function bindInteractiveControlHover(element: HTMLElement): void { + element.addEventListener('mouseenter', () => setInteractiveState(ctx, true)); + element.addEventListener('mouseleave', () => setInteractiveState(ctx, false)); + } + function remove(id: string): void { clearTimer(id); store.remove(id); @@ -251,6 +288,7 @@ export function createOverlayNotificationRenderer( button.type = 'button'; button.className = 'overlay-notification-action'; button.textContent = action.label; + bindInteractiveControlHover(button); button.addEventListener('click', () => { window.electronAPI.sendOverlayNotificationAction?.(entry.id, action.id, { noteId: action.noteId, @@ -312,6 +350,9 @@ export function createOverlayNotificationRenderer( closeButton.className = 'overlay-notification-close'; closeButton.setAttribute('aria-label', 'Dismiss notification'); closeButton.textContent = '×'; + if (hasExplicitNotificationActions(entry)) { + bindInteractiveControlHover(closeButton); + } closeButton.addEventListener('click', () => remove(entry.id)); card.replaceChildren(leadingEl, createContent(entry), closeButton); @@ -320,6 +361,7 @@ export function createOverlayNotificationRenderer( function render(): void { const visible = store.visible(); const visibleIds = new Set(visible.map((entry) => entry.id)); + const hasInteractiveCard = visible.some(hasExplicitNotificationActions); ctx.dom.overlayNotificationStack.classList.toggle( 'hidden', visible.length === 0 && leaving.size === 0, @@ -345,22 +387,25 @@ export function createOverlayNotificationRenderer( if (!card) { card = document.createElement('section'); card.classList.add('entering'); + watchEnterAnimation(card); cards.set(entry.id, card); } populateCard(card, entry); - // Appending an element already in the stack just moves it, keeping visible order - // without restarting its enter animation. - ctx.dom.overlayNotificationStack.append(card); + appendCardIfNeeded(card); } if (visible.length === 0 && leaving.size === 0) { setInteractiveState(ctx, false); + } else if (!hasInteractiveCard && ctx.state.isOverOverlayNotification) { + setInteractiveState(ctx, false); } options.onChanged?.(); } ctx.dom.overlayNotificationStack.addEventListener('mouseenter', () => { - setInteractiveState(ctx, true); + if (store.visible().some(hasExplicitNotificationActions)) { + setInteractiveState(ctx, true); + } }); ctx.dom.overlayNotificationStack.addEventListener('mouseleave', () => { setInteractiveState(ctx, false);