From d609de58f023787c51eb1ea6625b9e11b7a35b45 Mon Sep 17 00:00:00 2001 From: Huang Xin Date: Wed, 29 Apr 2026 15:32:54 +0800 Subject: [PATCH] fix(reader): preserve position when toggling scrolled mode, closes #3987 (#3996) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The paginator's scrolled-mode scroll handler is debounced 250 ms, so #anchor and #primaryIndex can lag behind the user's actual viewport. Toggling out of scrolled mode within that window made render() → scrollToAnchor(#anchor) restore the stale anchor, reverting the position to a previously visible section. Update foliate-js to flush the pending scroll state before flow leaves 'scrolled', and add regression coverage for the multi-section toggle path. --- .../.claude/memory/bug-patterns.md | 9 +++- .../paginator-multiview.browser.test.ts | 44 +++++++++++++++++++ .../src/app/reader/hooks/useBookShortcuts.ts | 8 ++-- packages/foliate-js | 2 +- 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/apps/readest-app/.claude/memory/bug-patterns.md b/apps/readest-app/.claude/memory/bug-patterns.md index 4b55cd0d..26eb39c8 100644 --- a/apps/readest-app/.claude/memory/bug-patterns.md +++ b/apps/readest-app/.claude/memory/bug-patterns.md @@ -98,7 +98,14 @@ **Fix Strategy:** Use physical `view.renderer.page`/`view.renderer.pages` instead of estimated section metadata. Check boundary conditions (0-indexed vs 1-indexed, inclusive vs exclusive). -### 11. Multiview Paginator Side Effects +### 11. Debounced State Stale on User-Initiated Layout Change +**Pattern:** A scroll/resize handler is debounced for performance, but during the debounce window any code path that re-runs layout based on saved state (e.g. `#anchor`, `#primaryIndex`) sees stale values. +**Example:** +- Scrolled-mode toggle reverted to previous chapter (#3987): the paginator's scroll handler is debounced 250 ms, so toggling `flow=scrolled → flow=paginated` within that window made `render() → scrollToAnchor(#anchor)` restore the anchor from before the user scrolled into the next section. Both `#anchor` and `#primaryIndex` were stale together, sending the position back. + +**Fix Strategy:** When an external trigger forces a re-render (here, `setAttribute('flow', ...)`), flush the debounced state synchronously *before* changing the layout. In paginator.js this means overriding `setAttribute` and calling `#detectPrimaryView()` + `#getVisibleRange()` while `this.scrolled` is still true. + +### 12. Multiview Paginator Side Effects **Pattern:** The multiview paginator (e925e9d+) loads adjacent sections in background. Events from these loads can interfere with user interactions on the primary section. **Examples:** - `load` event from adjacent section triggers `docLoadHandler` which re-adds ALL annotations, overwriting drag edits diff --git a/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts b/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts index 56074cf1..371cb6da 100644 --- a/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts +++ b/apps/readest-app/src/__tests__/document/paginator-multiview.browser.test.ts @@ -489,6 +489,50 @@ describe('Paginator multi-view architecture (browser)', () => { expect(paginator.scrolled).toBe(true); expect(paginator.primaryIndex).toBe(idx); }); + + // Regression for issue #3987: toggling scrolled mode off shortly after + // scrolling into the next section reverted the position to the previous + // section because the debounced scroll handler had not yet updated + // #primaryIndex / #anchor. + it('should not revert to previous section when toggling scrolled mode off mid-scroll', async () => { + paginator = createPaginator(); + paginator.open(book); + paginator.setAttribute('flow', 'scrolled'); + + const linearIndices = book + .sections!.map((s, i) => (s.linear !== 'no' ? i : -1)) + .filter((i) => i >= 0); + if (linearIndices.length < 2) return; + + const startIdx = linearIndices[0]!; + const nextIdx = linearIndices[1]!; + + const stabilized = waitForStabilized(paginator); + await paginator.goTo({ index: startIdx }); + await stabilized; + await waitForFillComplete(paginator); + + const contents = paginator.getContents(); + if (!contents.some((c) => c.index === nextIdx)) return; + + const container = paginator.shadowRoot!.getElementById('container')!; + const viewElements = Array.from(container.children).filter((c): c is HTMLElement => + Boolean((c as HTMLElement).querySelector?.('iframe')), + ); + expect(viewElements.length).toBeGreaterThanOrEqual(2); + const firstViewHeight = viewElements[0]!.getBoundingClientRect().height; + expect(firstViewHeight).toBeGreaterThan(0); + + // Scroll past the first section into the second, then immediately + // toggle scrolled mode off without giving the 250 ms debounce a chance + // to fire. + container.scrollTop = firstViewHeight + 100; + const stabilized2 = waitForStabilized(paginator); + paginator.setAttribute('flow', 'paginated'); + await stabilized2; + + expect(paginator.primaryIndex).toBe(nextIdx); + }); }); }); diff --git a/apps/readest-app/src/app/reader/hooks/useBookShortcuts.ts b/apps/readest-app/src/app/reader/hooks/useBookShortcuts.ts index 11e25084..ffae549c 100644 --- a/apps/readest-app/src/app/reader/hooks/useBookShortcuts.ts +++ b/apps/readest-app/src/app/reader/hooks/useBookShortcuts.ts @@ -12,7 +12,6 @@ import { setShortcutsDialogVisible } from '@/components/KeyboardShortcutsHelp'; import { MAX_ZOOM_LEVEL, MIN_ZOOM_LEVEL, ZOOM_STEP } from '@/services/constants'; import { getParagraphActionForKey } from '@/utils/paragraphPresentation'; import { viewPagination } from './usePagination'; -import { getStyles } from '@/utils/style'; import useShortcuts from '@/hooks/useShortcuts'; import useBooksManager from './useBooksManager'; import { getReadingRulerMoveDirection } from '../utils/readingRuler'; @@ -56,6 +55,7 @@ const useBookShortcuts = ({ sideBarBookKey, bookKeys }: UseBookShortcutsProps) = const flowMode = viewSettings.scrolled ? 'scrolled' : 'paginated'; getView(sideBarBookKey)?.renderer.setAttribute('flow', flowMode); } + return true; }; const switchSideBar = () => { @@ -210,12 +210,10 @@ const useBookShortcuts = ({ sideBarBookKey, bookKeys }: UseBookShortcutsProps) = const view = getView(sideBarBookKey); const bookData = getBookData(sideBarBookKey); const viewSettings = getViewSettings(sideBarBookKey)!; - viewSettings!.zoomLevel = zoomLevel; - setViewSettings(sideBarBookKey, viewSettings!); if (bookData?.isFixedLayout) { view?.renderer.setAttribute('scale-factor', zoomLevel); - } else { - view?.renderer.setStyles?.(getStyles(viewSettings!)); + viewSettings!.zoomLevel = zoomLevel; + setViewSettings(sideBarBookKey, viewSettings!); } }; diff --git a/packages/foliate-js b/packages/foliate-js index 9f12ba92..f860916a 160000 --- a/packages/foliate-js +++ b/packages/foliate-js @@ -1 +1 @@ -Subproject commit 9f12ba92905e4d0ca7b82fd908a01a317e0f5257 +Subproject commit f860916a2bfde622448a1cf9e37c87e07bdd150d