Skip to content

docs(tabs): say which of the three scroll-position fields to use - #443

Merged
PathGao merged 1 commit into
masterfrom
docs/tab-scroll-position-fields
Aug 3, 2026
Merged

docs(tabs): say which of the three scroll-position fields to use#443
PathGao merged 1 commit into
masterfrom
docs/tab-scroll-position-fields

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Tab records the reading position three times — scrollTop, scrollPercentage, anchorLine — and says nothing about why one concept needs three fields or which one a caller should read. They are not interchangeable, they are not kept in step with each other, and the names do not tell you which is authoritative.

Comments only. npm test 559/559, npm run check 636 files / 0 errors, npm run build clean, all measured on 5fd9b8f.

They are a fallback cascade, not three copies

Re-activating a tab tries them most-precise-first and stops at the first that resolves — MarkdownViewer.svelte:1131-1163:

if (tab.anchorLine > 0) {  findAnchorElement(body, tab.anchorLine)  scrolled = true; }
if (!scrolled) {
  if (body.scrollHeight > body.clientHeight && tab.scrollPercentage > 0) { …proportional… }
  else { body.scrollTop = tab.scrollTop; }
}

The editor runs the same idea with a different first entry and one fewer step — editorViewState, then anchorLine via revealLineNearTop, then scrollPercentage (components/Editor.svelte:290-311). It never reads scrollTop: grep -n scrollTop src/lib/components/Editor.svelte returns one line, e.scrollTopChanged.

field what it is survives breaks on
anchorLine a source line number re-render, fold, resize, pane swap the document being replaced
scrollPercentage 0–1 of the scrollable range a container of another height any content change
scrollTop raw pixels in the preview container nothing but the same DOM at the same size everything else

The history is the clearest statement of why each was added. scrollTop is the original (bf04daa, "cursor/scroll persists between tabs"). scrollPercentage arrived as edfb555, "roughly match viewer-editor scroll location" — a ratio, because the two panes have different heights. anchorLine is #420 (d9d5447), whose first sentence is the failure the ratio produced: "Switching tabs and coming back landed on a rough percentage of the document instead of where you left off."

anchorLine is a source line both ways: the preview pins it at PREVIEW_ANCHOR_OFFSET = 60px (utils/previewAnchor.ts) and the editor at two lines below the viewport top (Editor.svelte:640 writes startLine + 2, :296 restores anchorLine - 2). That is why it is the only one that means anything across a pane swap — it names a place in the text, not a place on screen.

Split-view scroll sync does not use any of the three. It is live DOM→DOM through utils/scrollSync.ts (#442) and reads the elements' own scrollTop, not the tab's.

The part that is worth the comment

Three writers, each updating a different subset:

writer scrollTop scrollPercentage anchorLine
user scroll of the preview (MarkdownViewer.svelte:1281-1295)
programmatic scroll — sync, scroll-history (:1274-1278)
editor teardown (Editor.svelte:626-643)

So the most recently written field is not the most precise one, and one of them can be current while its neighbours are stale. That is the sentence a reader needs, and it is not derivable from the names. scrollTop also has a second, unrelated consumer — isScrolled = scrollTop > 0 (MarkdownViewer.svelte:252) drives the title bar's .scrolled shadow (TitleBar.svelte:344) — which is why the programmatic branch keeps it up to date at all.

Also worth knowing and not written down anywhere: assigning any of the three scrolls nothing. The preview restore is an $effect keyed on activeTabId and markdownBody with its body untracked, so it runs on tab activation and body remount, not on write.

All three documented, in one block

One block above scrollPercentage/anchorLine, plus a one-line pointer on scrollTop. scrollTop sits five fields away from the other two in the interface and is not moved next to them — reordering an interface is a bigger diff than the comment it would tidy.

Does not fit cleanly — reported, not fixed

navigate() resets the one field the cascade consults last. Following a Markdown link reuses the tab, and tabs.svelte.ts:786 ends that with:

tab.isDirty = false;
tab.scrollTop = 0;

scrollPercentage and anchorLine are left holding the previous document's values, and it is anchorLine the restore reads first. The scrollTop = 0 branch is reachable only when the anchor fails to resolve and scrollPercentage is 0 — so for any tab that had been scrolled before the user clicked the link, the reset cannot take effect.

The consequence is deferred rather than immediate: the restore effect does not re-run on same-tab navigation (activeTabId is unchanged, and the <article bind:this={markdownBody}> at MarkdownViewer.svelte:3299 is not inside a block that remounts on a path change), so nothing jumps at the moment of navigation. It is the next activation of that tab that resolves the old document's anchorLine against the new document and lands at whatever block now occupies that line number.

I have evidenced this from the code, not reproduced it in the running app. The fix is a behaviour change — clearing all three, or moving the reset into a single "new document in this tab" helper — and belongs in its own PR.

Secondary, same shape: the programmatic-scroll early return at MarkdownViewer.svelte:1274-1278 skips scrollPercentage/anchorLine as a side effect of the feedback-loop guard it shares a branch with (the return also skips syncEditorToPreviewScroll). So in split view, scrolling the editor moves the preview but leaves the tab's anchor where the last user preview scroll put it. Whether that is intended is not recoverable from the code, so the comment states the mechanism and not a rationale.

Not traced

  • Not reproduced in the running app; this is a source-level trace.
  • scrollHistory / scrollFuture (MarkdownViewer.svelte:2561-2587) — the in-preview scroll back/forward stack. Component-local, not on Tab, and confirmed only to the extent that it sets isProgrammaticScroll and so lands in the row above.
  • editorViewState beyond its position in the editor's cascade. It is Monaco's opaque blob and carries selection and folding as well as scroll, which makes it a different kind of thing from these three.
  • The interaction between an anchor restore and a collapsedHeaders fold state applied in the same activation.

🤖 Generated with Claude Code

`Tab` records the reading position three times - `scrollTop`,
`scrollPercentage`, `anchorLine` - with nothing saying why one concept
needs three fields or which one a caller should read.

They are three increasingly degraded answers to the same question, tried
in order until one resolves: a source line, a fraction of the scrollable
range, then raw pixels. Each survives a different kind of change, and
each is written by a different subset of the three writers, so the most
recently written is not the most precise.

One block above `scrollPercentage`/`anchorLine`, plus a one-line pointer
on `scrollTop`, which sits apart from them in the interface.

Comments only. No behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao merged commit 6b58cd5 into master Aug 3, 2026
4 checks passed
@PathGao
PathGao deleted the docs/tab-scroll-position-fields branch August 3, 2026 11:29
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