diff --git a/apps/readest-app/src/__tests__/app/reader/utils/deferredAction.test.ts b/apps/readest-app/src/__tests__/app/reader/utils/deferredAction.test.ts new file mode 100644 index 00000000..4fb5751a --- /dev/null +++ b/apps/readest-app/src/__tests__/app/reader/utils/deferredAction.test.ts @@ -0,0 +1,94 @@ +import { describe, test, expect, vi } from 'vitest'; +import { + cancelDeferredAction, + createDeferredActionState, + flushDeferredAction, + runOrDeferAction, +} from '@/app/reader/utils/deferredAction'; + +describe('deferredAction', () => { + test('runs action immediately when shouldDefer is false', () => { + const state = createDeferredActionState(); + const action = vi.fn(); + + runOrDeferAction(state, false, action); + + expect(action).toHaveBeenCalledTimes(1); + expect(state.pending).toBeNull(); + }); + + test('stores action without running when shouldDefer is true', () => { + const state = createDeferredActionState(); + const action = vi.fn(); + + runOrDeferAction(state, true, action); + + expect(action).not.toHaveBeenCalled(); + expect(state.pending).toBe(action); + }); + + test('flushDeferredAction runs the latest deferred action exactly once', () => { + const state = createDeferredActionState(); + const action = vi.fn(); + + runOrDeferAction(state, true, action); + flushDeferredAction(state); + + expect(action).toHaveBeenCalledTimes(1); + expect(state.pending).toBeNull(); + + flushDeferredAction(state); + expect(action).toHaveBeenCalledTimes(1); + }); + + test('successive defers replace the pending action so flush runs only the last one', () => { + const state = createDeferredActionState(); + const first = vi.fn(); + const second = vi.fn(); + + runOrDeferAction(state, true, first); + runOrDeferAction(state, true, second); + flushDeferredAction(state); + + expect(first).not.toHaveBeenCalled(); + expect(second).toHaveBeenCalledTimes(1); + }); + + test('cancelDeferredAction discards a pending action without running it', () => { + const state = createDeferredActionState(); + const action = vi.fn(); + + runOrDeferAction(state, true, action); + cancelDeferredAction(state); + flushDeferredAction(state); + + expect(action).not.toHaveBeenCalled(); + }); + + test('Android long-press scenario: selection-then-touchend runs the action once', () => { + // Models the Annotator flow on Android: + // 1. touchstart -> androidTouchEnd = false (defer state cleared) + // 2. selectionchange -> handleQuickAction (deferred because !androidTouchEnd) + // 3. touchend -> androidTouchEnd = true, flushDeferredAction + const state = createDeferredActionState(); + const quickAction = vi.fn(); + let androidTouchEnd = false; + + // touchstart + cancelDeferredAction(state); + androidTouchEnd = false; + + // selectionchange triggers the quick action + runOrDeferAction(state, !androidTouchEnd, quickAction); + expect(quickAction).not.toHaveBeenCalled(); + + // touchend: gate opens, pending action fires + androidTouchEnd = true; + flushDeferredAction(state); + expect(quickAction).toHaveBeenCalledTimes(1); + + // A subsequent stray touchend must not re-run the action + flushDeferredAction(state); + expect(quickAction).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx b/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx index b2c937fe..234f3fbb 100644 --- a/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx +++ b/apps/readest-app/src/app/reader/components/annotator/Annotator.tsx @@ -26,6 +26,12 @@ import { getPopupPosition, getPosition, getTextFromRange } from '@/utils/sel'; import { eventDispatcher } from '@/utils/event'; import { findTocItemBS } from '@/services/nav'; import { throttle } from '@/utils/throttle'; +import { + cancelDeferredAction, + createDeferredActionState, + flushDeferredAction, + runOrDeferAction, +} from '../../utils/deferredAction'; import { runSimpleCC } from '@/utils/simplecc'; import { getWordCount } from '@/utils/word'; import { getIndexFromCfi, isCfiInLocation } from '@/utils/cfi'; @@ -95,6 +101,10 @@ const Annotator: React.FC<{ bookKey: string }> = ({ bookKey }) => { settings.globalReadSettings.highlightStyles[selectedStyle], ); const androidTouchEndRef = useRef(false); + // Holds a quick action that fired while the user is still touching the screen + // (Android long-press selects text via selectionchange before touchend). The + // pending action runs on touchend so popups don't open under an active touch. + const deferredQuickActionRef = useRef(createDeferredActionState()); const showingPopup = showAnnotPopup || @@ -256,11 +266,13 @@ const Annotator: React.FC<{ bookKey: string }> = ({ bookKey }) => { const ev = event.detail as NativeTouchEventType; if (ev.type === 'touchstart') { androidTouchEndRef.current = false; + cancelDeferredAction(deferredQuickActionRef.current); handleTouchStart(); } else if (ev.type === 'touchend') { androidTouchEndRef.current = true; handleTouchEnd(); handlePointerUp(doc, index); + flushDeferredAction(deferredQuickActionRef.current); } }; @@ -489,32 +501,41 @@ const Annotator: React.FC<{ bookKey: string }> = ({ bookKey }) => { const handleQuickAction = () => { const action = viewSettings.annotationQuickAction; - if (appService?.isAndroidApp && !androidTouchEndRef.current) return; - switch (action) { - case 'copy': - handleCopy(false); - handleDismissPopupAndSelection(); - break; - case 'highlight': - // highlight is already applied in instant annotating - handleDismissPopupAndSelection(); - break; - case 'search': - handleSearch(); - break; - case 'dictionary': - handleDictionary(); - break; - case 'wikipedia': - handleWikipedia(); - break; - case 'translate': - handleTranslation(); - break; - case 'tts': - handleSpeakText(true); - break; - } + const runAction = () => { + switch (action) { + case 'copy': + handleCopy(false); + handleDismissPopupAndSelection(); + break; + case 'highlight': + // highlight is already applied in instant annotating + handleDismissPopupAndSelection(); + break; + case 'search': + handleSearch(); + break; + case 'dictionary': + handleDictionary(); + break; + case 'wikipedia': + handleWikipedia(); + break; + case 'translate': + handleTranslation(); + break; + case 'tts': + handleSpeakText(true); + break; + } + }; + // On Android, a long-press fires selectionchange (and this handler) while + // the finger is still down. Defer until touchend so popups aren't dismissed + // by the in-progress touch (closes #3935). + runOrDeferAction( + deferredQuickActionRef.current, + !!appService?.isAndroidApp && !androidTouchEndRef.current, + runAction, + ); }; useEffect(() => { diff --git a/apps/readest-app/src/app/reader/utils/deferredAction.ts b/apps/readest-app/src/app/reader/utils/deferredAction.ts new file mode 100644 index 00000000..4a02f4c1 --- /dev/null +++ b/apps/readest-app/src/app/reader/utils/deferredAction.ts @@ -0,0 +1,35 @@ +// Tiny helper for actions that need to wait for an asynchronous "ready" signal +// before running. Used by the Annotator on Android: a long-press selects text +// while the finger is still down, but the quick action (translate/dictionary/ +// tts/etc.) must not fire until the user lifts (touchend), or the popup it +// opens would be dismissed by the in-progress touch. + +export interface DeferredActionState { + pending: (() => void) | null; +} + +export const createDeferredActionState = (): DeferredActionState => ({ pending: null }); + +export const runOrDeferAction = ( + state: DeferredActionState, + shouldDefer: boolean, + action: () => void, +): void => { + if (shouldDefer) { + state.pending = action; + return; + } + state.pending = null; + action(); +}; + +export const flushDeferredAction = (state: DeferredActionState): void => { + const fn = state.pending; + if (!fn) return; + state.pending = null; + fn(); +}; + +export const cancelDeferredAction = (state: DeferredActionState): void => { + state.pending = null; +};