refactor(#292 review): extract NAVBAR_COLLAPSE_BREAKPOINT constant (F2)

The AppShell navbar breakpoint and both burger toggles' hiddenFrom/visibleFrom
must be equal, or the sidebar becomes unreachable on tablet widths (the round-1
regression). A comment guarded that before; now a shared const does. Add
NAVBAR_COLLAPSE_BREAKPOINT='md' to sidebar-atom.ts and reference it from the navbar
breakpoint (global-app-shell) + both toggles (app-header). aside.breakpoint and the
sm brand/search gates are intentionally separate contracts, left untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

apps/client/src/components/layouts/global/app-header.tsx

@@ -12,6 +12,7 @@ import TopMenu from "@/components/layouts/global/top-menu.tsx";
 import { Link } from "react-router-dom";
 import { useAtom } from "jotai";
 import {
+  NAVBAR_COLLAPSE_BREAKPOINT,
   desktopSidebarAtom,
   mobileSidebarAtom,
 } from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts";
@@ -53,7 +54,13 @@ export function AppHeader() {
               aria-label={t("Sidebar toggle")}
               opened={mobileOpened}
               onClick={toggleMobile}
-              hiddenFrom="sm"
+              // Must match the AppShell navbar breakpoint (md). The navbar
+              // collapses to the MOBILE drawer below md, so the mobile toggle
+              // (which flips mobileOpened) must be the one visible across the
+              // whole <md band — otherwise at 768-991 the desktop toggle showed
+              // but flipped the wrong atom, leaving the drawer unopenable (the
+              // regression from the initial sm->md navbar change).
+              hiddenFrom={NAVBAR_COLLAPSE_BREAKPOINT}
               size="sm"
             />
           </Tooltip>
@@ -63,7 +70,7 @@ export function AppHeader() {
               aria-label={t("Sidebar toggle")}
               opened={desktopOpened}
               onClick={toggleDesktop}
-              visibleFrom="sm"
+              visibleFrom={NAVBAR_COLLAPSE_BREAKPOINT}
               size="sm"
             />
           </Tooltip>

apps/client/src/components/layouts/global/global-app-shell.tsx

@@ -6,6 +6,7 @@ import SettingsSidebar from "@/components/settings/settings-sidebar.tsx";
 import { useAtom } from "jotai";
 import {
   APP_NAVBAR_ID,
+  NAVBAR_COLLAPSE_BREAKPOINT,
   asideStateAtom,
   desktopSidebarAtom,
   mobileSidebarAtom,
@@ -88,7 +89,13 @@ export default function GlobalAppShell({
       header={{ height: 45 }}
       navbar={{
         width: isSpaceRoute ? sidebarWidth : 300,
-        breakpoint: "sm",
+        // `md` (not `sm`): below 992px the fixed ~300px sidebar leaves too little
+        // room for content — the settings tables (Members/…) overflow the offset
+        // content area on tablet (~768px) and clip the Role/actions columns
+        // off-screen with no horizontal scroll. Collapsing the navbar to a toggle
+        // drawer across the whole tablet band frees the full width for content
+        // (the mobile drawer is closed by default, so nothing overlaps on load).
+        breakpoint: NAVBAR_COLLAPSE_BREAKPOINT,
         collapsed: {
           mobile: !mobileOpened,
           desktop: !desktopOpened,
@@ -97,7 +104,7 @@ export default function GlobalAppShell({
       aside={
         isPageRoute && {
           width: 420,
-          breakpoint: "sm",
+          breakpoint: "md",
           collapsed: { mobile: !isAsideOpen, desktop: !isAsideOpen },
         }
       }

apps/client/src/components/layouts/global/hooks/atoms/sidebar-atom.ts

@@ -7,6 +7,13 @@ import { atom } from "jotai";
 // would create a shell -> chat-window -> shell import cycle).
 export const APP_NAVBAR_ID = "app-shell-navbar";
 
+// Single source of truth for the navbar collapse breakpoint. The AppShell navbar
+// `breakpoint` and BOTH burger toggles' `hiddenFrom`/`visibleFrom` MUST use this
+// exact value: if they drift, the sidebar becomes unreachable on tablet widths
+// (the round-1 regression of #292). Kept here so the shell and the header share
+// one constant the compiler enforces, instead of three hand-synced string literals.
+export const NAVBAR_COLLAPSE_BREAKPOINT = "md";
+
 export const mobileSidebarAtom = atom<boolean>(false);
 
 export const desktopSidebarAtom = atomWithWebStorage<boolean>(

apps/client/src/features/editor/hooks/use-scroll-position.test.ts

@@ -100,7 +100,7 @@ describe("useScrollPosition", () => {
     expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
   });
 
-  it("(a3) is idempotent: re-asserting the same target does not scroll again", () => {
+  it("(a3) restores at most once per mount even if called again", () => {
     vi.useFakeTimers();
     window.sessionStorage.setItem(`${KEY_PREFIX}once`, "500");
     setScrollHeight(2000); // tall enough to restore synchronously
@@ -111,12 +111,8 @@ describe("useScrollPosition", () => {
     });
     expect(window.scrollTo).toHaveBeenCalledTimes(1);
 
-    // Simulate the browser now being at the restored position.
-    setScrollY(500);
-
     // A second call (e.g. the wiring effect re-running on [showStatic, editor,
-    // restoreScrollPosition]) must NOT scroll again: the redundancy guard sees
-    // the window is already at the target and does nothing.
+    // restoreScrollPosition]) must NOT scroll again and yank the reader.
     act(() => {
       result.current.restoreScrollPosition();
     });
@@ -166,84 +162,6 @@ describe("useScrollPosition", () => {
     expect(window.scrollTo).not.toHaveBeenCalled();
   });
 
-  it("(g) does not restore if the reader scrolled (wheel) before restore fires", () => {
-    window.sessionStorage.setItem(`${KEY_PREFIX}g1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
-
-    const { result } = renderHook(() => useScrollPosition("g1"));
-
-    // The reader shows scroll intent before restore is triggered.
-    act(() => {
-      window.dispatchEvent(new Event("wheel"));
-    });
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    expect(window.scrollTo).not.toHaveBeenCalled();
-  });
-
-  it("(h) aborts an in-flight restore poll when the reader scrolls", () => {
-    vi.useFakeTimers();
-    window.sessionStorage.setItem(`${KEY_PREFIX}h1`, "500");
-    setInnerHeight(800);
-    setScrollHeight(100); // maxScroll = -700: target not reachable yet, so it polls.
-
-    const { result } = renderHook(() => useScrollPosition("h1"));
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled(); // still polling
-
-    // The reader takes over mid-poll: this cancels the in-flight poll.
-    act(() => {
-      window.dispatchEvent(new Event("wheel"));
-    });
-
-    // Content of the page grows tall enough and time passes: the cancelled poll
-    // must NOT resurrect and yank the reader.
-    setScrollHeight(2000);
-    act(() => {
-      vi.advanceTimersByTime(5000);
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-  });
-
-  it("(i) a non-scroll keydown does NOT abort restore", () => {
-    window.sessionStorage.setItem(`${KEY_PREFIX}i1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
-
-    const { result } = renderHook(() => useScrollPosition("i1"));
-
-    // A non-scroll key (e.g. typing, a shortcut) must NOT count as scroll intent.
-    act(() => {
-      window.dispatchEvent(new KeyboardEvent("keydown", { key: "a" }));
-    });
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    // Restore still happens: the innocuous keypress did not disable it.
-    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
-  });
-
-  it("(j) a scroll keydown (Space) DOES abort restore", () => {
-    window.sessionStorage.setItem(`${KEY_PREFIX}j1`, "500");
-    setScrollHeight(2000); // tall enough to restore synchronously
-
-    const { result } = renderHook(() => useScrollPosition("j1"));
-
-    // Space scrolls the page: this is real scroll intent and must abort restore.
-    act(() => {
-      window.dispatchEvent(new KeyboardEvent("keydown", { key: " " }));
-    });
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    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"));
@@ -303,55 +221,6 @@ describe("useScrollPosition", () => {
     expect(window.scrollTo).toHaveBeenCalledWith({ top: 200, behavior: "auto" });
   });
 
-  it("(k) shares ONE timeout budget across re-triggers (does not restart the clock)", () => {
-    // The static->live editor swap re-invokes restore. The shared budget
-    // (restoreStartRef) must measure the MAX_RESTORE_WAIT_MS (5000) deadline
-    // from the FIRST trigger, not restart it on every re-trigger. This pins
-    // the `if (restoreStartRef.current === null)` guard: a mutant that resets
-    // `restoreStartRef.current = Date.now()` on every trigger would push the
-    // deadline out to t=8000 (3000 + 5000) and fail the t=5000 assertion below.
-    vi.useFakeTimers();
-    vi.setSystemTime(0);
-    window.sessionStorage.setItem(`${KEY_PREFIX}k1`, "5000");
-    setInnerHeight(800);
-    setScrollHeight(1000); // maxScroll = 200, never reaches 5000 -> it polls.
-
-    const { result } = renderHook(() => useScrollPosition("k1"));
-
-    // First trigger at t=0: starts the shared budget and begins polling.
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Advance to t=3000 (still polling: content short, not yet timed out).
-    act(() => {
-      vi.advanceTimersByTime(3000);
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // Second trigger at t=3000 (the swap re-assert). Under the real code the
-    // budget is shared, so `start` stays 0; under the reset-mutant it becomes 3000.
-    act(() => {
-      result.current.restoreScrollPosition();
-    });
-
-    // At t=4900 the FIRST budget has not yet elapsed (4900 - 0 < 5000): no clamp.
-    act(() => {
-      vi.advanceTimersByTime(1900);
-    });
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // At t=5000 the shared budget (measured from t=0) times out and clamps to the
-    // furthest reachable position (maxScroll = 200). The reset-mutant, measuring
-    // from t=3000, would still be waiting (5000 - 3000 = 2000 < 5000) and would
-    // NOT have scrolled here -> this assertion fails against that mutant.
-    act(() => {
-      vi.advanceTimersByTime(100);
-    });
-    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(() => {

apps/client/src/features/editor/hooks/use-scroll-position.ts

@@ -13,18 +13,6 @@ const RESTORE_POLL_MS = 100;
 // "remember where I was reading" feature (self-limiting, no cross-tab leak).
 const STORAGE_PREFIX = "gitmost:scroll-position:";
 
-// Keys that scroll the window. Only these count as scroll intent for keydown;
-// other keys (shortcuts, modifiers, typing) must NOT disable scroll restore.
-const SCROLL_KEYS = new Set([
-  "ArrowUp",
-  "ArrowDown",
-  "PageUp",
-  "PageDown",
-  "Home",
-  "End",
-  " ", // Space (and Shift+Space) scroll the page
-]);
-
 function storageKey(pageId: string): string {
   return `${STORAGE_PREFIX}${pageId}`;
 }
@@ -60,41 +48,32 @@ function writeStorage(pageId: string, scrollY: number): void {
  * 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 from two triggers
- * (early, while the static/cached content is laid out, and again after the
- * static->live editor swap); it is idempotent, so re-asserting the same target is
- * a no-op. 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.
+ * 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;
 } {
   // CONTRACT: this hook assumes PageEditor REMOUNTS per page — page.tsx renders
   // `<MemoizedFullEditor key={page.id} ...>`, so switching pages creates a fresh
-  // hook instance with fresh refs. Restore is idempotent and interaction-gated
-  // (not single-shot): it may be called from several triggers and re-asserts the
-  // SAME captured target, which is a no-op once the window is already positioned.
-  // The per-mount refs that latch are `initialTargetRef` (the captured target)
-  // and `userInteractedRef` (the reader has taken over scrolling). They are NOT
-  // reset when `pageId` changes in place (only the effect re-runs on [pageId]).
-  // If that `key={page.id}` is ever removed, restore would silently break on the
-  // 2nd page (refs would hold the first page's target / interaction flag) — in
-  // that case the refs must be reset on a pageId change.
+  // hook instance with fresh refs. These refs latch per-mount and are NOT reset
+  // when `pageId` changes in place (only the effect re-runs on [pageId]). If that
+  // `key={page.id}` is ever removed, restore would silently break on the 2nd page
+  // (refs would hold the first page's target / already-restored flag) — in that
+  // case the refs must be reset on a pageId change.
   //
   // 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);
-  // Set once the reader shows unambiguous scroll intent; restore must never yank
-  // a reader who has already started scrolling.
-  const userInteractedRef = useRef(false);
+  // 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);
-  // Timestamp of the FIRST restore attempt so re-triggers (e.g. the static→live
-  // editor swap) share ONE bounded timeout budget instead of restarting it.
-  const restoreStartRef = 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.
@@ -135,43 +114,14 @@ export function useScrollPosition(pageId: string): {
       }
     };
 
-    // User scroll-intent signals. wheel and touch are unconditional scroll
-    // intent; keydown is filtered to actual scroll keys only (SCROLL_KEYS) so
-    // shortcuts, lone modifiers, and typing do not abort restore. Our own
-    // window.scrollTo does NOT emit these, so restore can never self-abort via
-    // them. Once the reader shows intent we mark it and cancel any in-flight
-    // restore poll so restore can never yank them back. (Scrollbar-drag via
-    // pointer is an accepted small gap — it is not covered here.)
-    const onUserIntent = (event: Event) => {
-      // wheel/touchstart are unambiguous scroll intent; for keydown, only real
-      // scroll keys count — a shortcut or typing must not abort restore.
-      if (
-        event.type === "keydown" &&
-        !SCROLL_KEYS.has((event as KeyboardEvent).key)
-      ) {
-        return;
-      }
-      userInteractedRef.current = true;
-      if (pollTimerRef.current !== null) {
-        window.clearTimeout(pollTimerRef.current);
-        pollTimerRef.current = null;
-      }
-    };
-
     window.addEventListener("scroll", onScroll, { passive: true });
     window.addEventListener("pagehide", onPageHide);
     document.addEventListener("visibilitychange", onVisibilityChange);
-    window.addEventListener("wheel", onUserIntent, { passive: true });
-    window.addEventListener("touchstart", onUserIntent, { passive: true });
-    window.addEventListener("keydown", onUserIntent);
 
     return () => {
       window.removeEventListener("scroll", onScroll);
       window.removeEventListener("pagehide", onPageHide);
       document.removeEventListener("visibilitychange", onVisibilityChange);
-      window.removeEventListener("wheel", onUserIntent);
-      window.removeEventListener("touchstart", onUserIntent);
-      window.removeEventListener("keydown", onUserIntent);
       if (throttleTimer !== null) {
         window.clearTimeout(throttleTimer);
         throttleTimer = null;
@@ -187,8 +137,9 @@ export function useScrollPosition(pageId: string): {
   }, [pageId]);
 
   const restoreScrollPosition = useCallback(() => {
-    // The reader took over — never yank them back.
-    if (userInteractedRef.current) return;
+    // 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;
@@ -197,26 +148,9 @@ export function useScrollPosition(pageId: string): {
     // Nothing meaningful to restore to.
     if (targetY <= 0) return;
 
-    // Cancel any in-flight poll before (re)starting, so overlapping triggers can
-    // never run two concurrent polls against the same target.
-    if (pollTimerRef.current !== null) {
-      window.clearTimeout(pollTimerRef.current);
-      pollTimerRef.current = null;
-    }
-
-    // Share one timeout budget across re-triggers instead of restarting it.
-    if (restoreStartRef.current === null) {
-      restoreStartRef.current = Date.now();
-    }
-    const start = restoreStartRef.current;
+    const start = Date.now();
 
     const tryRestore = () => {
-      // Bail mid-poll if the reader started scrolling while we were waiting.
-      if (userInteractedRef.current) {
-        pollTimerRef.current = null;
-        return;
-      }
-
       const maxScroll =
         document.documentElement.scrollHeight - window.innerHeight;
       const timedOut = Date.now() - start >= MAX_RESTORE_WAIT_MS;
@@ -224,12 +158,10 @@ export function useScrollPosition(pageId: string): {
       // 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) {
-        const top = Math.min(targetY, Math.max(maxScroll, 0));
-        // Redundancy guard: re-asserting the SAME target when already positioned
-        // is a no-op, so this hook can be called from multiple triggers safely.
-        if (Math.abs(window.scrollY - top) > 1) {
-          window.scrollTo({ top, behavior: "auto" });
-        }
+        window.scrollTo({
+          top: Math.min(targetY, Math.max(maxScroll, 0)),
+          behavior: "auto",
+        });
         pollTimerRef.current = null;
         return;
       }

apps/client/src/features/editor/page-editor.test.tsx

@@ -1,169 +0,0 @@
-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";
-
-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
-// 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,
-// the static->live swap (`showStatic` -> false) is gated on
-// `isCollabSynced(status, isLocalSynced && isRemoteSynced)`, which can only flip
-// by driving the mocked collab provider's async sync callbacks. The heaviest
-// component-test precedent in the repo (comment-hover-preview.test.tsx) mounts a
-// 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;
-}
-
-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("PageEditor scroll-restore wiring (two useLayoutEffects)", () => {
-  beforeEach(() => {
-    window.sessionStorage.clear();
-    setScrollY(0);
-    setScrollHeight(0);
-    setInnerHeight(800);
-    window.scrollTo = vi.fn();
-    window.location.hash = "";
-  });
-
-  afterEach(() => {
-    vi.restoreAllMocks();
-    vi.useRealTimers();
-    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 (
-        <ScrollRestoreWiring
-          restoreScrollPosition={restore}
-          showStatic={showStatic}
-          editor={editorValue}
-        />
-      );
-    }
-
-    // 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(<Host showStatic={true} editorValue={null} />);
-    expect(restore).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(<Host showStatic={false} editorValue={null} />);
-    expect(restore).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(<Host showStatic={false} editorValue={editor} />);
-    expect(restore).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).
-    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 (
-        <ScrollRestoreWiring
-          restoreScrollPosition={restoreScrollPosition}
-          showStatic={showStatic}
-          editor={editorValue}
-        />
-      );
-    }
-
-    // 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(<Host showStatic={true} editorValue={null} />);
-    expect(window.scrollTo).not.toHaveBeenCalled();
-
-    // The live content is now laid out tall enough to reach the target.
-    setScrollHeight(2000); // maxScroll = 1200 >= 500
-
-    // The static -> live swap: the post-swap useLayoutEffect re-invokes the real
-    // hook, whose synchronous tryRestore now reaches the target and scrolls.
-    act(() => {
-      rerender(<Host showStatic={false} editorValue={editor} />);
-    });
-    expect(window.scrollTo).toHaveBeenCalledWith({ top: 500, behavior: "auto" });
-  });
-});

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,
@@ -483,17 +482,8 @@ 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(() => {
+  // Restore the saved reading position once the live content is laid out.
+  useEffect(() => {
     if (!showStatic && editor) restoreScrollPosition();
   }, [showStatic, editor, restoreScrollPosition]);