mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-06-09 15:13:32 -07:00
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
<!-- read_when: changing overlay notification hover, macOS mouse passthrough, or notification actions -->
|
||||
|
||||
# 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.
|
||||
@@ -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<string>();
|
||||
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<string, string>;
|
||||
children: FakeElement[];
|
||||
classList: ReturnType<typeof createClassList>;
|
||||
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<string, Array<(event?: unknown) => 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();
|
||||
}
|
||||
});
|
||||
@@ -39,6 +39,7 @@ type FakeElement = {
|
||||
dataset: Record<string, string>;
|
||||
children: FakeElement[];
|
||||
classList: ReturnType<typeof createClassList>;
|
||||
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);
|
||||
|
||||
@@ -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', () => {
|
||||
if (store.visible().some(hasExplicitNotificationActions)) {
|
||||
setInteractiveState(ctx, true);
|
||||
}
|
||||
});
|
||||
ctx.dom.overlayNotificationStack.addEventListener('mouseleave', () => {
|
||||
setInteractiveState(ctx, false);
|
||||
|
||||
Reference in New Issue
Block a user