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();
|
||||
});
|
||||
});
|
||||
163
apps/client/src/features/editor/hooks/use-scroll-position.ts
Normal file
163
apps/client/src/features/editor/hooks/use-scroll-position.ts
Normal file
@@ -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 { searchSpotlight } from "@/features/search/constants.ts";
|
||||
import { useEditorScroll } from "./hooks/use-editor-scroll";
|
||||
import { useScrollPosition } from "./hooks/use-scroll-position";
|
||||
import { EditorLinkMenu } from "@/features/editor/components/link/link-menu";
|
||||
import ColumnsMenu from "@/features/editor/components/columns/columns-menu.tsx";
|
||||
import { TransclusionLookupProvider } from "@/features/editor/components/transclusion/transclusion-lookup-context";
|
||||
@@ -141,6 +142,7 @@ export default function PageEditor({
|
||||
[isComponentMounted],
|
||||
);
|
||||
const { handleScrollTo } = useEditorScroll({ canScroll });
|
||||
const { restoreScrollPosition } = useScrollPosition(pageId);
|
||||
// Providers only created once per pageId
|
||||
const providersRef = useRef<{
|
||||
local: IndexeddbPersistence;
|
||||
@@ -479,6 +481,11 @@ export default function PageEditor({
|
||||
}
|
||||
}, [yjsConnectionStatus, isSynced]);
|
||||
|
||||
// Restore the saved reading position once the live content is laid out.
|
||||
useEffect(() => {
|
||||
if (!showStatic && editor) restoreScrollPosition();
|
||||
}, [showStatic, editor, restoreScrollPosition]);
|
||||
|
||||
return (
|
||||
<TransclusionLookupProvider>
|
||||
<PageEmbedLookupProvider>
|
||||
|
||||
Reference in New Issue
Block a user