feat(editor): restore reading scroll position on reload (#266)
Adds useScrollPosition(pageId): saves window.scrollY to sessionStorage (key gitmost:scroll-position:<pageId>) on throttled scroll / pagehide / visibilitychange / cleanup, capturing the previously-saved value synchronously at mount before any handler can overwrite it with the fresh 0. restoreScrollPosition() (wired in page-editor.tsx to fire once the live content is laid out, !showStatic && editor) yields to a #hash anchor, then polls the document height and scrolls to the saved Y once the content is tall enough, with a 5s timeout clamped to the max reachable position. All storage access is try/caught so a disabled/quota'd Storage never breaks the page. The in-flight restore poll is held in a ref and cancelled on unmount, so a fast SPA navigation can't scroll the next page. closes #266 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,198 @@
|
|||||||
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
|
import { renderHook, act } from "@testing-library/react";
|
||||||
|
import { useScrollPosition } from "./use-scroll-position";
|
||||||
|
|
||||||
|
const KEY_PREFIX = "gitmost:scroll-position:";
|
||||||
|
|
||||||
|
function setScrollY(value: number): void {
|
||||||
|
Object.defineProperty(window, "scrollY", {
|
||||||
|
configurable: true,
|
||||||
|
value,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function setScrollHeight(value: number): void {
|
||||||
|
Object.defineProperty(document.documentElement, "scrollHeight", {
|
||||||
|
configurable: true,
|
||||||
|
value,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function setInnerHeight(value: number): void {
|
||||||
|
Object.defineProperty(window, "innerHeight", {
|
||||||
|
configurable: true,
|
||||||
|
value,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("useScrollPosition", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
window.sessionStorage.clear();
|
||||||
|
setScrollY(0);
|
||||||
|
setScrollHeight(0);
|
||||||
|
setInnerHeight(800);
|
||||||
|
// jsdom does not implement window.scrollTo; stub it.
|
||||||
|
window.scrollTo = vi.fn();
|
||||||
|
// Ensure no anchor leaks between tests.
|
||||||
|
window.location.hash = "";
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
vi.useRealTimers();
|
||||||
|
window.location.hash = "";
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(a) saves window.scrollY to sessionStorage under the pageId key, throttled", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
const { unmount } = renderHook(() => useScrollPosition("p1"));
|
||||||
|
|
||||||
|
// Leading-edge save fires immediately.
|
||||||
|
setScrollY(123);
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event("scroll"));
|
||||||
|
});
|
||||||
|
expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("123");
|
||||||
|
|
||||||
|
// Within the throttle window the next scroll is suppressed.
|
||||||
|
setScrollY(456);
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event("scroll"));
|
||||||
|
});
|
||||||
|
expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("123");
|
||||||
|
|
||||||
|
// After the throttle window elapses, the next scroll persists again.
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(250);
|
||||||
|
});
|
||||||
|
setScrollY(789);
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event("scroll"));
|
||||||
|
});
|
||||||
|
expect(window.sessionStorage.getItem(`${KEY_PREFIX}p1`)).toBe("789");
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(b) does not restore when the URL has a #hash anchor", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}p2`, "500");
|
||||||
|
// Content is ALREADY tall enough (maxScroll = 2000 - 800 = 1200 >= 500), so
|
||||||
|
// without the hash guard tryRestore would call scrollTo synchronously on the
|
||||||
|
// first tick. The assertion below therefore genuinely proves the hash guard
|
||||||
|
// short-circuits before any scroll (not just that the poll has not fired).
|
||||||
|
setScrollHeight(2000);
|
||||||
|
window.location.hash = "#some-heading";
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("p2"));
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
vi.advanceTimersByTime(5000);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(f) cancels the in-flight restore poll on unmount (no scroll on the next page)", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}p7`, "500");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(100); // maxScroll = -700: target not reachable yet, so it polls.
|
||||||
|
|
||||||
|
const { result, unmount } = renderHook(() => useScrollPosition("p7"));
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
|
||||||
|
|
||||||
|
// Navigate away (the hook unmounts) BEFORE the content grows tall enough.
|
||||||
|
unmount();
|
||||||
|
|
||||||
|
// Content of the NEXT page becomes tall; advancing time must NOT resurrect
|
||||||
|
// the cancelled poll (without the cleanup it would scroll the new page).
|
||||||
|
setScrollHeight(2000);
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(5000);
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(c) does nothing when nothing is saved or the saved value is <= 0", () => {
|
||||||
|
// Nothing saved.
|
||||||
|
const a = renderHook(() => useScrollPosition("nope"));
|
||||||
|
act(() => {
|
||||||
|
a.result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Saved value <= 0.
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}zero`, "0");
|
||||||
|
const b = renderHook(() => useScrollPosition("zero"));
|
||||||
|
act(() => {
|
||||||
|
b.result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(d) scrolls to the saved Y once the content is tall enough", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}p4`, "500");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(100); // maxScroll = -700, target not yet reachable.
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("p4"));
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Still polling: content not laid out yet.
|
||||||
|
expect(window.scrollTo).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Content becomes tall enough: maxScroll = 2000 - 800 = 1200 >= 500.
|
||||||
|
setScrollHeight(2000);
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(100);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(d2) clamps to the max reachable position after the timeout", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
window.sessionStorage.setItem(`${KEY_PREFIX}p5`, "5000");
|
||||||
|
setInnerHeight(800);
|
||||||
|
setScrollHeight(1000); // maxScroll stays 200, never reaches 5000.
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useScrollPosition("p5"));
|
||||||
|
act(() => {
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Advance past the 5s timeout; restore should fire clamped to maxScroll.
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(5000);
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
|
||||||
|
});
|
||||||
|
|
||||||
|
it("(e) never throws when storage access throws", () => {
|
||||||
|
const err = new Error("storage denied");
|
||||||
|
vi.spyOn(window.sessionStorage, "getItem").mockImplementation(() => {
|
||||||
|
throw err;
|
||||||
|
});
|
||||||
|
vi.spyOn(window.sessionStorage, "setItem").mockImplementation(() => {
|
||||||
|
throw err;
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(() => {
|
||||||
|
const { result, unmount } = renderHook(() => useScrollPosition("p6"));
|
||||||
|
act(() => {
|
||||||
|
setScrollY(42);
|
||||||
|
window.dispatchEvent(new Event("scroll"));
|
||||||
|
result.current.restoreScrollPosition();
|
||||||
|
});
|
||||||
|
unmount();
|
||||||
|
}).not.toThrow();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,163 @@
|
|||||||
|
import { useCallback, useEffect, useRef } from "react";
|
||||||
|
|
||||||
|
// Throttle interval for persisting the scroll position while the user reads.
|
||||||
|
const SAVE_THROTTLE_MS = 250;
|
||||||
|
// Give up polling for the live content height after this long and restore to
|
||||||
|
// the furthest reachable position (handles "collab never finishes laying out").
|
||||||
|
const MAX_RESTORE_WAIT_MS = 5000;
|
||||||
|
// How often to re-check the document height while waiting for content to load.
|
||||||
|
const RESTORE_POLL_MS = 100;
|
||||||
|
|
||||||
|
// sessionStorage key prefix. sessionStorage survives an F5 in the same tab and
|
||||||
|
// is cleared on tab close, which is exactly the lifetime we want for an MVP
|
||||||
|
// "remember where I was reading" feature (self-limiting, no cross-tab leak).
|
||||||
|
const STORAGE_PREFIX = "gitmost:scroll-position:";
|
||||||
|
|
||||||
|
function storageKey(pageId: string): string {
|
||||||
|
return `${STORAGE_PREFIX}${pageId}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
// All storage access is wrapped: private mode / quota / disabled storage must
|
||||||
|
// never throw out of the hook and break the page.
|
||||||
|
function readStorage(pageId: string): number | null {
|
||||||
|
try {
|
||||||
|
const raw = window.sessionStorage.getItem(storageKey(pageId));
|
||||||
|
if (raw === null) return null;
|
||||||
|
const value = Number.parseInt(raw, 10);
|
||||||
|
return Number.isFinite(value) ? value : null;
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function writeStorage(pageId: string, scrollY: number): void {
|
||||||
|
try {
|
||||||
|
window.sessionStorage.setItem(storageKey(pageId), String(Math.round(scrollY)));
|
||||||
|
} catch {
|
||||||
|
// Silently ignore: storage unavailable (private mode / quota exceeded).
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Persists and restores the window scroll position per page so a reader keeps
|
||||||
|
* their place across a reload (F5) or reopening the document.
|
||||||
|
*
|
||||||
|
* Returns `restoreScrollPosition`, which the page editor calls once the live
|
||||||
|
* (non-static) content is laid out. The two scroll mechanisms are mutually
|
||||||
|
* exclusive: if the URL has a `#hash` anchor, the existing anchor-scroll logic
|
||||||
|
* wins and restore is a no-op.
|
||||||
|
*/
|
||||||
|
export function useScrollPosition(pageId: string): {
|
||||||
|
restoreScrollPosition: () => void;
|
||||||
|
} {
|
||||||
|
// The target Y captured synchronously at mount, BEFORE any scroll/visibility
|
||||||
|
// handler can overwrite the stored value with a fresh 0 (the page starts
|
||||||
|
// scrolled to top on load). `null` means "not yet captured".
|
||||||
|
const initialTargetRef = useRef<number | null>(null);
|
||||||
|
// Guards so restore runs at most once per page mount.
|
||||||
|
const hasRestoredRef = useRef(false);
|
||||||
|
// Holds the in-flight restore poll timer so the cleanup can cancel it: without
|
||||||
|
// this, a fast SPA navigation away mid-poll would let the old page's poll fire
|
||||||
|
// window.scrollTo against the NEW page's document (visible wrong-page scroll).
|
||||||
|
const pollTimerRef = useRef<number | null>(null);
|
||||||
|
|
||||||
|
// Capture the previously-saved value synchronously during render, before the
|
||||||
|
// effect below registers handlers that would persist the current (0) scrollY.
|
||||||
|
if (initialTargetRef.current === null) {
|
||||||
|
const saved = readStorage(pageId);
|
||||||
|
// Store 0 when nothing is saved so the "already captured" check (!== null)
|
||||||
|
// holds; restore treats targetY <= 0 as a no-op anyway.
|
||||||
|
initialTargetRef.current = saved ?? 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
let throttleTimer: number | null = null;
|
||||||
|
|
||||||
|
const save = () => {
|
||||||
|
writeStorage(pageId, window.scrollY);
|
||||||
|
};
|
||||||
|
|
||||||
|
// Throttle the high-frequency scroll handler: persist immediately on the
|
||||||
|
// leading edge, then at most once per SAVE_THROTTLE_MS.
|
||||||
|
const onScroll = () => {
|
||||||
|
if (throttleTimer !== null) return;
|
||||||
|
save();
|
||||||
|
throttleTimer = window.setTimeout(() => {
|
||||||
|
throttleTimer = null;
|
||||||
|
}, SAVE_THROTTLE_MS);
|
||||||
|
};
|
||||||
|
|
||||||
|
// pagehide fires on reload/navigation (more reliable than unload); save now.
|
||||||
|
const onPageHide = () => {
|
||||||
|
save();
|
||||||
|
};
|
||||||
|
|
||||||
|
// Save when the tab is being backgrounded — covers mobile where pagehide is
|
||||||
|
// not always emitted.
|
||||||
|
const onVisibilityChange = () => {
|
||||||
|
if (document.visibilityState === "hidden") {
|
||||||
|
save();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
window.addEventListener("scroll", onScroll, { passive: true });
|
||||||
|
window.addEventListener("pagehide", onPageHide);
|
||||||
|
document.addEventListener("visibilitychange", onVisibilityChange);
|
||||||
|
|
||||||
|
return () => {
|
||||||
|
window.removeEventListener("scroll", onScroll);
|
||||||
|
window.removeEventListener("pagehide", onPageHide);
|
||||||
|
document.removeEventListener("visibilitychange", onVisibilityChange);
|
||||||
|
if (throttleTimer !== null) {
|
||||||
|
window.clearTimeout(throttleTimer);
|
||||||
|
throttleTimer = null;
|
||||||
|
}
|
||||||
|
// Cancel any in-flight restore poll so it cannot scroll the next page.
|
||||||
|
if (pollTimerRef.current !== null) {
|
||||||
|
window.clearTimeout(pollTimerRef.current);
|
||||||
|
pollTimerRef.current = null;
|
||||||
|
}
|
||||||
|
// SPA navigation away from this page: persist the final position.
|
||||||
|
save();
|
||||||
|
};
|
||||||
|
}, [pageId]);
|
||||||
|
|
||||||
|
const restoreScrollPosition = useCallback(() => {
|
||||||
|
// Run at most once per page mount.
|
||||||
|
if (hasRestoredRef.current) return;
|
||||||
|
hasRestoredRef.current = true;
|
||||||
|
|
||||||
|
// Anchor priority: a `#hash` in the URL is handled by useEditorScroll.
|
||||||
|
if (window.location.hash) return;
|
||||||
|
|
||||||
|
const targetY = initialTargetRef.current ?? 0;
|
||||||
|
// Nothing meaningful to restore to.
|
||||||
|
if (targetY <= 0) return;
|
||||||
|
|
||||||
|
const start = Date.now();
|
||||||
|
|
||||||
|
const tryRestore = () => {
|
||||||
|
const maxScroll =
|
||||||
|
document.documentElement.scrollHeight - window.innerHeight;
|
||||||
|
const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
|
||||||
|
|
||||||
|
// Restore once the content is tall enough to reach the target, or bail out
|
||||||
|
// after the timeout and scroll as far as currently possible.
|
||||||
|
if (maxScroll >= targetY || timedOut) {
|
||||||
|
window.scrollTo({
|
||||||
|
top: Math.min(targetY, Math.max(maxScroll, 0)),
|
||||||
|
behavior: "auto",
|
||||||
|
});
|
||||||
|
pollTimerRef.current = null;
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Stored in a ref so the effect cleanup can cancel it on unmount.
|
||||||
|
pollTimerRef.current = window.setTimeout(tryRestore, RESTORE_POLL_MS);
|
||||||
|
};
|
||||||
|
|
||||||
|
tryRestore();
|
||||||
|
}, []);
|
||||||
|
|
||||||
|
return { restoreScrollPosition };
|
||||||
|
}
|
||||||
@@ -77,6 +77,7 @@ import { PageEditMode } from "@/features/user/types/user.types.ts";
|
|||||||
import { jwtDecode } from "jwt-decode";
|
import { jwtDecode } from "jwt-decode";
|
||||||
import { searchSpotlight } from "@/features/search/constants.ts";
|
import { searchSpotlight } from "@/features/search/constants.ts";
|
||||||
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
||||||
|
import { useScrollPosition } from "./hooks/use-scroll-position";
|
||||||
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
||||||
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
||||||
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
||||||
@@ -141,6 +142,7 @@ export default function PageEditor({
|
|||||||
[isComponentMounted],
|
[isComponentMounted],
|
||||||
);
|
);
|
||||||
const { handleScrollTo } = useEditorScroll({ canScroll });
|
const { handleScrollTo } = useEditorScroll({ canScroll });
|
||||||
|
const { restoreScrollPosition } = useScrollPosition(pageId);
|
||||||
// Providers only created once per pageId
|
// Providers only created once per pageId
|
||||||
const providersRef = useRef<{
|
const providersRef = useRef<{
|
||||||
local: IndexeddbPersistence;
|
local: IndexeddbPersistence;
|
||||||
@@ -479,6 +481,11 @@ export default function PageEditor({
|
|||||||
}
|
}
|
||||||
}, [yjsConnectionStatus, isSynced]);
|
}, [yjsConnectionStatus, isSynced]);
|
||||||
|
|
||||||
|
// Restore the saved reading position once the live content is laid out.
|
||||||
|
useEffect(() => {
|
||||||
|
if (!showStatic && editor) restoreScrollPosition();
|
||||||
|
}, [showStatic, editor, restoreScrollPosition]);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<TransclusionLookupProvider>
|
<TransclusionLookupProvider>
|
||||||
<PageEmbedLookupProvider>
|
<PageEmbedLookupProvider>
|
||||||
|
|||||||
Reference in New Issue
Block a user