diff --git a/.gitignore b/.gitignore index 2ce6029b..475a2b5d 100644 --- a/.gitignore +++ b/.gitignore @@ -8,7 +8,6 @@ # testing /coverage -.playwright-mcp # next.js /.next/ @@ -46,3 +45,4 @@ fastlane/report.xml # nix result* +.playwright-mcp/ diff --git a/apps/readest-app/src/app/reader/components/ViewMenu.tsx b/apps/readest-app/src/app/reader/components/ViewMenu.tsx index 88d994af..cb0edf39 100644 --- a/apps/readest-app/src/app/reader/components/ViewMenu.tsx +++ b/apps/readest-app/src/app/reader/components/ViewMenu.tsx @@ -285,14 +285,11 @@ const ViewMenu: React.FC = ({ bookKey, setIsDropdownOpen }) => { disabled={bookData.isFixedLayout} /> - {/* Temporarily disabled for production, will be ready after the merge of PR #3150 */} - {process.env.NODE_ENV !== 'production' && ( - - )} + diff --git a/apps/readest-app/src/app/reader/components/rsvp/RSVPControl.tsx b/apps/readest-app/src/app/reader/components/rsvp/RSVPControl.tsx index 58c8e92b..a7b503c7 100644 --- a/apps/readest-app/src/app/reader/components/rsvp/RSVPControl.tsx +++ b/apps/readest-app/src/app/reader/components/rsvp/RSVPControl.tsx @@ -133,7 +133,9 @@ const RSVPControl: React.FC = ({ bookKey }) => { useEffect(() => { return () => { if (controllerRef.current) { - controllerRef.current.shutdown(); + // Use stop() instead of shutdown() to preserve saved position across sessions + // shutdown() clears localStorage which loses the user's reading progress + controllerRef.current.stop(); controllerRef.current = null; } // Remove any existing RSVP highlight when component unmounts @@ -209,13 +211,13 @@ const RSVPControl: React.FC = ({ bookKey }) => { const choice = (e as CustomEvent).detail; setStartChoice(choice); - // If there's only one option (beginning), start directly - if (!choice.hasSavedPosition && !choice.hasSelection) { - controller.startFromBeginning(); - setIsActive(true); - } else { - // Show dialog for user to choose + // If there's a saved position or selection, show dialog for user to choose + if (choice.hasSavedPosition || choice.hasSelection) { setShowStartDialog(true); + } else { + // No saved position or selection - start from current page position + controller.startFromCurrentPosition(); + setIsActive(true); } }; @@ -234,27 +236,74 @@ const RSVPControl: React.FC = ({ bookKey }) => { (option: 'beginning' | 'saved' | 'current' | 'selection') => { setShowStartDialog(false); const controller = controllerRef.current; + const view = getView(bookKey); if (!controller) return; + // Handler for when we need to navigate to a different section for resume + const handleNavigateToResume = (e: Event) => { + const { cfi } = (e as CustomEvent<{ cfi: string }>).detail; + controller.removeEventListener('rsvp-navigate-to-resume', handleNavigateToResume); + + if (view && cfi) { + // Navigate to the saved position's section + view.goTo(cfi); + + // Wait for navigation, then reload and start RSVP + setTimeout(() => { + const progress = getProgress(bookKey); + if (progress?.location) { + controller.setCurrentCfi(progress.location); + } + controller.loadNextPageContent(); + // Small delay to ensure content is loaded + setTimeout(() => { + controller.start(); + setIsActive(true); + }, 100); + }, 500); + } + }; + switch (option) { case 'beginning': controller.startFromBeginning(); + setIsActive(true); break; case 'saved': + // Listen for navigation event in case saved position is in different section + controller.addEventListener('rsvp-navigate-to-resume', handleNavigateToResume); controller.startFromSavedPosition(); + // If startFromSavedPosition started directly (same section), setIsActive + // If it emitted navigate event, the handler above will setIsActive after navigation + if (!controller.currentState.active) { + // Navigation event was emitted, don't set active yet + } else { + setIsActive(true); + } + // Clean up listener after a timeout if not used + setTimeout(() => { + controller.removeEventListener('rsvp-navigate-to-resume', handleNavigateToResume); + }, 1000); break; - case 'current': + case 'current': { + // Refresh the CFI in case user scrolled since dialog opened + const currentProgress = getProgress(bookKey); + if (currentProgress?.location) { + controller.setCurrentCfi(currentProgress.location); + } controller.startFromCurrentPosition(); + setIsActive(true); break; + } case 'selection': if (startChoice?.selectionText) { controller.startFromSelection(startChoice.selectionText); } + setIsActive(true); break; } - setIsActive(true); }, - [startChoice], + [bookKey, getProgress, getView, startChoice], ); const handleClose = useCallback(() => { @@ -266,49 +315,54 @@ const RSVPControl: React.FC = ({ bookKey }) => { const handleRsvpStop = (e: Event) => { const stopPosition = (e as CustomEvent).detail; - if (stopPosition?.range && typeof stopPosition.docIndex === 'number') { + if (stopPosition && stopPosition.cfi) { try { - // Get the document from the renderer - const contents = view.renderer.getContents?.(); - const content = contents?.find((c) => c.index === stopPosition.docIndex); - const doc = content?.doc; + // Navigate to the word's CFI position + view.goTo(stopPosition.cfi); - if (doc && stopPosition.range) { - // Expand the range to include the full sentence - const sentenceRange = expandRangeToSentence(stopPosition.range, doc); + // Try to create a sentence highlight using the stored Range + if (typeof stopPosition.docIndex === 'number' && stopPosition.range) { + // Check if the original range is still valid + let rangeIsValid = false; + try { + const rangeText = stopPosition.range.toString(); + rangeIsValid = rangeText === stopPosition.text; + } catch { + rangeIsValid = false; + } - // Get CFI for navigation - MUST get this BEFORE navigating - const cfi = view.getCFI(stopPosition.docIndex, stopPosition.range); + if (rangeIsValid) { + // Get the document from the renderer + const contents = view.renderer.getContents?.(); + const content = contents?.find((c) => c.index === stopPosition.docIndex); + const doc = content?.doc; - // Get CFI for the sentence highlight - MUST get this BEFORE navigating - // because goTo() may re-render the document, invalidating the Range objects - const sentenceCfi = cfi ? view.getCFI(stopPosition.docIndex, sentenceRange) : null; - const sentenceText = sentenceRange.toString(); + if (doc) { + // Expand the range to include the full sentence + const sentenceRange = expandRangeToSentence(stopPosition.range, doc); + const sentenceCfi = view.getCFI(stopPosition.docIndex, sentenceRange); + const sentenceText = sentenceRange.toString(); - if (cfi) { - // Navigate to the position - view.goTo(cfi); + if (sentenceCfi) { + // Remove any previous RSVP highlight + removeRsvpHighlight(); - if (sentenceCfi) { - // Remove any previous RSVP highlight - removeRsvpHighlight(); + // Create a persistent highlight for the sentence + const highlight: BookNote = { + id: `rsvp-temp-${Date.now()}`, + type: 'annotation', + cfi: sentenceCfi, + text: sentenceText, + style: 'underline', + color: themeCode.primary, + note: '', + createdAt: Date.now(), + updatedAt: Date.now(), + }; - // Create a persistent highlight for the sentence using theme accent color - const highlight: BookNote = { - id: `rsvp-temp-${Date.now()}`, - type: 'annotation', - cfi: sentenceCfi, - text: sentenceText, - style: 'underline', - color: themeCode.primary, // Use theme accent color (same as ORP focal point) - note: '', - createdAt: Date.now(), - updatedAt: Date.now(), - }; - - tempHighlightRef.current = highlight; - view.addAnnotation(highlight); - // Note: highlight persists until next page, reader close, or new RSVP session + tempHighlightRef.current = highlight; + view.addAnnotation(highlight); + } } } } diff --git a/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx b/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx index e625a7bc..17d5dc4f 100644 --- a/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx +++ b/apps/readest-app/src/app/reader/components/rsvp/RSVPOverlay.tsx @@ -93,7 +93,7 @@ const RSVPOverlay: React.FC = ({ }; }, [controller, onRequestNextPage]); - // Keyboard shortcuts + // Keyboard shortcuts - use capture phase to intercept before native elements useEffect(() => { const handleKeyboard = (event: KeyboardEvent) => { if (!state.active) return; @@ -101,14 +101,17 @@ const RSVPOverlay: React.FC = ({ switch (event.key) { case ' ': event.preventDefault(); + event.stopPropagation(); controller.togglePlayPause(); break; case 'Escape': event.preventDefault(); + event.stopPropagation(); onClose(); break; case 'ArrowLeft': event.preventDefault(); + event.stopPropagation(); if (event.shiftKey) { controller.skipBackward(15); } else { @@ -117,6 +120,7 @@ const RSVPOverlay: React.FC = ({ break; case 'ArrowRight': event.preventDefault(); + event.stopPropagation(); if (event.shiftKey) { controller.skipForward(15); } else { @@ -125,17 +129,20 @@ const RSVPOverlay: React.FC = ({ break; case 'ArrowUp': event.preventDefault(); + event.stopPropagation(); controller.increaseSpeed(); break; case 'ArrowDown': event.preventDefault(); + event.stopPropagation(); controller.decreaseSpeed(); break; } }; - document.addEventListener('keydown', handleKeyboard); - return () => document.removeEventListener('keydown', handleKeyboard); + // Use capture phase to handle events before they reach dropdown/select elements + document.addEventListener('keydown', handleKeyboard, { capture: true }); + return () => document.removeEventListener('keydown', handleKeyboard, { capture: true }); }, [state.active, controller, onClose]); // Word display helpers @@ -184,14 +191,14 @@ const RSVPOverlay: React.FC = ({ // Chapter helpers const getCurrentChapterLabel = useCallback((): string => { - if (!currentChapterHref) return 'Select Chapter'; + if (!currentChapterHref) return _('Select Chapter'); const normalizedCurrent = currentChapterHref.split('#')[0]?.replace(/^\//, '') || ''; const chapter = flatChapters.find((c) => { const normalizedHref = c.href.split('#')[0]?.replace(/^\//, '') || ''; return normalizedHref === normalizedCurrent; }); - return chapter?.label || 'Select Chapter'; - }, [currentChapterHref, flatChapters]); + return chapter?.label || _('Select Chapter'); + }, [_, currentChapterHref, flatChapters]); const isChapterActive = useCallback( (href: string): boolean => { @@ -283,7 +290,7 @@ const RSVPOverlay: React.FC = ({ return (
= ({ className='relative h-2 cursor-pointer overflow-visible rounded bg-gray-500/30' onClick={handleProgressBarClick} onKeyDown={(e) => { + e.preventDefault(); + e.stopPropagation(); if (e.key === 'ArrowLeft') controller.skipBackward(); else if (e.key === 'ArrowRight') controller.skipForward(); }} diff --git a/apps/readest-app/src/services/rsvp/RSVPController.ts b/apps/readest-app/src/services/rsvp/RSVPController.ts index 1400d4a2..a6c9c0fb 100644 --- a/apps/readest-app/src/services/rsvp/RSVPController.ts +++ b/apps/readest-app/src/services/rsvp/RSVPController.ts @@ -1,6 +1,8 @@ import { FoliateView } from '@/types/view'; import { RsvpWord, RsvpState, RsvpPosition, RsvpStopPosition, RsvpStartChoice } from './types'; import { containsCJK, splitTextIntoWords } from './utils'; +import { compare as compareCFI } from 'foliate-js/epubcfi.js'; +import { XCFI } from '@/utils/xcfi'; const DEFAULT_WPM = 300; const MIN_WPM = 100; @@ -15,6 +17,7 @@ const POSITION_KEY_PREFIX = 'readest_rsvp_pos_'; export class RSVPController extends EventTarget { private view: FoliateView; private bookKey: string; + private bookId: string; // Book hash without session suffix, for persistent storage private currentCfi: string | null = null; private state: RsvpState = { @@ -37,6 +40,9 @@ export class RSVPController extends EventTarget { super(); this.view = view; this.bookKey = bookKey; + // Extract book ID (hash) from bookKey format: "{hash}-{sessionId}" + // Use only the hash for persistent position storage across sessions + this.bookId = bookKey.split('-')[0] || bookKey; this.loadSettings(); } @@ -120,7 +126,8 @@ export class RSVPController extends EventTarget { } private loadPositionFromStorage(): RsvpPosition | null { - const stored = localStorage.getItem(`${POSITION_KEY_PREFIX}${this.bookKey}`); + // Use bookId (without session suffix) for persistent position across sessions + const stored = localStorage.getItem(`${POSITION_KEY_PREFIX}${this.bookId}`); if (stored) { try { return JSON.parse(stored) as RsvpPosition; @@ -132,37 +139,71 @@ export class RSVPController extends EventTarget { } private savePositionToStorage(): void { - if (!this.currentCfi) return; if (this.state.words.length === 0) return; + const currentWord = this.state.words[this.state.currentIndex]; + if (!currentWord) return; + + // Use the word's CFI if available, otherwise fall back to section CFI + const cfi = currentWord.cfi || this.currentCfi; + if (!cfi) return; + const position: RsvpPosition = { - cfi: this.currentCfi, - wordIndex: this.state.currentIndex, - wordText: this.state.words[this.state.currentIndex]?.text || '', + cfi: cfi, + wordText: currentWord.text, }; - localStorage.setItem(`${POSITION_KEY_PREFIX}${this.bookKey}`, JSON.stringify(position)); + // Use bookId (without session suffix) for persistent position across sessions + localStorage.setItem(`${POSITION_KEY_PREFIX}${this.bookId}`, JSON.stringify(position)); } private clearPositionFromStorage(): void { - localStorage.removeItem(`${POSITION_KEY_PREFIX}${this.bookKey}`); + // Use bookId (without session suffix) for persistent position across sessions + localStorage.removeItem(`${POSITION_KEY_PREFIX}${this.bookId}`); } - private extractBaseCfi(cfi: string): string { - const inner = cfi.replace(/^epubcfi\(/, '').replace(/\)$/, ''); - const parts = inner.split(','); - let basePath = parts[0]!; - const match = basePath.match(/^(.*\][^\/]*)/); - if (match) { - basePath = match[1]!; + private getSpineIndex(cfi: string): number { + try { + return XCFI.extractSpineIndex(cfi); + } catch { + return -1; } - return basePath; } private isSameSection(cfi1: string | null, cfi2: string | null): boolean { if (!cfi1 || !cfi2) return false; - const base1 = this.extractBaseCfi(cfi1); - const base2 = this.extractBaseCfi(cfi2); - return base1 === base2; + const spine1 = this.getSpineIndex(cfi1); + const spine2 = this.getSpineIndex(cfi2); + return spine1 >= 0 && spine1 === spine2; + } + + private findWordIndexByCfi(words: RsvpWord[], targetCfi: string): number { + // First try exact CFI match + for (let i = 0; i < words.length; i++) { + const word = words[i]; + if (word?.cfi === targetCfi) { + return i; + } + } + + // Check if target is in same section as any word + const targetSpineIndex = this.getSpineIndex(targetCfi); + if (targetSpineIndex < 0) return -1; + + // Find the first word at or after the target position using CFI compare + for (let i = 0; i < words.length; i++) { + const word = words[i]; + if (!word?.cfi) continue; + + // Must be in the same section + if (this.getSpineIndex(word.cfi) !== targetSpineIndex) continue; + + // Use compareCFI to find first word at or after target + if (compareCFI(word.cfi, targetCfi) >= 0) { + return i; + } + } + + return -1; } start(retryCount = 0): void { @@ -183,11 +224,27 @@ export class RSVPController extends EventTarget { this.pendingStartWordIndex = null; } else { const savedPosition = this.loadPositionFromStorage(); - if (savedPosition && this.isSameSection(savedPosition.cfi, this.currentCfi)) { - if (savedPosition.wordIndex < words.length) { - if (words[savedPosition.wordIndex]?.text === savedPosition.wordText) { - startIndex = savedPosition.wordIndex; - resumedFromIndex = savedPosition.wordIndex; + if (savedPosition) { + // Try CFI-based position recovery first + if (savedPosition.cfi) { + const cfiIndex = this.findWordIndexByCfi(words, savedPosition.cfi); + if (cfiIndex >= 0) { + startIndex = cfiIndex; + resumedFromIndex = cfiIndex; + } else { + // CFI not found, try text match as fallback + const textMatchIndex = words.findIndex((w) => w.text === savedPosition.wordText); + if (textMatchIndex >= 0) { + startIndex = textMatchIndex; + resumedFromIndex = textMatchIndex; + } + } + } else { + // Legacy position without CFI - try text match + const textMatchIndex = words.findIndex((w) => w.text === savedPosition.wordText); + if (textMatchIndex >= 0) { + startIndex = textMatchIndex; + resumedFromIndex = textMatchIndex; } } } @@ -265,16 +322,19 @@ export class RSVPController extends EventTarget { stop(): void { this.savePositionToStorage(); - const stopPosition: RsvpStopPosition | null = - this.state.words.length > 0 - ? { - wordIndex: this.state.currentIndex, - totalWords: this.state.words.length, - text: this.state.words[this.state.currentIndex]?.text || '', - range: this.state.words[this.state.currentIndex]?.range, - docIndex: this.state.words[this.state.currentIndex]?.docIndex, - } - : null; + let stopPosition: RsvpStopPosition | null = null; + if (this.state.words.length > 0) { + const currentWord = this.state.words[this.state.currentIndex]; + + stopPosition = { + wordIndex: this.state.currentIndex, + totalWords: this.state.words.length, + text: currentWord?.text || '', + range: currentWord?.range, + docIndex: currentWord?.docIndex, + cfi: currentWord?.cfi, + }; + } this.dispatchEvent(new CustomEvent('rsvp-stop', { detail: stopPosition })); @@ -293,20 +353,17 @@ export class RSVPController extends EventTarget { } requestStart(selectionText?: string): void { - const words = this.extractWordsWithRanges(); - const firstVisibleWordIndex = this.findFirstVisibleWordIndex(words); - const savedPosition = this.loadPositionFromStorage(); - const hasSavedPosition = !!( - savedPosition && this.isSameSection(savedPosition.cfi, this.currentCfi) - ); + // Show Resume option if we have a saved position with a valid CFI + // We don't require it to be in the same section - user may want to resume + // from where they left off even if they've navigated elsewhere + const hasSavedPosition = !!savedPosition?.cfi; const hasSelection = !!selectionText && selectionText.trim().length > 0; const startChoice: RsvpStartChoice = { hasSavedPosition, hasSelection, selectionText: selectionText?.trim(), - firstVisibleWordIndex, }; this.dispatchEvent(new CustomEvent('rsvp-start-choice', { detail: startChoice })); @@ -319,6 +376,25 @@ export class RSVPController extends EventTarget { } startFromSavedPosition(): void { + const savedPosition = this.loadPositionFromStorage(); + if (!savedPosition?.cfi) { + // No saved position, start from beginning + this.start(); + return; + } + + // Check if saved position is in a different section + if (!this.isSameSection(savedPosition.cfi, this.currentCfi)) { + // Need to navigate to the saved section first + // Emit event for React component to handle navigation + this.dispatchEvent( + new CustomEvent('rsvp-navigate-to-resume', { + detail: { cfi: savedPosition.cfi }, + }), + ); + return; + } + this.pendingStartWordIndex = null; this.start(); } @@ -326,8 +402,17 @@ export class RSVPController extends EventTarget { startFromCurrentPosition(): void { this.clearPositionFromStorage(); const words = this.extractWordsWithRanges(); - const firstVisibleIndex = this.findFirstVisibleWordIndex(words); - this.pendingStartWordIndex = firstVisibleIndex > 0 ? firstVisibleIndex : null; + + // Use CFI-based matching to find the first word at current page position + let startIndex = 0; + if (this.currentCfi) { + const cfiIndex = this.findWordIndexByCfi(words, this.currentCfi); + if (cfiIndex >= 0) { + startIndex = cfiIndex; + } + } + + this.pendingStartWordIndex = startIndex > 0 ? startIndex : null; this.start(); } @@ -339,39 +424,6 @@ export class RSVPController extends EventTarget { this.start(); } - private findFirstVisibleWordIndex(words: RsvpWord[]): number { - if (words.length === 0) return 0; - - try { - const renderer = this.view.renderer; - const scrollStart = renderer.start ?? 0; - const pageSize = renderer.size ?? 0; - - if (pageSize > 0) { - const visibleStart = scrollStart - pageSize; - const visibleEnd = scrollStart; - - for (let i = 0; i < words.length; i++) { - const word = words[i]; - if (word?.range) { - try { - const rect = word.range.getBoundingClientRect(); - if (rect.width > 0 && rect.left >= visibleStart && rect.left < visibleEnd) { - return i; - } - } catch { - continue; - } - } - } - } - } catch { - // Fall through to return 0 - } - - return 0; - } - private findWordIndexBySelection(words: RsvpWord[], selectionText: string): number { if (!selectionText || words.length === 0) return -1; @@ -588,7 +640,6 @@ export class RSVPController extends EventTarget { if (node.nodeType === Node.TEXT_NODE) { const text = node.textContent || ''; const nodeWords = splitTextIntoWords(text); - console.log('Extracted words from text node:', nodeWords); let offset = 0; for (const word of nodeWords) { @@ -600,12 +651,22 @@ export class RSVPController extends EventTarget { range.setStart(node, wordStart); range.setEnd(node, wordStart + word.length); + // Generate CFI for this word for position tracking + let cfi: string | undefined; + try { + cfi = this.view.getCFI(docIndex, range); + } catch { + // CFI generation failed, will fall back to word index + cfi = undefined; + } + words.push({ text: word, orpIndex: this.calculateORP(word), pauseMultiplier: this.getPauseMultiplier(word), range, docIndex, + cfi, }); } catch { words.push({ diff --git a/apps/readest-app/src/services/rsvp/types.ts b/apps/readest-app/src/services/rsvp/types.ts index 7d0a1efd..c5ef30b4 100644 --- a/apps/readest-app/src/services/rsvp/types.ts +++ b/apps/readest-app/src/services/rsvp/types.ts @@ -4,6 +4,7 @@ export interface RsvpWord { pauseMultiplier: number; range?: Range; docIndex?: number; + cfi?: string; // Canonical Fragment Identifier for precise position tracking } export interface RsvpState { @@ -19,7 +20,6 @@ export interface RsvpState { export interface RsvpPosition { cfi: string; - wordIndex: number; wordText: string; } @@ -29,11 +29,11 @@ export interface RsvpStopPosition { text: string; range?: Range; docIndex?: number; + cfi?: string; // Canonical Fragment Identifier for the stop position } export interface RsvpStartChoice { hasSavedPosition: boolean; hasSelection: boolean; selectionText?: string; - firstVisibleWordIndex: number; }