From 6405ba31c8fbbc4cfbbff193b15a117c10dea160 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Fri, 29 May 2026 16:22:30 +0800 Subject: [PATCH] fix(reader): keep TOC scrolled to the current chapter on refresh (#4353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a hard refresh the TOC sidebar occasionally (~1 in 10) scrolled to and highlighted the current chapter, then rewound to the very top of the list. It is a scroll-position race in TOCView, not a progress/sectionHref reset (the reading position stays correct throughout). OverlayScrollbars resets the wrapped Virtuoso viewport's scrollTop to 0 when it initializes (deferred). Its `initialized` callback re-scrolled only to `initialScrollTarget.index`, captured at mount — and on a fresh refresh `progress` is not available yet, so that index is 0 and the reset is never corrected. Whether OverlayScrollbars initializes before or after the auto-scroll to the reading position is the timing race that made it intermittent. Re-apply the scroll to the current active item (via refs mirroring the live flatItems/activeHref) in the `initialized` callback, falling back to the mount-time index. Refs are used because OverlayScrollbars binds the callback at mount and fires it later, so it must read the latest active item. Co-authored-by: Claude Opus 4.8 (1M context) --- .../src/__tests__/components/TOCView.test.tsx | 146 ++++++++++++++++++ .../app/reader/components/sidebar/TOCView.tsx | 20 ++- 2 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 apps/readest-app/src/__tests__/components/TOCView.test.tsx diff --git a/apps/readest-app/src/__tests__/components/TOCView.test.tsx b/apps/readest-app/src/__tests__/components/TOCView.test.tsx new file mode 100644 index 00000000..f9ade085 --- /dev/null +++ b/apps/readest-app/src/__tests__/components/TOCView.test.tsx @@ -0,0 +1,146 @@ +import { render, act, cleanup } from '@testing-library/react'; +import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vitest'; + +import type { TOCItem } from '@/libs/document'; + +// ---------- Shared mutable test state (captured by the mock factories) ---------- +let scrollToIndexSpy: Mock<(arg: unknown) => void>; +// Only the FIRST (mount-time) `initialized` callback is captured, mirroring how +// OverlayScrollbars binds its event handlers when it initializes the viewport. +// A fix that relied on a fresher render closure would pass against the latest +// callback but still break in the real app — so the test forces a ref-based fix. +let capturedInitialized: + | ((instance: { elements: () => { viewport: HTMLElement } }) => void) + | undefined; +let mockProgress: { sectionHref: string; location: string } | null; + +// ---------- Mocks ---------- +vi.mock('@/store/readerStore', () => ({ + useReaderStore: () => ({ + getView: () => undefined, + getViewSettings: () => ({ isEink: false }), + getProgress: () => mockProgress, + }), +})); + +vi.mock('@/store/sidebarStore', () => ({ + useSidebarStore: () => ({ sideBarBookKey: 'book1', isSideBarVisible: true }), +})); + +vi.mock('@/services/nav', () => ({ findParentPath: () => [] })); + +vi.mock('@/utils/event', () => ({ + eventDispatcher: { dispatch: vi.fn(), on: vi.fn(), off: vi.fn() }, +})); + +vi.mock('@/utils/misc', () => ({ getContentMd5: (s: string) => s })); + +vi.mock('@/app/reader/hooks/useTextTranslation', () => ({ + useTextTranslation: () => {}, +})); + +// Virtuoso is replaced with a stub that exposes a spy-able `scrollToIndex` +// through the imperative handle and hands TOCView a scroller element. +vi.mock('react-virtuoso', async () => { + const ReactMod = await import('react'); + return { + Virtuoso: ReactMod.forwardRef( + ( + props: { scrollerRef?: (el: HTMLElement | Window | null) => void }, + ref: React.Ref, + ) => { + ReactMod.useImperativeHandle(ref, () => ({ + scrollToIndex: (arg: unknown) => scrollToIndexSpy(arg), + })); + ReactMod.useEffect(() => { + props.scrollerRef?.(document.createElement('div')); + }, []); + return null; + }, + ), + }; +}); + +// Capture the OverlayScrollbars `initialized` callback so the test can fire it +// on demand (the real hook fires it after a deferred, timing-dependent init). +vi.mock('overlayscrollbars-react', () => ({ + useOverlayScrollbars: (opts: { + events?: { initialized?: (i: { elements: () => { viewport: HTMLElement } }) => void }; + }) => { + if (!capturedInitialized) capturedInitialized = opts.events?.initialized; + return [vi.fn(), () => undefined]; + }, +})); + +// eslint-disable-next-line import/first +import TOCView from '@/app/reader/components/sidebar/TOCView'; + +const makeFlatToc = (count: number): TOCItem[] => + Array.from({ length: count }, (_, i) => ({ + id: i + 1, + label: `Chapter ${i}`, + href: `ch${i}.html`, + index: i, + })); + +const fireOverlayScrollbarsInitialized = () => { + act(() => { + capturedInitialized?.({ elements: () => ({ viewport: document.createElement('div') }) }); + }); +}; + +beforeEach(() => { + scrollToIndexSpy = vi.fn<(arg: unknown) => void>(); + capturedInitialized = undefined; + mockProgress = null; + // Run rAF synchronously so the callback's scroll happens inside act(). + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { + cb(0); + return 0; + }); +}); + +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +describe('TOCView — OverlayScrollbars init does not rewind the TOC to the top', () => { + it('re-applies the auto-scroll to the active item when OverlayScrollbars initializes after the reading position arrives', () => { + const toc = makeFlatToc(8); + const activeHref = 'ch5.html'; + + // Fresh refresh: TOCView mounts before the first relocate, so `progress` + // (and thus the mount-time initialScrollTarget) has no reading position. + mockProgress = null; + const { rerender } = render(); + + // The relocate arrives → the normal auto-scroll effect centers the active + // chapter. (OverlayScrollbars' deferred init, which resets scrollTop to 0, + // has not fired yet.) + mockProgress = { sectionHref: activeHref, location: 'epubcfi(/6/12!/4/1:0)' }; + act(() => { + rerender(); + }); + + // Ignore that first scroll; we only care whether the OverlayScrollbars init + // (which clobbers scrollTop) re-applies it instead of stranding the top. + scrollToIndexSpy.mockClear(); + + fireOverlayScrollbarsInitialized(); + + expect(scrollToIndexSpy).toHaveBeenCalledWith(expect.objectContaining({ index: 5 })); + }); + + it('does not force a scroll on OverlayScrollbars init when there is no reading position', () => { + const toc = makeFlatToc(8); + + mockProgress = null; + render(); + + scrollToIndexSpy.mockClear(); + fireOverlayScrollbarsInitialized(); + + expect(scrollToIndexSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/readest-app/src/app/reader/components/sidebar/TOCView.tsx b/apps/readest-app/src/app/reader/components/sidebar/TOCView.tsx index aa0294bf..806aff74 100644 --- a/apps/readest-app/src/app/reader/components/sidebar/TOCView.tsx +++ b/apps/readest-app/src/app/reader/components/sidebar/TOCView.tsx @@ -77,6 +77,11 @@ const TOCView: React.FC<{ // viewport-wrap scrollTop reset) flips userScrolledRef in that window it // would otherwise suppress the auto-scroll once progress finally arrives. const initialAutoScrollProcessedRef = useRef(false); + // Mirror the latest active item + flat list so the OverlayScrollbars + // `initialized` callback (created at mount but fired after a deferred, + // timing-dependent delay) can re-center on the *current* reading position. + const activeHrefRef = useRef(null); + const flatItemsRef = useRef([]); // OverlayScrollbars + Virtuoso integration (same pattern as Bookshelf) const osRootRef = useRef(null); @@ -89,7 +94,17 @@ const TOCView: React.FC<{ const { viewport } = instance.elements(); viewport.style.overflowX = 'var(--os-viewport-overflow-x)'; viewport.style.overflowY = 'var(--os-viewport-overflow-y)'; - const target = initialScrollTarget.index; + // OverlayScrollbars resets the wrapped viewport's scrollTop to 0 as it + // initializes. On a fresh refresh the auto-scroll to the reading + // position may already have run by now, so re-apply it here — using the + // *current* active item, since initialScrollTarget was captured at mount + // when progress was usually not yet available (index 0). Without this + // the TOC rewinds to the very top on ~1 in 10 refreshes, depending on + // whether this deferred init lands before or after the auto-scroll. + const activeIdx = activeHrefRef.current + ? flatItemsRef.current.findIndex((f) => f.item.href === activeHrefRef.current) + : -1; + const target = activeIdx > 0 ? activeIdx : initialScrollTarget.index; if (target > 0) { requestAnimationFrame(() => { virtuosoRef.current?.scrollToIndex({ @@ -147,6 +162,9 @@ const TOCView: React.FC<{ const activeHref = progress?.sectionHref ?? null; const flatItems = useMemo(() => flattenTOC(toc, expandedItems), [toc, expandedItems]); + // Keep the refs read by the OverlayScrollbars `initialized` callback current. + activeHrefRef.current = activeHref; + flatItemsRef.current = flatItems; const handleToggleExpand = useCallback((item: TOCItem) => { const itemId = getItemIdentifier(item);