forked from akai/readest
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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!);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
+1
-1
Submodule packages/foliate-js updated: 9f12ba9290...f860916a2b
Reference in New Issue
Block a user