From 743fe0369e2f50ea2f001d8f2087f372e1d6eb55 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. - Tests for hasSavedReadingPosition. Co-Authored-By: Claude Opus 4.8 --- .../editor/hooks/use-scroll-position.test.ts | 22 ++++++++++++++++++- .../editor/hooks/use-scroll-position.ts | 11 ++++++++++ .../src/features/editor/title-editor.tsx | 21 ++++++++++++++++-- 3 files changed, 51 insertions(+), 3 deletions(-) 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/title-editor.tsx b/apps/client/src/features/editor/title-editor.tsx index 0b1fb924..fa8bc294 100644 --- a/apps/client/src/features/editor/title-editor.tsx +++ b/apps/client/src/features/editor/title-editor.tsx @@ -1,5 +1,5 @@ import "@/features/editor/styles/index.css"; -import React, { useCallback, useEffect, useState } from "react"; +import React, { useCallback, useEffect, useRef, useState } from "react"; import { EditorContent, useEditor } from "@tiptap/react"; import { Document } from "@tiptap/extension-document"; import { Heading } from "@tiptap/extension-heading"; @@ -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 { hasSavedReadingPosition } from "@/features/editor/hooks/use-scroll-position"; export interface TitleEditorProps { pageId: string; @@ -54,6 +55,15 @@ export function TitleEditor({ const [activePageId, setActivePageId] = useState(pageId); const currentPageEditMode = useAtomValue(currentPageEditModeAtom); + // Captured synchronously on first render (before any scroll handler can clobber + // the stored value): does this page have a saved reading position that will be + // restored (scrolling away from the top)? If so we must NOT auto-focus the + // title, or the caret lands in the now-off-screen title. + const hasSavedScrollRef = useRef(null); + if (hasSavedScrollRef.current === null) { + hasSavedScrollRef.current = hasSavedReadingPosition(pageId); + } + const titleEditor = useEditor({ extensions: [ Document.extend({ @@ -168,10 +178,17 @@ export function TitleEditor({ }, [pageId, title, titleEditor]); useEffect(() => { + // Skip title auto-focus when a saved reading position will be restored: the + // viewport scrolls away from the top, so focusing the top-of-page title would + // drop the caret off-screen (the first keystroke would go into the hidden title). + if (hasSavedScrollRef.current) return; setTimeout(() => { // guard against Cannot access view['hasFocus'] error if (!titleEditor?.isInitialized) return; - titleEditor?.commands?.focus("end"); + // Focus WITHOUT scrolling: TipTap's focus scrolls the focused node into view + // by default, and the title sits at the very top of the page — which would + // fight scroll-position restoration. We only want the caret placed. + titleEditor?.commands?.focus("end", { scrollIntoView: false }); }, 300); }, [titleEditor]);