fix(reader): keep TOC scrolled to the current chapter on refresh (#4353)

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) <noreply@anthropic.com>
2026-05-29 16:22:30 +08:00
parent 4988a53cba
commit 6405ba31c8
2 changed files with 165 additions and 1 deletions
@@ -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<unknown>,
+      ) => {
+        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(<TOCView bookKey='book1' toc={toc} />);
+
+    // 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(<TOCView bookKey='book1' toc={toc} />);
+    });
+
+    // 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(<TOCView bookKey='book1' toc={toc} />);
+
+    scrollToIndexSpy.mockClear();
+    fireOverlayScrollbarsInitialized();
+
+    expect(scrollToIndexSpy).not.toHaveBeenCalled();
+  });
+});
@@ -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<string | null>(null);
+  const flatItemsRef = useRef<FlatTOCItem[]>([]);

  // OverlayScrollbars + Virtuoso integration (same pattern as Bookshelf)
  const osRootRef = useRef<HTMLDivElement>(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);