Compare commits

..

1 Commits

Author SHA1 Message Date
claude_code 4af21494af fix(editor): stop the title auto-focus from yanking scroll on reload (#266)
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 <noreply@anthropic.com>
2026-07-03 05:15:28 +03:00
7 changed files with 138 additions and 43 deletions
@@ -1,14 +1,7 @@
import { describe, it, expect, vi } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import { render, screen } from "@testing-library/react";
import { MantineProvider } from "@mantine/core";
// Spy on the markdown renderer so we can assert it is NOT called while the block
// is collapsed (the #302 fix) and IS called once on expand. The count/fallback
// tests don't depend on real markdown, so a light stub is safe.
vi.mock("@/features/ai-chat/utils/markdown.ts", () => ({
renderChatMarkdown: vi.fn((md: string) => `<p>${md}</p>`),
}));
// Stub react-i18next so `t` returns the key with `{{count}}` interpolated. This
// keeps the assertions on the component's OWN count logic (authoritative vs
// estimate) rather than on translation, and mirrors the t-mock pattern used by
@@ -24,7 +17,6 @@ vi.mock("react-i18next", () => ({
import ReasoningBlock from "./reasoning-block";
import { estimateTokens } from "@/features/ai-chat/utils/count-stream-tokens.ts";
import { renderChatMarkdown } from "@/features/ai-chat/utils/markdown.ts";
// matchMedia (read by MantineProvider) is stubbed globally in vitest.setup.ts.
@@ -70,18 +62,4 @@ describe("ReasoningBlock", () => {
// either way the text is present in the document.
expect(screen.getByText(/reasoning/)).toBeDefined();
});
it("does not parse the reasoning markdown while collapsed; parses on expand (#302)", () => {
const renderSpy = vi.mocked(renderChatMarkdown);
renderSpy.mockClear();
renderBlock({ text: "**bold** reasoning", tokens: 5 });
// Collapsed is the default. The expensive markdown parse (marked + DOMPurify)
// must NOT run for the hidden body — that O(n^2) re-parse on every streamed
// delta is exactly what froze the chat (#302). The collapsed body shows the
// cheap raw-text fallback instead.
expect(renderSpy).not.toHaveBeenCalled();
// Expanding parses the current text exactly once (a user-initiated click).
fireEvent.click(screen.getByRole("button"));
expect(renderSpy).toHaveBeenCalledTimes(1);
});
});
@@ -34,19 +34,15 @@ function ReasoningBlock({ text, tokens }: ReasoningBlockProps) {
// Authoritative count wins; otherwise estimate live from the streamed text.
const count = tokens && tokens > 0 ? tokens : estimateTokens(text);
const trimmed = text.trim();
// Parse the reasoning markdown ONLY while the block is expanded. Collapsed is the
// default and the common case during a long "thinking" stream: reasoning text
// streams in and grows with every throttled delta (~20Hz), so a `[trimmed]`-only
// memo re-parses the whole, ever-growing text (marked + DOMPurify) on every delta
// — an O(n²) storm that pins the main thread and freezes the chat, all for a block
// the user isn't even looking at (the html is only shown inside <Collapse in={open}>
// below). Gating on `open` skips that hidden parsing entirely; expanding parses the
// current text once (an instant, user-initiated click), and further streaming while
// open is the normal per-delta append render, like the answer.
// Memoize the markdown render so toggling `open` (or a parent re-render caused
// by an unrelated streamed delta) does not re-parse the reasoning text; it
// recomputes only when the reasoning text itself changes (while it streams in).
// collapseBlankLines collapses the blank-line gaps the model emits between every
// list item / paragraph so the reasoning renders compactly (tight lists, joined
// paragraphs) — ONLY here, not in the normal answer.
const html = useMemo(
() =>
open && trimmed ? renderChatMarkdown(collapseBlankLines(trimmed), {}) : "",
[open, trimmed],
() => (trimmed ? renderChatMarkdown(collapseBlankLines(trimmed), {}) : ""),
[trimmed],
);
return (
@@ -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);
});
});
@@ -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.
@@ -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();
});
});
@@ -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
* `<MemoizedFullEditor key={page.id}>`), 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<boolean | null>(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]);
}
@@ -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 () => {