diff --git a/scripts/jumpToSelectedFragment.test.ts b/scripts/jumpToSelectedFragment.test.ts index 6dca56a2..1255f962 100644 --- a/scripts/jumpToSelectedFragment.test.ts +++ b/scripts/jumpToSelectedFragment.test.ts @@ -212,7 +212,7 @@ test('the outline and the context menu share one line-to-editor jump', () => { // singleImplementationConvention.test.ts exists to catch: the two would // scroll differently, focus differently, and only one of them would clamp. const revealHeader = functionSource(editorSource, 'revealHeader'); - assert.match(revealHeader, /revealSourceRange\(lineNumber, lineNumber\)/); + assert.match(revealHeader, /revealSourceRange\(lineNumber, lineNumber, 'top'\)/); assert.doesNotMatch(revealHeader, /setSelection\(\{[\s\S]*?startColumn: 1/); }); @@ -231,7 +231,7 @@ test('a jump asked for before Monaco has loaded is queued, not dropped', () => { // frames after the component does — and the preview calls in immediately // after flipping into edit mode. const reveal = functionSource(editorSource, 'revealSourceRange'); - assert.match(reveal, /if \(!editorReady \|\| !editor\) \{\s*\n\s*pendingReveal = \{ startLine, endLine \};/); + assert.match(reveal, /if \(!editorReady \|\| !editor\) \{\s*\n\s*pendingReveal = \{ startLine, endLine, placement \};/); // Spent after the view-state / anchor-line restore, so an explicit "edit // this fragment" wins over the position the tab was left at. @@ -396,7 +396,7 @@ test('the outline is fed body lines from both panes', () => { // lines (`getPreviewScrollAnchor`); the editor's has to be converted, or the // highlighted heading changes depending on which pane you scrolled. const fromEditor = functionSource(viewerSource, 'handleEditorScrollSync'); - assert.match(fromEditor, /tocActiveLine = lineCoords\.toRendererLine\(position\.line\)/); + assert.match(fromEditor, /tocActiveLine = tabAnchorForEditorTopLine\(lineCoords, asBufferLine\(position\.line\)\)/); const fromPreview = functionSource(viewerSource, 'getPreviewScrollAnchor'); assert.doesNotMatch(fromPreview, /toBufferLine|toRendererLine/, 'already a body line'); diff --git a/scripts/tocFollowsEditor.test.ts b/scripts/tocFollowsEditor.test.ts index 542e15df..842f653a 100644 --- a/scripts/tocFollowsEditor.test.ts +++ b/scripts/tocFollowsEditor.test.ts @@ -23,7 +23,7 @@ import test from 'node:test'; import { functionSource, readSource, sliceBetween } from './sourceTree.js'; import { activeTocIdForLine, sourceLineOf, type TocLineEntry } from '../src/lib/utils/tocFollow.js'; -import { asBufferLine, asRendererLine, lineCoordinates } from '../src/lib/utils/lineCoordinates.js'; +import { asBufferLine, asRendererLine, lineCoordinates, tabAnchorForEditorTopLine } from '../src/lib/utils/lineCoordinates.js'; /** * Every line in the outline is a BODY line, because the outline is built out of @@ -114,13 +114,16 @@ test('the editor position reaches the outline whether or not scroll sync is on', // The line is converted on the way in: `position` is in the editor's // numbering and the outline is built from `data-sourcepos`, which counts // from the body. See jumpToSelectedFragment.test.ts for that boundary. + // It goes through `tabAnchorForEditorTopLine`, the crossing the tab's own + // reading position takes, so the outline follows the line the reader is + // on rather than the one the viewport cuts in half (#744). const handler = functionSource(viewerSource, 'handleEditorScrollSync'); const record = handler.indexOf('tocActiveLine ='); const syncCheck = handler.indexOf('isScrollSynced'); assert.ok(record !== -1 && syncCheck !== -1, 'both statements must still be here'); assert.ok(record < syncCheck, 'the outline is fed before the sync check, not inside it'); - assert.match(handler, /tocActiveLine = lineCoords\.toRendererLine\(position\.line\)/); + assert.match(handler, /tocActiveLine = tabAnchorForEditorTopLine\(lineCoords, asBufferLine\(position\.line\)\)/); }); test('the editor position is converted before the outline compares it', () => { @@ -131,16 +134,26 @@ test('the editor position is converted before the outline compares it', () => { const coords = lineCoordinates(raw); assert.equal(coords.frontMatterLines, 6, 'the fixture must actually have front matter'); - // The reader is on the last line of the intro section: body line 20, which - // is buffer line 26 because six lines of YAML sit above it. - const position = asBufferLine(20 + coords.frontMatterLines); - assert.equal(activeTocIdForLine(OUTLINE, coords.toRendererLine(position)), 'intro'); + // The top of the editor's viewport is body line 18, which is buffer line + // 24 because six lines of YAML sit above it. The reader's anchor sits two + // lines below that, on body line 20: the last line of the intro section. + const topLine = asBufferLine(18 + coords.frontMatterLines); + assert.equal(activeTocIdForLine(OUTLINE, tabAnchorForEditorTopLine(coords, topLine)), 'intro'); // Handed over unconverted it is past the next heading, and the outline // highlights the section the reader has not reached yet. That is the whole // defect, and it is invisible in a document without front matter — which is // every other fixture in this file. - assert.equal(activeTocIdForLine(OUTLINE, asRendererLine(position)), 'setup'); + assert.equal(activeTocIdForLine(OUTLINE, asRendererLine(topLine)), 'setup'); + + // And with the top line one short of a heading — the heading is the first + // line fully on screen, the line above it is the one the viewport cuts — + // the anchor is on the heading and the outline has moved on. Fed the raw + // top line it would still say `intro`, one entry behind what the reader + // sees at the top of the editor. + const cutLine = asBufferLine(19 + coords.frontMatterLines); + assert.equal(activeTocIdForLine(OUTLINE, tabAnchorForEditorTopLine(coords, cutLine)), 'setup'); + assert.equal(activeTocIdForLine(OUTLINE, coords.toRendererLine(cutLine)), 'intro'); }); test('the preview feeds the same state, off the line it already measures', () => { diff --git a/src/lib/MarkdownViewer.svelte b/src/lib/MarkdownViewer.svelte index c843b714..7f56202d 100644 --- a/src/lib/MarkdownViewer.svelte +++ b/src/lib/MarkdownViewer.svelte @@ -62,8 +62,10 @@ import { type OffsetLayoutNode, } from './utils/previewAnchor.js'; import { + asBufferLine, asRendererLine, lineCoordinates, + tabAnchorForEditorTopLine, type BufferLine, type BufferLineRange, type RendererLine, @@ -1660,8 +1662,15 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu let tocActiveLine = $state(null); function handleEditorScrollSync(position: ScrollSyncPosition) { - // The outline is built from `data-sourcepos`, so it counts from the body. - if (position.line !== undefined) tocActiveLine = lineCoords.toRendererLine(position.line); + // The line the tab would record as its reading position, not the line + // the viewport cuts in half: `tabAnchorForEditorTopLine` is the one + // crossing from a Monaco top line into the outline's numbering (it + // counts from the body, and sits `EDITOR_ANCHOR_LINE_OFFSET` lines + // down). Handing over the raw top line left the outline one entry + // behind whenever a heading was the first line on screen (#744). + if (position.line !== undefined) { + tocActiveLine = tabAnchorForEditorTopLine(lineCoords, asBufferLine(position.line)); + } if (tabManager.activeTab?.isScrollSynced) { scrollPreviewToSyncPosition(position); diff --git a/src/lib/components/Editor.svelte b/src/lib/components/Editor.svelte index 9b175350..42862613 100644 --- a/src/lib/components/Editor.svelte +++ b/src/lib/components/Editor.svelte @@ -838,9 +838,9 @@ // After the view-state / anchor-line restore above, deliberately: an // explicit "edit this fragment" beats the position the tab was left at. if (pendingReveal) { - const { startLine, endLine } = pendingReveal; + const { startLine, endLine, placement } = pendingReveal; pendingReveal = null; - revealSourceRange(startLine, endLine); + revealSourceRange(startLine, endLine, placement); } return () => { @@ -2350,7 +2350,14 @@ * put it, a jump issued in the same turn as the switch into edit mode is * simply dropped. `onMount` spends it once the editor is up. */ - let pendingReveal: { startLine: number; endLine: number } | null = null; + /** + * Where a revealed range lands in the viewport. The outline asks for `top`: + * the preview answers the same click by putting the heading just under its + * top edge, and with scroll sync off the two panes then show the same + * place. The context menu's Edit keeps `center`, the reading it had. + */ + type RevealPlacement = 'center' | 'top'; + let pendingReveal: { startLine: number; endLine: number; placement: RevealPlacement } | null = null; /** * Put the reader on `startLine`..`endLine` of the buffer. @@ -2366,9 +2373,9 @@ * fades on a timer would be a second highlighting mechanism doing what this * one already does, and would leave the caret at the top of the file. */ - export function revealSourceRange(startLine: number, endLine: number) { + export function revealSourceRange(startLine: number, endLine: number, placement: RevealPlacement = 'center') { if (!editorReady || !editor) { - pendingReveal = { startLine, endLine }; + pendingReveal = { startLine, endLine, placement }; return; } @@ -2384,7 +2391,8 @@ const start = Math.min(Math.max(1, Math.trunc(startLine)), lastLine); const end = Math.min(Math.max(start, Math.trunc(endLine)), lastLine); - editor.revealLineInCenterIfOutsideViewport(start, monaco.editor.ScrollType.Smooth); + if (placement === 'top') editor.revealLineNearTop(start, monaco.editor.ScrollType.Smooth); + else editor.revealLineInCenterIfOutsideViewport(start, monaco.editor.ScrollType.Smooth); editor.setSelection({ startLineNumber: start, startColumn: 1, @@ -2406,7 +2414,7 @@ if (!model) return; const lineNumber = sourceLine ?? 0; if (Number.isInteger(lineNumber) && lineNumber > 0) { - revealSourceRange(lineNumber, lineNumber); + revealSourceRange(lineNumber, lineNumber, 'top'); return; } @@ -2416,13 +2424,13 @@ const match = model.findNextMatch(regex.source, { lineNumber: 1, column: 1 }, true, false, null, true); if (match) { - editor.revealLineInCenterIfOutsideViewport(match.range.startLineNumber, monaco.editor.ScrollType.Smooth); + editor.revealLineNearTop(match.range.startLineNumber, monaco.editor.ScrollType.Smooth); editor.setSelection(match.range); editor.focus(); } else { const fallbackMatch = model.findNextMatch(escapedText, { lineNumber: 1, column: 1 }, false, false, null, false); if (fallbackMatch) { - editor.revealLineInCenterIfOutsideViewport(fallbackMatch.range.startLineNumber, monaco.editor.ScrollType.Smooth); + editor.revealLineNearTop(fallbackMatch.range.startLineNumber, monaco.editor.ScrollType.Smooth); editor.setSelection(fallbackMatch.range); editor.focus(); }