From ae3f0c76892dc3e95cd3222f0d730cdaae3f0293 Mon Sep 17 00:00:00 2001 From: PathGao Date: Sat, 8 Aug 2026 16:17:57 +0800 Subject: [PATCH] fix(toc): stop the floating outline from sitting on the text it covers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **A. The overlap test was asking about the wrong pane.** The outline is positioned against the layout container, so it lands on whichever pane holds that edge — and the editor is always the first child, so it takes the left edge whenever it is on screen. But `isOverhanging` always measured the VIEWER pane against the preview's centred content width. That is only the right question in reading mode: reading viewer alone → preview on both sides ✓ measured split editor | viewer → editor on the left ✗ measured the preview editing only viewer is `flex: 0` → editor on both sides ✗ viewerWidth is 0 In editing-only mode `viewerWidth > 0` is false, so the expression fell through to "no overlap" while the panel sat on top of the code with neither the shadow nor the border that exist to say so. The test now answers "is the preview underneath me at all" first, and only reaches for arithmetic when it is. Moved to `utils/tocOverlay.ts` with the six cases under test. **B. An outline that covers the text now gets out of the way.** This is what #176 is actually about. Unpinned does not mean auto-hiding, it means "do not reflow the text" — the panel is supposed to fall into the gutter beside the centred preview. With the defaults that gutter is only wide enough at 1360px of viewer width (240 > (W - 880) / 2), which is wider than most windows and twice as wide as split view can offer. So for most readers the floating mode has covered their text since the day it shipped, and there was no way to dismiss it except the toggle it came from. It now collapses when you pick an entry, and when you reach past it to touch what it was covering. Both paths are gated on the same predicate A repairs: pinned it is a sidebar, and one genuinely sitting in the margin harms nothing and keeps its old behaviour. **The highlight had to stop being owned by the outline.** A jump marks the heading in the preview and leaves the mark until the reader moves — but the listeners that clear it belonged to the outline, and a new instance starts with its own `activeTargetEl` and cannot see what an earlier one marked. Hiding the outline right after a jump therefore stranded the mark forever. That was already reachable by hand; B makes it routine. The clearing is handed to listeners that outlive the component, deferred past the jump's own smooth scroll for the reason `clickLock` exists. Refs #176 --- scripts/previewWidth.test.ts | 8 ++- scripts/tocOverlay.test.ts | 106 ++++++++++++++++++++++++++++++++++ src/lib/MarkdownViewer.svelte | 52 +++++++++++++++-- src/lib/components/Toc.svelte | 40 ++++++++++++- src/lib/utils/tocOverlay.ts | 65 +++++++++++++++++++++ 5 files changed, 264 insertions(+), 7 deletions(-) create mode 100644 scripts/tocOverlay.test.ts create mode 100644 src/lib/utils/tocOverlay.ts diff --git a/scripts/previewWidth.test.ts b/scripts/previewWidth.test.ts index 3a014144..19427fb4 100644 --- a/scripts/previewWidth.test.ts +++ b/scripts/previewWidth.test.ts @@ -16,6 +16,7 @@ import { const settingsSource = readSource(new URL('../src/lib/stores/settings.svelte.ts', import.meta.url)); const viewerSource = readSource(new URL('../src/lib/MarkdownViewer.svelte', import.meta.url)); const settingsComponentSource = readSource(new URL('../src/lib/components/Settings.svelte', import.meta.url)); +const tocOverlaySource = readSource(new URL('../src/lib/utils/tocOverlay.ts', import.meta.url)); test('preview width defaults and clamps persisted numeric values', () => { assert.equal(DEFAULT_PREVIEW_MAX_WIDTH, 880); @@ -65,7 +66,12 @@ test('settings load, persist, and reset the preview width through one normalizer test('preview layout derives width and ToC geometry from the same preference', () => { assert.match(viewerSource, /getPreviewContentWidth\(settings\.previewMaxWidth, isFullWidth\)/); - assert.match(viewerSource, /viewerWidth - previewContentWidth/); + // The gutter arithmetic moved to `tocOverlay.ts` with #176, which added the + // question the inline expression could not answer: whether the pane under + // the outline is the preview at all. Both halves still feed on this same + // preference — see scripts/tocOverlay.test.ts for the geometry itself. + assert.match(viewerSource, /isTocOverhanging\(\{[\s\S]*?previewContentWidth,[\s\S]*?\}\)/); + assert.match(tocOverlaySource, /input\.viewerWidth - input\.previewContentWidth/); assert.match(viewerSource, /--preview-max-width:/); assert.match(viewerSource, /max-width: var\(--preview-max-width, 880px\)/); }); diff --git a/scripts/tocOverlay.test.ts b/scripts/tocOverlay.test.ts new file mode 100644 index 00000000..064beef1 --- /dev/null +++ b/scripts/tocOverlay.test.ts @@ -0,0 +1,106 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { readSource } from './sourceTree.js'; + +import { isTocOverPreview, isTocOverhanging } from '../src/lib/utils/tocOverlay.ts'; + +const DEFAULTS = { + // TOC_WIDTH_RANGE.default and DEFAULT_PREVIEW_MAX_WIDTH. + tocWidth: 240, + previewContentWidth: 880 as number | null, + isFullWidth: false, +}; + +const reading = { isEditing: false, isSplit: false } as const; +const editingOnly = { isEditing: true, isSplit: false } as const; +const split = { isEditing: true, isSplit: true } as const; + +test('the pane under the outline follows from the panes that are rendered', () => { + // The editor is always the first child, so it owns the left edge whenever + // it is on screen. + assert.equal(isTocOverPreview({ ...reading, tocSide: 'left' }), true); + assert.equal(isTocOverPreview({ ...reading, tocSide: 'right' }), true); + + assert.equal(isTocOverPreview({ ...split, tocSide: 'left' }), false); + assert.equal(isTocOverPreview({ ...split, tocSide: 'right' }), true); + + // The viewer pane is `flex: 0` here — neither side lands on the preview. + assert.equal(isTocOverPreview({ ...editingOnly, tocSide: 'left' }), false); + assert.equal(isTocOverPreview({ ...editingOnly, tocSide: 'right' }), false); +}); + +test('in reading mode the gutter decides, and the default one is not wide enough', () => { + const at = (viewerWidth: number) => + isTocOverhanging({ ...DEFAULTS, ...reading, tocSide: 'left', viewerWidth }); + + // 240 > (W - 880) / 2 ⟺ W < 1360. This is the finding in #176: the + // unpinned outline covers the text in any window narrower than that, which + // is most of them. + assert.equal(at(1359), true); + assert.equal(at(1360), false); + assert.equal(at(1600), false); + assert.equal(at(1200), true); +}); + +test('a full-width preview has no gutter at all', () => { + assert.equal( + isTocOverhanging({ + ...DEFAULTS, + ...reading, + tocSide: 'left', + isFullWidth: true, + previewContentWidth: null, + viewerWidth: 3000, + }), + true, + ); +}); + +test('covering the editor always counts, however wide the window is', () => { + // The regression #176 turns on: `viewerWidth` is 0 while the viewer pane is + // collapsed, so the old expression fell through to "no overlap" and the + // panel sat on the code with no shadow to say so. + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...editingOnly, tocSide: 'left', viewerWidth: 0 }), + true, + ); + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...editingOnly, tocSide: 'right', viewerWidth: 0 }), + true, + ); + // Split view is the same defect wearing a different hat: the outline is over + // the editor, but the measurement was taken from the preview. + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...split, tocSide: 'left', viewerWidth: 2000 }), + true, + ); + // The right-hand side in split view really is over the preview, so it goes + // back to arithmetic. + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...split, tocSide: 'right', viewerWidth: 2000 }), + false, + ); +}); + +test('a narrow preview keeps the floor at 50px rather than going negative', () => { + // (600 - 880) / 2 is negative; without the floor any outline would count as + // overhanging, including one narrower than the panel it is compared with. + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...reading, tocSide: 'left', viewerWidth: 600, tocWidth: 40 }), + false, + ); + assert.equal( + isTocOverhanging({ ...DEFAULTS, ...reading, tocSide: 'left', viewerWidth: 600, tocWidth: 60 }), + true, + ); +}); + +test('the outline collapses itself only when it is in the way', () => { + const viewer = readSource('src/lib/MarkdownViewer.svelte'); + // Both auto-collapse paths are gated on the same predicate, so a window wide + // enough to hold the outline beside the text keeps the old behaviour. + assert.match(viewer, /isOverhanging && !settings\.pinnedToc/); + // Click-outside must not fight the toggle button, which owns its own click. + assert.match(viewer, /tocToggleEl\?\.contains\(target\)/); +}); diff --git a/src/lib/MarkdownViewer.svelte b/src/lib/MarkdownViewer.svelte index 073bc30b..9df3204e 100644 --- a/src/lib/MarkdownViewer.svelte +++ b/src/lib/MarkdownViewer.svelte @@ -89,6 +89,7 @@ import { import { tabManager, type Tab } from './stores/tabs.svelte.js'; import { snapshotTab } from './utils/tabTransfer.js'; import { adjustPreviewMaxWidth, getPreviewContentWidth, getStoredPreviewFullWidth } from './utils/previewWidth.js'; +import { isTocOverhanging } from './utils/tocOverlay.js'; import { getScrollSyncPositionFromPixels, getScrollTopForSyncPosition, @@ -302,9 +303,19 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu // granularity of the numeric settings input, and arrow keys move the splitter 16px. const TOC_RESIZE_STEP = 16; let isTocResizing = $state(false); + let tocWrapperEl = $state(null); + let tocToggleEl = $state(null); let previewContentWidth = $derived(getPreviewContentWidth(settings.previewMaxWidth, isFullWidth)); let isOverhanging = $derived( - isFullWidth || (viewerWidth > 0 && previewContentWidth !== null && settings.tocWidth > Math.max(50, (viewerWidth - previewContentWidth) / 2)), + isTocOverhanging({ + isEditing, + isSplit, + tocSide: settings.tocSide, + isFullWidth, + viewerWidth, + previewContentWidth, + tocWidth: settings.tocWidth, + }), ); $effect(() => { @@ -312,6 +323,28 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu localStorage.removeItem('isFullWidth'); }); + /** + * Reaching past a floating outline to touch what it is covering is a request + * for it to move. Only while it IS covering something: pinned it is a + * sidebar, and one sitting in the margin is not in anybody's way. + */ + $effect(() => { + if (!settings.showToc || settings.pinnedToc || !isOverhanging) return; + const dismiss = (e: PointerEvent) => { + const target = e.target as Node | null; + if (!target) return; + // The toggle button owns its own click; closing here as well would + // open and shut the panel in one gesture. Anything inside the panel — + // the resize handle included — is use, not dismissal. + if (tocWrapperEl?.contains(target) || tocToggleEl?.contains(target)) return; + settings.showToc = false; + }; + // Capture, so a handler that stops propagation on its way up cannot leave + // the outline stranded over the text. + window.addEventListener('pointerdown', dismiss, { passive: true, capture: true }); + return () => window.removeEventListener('pointerdown', dismiss, { capture: true }); + }); + import { parseAndApplyVscodeTheme, clearVscodeTheme } from './utils/theme'; // Theme State @@ -3599,8 +3632,9 @@ import { createDocumentSession, type LoadMarkdownOptions } from './sessions/docu {#if isMarkdown && !showHome}
- {#if settings.showToc} -
copyHeadingReference(text, slug)} onjump={(id: string, text: string, sourceLine: number | null) => { + // A floating outline that is covering the text has done its + // job the moment you pick an entry: it exists to be called + // up, used once and dismissed. Pinned it is a permanent + // sidebar and stays; not overhanging it is sitting in the + // margin harming nothing, and closing it would take away a + // behaviour that was already fine. + if (isOverhanging && !settings.pinnedToc) settings.showToc = false; if (isEditing && editorPane) { // Same renderer-to-buffer shift as the context menu: the // outline reads `data-sourcepos` too, and has been landing diff --git a/src/lib/components/Toc.svelte b/src/lib/components/Toc.svelte index 48fc9102..6134aff6 100644 --- a/src/lib/components/Toc.svelte +++ b/src/lib/components/Toc.svelte @@ -42,6 +42,8 @@ // when user clicks a toc entry, lock active id until scroll catches up let clickLock: string | null = null; let clickLockTimer: ReturnType | null = null; + /** How long a jump's own smooth scroll is given to settle. */ + const CLICK_LOCK_MS = 600; $effect(() => { if (htmlContent && markdownBody) { @@ -167,6 +169,41 @@ activeTargetEl = null; } + /** + * The highlight is worn by an element in the PREVIEW, which outlives this + * component. Going away while one is showing — the outline collapsing itself + * after a jump, or the reader hiding it by hand — used to leave that mark on + * the heading with nothing able to clear it: the listeners left with the + * component, and a later instance starts with its own `activeTargetEl` and + * cannot see what an earlier one marked. So hand the clearing over to + * listeners that belong to nobody, and take them all off the first time the + * reader moves. + */ + function releaseStrandedHighlight(el: HTMLElement) { + const stranded = activeTargetEl; + if (!stranded) return; + activeTargetEl = null; + + const clear = () => { + stranded.classList.remove('toc-target-active'); + el.removeEventListener('scroll', clear); + el.removeEventListener('pointerdown', clear); + window.removeEventListener('keydown', clear); + }; + const listen = () => { + el.addEventListener('scroll', clear, { passive: true }); + el.addEventListener('pointerdown', clear, { passive: true }); + window.addEventListener('keydown', clear, { passive: true }); + }; + + // Exactly what `clickLock` is for, and the reason it cannot simply be + // read here: the jump's own smooth scroll is still running, and it must + // not be mistaken for the reader scrolling away from what they just + // asked to see. The timer outlives the component, the lock does not. + if (clickLock) setTimeout(listen, CLICK_LOCK_MS); + else listen(); + } + function handleScroll() { // The lock is here for the jump's OWN smooth scroll, which would // otherwise clear the highlight before the reader has seen it. @@ -242,6 +279,7 @@ el.removeEventListener('scroll', handleScroll); el.removeEventListener('pointerdown', handleReaderAction); window.removeEventListener('keydown', handleReaderAction); + releaseStrandedHighlight(el); }; } }); @@ -273,7 +311,7 @@ // release lock after scroll settles if (clickLockTimer) clearTimeout(clickLockTimer); - clickLockTimer = setTimeout(() => { clickLock = null; }, 600); + clickLockTimer = setTimeout(() => { clickLock = null; }, CLICK_LOCK_MS); } } diff --git a/src/lib/utils/tocOverlay.ts b/src/lib/utils/tocOverlay.ts new file mode 100644 index 00000000..86c7ca9e --- /dev/null +++ b/src/lib/utils/tocOverlay.ts @@ -0,0 +1,65 @@ +export type TocSide = 'left' | 'right'; + +export interface TocPlacement { + isEditing: boolean; + isSplit: boolean; + tocSide: TocSide; +} + +export interface TocOverhangInput extends TocPlacement { + isFullWidth: boolean; + /** Client width of the VIEWER pane. Zero while that pane is collapsed. */ + viewerWidth: number; + /** The preview's centred content width, or null when it fills the pane. */ + previewContentWidth: number | null; + tocWidth: number; +} + +/** + * The narrowest gutter the outline may share with the text before it counts as + * covering it. Below this the "gap" reads as a collision either way. + */ +const MIN_GUTTER = 50; + +/** + * Is the preview the thing underneath the outline? + * + * The outline is positioned against the LAYOUT container, not against the pane + * it happens to land on. "Is there room beside the text?" is therefore only the + * right question when the pane underneath is the preview: the preview centres + * its content and leaves a gutter either side, while the editor fills its pane + * edge to edge and has no gutter to lend. + * + * Which pane is underneath follows from which panes are rendered, because the + * editor is always the first child and so takes the left edge whenever it is on + * screen at all: + * + * reading viewer alone → preview on both sides + * split editor | viewer → editor on the left, preview on the right + * editing only viewer is `flex: 0` → editor on both sides + */ +export function isTocOverPreview({ isEditing, isSplit, tocSide }: TocPlacement): boolean { + const editorVisible = isSplit || isEditing; + const viewerVisible = isSplit || !isEditing; + return tocSide === 'right' ? viewerVisible : !editorVisible; +} + +/** + * Does the outline sit ON TOP of what the reader is reading? + * + * This drives the shadow and border that tell the reader the panel is floating + * over their text rather than beside it, and it gates the auto-collapse: an + * outline that is not covering anything has no reason to get out of the way. + * + * Measuring the viewer pane was only ever right in reading mode. In the other + * two the outline covers the editor, and in editing-only mode `viewerWidth` is + * 0, so the old test answered "no overlap" while the panel sat on the code. + */ +export function isTocOverhanging(input: TocOverhangInput): boolean { + // Nothing under it centres its content, so there is no gutter to fall into. + if (!isTocOverPreview(input)) return true; + if (input.isFullWidth) return true; + if (input.viewerWidth <= 0 || input.previewContentWidth === null) return false; + const gutter = (input.viewerWidth - input.previewContentWidth) / 2; + return input.tocWidth > Math.max(MIN_GUTTER, gutter); +}