akai/readest
2
1
Fork 1
Code Issues Pull Requests Actions 10 Packages Projects Releases Wiki Activity

fix(reader): collapse TOC to the current chapter's path by default (#4366)

Browse Source
This commit is contained in:
Huang Xin
2026-05-30 17:37:59 +08:00
committed by GitHub
parent 92b3c9db48
commit 11666be5ee
3 changed files with 213 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/readest-app/src/__tests__/app/reader/toc-view-init.test.ts
+147 -82
View File
@@ -1,32 +1,6 @@
import { describe, it, expect, vi } from 'vitest';
import type { TOCItem } from '@/libs/document';
/**
* Regression test for TOC sidebar blank on initial book load.
*
* When a book opens at a position with no TOC entry (e.g., cover page),
* progress.sectionHref is undefined and expandParents is never called.
* For books with a nested TOC structure (chapters nested under a root
* container), the sidebar appears blank because expandedItems is empty
* and only the root item is shown.
*
* Fix: Initialize expandedItems with top-level TOC items that have subitems
* so chapters are visible immediately on sidebar open.
*/
// Mirrors the logic in TOCView.tsx's getItemIdentifier
const getItemIdentifier = (item: TOCItem) => {
const href = item.href || '';
return `toc-item-${item.id}-${href}`;
};
// Mirrors the initialization logic to be added to TOCView.tsx
const getInitialExpandedItems = (toc: TOCItem[]): Set<string> => {
const topLevelWithSubitems = toc
.filter((item) => item.subitems?.length)
.map((item) => getItemIdentifier(item));
return topLevelWithSubitems.length > 0 ? new Set(topLevelWithSubitems) : new Set();
};
import { computeExpandedSet, getItemIdentifier } from '@/app/reader/components/sidebar/tocTree';
// Mirrors useFlattenedTOC logic in TOCView.tsx
const flattenTOC = (items: TOCItem[], expandedItems: Set<string>, depth = 0): TOCItem[] => {
@@ -41,28 +15,10 @@ const flattenTOC = (items: TOCItem[], expandedItems: Set<string>, depth = 0): TO
return result;
};
// Helpers mirrored from TOCView.tsx for the initial-scroll-target logic.
// These drive the Virtuoso `initialTopMostItemIndex` prop, which avoids the
// race where a setTimeout-based scrollToIndex fires before Virtuoso has
// finished its first layout pass.
const findParentPath = (items: TOCItem[], href: string, path: TOCItem[] = []): TOCItem[] => {
for (const item of items) {
const newPath = [...path, item];
if (item.href === href) return path;
if (item.subitems) {
const found = findParentPath(item.subitems, href, newPath);
if (found.length > 0) return found;
}
}
return [];
};
const computeExpandedSet = (toc: TOCItem[], href: string | undefined): Set<string> => {
const topLevel = toc.filter((item) => item.subitems?.length).map(getItemIdentifier);
const parents = href ? findParentPath(toc, href).map(getItemIdentifier).filter(Boolean) : [];
return new Set([...topLevel, ...parents]);
};
// Mirrors the initial-scroll-target logic in TOCView.tsx (uses the real
// computeExpandedSet). This drives the Virtuoso `initialTopMostItemIndex`
// prop, which avoids the race where a setTimeout-based scrollToIndex fires
// before Virtuoso has finished its first layout pass.
const getInitialScrollTarget = (
toc: TOCItem[],
href: string | undefined,
@@ -74,6 +30,15 @@ const getInitialScrollTarget = (
return { index: idx > 0 ? idx : 0, expanded };
};
/**
* Auto-expansion policy (computeExpandedSet).
*
* Only the ancestors of the current reading location are "necessary" to
* expand — a deep, multi-volume hierarchy must otherwise stay collapsed so
* it is easy to scan (issue #4059). The single exception is a TOC wrapped in
* one root container: collapsing it would reduce the sidebar to a single
* uninformative row, so that lone root is expanded as a fallback.
*/
describe('TOC sidebar initialization', () => {
const nestedTOC: TOCItem[] = [
{
@@ -94,54 +59,90 @@ describe('TOC sidebar initialization', () => {
{ id: 3, label: 'Chapter 3', href: 'ch3.html' } as unknown as TOCItem,
];
describe('before fix (demonstrates the bug)', () => {
it('nested TOC with empty expandedItems only shows root item, not chapters', () => {
const expandedItems = new Set<string>(); // Empty initial state (the bug)
const flatItems = flattenTOC(nestedTOC, expandedItems);
// Only root "Book" shows — chapters are hidden
expect(flatItems).toHaveLength(1);
expect(flatItems[0]!.label).toBe('Book');
});
});
// Mirrors a multi-volume collection ("文学必读合集20册" in issue #4059):
// several top-level volume containers, each with their own chapters.
const multiVolumeTOC: TOCItem[] = [
{
id: 0,
label: 'Volume 1',
href: 'v1.html',
subitems: [
{ id: 1, label: 'V1 Chapter 1', href: 'v1c1.html' },
{ id: 2, label: 'V1 Chapter 2', href: 'v1c2.html' },
],
},
{
id: 3,
label: 'Volume 2',
href: 'v2.html',
subitems: [
{ id: 4, label: 'V2 Chapter 1', href: 'v2c1.html' },
{ id: 5, label: 'V2 Chapter 2', href: 'v2c2.html' },
],
},
{
id: 6,
label: 'Volume 3',
href: 'v3.html',
subitems: [{ id: 7, label: 'V3 Chapter 1', href: 'v3c1.html' }],
},
] as unknown as TOCItem[];
describe('after fix (initialization effect behavior)', () => {
it('nested TOC: getInitialExpandedItems expands the root container', () => {
const expandedItems = getInitialExpandedItems(nestedTOC);
expect(expandedItems.size).toBe(1);
expect(expandedItems.has(getItemIdentifier(nestedTOC[0]!))).toBe(true);
describe('single-root fallback (prevents a blank sidebar)', () => {
it('nested TOC with no current location expands only the lone root container', () => {
const expanded = computeExpandedSet(nestedTOC, undefined);
expect(expanded.size).toBe(1);
expect(expanded.has(getItemIdentifier(nestedTOC[0]!))).toBe(true);
});
it('nested TOC: with initialized expandedItems, all chapters are visible', () => {
const expandedItems = getInitialExpandedItems(nestedTOC);
const flatItems = flattenTOC(nestedTOC, expandedItems);
it('nested TOC: with the root expanded, all chapters are visible', () => {
const expanded = computeExpandedSet(nestedTOC, undefined);
const flatItems = flattenTOC(nestedTOC, expanded);
// Root + 3 chapters = 4 items
expect(flatItems).toHaveLength(4);
expect(flatItems[1]!.label).toBe('Chapter 1');
expect(flatItems[2]!.label).toBe('Chapter 2');
expect(flatItems[3]!.label).toBe('Chapter 3');
});
it('flat TOC: getInitialExpandedItems returns empty set (no change)', () => {
const expandedItems = getInitialExpandedItems(flatTOC);
expect(expandedItems.size).toBe(0);
it('flat TOC with no current location expands nothing', () => {
const expanded = computeExpandedSet(flatTOC, undefined);
expect(expanded.size).toBe(0);
});
it('flat TOC: all chapters visible regardless of expandedItems', () => {
const expandedItems = getInitialExpandedItems(flatTOC);
const flatItems = flattenTOC(flatTOC, expandedItems);
expect(flatItems).toHaveLength(3);
it('empty TOC produces an empty expanded set', () => {
expect(computeExpandedSet([], undefined).size).toBe(0);
});
});
describe('multi-volume TOC stays collapsed (issue #4059)', () => {
it('expands nothing when there is no current location', () => {
const expanded = computeExpandedSet(multiVolumeTOC, undefined);
expect(expanded.size).toBe(0);
});
it('non-empty expandedItems is preserved (no re-initialization)', () => {
const existingItems = new Set(['toc-item-1-ch1.html']);
// The effect uses: if (prev.size > 0) return prev
const result = existingItems.size > 0 ? existingItems : getInitialExpandedItems(nestedTOC);
expect(result).toBe(existingItems); // Same reference - not re-initialized
it('shows only the collapsed volume rows when nothing is expanded', () => {
const expanded = computeExpandedSet(multiVolumeTOC, undefined);
const flatItems = flattenTOC(multiVolumeTOC, expanded);
expect(flatItems.map((i) => i.label)).toEqual(['Volume 1', 'Volume 2', 'Volume 3']);
});
it('empty TOC produces empty expandedItems', () => {
const expandedItems = getInitialExpandedItems([]);
expect(expandedItems.size).toBe(0);
it('expands only the current volume, leaving the others collapsed', () => {
const expanded = computeExpandedSet(multiVolumeTOC, 'v2c1.html');
expect(expanded.has(getItemIdentifier(multiVolumeTOC[1]!))).toBe(true); // Volume 2
expect(expanded.has(getItemIdentifier(multiVolumeTOC[0]!))).toBe(false); // Volume 1
expect(expanded.has(getItemIdentifier(multiVolumeTOC[2]!))).toBe(false); // Volume 3
});
it('only the current volume reveals its chapters in the flattened list', () => {
const expanded = computeExpandedSet(multiVolumeTOC, 'v2c1.html');
const flatItems = flattenTOC(multiVolumeTOC, expanded);
expect(flatItems.map((i) => i.label)).toEqual([
'Volume 1',
'Volume 2',
'V2 Chapter 1',
'V2 Chapter 2',
'Volume 3',
]);
});
});
});
@@ -494,3 +495,67 @@ describe('TOC scroll effect retries when flatItems is stale', () => {
});
});
});
/**
* Regression test for issue #4059 follow-up: collapsed-by-default TOC drops
* the initial auto-scroll.
*
* With the current volume collapsed at mount (pinned sidebar mounts before
* the first relocate), the post-mount progress effect expands that volume,
* growing the virtualized list from a few rows to dozens. That growth makes
* Virtuoso emit a synthetic onScroll. The old handler treated ANY scroll
* while a scroll was queued as "the user took over" and cleared
* pendingScrollRef — so the one-shot auto-scroll to the current chapter was
* silently dropped and the TOC stayed near the top.
*
* Fix: ignore a queued-state scroll unless a real user gesture
* (wheel/touch/pointer/key) preceded it.
*/
describe('TOC onScroll distinguishes user scroll from expansion shift', () => {
type Refs = { pendingScroll: boolean; userScrolled: boolean };
// Mirrors the onScroll handler BEFORE the fix: any scroll clears pending.
const runOnScrollOld = (refs: Refs): void => {
refs.pendingScroll = false;
refs.userScrolled = true;
};
// Mirrors the onScroll handler AFTER the fix: a scroll arriving while an
// auto-scroll is queued is ignored unless a real user gesture preceded it.
const runOnScrollNew = (refs: Refs, userInput: boolean): void => {
if (refs.pendingScroll && !userInput) return;
refs.pendingScroll = false;
refs.userScrolled = true;
};
describe('before fix (demonstrates the bug)', () => {
it('a synthetic expansion scroll clears the queued auto-scroll', () => {
const refs: Refs = { pendingScroll: true, userScrolled: false };
runOnScrollOld(refs);
expect(refs.pendingScroll).toBe(false); // BUG: queued auto-scroll lost
});
});
describe('after fix', () => {
it('preserves the queued auto-scroll on a no-gesture expansion scroll', () => {
const refs: Refs = { pendingScroll: true, userScrolled: false };
runOnScrollNew(refs, false);
expect(refs.pendingScroll).toBe(true);
expect(refs.userScrolled).toBe(false);
});
it('lets a real user gesture cancel the queued auto-scroll', () => {
const refs: Refs = { pendingScroll: true, userScrolled: false };
runOnScrollNew(refs, true);
expect(refs.pendingScroll).toBe(false);
expect(refs.userScrolled).toBe(true);
});
it('records user scrolls normally once nothing is queued', () => {
const refs: Refs = { pendingScroll: false, userScrolled: false };
// No gesture flag needed: with nothing queued the guard does not apply.
runOnScrollNew(refs, false);
expect(refs.userScrolled).toBe(true);
});
});
});
apps/readest-app/src/app/reader/components/sidebar/TOCView.tsx
+44 -16
View File
@@ -6,15 +6,10 @@ import 'overlayscrollbars/overlayscrollbars.css';
import { TOCItem } from '@/libs/document';
import { useReaderStore } from '@/store/readerStore';
import { useSidebarStore } from '@/store/sidebarStore';
import { findParentPath } from '@/services/nav';
import { eventDispatcher } from '@/utils/event';
import { useTextTranslation } from '../../hooks/useTextTranslation';
import { FlatTOCItem, StaticListRow } from './TOCItem';
const getItemIdentifier = (item: TOCItem) => {
const href = item.href || '';
return `toc-item-${item.id}-${href}`;
};
import { computeExpandedSet, getItemIdentifier } from './tocTree';
const flattenTOC = (items: TOCItem[], expandedItems: Set<string>, depth = 0): FlatTOCItem[] => {
const result: FlatTOCItem[] = [];
@@ -28,12 +23,6 @@ const flattenTOC = (items: TOCItem[], expandedItems: Set<string>, depth = 0): Fl
return result;
};
const computeExpandedSet = (toc: TOCItem[], href: string | undefined): Set<string> => {
const topLevel = toc.filter((item) => item.subitems?.length).map(getItemIdentifier);
const parents = href ? findParentPath(toc, href).map(getItemIdentifier).filter(Boolean) : [];
return new Set([...topLevel, ...parents]);
};
const setsHaveSameContents = (a: Set<string>, b: Set<string>): boolean => {
if (a.size !== b.size) return false;
for (const item of a) if (!b.has(item)) return false;
@@ -82,6 +71,11 @@ const TOCView: React.FC<{
// timing-dependent delay) can re-center on the *current* reading position.
const activeHrefRef = useRef<string | null>(null);
const flatItemsRef = useRef<FlatTOCItem[]>([]);
// True once the reader has genuinely driven the list (wheel/touch/pointer/
// key). Auto-expanding the current volume on open grows the list and fires a
// synthetic scroll event; without a real gesture behind it, that scroll must
// not be mistaken for the user taking over and cancel the queued auto-scroll.
const userInputRef = useRef(false);
// OverlayScrollbars + Virtuoso integration (same pattern as Bookshelf)
const osRootRef = useRef<HTMLDivElement>(null);
@@ -126,6 +120,27 @@ const TOCView: React.FC<{
return () => osInstance()?.destroy();
}, [scroller, initialize, osInstance]);
// Flag real user gestures so onScroll can tell them apart from the synthetic
// scroll emitted when the current volume auto-expands on open.
useEffect(() => {
if (!scroller) return;
const markUserInput = () => {
userInputRef.current = true;
};
const passiveCapture = { capture: true, passive: true } as const;
const capture = { capture: true } as const;
scroller.addEventListener('wheel', markUserInput, passiveCapture);
scroller.addEventListener('touchstart', markUserInput, passiveCapture);
scroller.addEventListener('pointerdown', markUserInput, passiveCapture);
scroller.addEventListener('keydown', markUserInput, capture);
return () => {
scroller.removeEventListener('wheel', markUserInput, passiveCapture);
scroller.removeEventListener('touchstart', markUserInput, passiveCapture);
scroller.removeEventListener('pointerdown', markUserInput, passiveCapture);
scroller.removeEventListener('keydown', markUserInput, capture);
};
}, [scroller]);
const handleScrollerRef = useCallback((el: HTMLElement | Window | null) => {
setScroller(el instanceof HTMLElement ? el : null);
}, []);
@@ -229,6 +244,15 @@ const TOCView: React.FC<{
const distance = Math.abs(idx - visibleCenterRef.current);
const behavior = isEink || distance > 16 ? 'auto' : 'smooth';
virtuosoRef.current?.scrollToIndex({ index: idx, align: 'center', behavior });
// When the current volume auto-expands on open, the list grows by dozens of
// rows in this same commit. Virtuoso scrolls before measuring the new rows,
// so a single scrollToIndex lands short. Re-assert on the next frame (once
// they're measured) for the instant-jump case so the chapter ends centered.
if (behavior === 'auto') {
requestAnimationFrame(() => {
virtuosoRef.current?.scrollToIndex({ index: idx, align: 'center', behavior: 'auto' });
});
}
pendingScrollRef.current = false;
}, [flatItems, activeHref, isSideBarVisible, isEink]);
@@ -247,10 +271,14 @@ const TOCView: React.FC<{
visibleCenterRef.current = Math.floor((startIndex + endIndex) / 2);
}}
onScroll={() => {
// A scroll arriving while a pending auto-scroll is still
// queued (idx === -1, waiting on flatItems to expand) means
// the user is now driving — drop the queued auto-scroll so
// the next render doesn't yank them away.
// A scroll arriving while a pending auto-scroll is still queued
// (idx === -1, waiting on flatItems to expand) normally means the
// user is now driving — drop the queued auto-scroll so the next
// render doesn't yank them away. But auto-expanding the current
// volume on open grows the list and emits a synthetic scroll with
// no gesture behind it; ignore that so the initial auto-scroll
// survives. A real user scroll still cancels it via userInputRef.
if (pendingScrollRef.current && !userInputRef.current) return;
pendingScrollRef.current = false;
userScrolledRef.current = true;
if (scrollCooldownRef.current) clearTimeout(scrollCooldownRef.current);
apps/readest-app/src/app/reader/components/sidebar/tocTree.ts
+22
View File
@@ -0,0 +1,22 @@
import { TOCItem } from '@/libs/document';
import { findParentPath } from '@/services/nav';
export const getItemIdentifier = (item: TOCItem) => {
const href = item.href || '';
return `toc-item-${item.id}-${href}`;
};
// Decide which TOC nodes to auto-expand. Only the ancestors of the current
// reading location are "necessary" — expanding just that path reveals where
// the reader is while leaving the rest of a deep, multi-volume hierarchy
// collapsed and easy to scan (issue #4059). When there's no resolvable reading
// position yet, keep everything collapsed, except a lone wrapping root
// container — otherwise the sidebar would show a single uninformative row.
export const computeExpandedSet = (toc: TOCItem[], href: string | undefined): Set<string> => {
const parents = href ? findParentPath(toc, href).map(getItemIdentifier) : [];
if (parents.length) return new Set(parents);
if (toc.length === 1 && toc[0]?.subitems?.length) {
return new Set([getItemIdentifier(toc[0])]);
}
return new Set();
};