diff --git a/apps/client/src/features/editor/hooks/use-scroll-position.ts b/apps/client/src/features/editor/hooks/use-scroll-position.ts
index e6500b9d..8c2459fb 100644
--- a/apps/client/src/features/editor/hooks/use-scroll-position.ts
+++ b/apps/client/src/features/editor/hooks/use-scroll-position.ts
@@ -1,4 +1,5 @@
-import { useCallback, useEffect, useRef } from "react";
+import { useCallback, useEffect, useLayoutEffect, useRef } from "react";
+import type { Editor } from "@tiptap/react";
// Throttle interval for persisting the scroll position while the user reads.
const SAVE_THROTTLE_MS = 250;
@@ -243,3 +244,37 @@ export function useScrollPosition(pageId: string): {
return { restoreScrollPosition };
}
+
+/**
+ * Wires `useScrollPosition` to the page editor's static->live swap lifecycle.
+ *
+ * Extracted from PageEditor so the exact restore triggers (their deps and the
+ * post-swap `&& editor` guard) are directly unit-testable rather than mirrored.
+ * Behaviour is unchanged: `restoreScrollPosition` is idempotent, so re-asserting
+ * the same target from either trigger is a no-op.
+ *
+ * @param pageId the page whose scroll position is persisted/restored.
+ * @param editor the tiptap editor instance, or `null` until it is ready.
+ * @param showStatic whether the static (cached) content is still shown.
+ */
+export function useScrollRestoreOnSwap(
+ pageId: string,
+ editor: Editor | null,
+ showStatic: boolean,
+): void {
+ const { restoreScrollPosition } = useScrollPosition(pageId);
+
+ // Restore as early as the static (cached) content is laid out, before paint,
+ // so the reader's position is applied without a visible jump. Aborts itself if
+ // the reader has already started scrolling (handled inside the hook).
+ useLayoutEffect(() => {
+ restoreScrollPosition();
+ }, [restoreScrollPosition]);
+
+ // Re-assert once after the static -> live editor swap in case the swap reset
+ // the window scroll. Idempotent: a no-op when the position is already correct,
+ // and a no-op after the reader has interacted.
+ useLayoutEffect(() => {
+ if (!showStatic && editor) restoreScrollPosition();
+ }, [showStatic, editor, restoreScrollPosition]);
+}
diff --git a/apps/client/src/features/editor/page-editor.test.tsx b/apps/client/src/features/editor/page-editor.test.tsx
index 123045c6..57d3fab8 100644
--- a/apps/client/src/features/editor/page-editor.test.tsx
+++ b/apps/client/src/features/editor/page-editor.test.tsx
@@ -1,15 +1,16 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, act } from "@testing-library/react";
-import { useLayoutEffect, useState } from "react";
-import { useScrollPosition } from "./hooks/use-scroll-position";
+import type { Editor } from "@tiptap/react";
+import { useScrollRestoreOnSwap } from "./hooks/use-scroll-position";
const KEY_PREFIX = "gitmost:scroll-position:";
// NOTE ON SCOPE (F2 — reviewer-approved lighter variant).
//
-// The real UX wiring lives in page-editor.tsx as two useLayoutEffects around the
-// useScrollPosition hook. A FULL PageEditor component test is impractical here and
-// has no precedent in this client: PageEditor directly constructs a
+// The real UX wiring lives in the exported `useScrollRestoreOnSwap` hook (two
+// useLayoutEffects around useScrollPosition), which PageEditor calls with the
+// same signature. A FULL PageEditor component test is impractical here and has no
+// precedent in this client: PageEditor directly constructs a
// HocuspocusProviderWebsocket + IndexeddbPersistence, a tiptap `useEditor` with
// collab extensions, reads jotai atoms, react-router params, the shared
// `queryClient` from main.tsx, i18n, and mounts ~12 editor menu children. Worse,
@@ -20,35 +21,17 @@ const KEY_PREFIX = "gitmost:scroll-position:";
// single leaf component with ONE mocked query; nothing mounts a feature root of
// this weight. Reproducing all of that would test the mocks, not the wiring.
//
-// So this file tests the same integration at the level that carries the real
-// contract: the two useLayoutEffect blocks are reproduced VERBATIM from
-// page-editor.tsx (the early pre-paint restore, and the post-swap re-assert with
-// deps [showStatic, editor]) and exercised against the REAL useScrollPosition
-// hook. If page-editor's wiring regresses (e.g. the swap effect drops the
-// `&& editor` guard or its deps), the mirror below regresses in lockstep.
-
-// Mirror of page-editor.tsx lines ~489-498 (the two scroll-restore useLayoutEffects).
-function ScrollRestoreWiring({
- restoreScrollPosition,
- showStatic,
- editor,
-}: {
- restoreScrollPosition: () => void;
- showStatic: boolean;
- editor: unknown | null;
-}) {
- // Restore as early as the static (cached) content is laid out, before paint.
- useLayoutEffect(() => {
- restoreScrollPosition();
- }, [restoreScrollPosition]);
-
- // Re-assert once after the static -> live editor swap.
- useLayoutEffect(() => {
- if (!showStatic && editor) restoreScrollPosition();
- }, [showStatic, editor, restoreScrollPosition]);
-
- return null;
-}
+// So this file tests the REAL `useScrollRestoreOnSwap` hook — the exact code
+// PageEditor imports and calls — driving its `showStatic`/`editor` inputs the way
+// the swap does. Because it exercises the real hook (not a copy), dropping the
+// `&& editor` guard or changing the effect deps makes these tests fail; they
+// guard the production code directly (verified: removing `&& editor` reddens the
+// first test).
+//
+// Both tests observe the real effect via `window.scrollTo`. The stubbed
+// `window.scrollTo` never mutates `window.scrollY`, and the target is left
+// unreached, so every restore invocation that passes the guard yields exactly one
+// `scrollTo` call — making the call count a faithful proxy for restore invocations.
function setScrollY(value: number): void {
Object.defineProperty(window, "scrollY", { configurable: true, value });
@@ -63,7 +46,25 @@ function setInnerHeight(value: number): void {
Object.defineProperty(window, "innerHeight", { configurable: true, value });
}
-describe("PageEditor scroll-restore wiring (two useLayoutEffects)", () => {
+// Minimal stand-in for the tiptap editor: the hook only truthiness-checks it.
+const fakeEditor = { id: "editor" } as unknown as Editor;
+
+// Thin host that calls the REAL hook so a rerender drives showStatic/editor
+// exactly like the page-editor swap does.
+function Host({
+ pageId,
+ showStatic,
+ editor,
+}: {
+ pageId: string;
+ showStatic: boolean;
+ editor: Editor | null;
+}) {
+ useScrollRestoreOnSwap(pageId, editor, showStatic);
+ return null;
+}
+
+describe("PageEditor scroll-restore wiring (useScrollRestoreOnSwap)", () => {
beforeEach(() => {
window.sessionStorage.clear();
setScrollY(0);
@@ -79,81 +80,52 @@ describe("PageEditor scroll-restore wiring (two useLayoutEffects)", () => {
window.location.hash = "";
});
- it("re-invokes restoreScrollPosition after the swap, with the [showStatic, editor] deps", () => {
- // A referentially STABLE spy, mirroring page-editor where restoreScrollPosition
- // is a useCallback([]) — so the early effect (dep [restoreScrollPosition]) runs
- // exactly once and does NOT re-fire on every render.
- const restore = vi.fn();
-
- const editor = { id: "editor" };
-
- // Host owns the swap state so we can drive showStatic/editor like page-editor.
- function Host({
- showStatic,
- editorValue,
- }: {
- showStatic: boolean;
- editorValue: unknown | null;
- }) {
- return (
-
- );
- }
+ it("re-invokes restore after the swap, with the [showStatic, editor] deps/guard", () => {
+ // Target is immediately reachable, so each restore that passes the guard
+ // scrolls synchronously. `window.scrollY` stays 0 (stubbed scrollTo never
+ // updates it), so scrollTo is called once per effective restore — a proxy for
+ // the restore invocation count.
+ window.sessionStorage.setItem(`${KEY_PREFIX}guard`, "500");
+ setInnerHeight(800);
+ setScrollHeight(2000); // maxScroll = 1200 >= 500: reachable, no polling.
// Pre-swap: static content shown, live editor not ready. Only the early
// pre-paint restore fires; the post-swap effect's guard (!showStatic) blocks it.
- const { rerender } = render();
- expect(restore).toHaveBeenCalledTimes(1);
+ const { rerender } = render(
+ ,
+ );
+ expect(window.scrollTo).toHaveBeenCalledTimes(1);
// Collab reports synced (showStatic flips false) but the editor is not ready
- // yet: the swap effect runs but the `&& editor` guard must keep it a no-op.
- // (Pins the guard: dropping `&& editor` would restore against a null editor.)
- rerender();
- expect(restore).toHaveBeenCalledTimes(1);
+ // yet: the swap effect re-runs (deps [showStatic, editor] changed) but the
+ // `&& editor` guard must keep it a no-op. The early effect does NOT re-fire
+ // (its dep [restoreScrollPosition] is a stable useCallback([])).
+ // (Pins the guard: dropping `&& editor` would restore against a null editor,
+ // producing a 2nd scrollTo and failing this expectation.)
+ rerender();
+ expect(window.scrollTo).toHaveBeenCalledTimes(1);
// The static -> live swap completes (showStatic false AND editor present): the
- // post-swap effect re-asserts the restore exactly once more. The early effect
- // does NOT re-fire (restore identity is stable), so this second call is driven
- // solely by the [showStatic, editor] deps changing.
- rerender();
- expect(restore).toHaveBeenCalledTimes(2);
+ // post-swap effect re-asserts the restore exactly once more, driven solely by
+ // the [showStatic, editor] deps changing.
+ rerender();
+ expect(window.scrollTo).toHaveBeenCalledTimes(2);
});
it("the post-swap re-assert drives a REAL restore (window.scrollTo) via the hook", () => {
- // End-to-end through the real useScrollPosition: the swap re-invocation is the
- // CAUSE of the scroll (nothing scrolls before it).
+ // End-to-end through the real useScrollPosition (inside the hook): the swap
+ // re-invocation is the CAUSE of the scroll (nothing scrolls before it).
vi.useFakeTimers();
window.sessionStorage.setItem(`${KEY_PREFIX}peg`, "500");
setInnerHeight(800);
setScrollHeight(100); // maxScroll = -700: target not reachable yet -> polls.
- const editor = { id: "editor" };
-
- function Host({
- showStatic,
- editorValue,
- }: {
- showStatic: boolean;
- editorValue: unknown | null;
- }) {
- const { restoreScrollPosition } = useScrollPosition("peg");
- return (
-
- );
- }
-
// Pre-swap: the early restore runs but content is too short, so it starts
// polling (a pending timer) without scrolling. We never advance timers, so the
// early poll cannot fire on its own — isolating the swap as the sole cause.
- const { rerender } = render();
+ const { rerender } = render(
+ ,
+ );
expect(window.scrollTo).not.toHaveBeenCalled();
// The live content is now laid out tall enough to reach the target.
@@ -162,7 +134,7 @@ describe("PageEditor scroll-restore wiring (two useLayoutEffects)", () => {
// The static -> live swap: the post-swap useLayoutEffect re-invokes the real
// hook, whose synchronous tryRestore now reaches the target and scrolls.
act(() => {
- rerender();
+ rerender();
});
expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
});
diff --git a/apps/client/src/features/editor/page-editor.tsx b/apps/client/src/features/editor/page-editor.tsx
index ada24872..b001051f 100644
--- a/apps/client/src/features/editor/page-editor.tsx
+++ b/apps/client/src/features/editor/page-editor.tsx
@@ -2,7 +2,6 @@ import "@/features/editor/styles/index.css";
import React, {
useCallback,
useEffect,
- useLayoutEffect,
useMemo,
useRef,
useState,
@@ -79,7 +78,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 { useScrollRestoreOnSwap } 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";
@@ -144,7 +143,6 @@ export default function PageEditor({
[isComponentMounted],
);
const { handleScrollTo } = useEditorScroll({ canScroll });
- const { restoreScrollPosition } = useScrollPosition(pageId);
// Providers only created once per pageId
const providersRef = useRef<{
local: IndexeddbPersistence;
@@ -483,19 +481,10 @@ export default function PageEditor({
}
}, [yjsConnectionStatus, isSynced]);
- // Restore as early as the static (cached) content is laid out, before paint,
- // so the reader's position is applied without a visible jump. Aborts itself if
- // the reader has already started scrolling (handled inside the hook).
- useLayoutEffect(() => {
- restoreScrollPosition();
- }, [restoreScrollPosition]);
-
- // Re-assert once after the static -> live editor swap in case the swap reset
- // the window scroll. Idempotent: a no-op when the position is already correct,
- // and a no-op after the reader has interacted.
- useLayoutEffect(() => {
- if (!showStatic && editor) restoreScrollPosition();
- }, [showStatic, editor, restoreScrollPosition]);
+ // Restore the reader's scroll position across the static -> live editor swap.
+ // The wiring (early pre-paint restore + post-swap re-assert) lives in the hook
+ // so its triggers/guard are directly unit-testable.
+ useScrollRestoreOnSwap(pageId, editor, showStatic);
return (