fix: keep the page still when a new highlight's note opens by gnapse · Pull Request #150 · klipstack/klipstack · GitHub
Skip to content

fix: keep the page still when a new highlight's note opens - #150

Open
gnapse wants to merge 2 commits into
mainfrom
fix/note-editor-focus-scroll
Open

gnapse wants to merge 2 commits into
mainfrom
fix/note-editor-focus-scroll

Conversation

@gnapse

@gnapse gnapse commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Why

Sometimes "Highlight and add a note" scrolled the reader up to the top of the article, and the empty note waited, focused, further down. A new note's editor appears at the top of the article, invisible, until the margin measures it on the next frame. In about half of the tries it took focus while it was still up there, and the browser could scroll to the caret.

What changes

  • In the margin, a new note's editor takes focus only once it stands beside its highlight, one frame later than before. Editing an existing note, the floating editor, the phone sheet and Home are unchanged.
  • An editor that already has focus keeps its caret if the margin lays out again while you type.
  • The textarea fallback (editor chunk unavailable, e.g. offline) focuses itself the same way. Its native autoFocus acts only on mount, so it would have stayed unfocused.

Verification

  • The extended e2e test "Highlight and add a note" adds five notes further down the article. For each one it checks where the editor stands when it takes focus and that the page did not scroll. Against the production build:
    • with origin/main's components it failed 3 of 3 runs ("editor at -996");
    • with the fix it passed 3 of 3.
  • All of highlights.spec.ts, home-highlights.spec.ts and the notebook specs pass, and pnpm verify passes.
  • Fallback: a scratch spec that blocks the editor chunk showed the textarea focused beside its highlight with the page still. Without the change, it never took focus. Not kept: it needs service workers blocked.
  • Fresh-context review ran (task, tests, public repo). Fixed from it: the exact focus count failed under the dev server's StrictMode, which focuses twice; the fallback focus; more notes and a scroll check in the test. Dismissed: an editor whose highlight is never measured is never focused. That case is theoretical, it was invisible before too, and the next click closes it with nothing lost.
  • Not verified: the browser's own scroll to the caret. Automated Chromium and WebKit never scrolled, even with the editor focused 2,000 px above the screen, so the test checks the cause: where the editor is when it takes focus.

gnapse and others added 2 commits October 21, 2026 01:41
A new highlight's note editor mounted in the margin at top 0 (the article's
top, invisible) and took focus there, before the margin's next-frame layout
moved it beside its highlight. When the browser revealed the caret in that
window, the page jumped to the top of the article, with the editor focused
further down. MarginNotes now tells the editor whether it is placed, and the
editor takes the keyboard only then.

Amp-Thread-ID: https://ampcode.com/threads/T-01a15bff-c2df-705d-908e-92fabd2f12ff
Co-authored-by: Amp <amp@ampcode.com>
…stays put

The textarea that stands in for the editor when its chunk cannot load used
the native autoFocus, which acts only on mount: in the margin it now mounts
before it is placed, so it focuses from an effect instead, as the editor does.
The regression test checks each note on its own (the dev server's StrictMode
focuses twice), over five notes, and that the page did not scroll.

Amp-Thread-ID: https://ampcode.com/threads/T-01a15bff-c2df-705d-908e-92fabd2f12ff
Co-authored-by: Amp <amp@ampcode.com>
@gnapse
gnapse enabled auto-merge (squash) October 21, 2026 02:22

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant