fix(rsvp): resume at stop word, prevent section replay, restore full context (#3960)

This commit is contained in:
Lex Moulton
2026-04-27 21:02:40 -04:00
committed by GitHub
parent 6fcda66b60
commit 6d5e59c79a
7 changed files with 302 additions and 78 deletions
@@ -0,0 +1,203 @@
'use client';
import { render, act, cleanup } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
import RSVPControl from '@/app/reader/components/rsvp/RSVPControl';
import { eventDispatcher } from '@/utils/event';
// ---------- Shared mutable test state ----------
// These are captured by closure in the mock factories below.
// Each beforeEach resets them.
let primaryIndex = 4;
const viewRelocateListeners: EventListener[] = [];
const loadedSections: number[] = [];
const controllerEventListeners = new Map<string, EventListener[]>();
let capturedOnRequestNextPage: (() => Promise<void>) | null = null;
// ---------- Mocks ----------
vi.mock('@/app/reader/components/rsvp/RSVPOverlay', () => ({
default: vi.fn(({ onRequestNextPage }: { onRequestNextPage: () => Promise<void> }) => {
capturedOnRequestNextPage = onRequestNextPage;
return null;
}),
}));
vi.mock('@/app/reader/components/rsvp/RSVPStartDialog', () => ({
default: () => null,
}));
vi.mock('@/hooks/useTranslation', () => ({
useTranslation: () => (s: string) => s,
}));
vi.mock('@/context/EnvContext', () => ({
useEnv: () => ({ envConfig: {} }),
}));
vi.mock('@/store/readerStore', () => ({
useReaderStore: () => ({
getView: () => mockView,
getProgress: () => ({
location: 'epubcfi(/6/8!/4/1:0)',
sectionHref: `ch${primaryIndex}.xhtml`,
}),
}),
}));
vi.mock('@/store/bookDataStore', () => ({
useBookDataStore: () => ({
getBookData: () => ({ book: { format: 'EPUB' }, bookDoc: { toc: [] } }),
getConfig: () => null,
setConfig: vi.fn(),
saveConfig: vi.fn(),
}),
}));
vi.mock('@/store/settingsStore', () => ({
useSettingsStore: () => ({ settings: {} }),
}));
vi.mock('@/store/themeStore', () => ({
useThemeStore: () => ({ themeCode: { primary: '#000' } }),
}));
// RSVPController mock: fires rsvp-start-choice immediately from requestStart(),
// and tracks loadNextPageContent calls via the shared loadedSections array.
// Must use a regular function (not arrow function) so it can be called with `new`.
vi.mock('@/services/rsvp', () => ({
// eslint-disable-next-line prefer-arrow-callback
RSVPController: vi.fn(function RSVPControllerMock() {
return {
seedPosition: vi.fn(),
setCurrentCfi: vi.fn(),
requestStart: vi.fn(() => {
const event = new CustomEvent('rsvp-start-choice', {
detail: { hasSavedPosition: false, hasSelection: false },
});
(controllerEventListeners.get('rsvp-start-choice') ?? []).forEach((h) => h(event));
}),
startFromCurrentPosition: vi.fn(),
stop: vi.fn(),
loadNextPageContent: vi.fn(() => {
loadedSections.push(primaryIndex);
}),
getStoredPosition: vi.fn(() => null),
get currentState() {
return { active: true };
},
addEventListener: vi.fn((type: string, listener: EventListener) => {
if (!controllerEventListeners.has(type)) controllerEventListeners.set(type, []);
controllerEventListeners.get(type)!.push(listener);
}),
removeEventListener: vi.fn((type: string, listener: EventListener) => {
const arr = controllerEventListeners.get(type) ?? [];
controllerEventListeners.set(
type,
arr.filter((l) => l !== listener),
);
}),
};
}),
buildRsvpExitConfigUpdate: vi.fn(() => ({})),
}));
// ---------- Mock FoliateView ----------
const mockView = {
renderer: {
get primaryIndex() {
return primaryIndex;
},
get atEnd() {
return false;
},
// renderer.goTo is called directly with the exact target section index,
// bypassing the reverted primaryIndex that nextSection() would use.
goTo: vi.fn(async ({ index }: { index: number }) => {
const prevIdx = primaryIndex;
// #goTo sets #primaryIndex = index before the navigation relocate fires
primaryIndex = index;
const navEvent = new CustomEvent('relocate', {
detail: {
section: { current: index },
tocItem: { href: `ch${index}.xhtml` },
},
});
[...viewRelocateListeners].forEach((l) => l(navEvent));
// After navigation, a scroll event fires and #detectPrimaryView()
// reverts #primaryIndex to the old section (same revert behaviour
// as before, but now we navigated to the correct section in the first place)
primaryIndex = prevIdx;
}),
getContents: vi.fn(() => []),
},
addEventListener: vi.fn((type: string, listener: EventListener) => {
if (type === 'relocate') viewRelocateListeners.push(listener);
}),
removeEventListener: vi.fn((type: string, listener: EventListener) => {
if (type === 'relocate') {
const idx = viewRelocateListeners.indexOf(listener);
if (idx >= 0) viewRelocateListeners.splice(idx, 1);
}
}),
book: { format: 'EPUB' },
getCFI: vi.fn(() => null),
addAnnotation: vi.fn(),
resolveCFI: vi.fn(),
};
// ---------- Tests ----------
describe('RSVPControl — section advance tracking', () => {
beforeEach(() => {
primaryIndex = 4;
loadedSections.length = 0;
viewRelocateListeners.length = 0;
controllerEventListeners.clear();
capturedOnRequestNextPage = null;
});
afterEach(() => {
cleanup();
});
test('advances directly to rsvpSectionRef+1, loading section 6 on second advance', async () => {
render(
<RSVPControl bookKey='test-book' gridInsets={{ top: 0, bottom: 0, left: 0, right: 0 }} />,
);
// Start RSVP — handleStart auto-starts without showing a dialog because
// the mock controller fires rsvp-start-choice with no saved position
await act(async () => {
eventDispatcher.dispatch('rsvp-start', { bookKey: 'test-book' });
await new Promise<void>((r) => setTimeout(r, 20));
});
// RSVPOverlay should now be mounted and onRequestNextPage captured
expect(capturedOnRequestNextPage).not.toBeNull();
// First section advance: rsvpSectionRef starts at 4 → goTo(5)
await act(async () => {
await capturedOnRequestNextPage!();
});
expect(loadedSections[0]).toBe(5); // section 5 correctly loaded on first advance
expect(primaryIndex).toBe(4); // confirms primaryIndex has reverted after navigation
expect(mockView.renderer.goTo).toHaveBeenNthCalledWith(1, { index: 5 });
// Second advance: rsvpSectionRef is now 5 → goTo(6).
// Without the fix, nextSection() used the reverted primaryIndex=4 and
// navigated to section 5 again, causing a stale-section freeze.
await act(async () => {
await capturedOnRequestNextPage!();
});
expect(loadedSections).toEqual([5, 6]);
expect(mockView.renderer.goTo).toHaveBeenNthCalledWith(2, { index: 6 });
});
});
@@ -0,0 +1,25 @@
import { describe, expect, test } from 'vitest';
import { buildRsvpExitConfigUpdate } from '@/services/rsvp/persistence';
describe('buildRsvpExitConfigUpdate', () => {
test('pins location to the RSVP word CFI when present', () => {
const rsvpPosition = {
cfi: 'epubcfi(/6/8!/4/2/1:42)',
wordText: 'somewhere',
};
const update = buildRsvpExitConfigUpdate(rsvpPosition);
expect(update.rsvpPosition).toEqual(rsvpPosition);
expect(update.location).toBe('epubcfi(/6/8!/4/2/1:42)');
});
test('omits location when the RSVP word has no CFI', () => {
const rsvpPosition = { cfi: '', wordText: 'no-cfi' };
const update = buildRsvpExitConfigUpdate(rsvpPosition);
expect(update.rsvpPosition).toEqual(rsvpPosition);
expect('location' in update).toBe(false);
});
});
@@ -6,11 +6,17 @@ import { useReaderStore } from '@/store/readerStore';
import { useBookDataStore } from '@/store/bookDataStore';
import { useSettingsStore } from '@/store/settingsStore';
import { useThemeStore } from '@/store/themeStore';
import { RSVPController, RsvpStartChoice, RsvpStopPosition } from '@/services/rsvp';
import {
RSVPController,
RsvpStartChoice,
RsvpStopPosition,
buildRsvpExitConfigUpdate,
} from '@/services/rsvp';
import { eventDispatcher } from '@/utils/event';
import { useEnv } from '@/context/EnvContext';
import { useTranslation } from '@/hooks/useTranslation';
import { BookNote } from '@/types/book';
import { BookNote, PageInfo } from '@/types/book';
import { TOCItem } from '@/libs/document';
import { Insets } from '@/types/misc';
import RSVPOverlay from './RSVPOverlay';
import RSVPStartDialog from './RSVPStartDialog';
@@ -116,6 +122,10 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
const [startChoice, setStartChoice] = useState<RsvpStartChoice | null>(null);
const controllerRef = useRef<RSVPController | null>(null);
const tempHighlightRef = useRef<BookNote | null>(null);
// renderer.primaryIndex reverts after navigation (paginator #detectPrimaryView),
// so track RSVP's actual section and chapter href in stable refs instead.
const rsvpSectionRef = useRef<number>(-1);
const rsvpChapterHrefRef = useRef<string | null>(null);
// Helper to remove any existing RSVP highlight
const removeRsvpHighlight = useCallback(() => {
@@ -141,6 +151,8 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
}
// Remove any existing RSVP highlight when component unmounts
removeRsvpHighlight();
rsvpSectionRef.current = -1;
rsvpChapterHrefRef.current = null;
};
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
@@ -198,6 +210,8 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
// Create controller if not exists
if (!controllerRef.current) {
controllerRef.current = new RSVPController(view, bookKey);
rsvpSectionRef.current = view.renderer.primaryIndex;
rsvpChapterHrefRef.current = progress?.sectionHref ?? null;
}
const controller = controllerRef.current;
@@ -385,13 +399,17 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
controller.stop();
}
// Persist RSVP position to BookConfig so it syncs to the cloud
// Persist RSVP position to BookConfig so it syncs to the cloud. Pin
// `location` to the RSVP word's CFI so the next normal-mode load resumes
// here instead of at a section boundary that a mid-RSVP relocate left
// behind in the auto-saved config.
const rsvpPosition = controller?.getStoredPosition();
if (rsvpPosition) {
const config = getConfig(bookKey);
if (config) {
setConfig(bookKey, { rsvpPosition });
saveConfig(envConfig, bookKey, { ...config, rsvpPosition }, settings);
const update = buildRsvpExitConfigUpdate(rsvpPosition);
setConfig(bookKey, update);
saveConfig(envConfig, bookKey, { ...config, ...update }, settings);
}
}
@@ -414,8 +432,11 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
const view = getView(bookKey);
if (!view) return;
const onRelocate = () => {
const onRelocate = (e: Event) => {
view.removeEventListener('relocate', onRelocate);
const detail = (e as CustomEvent).detail as { section?: PageInfo; tocItem?: TOCItem };
rsvpSectionRef.current = detail.section?.current ?? view.renderer.primaryIndex;
rsvpChapterHrefRef.current = detail.tocItem?.href ?? null;
const controller = controllerRef.current;
if (controller) {
const progress = getProgress(bookKey);
@@ -437,18 +458,30 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
removeRsvpHighlight();
const indexBefore = view.renderer.primaryIndex;
if (view.renderer.atEnd) {
controllerRef.current?.pause();
return;
}
const indexBefore =
rsvpSectionRef.current >= 0 ? rsvpSectionRef.current : view.renderer.primaryIndex;
let cleanup: ReturnType<typeof setTimeout> | null = null;
const onRelocate = (e: Event) => {
const detail = (e as CustomEvent).detail as { section?: PageInfo; tocItem?: TOCItem };
const newIndex = detail.section?.current ?? view.renderer.primaryIndex;
if (newIndex === indexBefore) return; // revert relocate — keep waiting
const onRelocate = () => {
view.removeEventListener('relocate', onRelocate);
if (cleanup) clearTimeout(cleanup);
const controller = controllerRef.current;
if (!controller) return;
// Pause at the end of the book instead of advancing
if (view.renderer.primaryIndex === indexBefore) {
controller.pause();
return;
}
rsvpSectionRef.current = newIndex;
rsvpChapterHrefRef.current = detail.tocItem?.href ?? null;
const progress = getProgress(bookKey);
if (progress?.location) {
@@ -456,15 +489,21 @@ const RSVPControl: React.FC<RSVPControlProps> = ({ bookKey, gridInsets }) => {
}
controller.loadNextPageContent();
};
view.addEventListener('relocate', onRelocate);
await view.renderer.nextSection?.();
cleanup = setTimeout(() => view.removeEventListener('relocate', onRelocate), 5000);
// Navigate directly to rsvpSectionRef.current + 1 rather than calling nextSection(),
// which uses renderer.primaryIndex internally. primaryIndex reverts to the previous
// section after navigation (#detectPrimaryView), so nextSection() would re-navigate
// to the already-current section and the onRelocate filter would discard the event.
await view.renderer.goTo({ index: rsvpSectionRef.current + 1 });
}, [bookKey, getProgress, getView, removeRsvpHighlight]);
// Get current chapter info
const progress = getProgress(bookKey);
const bookData = getBookData(bookKey);
const chapters = bookData?.bookDoc?.toc || [];
const currentChapterHref = progress?.sectionHref || null;
const currentChapterHref = rsvpChapterHrefRef.current ?? progress?.sectionHref ?? null;
// Use portal to render overlay at body level to avoid stacking context issues
const portalContainer = typeof document !== 'undefined' ? document.body : null;
@@ -34,20 +34,6 @@ const STORAGE_KEY_FONT_SIZE = 'readest_rsvp_fontsize';
const STORAGE_KEY_ORP_COLOR = 'readest_rsvp_orp_color';
const STORAGE_KEY_CONTEXT = 'readest_rsvp_context';
// Context window: render only a sliding window of words around the current index.
// Why: full-chapter rendering can be thousands of spans, and iOS WebKit's layout cost
// for getBoundingClientRect (used by auto-scroll) scales with DOM size, which throttles
// the word-advance interval well below the configured WPM.
const CONTEXT_WINDOW_SIZE = 500;
const CONTEXT_WINDOW_SLIDE_THRESHOLD = 100;
const computeContextWindow = (total: number, currentIndex: number) => {
if (total <= CONTEXT_WINDOW_SIZE) return { start: 0, end: total };
const half = Math.floor(CONTEXT_WINDOW_SIZE / 2);
const start = Math.max(0, Math.min(total - CONTEXT_WINDOW_SIZE, currentIndex - half));
return { start, end: start + CONTEXT_WINDOW_SIZE };
};
interface RSVPOverlayProps {
gridInsets: Insets;
controller: RSVPController;
@@ -107,11 +93,7 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
}
return 0;
});
const [contextWindow, setContextWindow] = useState(() =>
computeContextWindow(state.words.length, state.currentIndex),
);
const contextWordRef = useRef<HTMLSpanElement>(null);
const contextPanelRef = useRef<HTMLDivElement>(null);
const touchStartX = useRef(0);
const touchStartY = useRef(0);
const touchStartTime = useRef(0);
@@ -241,48 +223,11 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
}
}, [state]);
// Stable word list that only changes when contextWindow changes
const contextWords = useMemo(
() => state.words.slice(contextWindow.start, contextWindow.end),
[state.words, contextWindow],
);
// Slide/reset the context window as currentIndex moves or the chapter changes.
// Auto-scroll: keep highlighted word in view
useEffect(() => {
const total = state.words.length;
const cur = state.currentIndex;
setContextWindow((prev) => {
if (total <= CONTEXT_WINDOW_SIZE) {
if (prev.start === 0 && prev.end === total) return prev;
return { start: 0, end: total };
}
const currentSize = prev.end - prev.start;
const outOfBounds = cur < prev.start || cur >= prev.end;
const nearStartEdge = prev.start > 0 && cur < prev.start + CONTEXT_WINDOW_SLIDE_THRESHOLD;
const nearEndEdge = prev.end < total && cur >= prev.end - CONTEXT_WINDOW_SLIDE_THRESHOLD;
const sizeMismatch = currentSize !== CONTEXT_WINDOW_SIZE || prev.end > total;
if (!outOfBounds && !nearStartEdge && !nearEndEdge && !sizeMismatch) return prev;
return computeContextWindow(total, cur);
});
}, [state.currentIndex, state.words]);
// Auto-scroll: keep highlighted word away from top/bottom edges
useEffect(() => {
const panel = contextPanelRef.current;
const word = contextWordRef.current;
if (contextCollapsed || !panel || !word) return;
const panelRect = panel.getBoundingClientRect();
const wordRect = word.getBoundingClientRect();
const margin = panelRect.height * 0.15;
const topLine = panelRect.top + margin;
if (wordRect.top < topLine) {
panel.scrollTop -= topLine - wordRect.top;
} else if (wordRect.bottom > panelRect.bottom - margin) {
panel.scrollTop += wordRect.top - topLine;
}
}, [state.currentIndex, contextCollapsed, contextWindow]);
if (contextCollapsed) return;
contextWordRef.current?.scrollIntoView({ block: 'nearest', behavior: 'instant' });
}, [state.currentIndex, contextCollapsed]);
useEffect(() => {
if (!showChapterDropdown) return;
@@ -604,14 +549,13 @@ const RSVPOverlay: React.FC<RSVPOverlayProps> = ({
</button>
{!contextCollapsed && (
<div
ref={contextPanelRef}
className='max-h-[20vh] overflow-y-auto px-3 pb-3 md:px-4 md:pb-4'
onTouchStart={(e) => e.stopPropagation()}
onTouchEnd={(e) => e.stopPropagation()}
>
<div className='text-left text-base leading-relaxed md:text-lg'>
{contextWords.map((w, i) => {
const wordIndex = contextWindow.start + i;
{state.words.map((w, i) => {
const wordIndex = i;
const isCurrent = wordIndex === state.currentIndex;
return (
<span
@@ -1,2 +1,3 @@
export * from './types';
export * from './RSVPController';
export * from './persistence';
@@ -0,0 +1,12 @@
import { BookConfig } from '@/types/book';
import { RsvpPosition } from './types';
// Builds the BookConfig delta to persist when leaving RSVP. Pinning `location`
// to the RSVP word's CFI is what stops the next normal-mode load from resuming
// at the boundary CFI a mid-RSVP section transition wrote into the config.
export const buildRsvpExitConfigUpdate = (rsvpPosition: RsvpPosition): Partial<BookConfig> => {
if (rsvpPosition.cfi) {
return { rsvpPosition, location: rsvpPosition.cfi };
}
return { rsvpPosition };
};
+1 -1
View File
@@ -34,7 +34,7 @@ export interface Renderer extends HTMLElement {
nextSection?: () => Promise<void>;
prevSection?: () => Promise<void>;
render?: () => Promise<void>;
goTo: (params: { index: number; anchor?: number | RangeAnchor }) => void;
goTo: (params: { index: number; anchor?: number | RangeAnchor }) => Promise<void>;
setStyles?: (css: string) => void;
primaryIndex: number;
getContents: () => { doc: Document; index?: number; overlayer?: unknown }[];