diff --git a/apps/readest-app/src/__tests__/helpers/shortcuts.test.ts b/apps/readest-app/src/__tests__/helpers/shortcuts.test.ts index f5d0cedc..090461f8 100644 --- a/apps/readest-app/src/__tests__/helpers/shortcuts.test.ts +++ b/apps/readest-app/src/__tests__/helpers/shortcuts.test.ts @@ -56,6 +56,14 @@ describe('TTS navigation shortcuts', () => { }); }); +describe('Proofread selection shortcut (#4717)', () => { + it('binds alt+p alongside ctrl+p/cmd+p so it avoids the print conflict', async () => { + const shortcuts = await getDefaults(); + expect(shortcuts.onProofreadSelection.keys).toContain('alt+p'); + expect(shortcuts.onProofreadSelection.keys).toEqual(['ctrl+p', 'cmd+p', 'alt+p']); + }); +}); + describe('No identical keybinding lists across actions (#3675)', () => { // Pre-existing pairs where two actions intentionally share the exact // same key list — both handlers guard on runtime context. diff --git a/apps/readest-app/src/__tests__/paragraph-mode.test.tsx b/apps/readest-app/src/__tests__/paragraph-mode.test.tsx index 66e83b78..d2801910 100644 --- a/apps/readest-app/src/__tests__/paragraph-mode.test.tsx +++ b/apps/readest-app/src/__tests__/paragraph-mode.test.tsx @@ -381,6 +381,24 @@ describe('paragraph mode', () => { expect(onClose).toHaveBeenCalledTimes(1); }); + + it('exits when the toggle paragraph mode shortcut (Shift+P) is pressed (#4717)', async () => { + const onClose = vi.fn(); + await renderVisibleOverlay(onClose); + + fireEvent.keyDown(document.body, { key: 'P', shiftKey: true }); + + expect(onClose).toHaveBeenCalledTimes(1); + }); + + it('does not exit on an unrelated key while visible', async () => { + const onClose = vi.fn(); + await renderVisibleOverlay(onClose); + + fireEvent.keyDown(document.body, { key: 'x' }); + + expect(onClose).not.toHaveBeenCalled(); + }); }); describe('paragraph mode TTS sync', () => { diff --git a/apps/readest-app/src/__tests__/utils/shortcut-keys.test.ts b/apps/readest-app/src/__tests__/utils/shortcut-keys.test.ts index 21bc1ed0..9e34ee4a 100644 --- a/apps/readest-app/src/__tests__/utils/shortcut-keys.test.ts +++ b/apps/readest-app/src/__tests__/utils/shortcut-keys.test.ts @@ -1,5 +1,22 @@ import { describe, it, expect } from 'vitest'; -import { formatKeyForDisplay, filterPlatformKeys } from '../../utils/shortcutKeys'; +import { formatKeyForDisplay, filterPlatformKeys, matchesShortcut } from '../../utils/shortcutKeys'; + +const evt = ( + key: string, + modifiers: Partial<{ + ctrlKey: boolean; + altKey: boolean; + metaKey: boolean; + shiftKey: boolean; + }> = {}, +) => ({ + key, + ctrlKey: false, + altKey: false, + metaKey: false, + shiftKey: false, + ...modifiers, +}); describe('formatKeyForDisplay', () => { describe('Mac platform', () => { @@ -127,3 +144,40 @@ describe('filterPlatformKeys', () => { ]); }); }); + +describe('matchesShortcut', () => { + it('matches a shift+letter shortcut against an uppercased key event', () => { + expect(matchesShortcut(evt('P', { shiftKey: true }), ['shift+p'])).toBe(true); + }); + + it('does not match when a required modifier is missing', () => { + expect(matchesShortcut(evt('p'), ['shift+p'])).toBe(false); + }); + + it('does not match when an extra modifier is pressed', () => { + expect(matchesShortcut(evt('P', { shiftKey: true, ctrlKey: true }), ['shift+p'])).toBe(false); + }); + + it('treats alt and opt as the same modifier', () => { + expect(matchesShortcut(evt('p', { altKey: true }), ['alt+p'])).toBe(true); + expect(matchesShortcut(evt('p', { altKey: true }), ['opt+p'])).toBe(true); + }); + + it('treats cmd and meta as the same modifier', () => { + expect(matchesShortcut(evt('p', { metaKey: true }), ['cmd+p'])).toBe(true); + expect(matchesShortcut(evt('p', { metaKey: true }), ['meta+p'])).toBe(true); + }); + + it('matches when any key in the list matches', () => { + expect(matchesShortcut(evt('p', { altKey: true }), ['ctrl+p', 'cmd+p', 'alt+p'])).toBe(true); + }); + + it('matches symbol and space base keys', () => { + expect(matchesShortcut(evt(']', { ctrlKey: true }), ['ctrl+]'])).toBe(true); + expect(matchesShortcut(evt(' ', { shiftKey: true }), ['shift+ '])).toBe(true); + }); + + it('returns false for an empty key list', () => { + expect(matchesShortcut(evt('p', { shiftKey: true }), [])).toBe(false); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/paragraph/ParagraphOverlay.tsx b/apps/readest-app/src/app/reader/components/paragraph/ParagraphOverlay.tsx index 2cfc9afa..a0acda4f 100644 --- a/apps/readest-app/src/app/reader/components/paragraph/ParagraphOverlay.tsx +++ b/apps/readest-app/src/app/reader/components/paragraph/ParagraphOverlay.tsx @@ -13,6 +13,8 @@ import { ParagraphPresentation, } from '@/utils/paragraphPresentation'; import { getTextSubRange } from '@/services/tts/wordHighlight'; +import { loadShortcuts } from '@/helpers/shortcuts'; +import { matchesShortcut } from '@/utils/shortcutKeys'; import TTSFollowIndicator, { TtsSyncStatus } from '../tts/TTSFollowIndicator'; import { buildTtsHighlightCssText } from './paragraphTts'; @@ -342,6 +344,16 @@ const ParagraphOverlay: React.FC = ({ return; } + // The overlay swallows every keydown in the capture phase, so the global + // toggle shortcut (Shift+P by default) never reaches useShortcuts. Honor + // it here so the same shortcut that enters paragraph mode also exits it + // (#4717). + if (matchesShortcut(e, loadShortcuts().onToggleParagraphMode.keys)) { + e.preventDefault(); + onCloseRef.current?.(); + return; + } + const action = getParagraphActionForKey(e.key, activePresentation ?? viewSettings); if (action === 'next') { e.preventDefault(); diff --git a/apps/readest-app/src/helpers/shortcuts.ts b/apps/readest-app/src/helpers/shortcuts.ts index 201baaf7..17c0c9c1 100644 --- a/apps/readest-app/src/helpers/shortcuts.ts +++ b/apps/readest-app/src/helpers/shortcuts.ts @@ -134,7 +134,9 @@ const DEFAULT_SHORTCUTS = { section: 'Selection', }, onProofreadSelection: { - keys: ['ctrl+p', 'cmd+p'], + // alt+p is a print-free alternative on Windows/Linux, where ctrl+p is + // intercepted by the browser's print dialog (#4717). + keys: ['ctrl+p', 'cmd+p', 'alt+p'], description: _('Proofread Selection'), section: 'Selection', }, diff --git a/apps/readest-app/src/hooks/useShortcuts.ts b/apps/readest-app/src/hooks/useShortcuts.ts index 69c0fd0a..929aa3a9 100644 --- a/apps/readest-app/src/hooks/useShortcuts.ts +++ b/apps/readest-app/src/hooks/useShortcuts.ts @@ -1,5 +1,6 @@ import { useEffect, useState } from 'react'; import { loadShortcuts, ShortcutConfig } from '../helpers/shortcuts'; +import { matchesShortcut, ShortcutEventLike } from '../utils/shortcutKeys'; export type KeyActionHandlers = { [K in keyof ShortcutConfig]?: (event?: KeyboardEvent | MessageEvent) => void; @@ -17,45 +18,9 @@ const useShortcuts = (actions: KeyActionHandlers, dependencies: React.Dependency return () => window.removeEventListener('shortcutUpdate', handleShortcutUpdate); }, []); - const parseShortcut = (shortcut: string) => { - const keys = shortcut.toLowerCase().split('+'); - return { - ctrlKey: keys.includes('ctrl'), - altKey: keys.includes('alt') || keys.includes('opt'), - metaKey: keys.includes('meta') || keys.includes('cmd'), - shiftKey: keys.includes('shift'), - key: keys.find((k) => !['ctrl', 'alt', 'opt', 'meta', 'cmd', 'shift'].includes(k)), - }; - }; - - const isShortcutMatch = ( - shortcut: string, - key: string, - ctrlKey: boolean, - altKey: boolean, - metaKey: boolean, - shiftKey: boolean, - ) => { - const parsedShortcut = parseShortcut(shortcut); - return ( - parsedShortcut.key === key.toLowerCase() && - parsedShortcut.ctrlKey === ctrlKey && - parsedShortcut.altKey === altKey && - parsedShortcut.metaKey === metaKey && - parsedShortcut.shiftKey === shiftKey - ); - }; - - const processKeyEvent = ( - key: string, - ctrlKey: boolean, - altKey: boolean, - metaKey: boolean, - shiftKey: boolean, - event: KeyboardEvent | MessageEvent, - ) => { + const processKeyEvent = (eventLike: ShortcutEventLike, event: KeyboardEvent | MessageEvent) => { // FIXME: This is a temporary fix to disable Back button navigation - if (key === 'backspace') return true; + if (eventLike.key.toLowerCase() === 'backspace') return true; for (const [actionName, actionHandler] of Object.entries(actions)) { const shortcutKey = actionName as keyof ShortcutConfig; const handler = actionHandler as @@ -63,12 +28,7 @@ const useShortcuts = (actions: KeyActionHandlers, dependencies: React.Dependency | undefined; const shortcutEntry = shortcuts[shortcutKey as keyof ShortcutConfig]; // console.log('Checking action:', shortcutKey); - if ( - handler && - shortcutEntry?.keys?.some((shortcut) => - isShortcutMatch(shortcut, key, ctrlKey, altKey, metaKey, shiftKey), - ) - ) { + if (handler && shortcutEntry?.keys && matchesShortcut(eventLike, shortcutEntry.keys)) { if (handler(event)) { return true; } @@ -103,7 +63,7 @@ const useShortcuts = (actions: KeyActionHandlers, dependencies: React.Dependency event.preventDefault(); } - const handled = processKeyEvent(key.toLowerCase(), ctrlKey, altKey, metaKey, shiftKey, event); + const handled = processKeyEvent({ key, ctrlKey, altKey, metaKey, shiftKey }, event); // console.log('Key event handled:', key, handled); if (handled) event.preventDefault(); } else if ( @@ -112,7 +72,7 @@ const useShortcuts = (actions: KeyActionHandlers, dependencies: React.Dependency event.data.type === 'iframe-keydown' ) { const { key, ctrlKey, altKey, metaKey, shiftKey } = event.data; - processKeyEvent(key.toLowerCase(), ctrlKey, altKey, metaKey, shiftKey, event); + processKeyEvent({ key, ctrlKey, altKey, metaKey, shiftKey }, event); } }; diff --git a/apps/readest-app/src/utils/shortcutKeys.ts b/apps/readest-app/src/utils/shortcutKeys.ts index 0d31c7b1..8336a06d 100644 --- a/apps/readest-app/src/utils/shortcutKeys.ts +++ b/apps/readest-app/src/utils/shortcutKeys.ts @@ -64,6 +64,38 @@ export const formatKeyForDisplay = (key: string, isMac: boolean): string => { return [...modifiers, displayKey].join('+'); }; +export type ShortcutEventLike = Pick< + KeyboardEvent, + 'key' | 'ctrlKey' | 'altKey' | 'metaKey' | 'shiftKey' +>; + +const parseShortcut = (shortcut: string) => { + const keys = shortcut.toLowerCase().split('+'); + return { + ctrlKey: keys.includes('ctrl'), + altKey: keys.includes('alt') || keys.includes('opt'), + metaKey: keys.includes('meta') || keys.includes('cmd'), + shiftKey: keys.includes('shift'), + key: keys.find((k) => !MODIFIERS.has(k)), + }; +}; + +// Whether a keyboard event matches any of the given shortcut strings. `alt`/`opt` +// and `cmd`/`meta` are treated as equivalent, matching how shortcuts are authored. +export const matchesShortcut = (event: ShortcutEventLike, keys: string[]): boolean => { + const key = event.key.toLowerCase(); + return keys.some((shortcut) => { + const parsed = parseShortcut(shortcut); + return ( + parsed.key === key && + parsed.ctrlKey === event.ctrlKey && + parsed.altKey === event.altKey && + parsed.metaKey === event.metaKey && + parsed.shiftKey === event.shiftKey + ); + }); +}; + const MAC_MODIFIERS = new Set(['cmd', 'opt']); const OTHER_MODIFIERS = new Set(['ctrl', 'alt']);