From 4af21494af49f2c432e4beffd51b985479123cea Mon Sep 17 00:00:00 2001 From: claude_code Date: Fri, 3 Jul 2026 05:05:17 +0300 Subject: [PATCH] fix(editor): stop the title auto-focus from yanking scroll on reload (#266) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause (confirmed via Chrome DevTools on the live app): the reading-position restore jittered on reload — it landed at the saved spot, jumped to the top, then back. The jump was NOT a height collapse: the title editor auto-focuses ~300ms after mount, and TipTap's focus scrolls the focused node into view. Since the title sits at the top of the page, that yanked window scroll to the top. Minimal fix (the fast restore mechanism is left unchanged): - Focus the title with { scrollIntoView: false } so placing the caret no longer moves the viewport. - Skip the title auto-focus entirely when a saved reading position will be restored (otherwise the caret lands in the now-off-screen title). Exported hasSavedReadingPosition() as the single source of truth. - Extracted the decision into a testable useTitleAutofocus hook (which also adds a clearTimeout cleanup, fixing a pre-existing uncancelled/destroyed-editor timer), and covered it + hasSavedReadingPosition with unit tests. Co-Authored-By: Claude Opus 4.8 --- .../editor/hooks/use-scroll-position.test.ts | 22 +++++++- .../editor/hooks/use-scroll-position.ts | 11 ++++ .../editor/hooks/use-title-autofocus.test.ts | 50 +++++++++++++++++++ .../editor/hooks/use-title-autofocus.ts | 45 +++++++++++++++++ .../src/features/editor/title-editor.tsx | 9 +--- 5 files changed, 129 insertions(+), 8 deletions(-) create mode 100644 apps/client/src/features/editor/hooks/use-title-autofocus.test.ts create mode 100644 apps/client/src/features/editor/hooks/use-title-autofocus.ts diff --git a/apps/client/src/features/editor/hooks/use-scroll-position.test.ts b/apps/client/src/features/editor/hooks/use-scroll-position.test.ts index 06debfde..cbd49456 100644 --- a/apps/client/src/features/editor/hooks/use-scroll-position.test.ts +++ b/apps/client/src/features/editor/hooks/use-scroll-position.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { renderHook, act } from "@testing-library/react"; -import { useScrollPosition } from "./use-scroll-position"; +import { useScrollPosition, hasSavedReadingPosition } from "./use-scroll-position"; const KEY_PREFIX = "gitmost:scroll-position:"; @@ -372,3 +372,23 @@ describe("useScrollPosition", () => { }).not.toThrow(); }); }); + +describe("hasSavedReadingPosition", () => { + beforeEach(() => { + window.sessionStorage.clear(); + }); + + it("returns false when nothing is saved for the page", () => { + expect(hasSavedReadingPosition("none")).toBe(false); + }); + + it("returns false when the saved value is 0 (page stays at the top)", () => { + window.sessionStorage.setItem(`${KEY_PREFIX}zero`, "0"); + expect(hasSavedReadingPosition("zero")).toBe(false); + }); + + it("returns true when a positive position is saved", () => { + window.sessionStorage.setItem(`${KEY_PREFIX}deep`, "500"); + expect(hasSavedReadingPosition("deep")).toBe(true); + }); +}); 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 8c2459fb..b92caf40 100644 --- a/apps/client/src/features/editor/hooks/use-scroll-position.ts +++ b/apps/client/src/features/editor/hooks/use-scroll-position.ts @@ -57,6 +57,17 @@ function writeStorage(pageId: string, scrollY: number): void { } } +/** + * Whether a positive reading position is saved for this page — i.e. the page + * will be scrolled away from the top on load. Used by the title editor to avoid + * auto-focusing (and thus placing the caret in) the now-off-screen title. + * Returns false when nothing is saved or storage is unavailable. + */ +export function hasSavedReadingPosition(pageId: string): boolean { + const y = readStorage(pageId); + return typeof y === "number" && y > 0; +} + /** * Persists and restores the window scroll position per page so a reader keeps * their place across a reload (F5) or reopening the document. diff --git a/apps/client/src/features/editor/hooks/use-title-autofocus.test.ts b/apps/client/src/features/editor/hooks/use-title-autofocus.test.ts new file mode 100644 index 00000000..3f23b510 --- /dev/null +++ b/apps/client/src/features/editor/hooks/use-title-autofocus.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useTitleAutofocus } from "./use-title-autofocus"; + +const KEY_PREFIX = "gitmost:scroll-position:"; + +function fakeEditor(overrides = {}) { + return { isInitialized: true, commands: { focus: vi.fn() }, ...overrides } as any; +} + +describe("useTitleAutofocus", () => { + beforeEach(() => { + window.sessionStorage.clear(); + vi.useFakeTimers(); + }); + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it("skips auto-focus when a saved reading position exists", () => { + window.sessionStorage.setItem(`${KEY_PREFIX}saved`, "500"); + const editor = fakeEditor(); + renderHook(() => useTitleAutofocus(editor, "saved")); + act(() => vi.advanceTimersByTime(300)); + expect(editor.commands.focus).not.toHaveBeenCalled(); + }); + + it("auto-focuses a new page (no saved position) with scrollIntoView: false", () => { + const editor = fakeEditor(); + renderHook(() => useTitleAutofocus(editor, "fresh")); + act(() => vi.advanceTimersByTime(300)); + expect(editor.commands.focus).toHaveBeenCalledWith("end", { scrollIntoView: false }); + }); + + it("does not focus before initialization", () => { + const editor = fakeEditor({ isInitialized: false }); + renderHook(() => useTitleAutofocus(editor, "fresh2")); + act(() => vi.advanceTimersByTime(300)); + expect(editor.commands.focus).not.toHaveBeenCalled(); + }); + + it("cancels the pending focus on unmount", () => { + const editor = fakeEditor(); + const { unmount } = renderHook(() => useTitleAutofocus(editor, "fresh3")); + unmount(); + act(() => vi.advanceTimersByTime(300)); + expect(editor.commands.focus).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/client/src/features/editor/hooks/use-title-autofocus.ts b/apps/client/src/features/editor/hooks/use-title-autofocus.ts new file mode 100644 index 00000000..592fef6f --- /dev/null +++ b/apps/client/src/features/editor/hooks/use-title-autofocus.ts @@ -0,0 +1,45 @@ +import { useEffect, useRef } from "react"; +import type { Editor } from "@tiptap/react"; +import { hasSavedReadingPosition } from "./use-scroll-position"; + +// Delay before auto-focusing the title on load — guards a tiptap init race +// ("Cannot access view['hasFocus']" if focused too early). +const TITLE_AUTOFOCUS_DELAY_MS = 300; + +/** + * Auto-focus the page title shortly after mount — UNLESS a saved reading position + * will be restored (then the viewport scrolls away from the top, and focusing the + * top-of-page title would drop the caret off-screen). When it does focus, it uses + * `{ scrollIntoView: false }` so placing the caret never moves the viewport + * (tiptap's focus scrolls the focused node into view by default, which otherwise + * yanks the window to the top and fights scroll-position restoration). + * + * Extracted from TitleEditor so this exact decision is unit-testable. + * + * CONTRACT: relies on TitleEditor remounting per page (page.tsx renders + * ``), so `hasSavedScrollRef` is captured fresh + * per page. It is read synchronously on first render, before any scroll-save + * handler can clobber the stored value to 0 — matching `useScrollPosition`'s own + * synchronous capture of `initialTargetRef`. + */ +export function useTitleAutofocus( + titleEditor: Editor | null, + pageId: string, +): void { + const hasSavedScrollRef = useRef(null); + if (hasSavedScrollRef.current === null) { + hasSavedScrollRef.current = hasSavedReadingPosition(pageId); + } + + useEffect(() => { + if (hasSavedScrollRef.current) return; + const timer = setTimeout(() => { + // guard against "Cannot access view['hasFocus']" before init + if (!titleEditor?.isInitialized) return; + titleEditor?.commands?.focus("end", { scrollIntoView: false }); + }, TITLE_AUTOFOCUS_DELAY_MS); + // Clear the pending focus if the editor changes or the component unmounts + // (also fixes the previously-uncancelled timer). + return () => clearTimeout(timer); + }, [titleEditor]); +} diff --git a/apps/client/src/features/editor/title-editor.tsx b/apps/client/src/features/editor/title-editor.tsx index 0b1fb924..0a8534bf 100644 --- a/apps/client/src/features/editor/title-editor.tsx +++ b/apps/client/src/features/editor/title-editor.tsx @@ -28,6 +28,7 @@ import localEmitter from "@/lib/local-emitter.ts"; import { PageEditMode } from "@/features/user/types/user.types.ts"; import { searchSpotlight } from "@/features/search/constants.ts"; import { platformModifierKey } from "@/lib"; +import { useTitleAutofocus } from "@/features/editor/hooks/use-title-autofocus"; export interface TitleEditorProps { pageId: string; @@ -167,13 +168,7 @@ export function TitleEditor({ } }, [pageId, title, titleEditor]); - useEffect(() => { - setTimeout(() => { - // guard against Cannot access view['hasFocus'] error - if (!titleEditor?.isInitialized) return; - titleEditor?.commands?.focus("end"); - }, 300); - }, [titleEditor]); + useTitleAutofocus(titleEditor, pageId); useEffect(() => { return () => { -- 2.52.0