Skip to content

fix(preview): give each document its own heading fold state - #425

Merged
PathGao merged 1 commit into
masterfrom
fix/per-document-fold-state
Aug 3, 2026
Merged

fix(preview): give each document its own heading fold state#425
PathGao merged 1 commit into
masterfrom
fix/per-document-fold-state

Conversation

@PathGao

@PathGao PathGao commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Fold ## Introduction in one document and it folds in every other open document that has an ## Introduction too. Close the tab you folded it in and the key stays behind, so the next file containing that heading opens with the section already shut — and with nothing on screen that could explain why.

Why

The fold key is h.id || h.textContent?.trim() — the heading's id, comrak's slug, falling back to the text. That identifies a heading within a document and nowhere else: two files with the same title both get introduction.

MarkdownViewer.svelte held one Set per window and handed it to every render, to Toc.svelte and to FindBar.svelte. There is no .clear() and no reassignment to an empty set anywhere in the repo, so nothing ever reset it — not on tab switch, not on close.

Two things are visible today, and they are different sizes:

The outline reads the live Set, so it hides the folded section's children immediately in the other document — while that document's preview still shows them expanded.
The preview re-reads the Set on the next render of that document. A fresh open, a reload, a keystroke in split view. That is where "opened pre-folded" comes from.

452ec96 (#219) moved the key from heading text to the deduplicated id, which fixed collisions between duplicate headings inside one document. It could not fix this one: there was nowhere to hang per-document fold state.

The fix

Tab.collapsedHeaders, alongside scrollTop, scrollPercentage, anchorLine and editorViewState — the same class of per-document state, saved and restored on tab switch by the same mechanism (#79, #147, #420). Required rather than optional, like hasReplacementChars: every consumer calls .has() on it, so the compiler asks each of the five construction sites.

The viewer reads it through a derived:

let collapsedHeaders = $derived(activeTab?.collapsedHeaders ?? NO_COLLAPSED_HEADERS);

so a tab switch swaps the whole set at once, and the preview render, the outline and find all see the document on screen. Every write goes through one setCollapsedHeaders, so both toggle paths — the preview chevron (which is also what FindBar clicks to reveal a match inside a collapsed section) and the outline's fold button — land on the tab the user is looking at.

renderMarkdownPreview takes the fold set as an argument rather than reading the active tab, because three of its callers render a document that is not on screen: a window restore renders every restored tab, a tab arriving from another window renders itself, and the background completion of a >50KB file lands whenever it lands. Reading "the active tab" there would have traded a leak between documents for a rarer and worse one. documentSession looks the set up at render time, not at load time, so folding something while a large file is still loading is not undone when the full render arrives.

No document/model object, no ITextModel, no lifecycle abstraction (#391). The Set moved onto the tab; that is the whole change.

Decisions

Persistence across restart: no. The window snapshot carries scroll, and the obvious symmetry argument is real — Obsidian persists folds keyed by file path. But the restore reads each file fresh from disk, and a fold key describes a heading in a particular revision of a document. A file edited while Markpad was closed comes back with sections hidden that the user never folded in the text now in front of them — which is the failure this PR removes, reintroduced through the back door. Scroll degrades; you scroll. A fold hides text. It is a reasonable follow-up now that the state has a home, and it is a feature rather than part of this fix; serializeState says so where a future reader will look.

Reload of the same file (Live Mode external change, revision load, loadMarkdown): the tab keeps its set. Keys whose headings are gone never match anything and do nothing; headings that survive fold as they were. Clearing on reload would drop the user's folds every time the watcher fired.

Untitled buffers: work by construction. The state hangs off the tab, not off a path, which is precisely why a path -> Set map was not the answer — untitled tabs have no key, and they are the tabs most likely to share a heading with each other.

Split view: shows one document. collapsedHeaders follows the active tab, and the split render path passes that tab's own set. Nothing assumes two.

Cross-window move: arrives with everything open, joining content and editorViewState on the excluded list — the destination re-renders from source. Before this PR the arriving document inherited whatever the destination window had folded, which is the same bleed. Carrying it would need a new required field in the strict transfer validator; that is a change to make on its own terms.

Tests

scripts/foldStatePerDocument.test.ts runs the real toggleFold and handleLinkClick out of MarkdownViewer.svelte, the real visibleItems out of Toc.svelte and the real processMarkdownHtml, against the real TabManager. It asserts nothing about the text of an implementation file: what is checked is what a second document renders as.

The rune declarations are plucked too, not restated — the entire fix lives in one of them, and $derived is re-evaluated on read where $state is not, so the harness models that difference instead of papering over it. That is what lets the same file run against both versions.

On master 6 of 7 fail, all clean assertions: the second document must render with its own fold state, which is none — true !== false, and the outline returning ['introduction'] where ['introduction', 'details'] is expected. No crashes, no missing symbols. The one that passes (the folded document is still folded when you come back) passes on master for the trivial reason that a shared set still has the key.
setTabCollapsedHeaders writes to every tab 4 fail
The derived reads a fixed tab instead of the active one 4 fail
Final 7 / 7

ShimClassList.toggle in the test DOM returned nothing where the real DOMTokenList returns whether the token is present afterwards — the exact value the preview's fold handler reads to tell a fold from an unfold. Fixed, or every chevron click would have looked like an unfold in every future test.

editorPdfExport.test.ts pinned renderMarkdownPreview(rawContent, tab.path) down to the closing paren. Relaxed to allow trailing arguments, for the reason #418 gives: what that test is about is which buffer and which path get rendered before the print, not the arity of the renderer.

npm run check   438 files, 0 errors
npm test        547 / 547

No Rust touched.

Not covered

  • A fold still resets visually when you leave a tab and come back. {@html sanitizedHtml} rebuilds the preview from tab.content, the cached HTML, and the toggle handlers deliberately do not re-render — they toggle the class on the live DOM. So the cached string never learns about the fold until the document is re-rendered for some other reason. This is pre-existing, unchanged by this PR, and now visible as a disagreement with the outline, which does track the fold. Fixing it means either re-rendering on every toggle or re-applying the classes after the {@html} swap; both are a separate change.
  • No browser round trip. Everything above is exercised through the DOM shim and the real store, not in a running window.
  • Fold state is not persisted and not transferred between windows, by the decisions above.
  • Stale keys accumulate in a tab's set when a heading is renamed or deleted while the tab stays open. Bounded by how many headings the user folded, and a key that matches nothing does nothing — but nothing prunes them either.
  • The fixture is comrak-shaped, not comrak-produced, following the existing convention in renderProtocolFixtures.ts.

🤖 Generated with Claude Code

Fold `## Introduction` in one document and it folds in every other open
document that has an `## Introduction` too. Close the document you
folded it in and the key stays behind, so the next file containing that
heading opens with the section already shut - and with nothing on screen
that could explain why.

The fold key is the heading's id, comrak's slug, falling back to the
heading text. That identifies a heading *within* a document and nowhere
else: two files with the same title both get `introduction`. The viewer
held one `Set` per window and handed it to every render, to the table of
contents and to find, and nothing ever cleared it - not on tab switch,
not on close.

452ec96 (#219) moved the key from the heading text to the deduplicated
id, which fixed duplicate headings inside one document. It could not fix
this, because there was nowhere to hang per-document fold state.

There is now: `Tab.collapsedHeaders`, next to `scrollTop`,
`scrollPercentage`, `anchorLine` and `editorViewState`, which are the
same class of thing and live there for the same reason. The viewer reads
it through a derived, so a tab switch swaps the whole set at once and
the preview, the outline and find all see the document on screen. Writes
go through one `setCollapsedHeaders`, so both toggle paths - the preview
chevron, which is also what find clicks to reveal a match inside a
collapsed section, and the outline's fold button - land on the tab the
user is looking at.

`renderMarkdownPreview` takes the fold set as an argument rather than
reading the active tab, because three of its callers render a document
that is not on screen: a window restore renders every restored tab, a
tab arriving from another window renders itself, and the background
completion of a >50KB file lands whenever it lands. Reading "the active
tab" there would have replaced a leak between documents with a rarer,
worse one.

Not persisted in the window snapshot, unlike scroll. The restore reads
each file fresh from disk, and a fold key describes a heading in a
particular revision of a document; a file edited while Markpad was
closed would come back with sections hidden that the user never folded
in the text now in front of them, which is the failure this removes.
Scroll degrades - you scroll. A fold hides text. Obsidian does persist
folds, keyed by path; the same is a reasonable follow-up now that the
state has a home, and it is a feature rather than part of this fix.

A reload of the same file keeps the tab's set: keys whose headings are
gone simply never match, and the headings that survive fold as they
were. Cross-window moves arrive with everything open, matching `content`
and `editorViewState`, which are also rebuilt at the destination.

`ShimClassList.toggle` in the test DOM returned nothing where the real
`DOMTokenList` returns whether the token is now present - the value the
preview's fold handler reads to tell a fold from an unfold.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@PathGao
PathGao force-pushed the fix/per-document-fold-state branch from f40c51b to dd7b8f2 Compare August 3, 2026 07:21
@PathGao
PathGao merged commit c53a1b6 into master Aug 3, 2026
4 checks passed
@PathGao
PathGao deleted the fix/per-document-fold-state branch August 3, 2026 09:26
PathGao pushed a commit that referenced this pull request Aug 3, 2026
…er one

#425 gave each document its own heading-fold state by moving the set from
the window onto the tab. A tab is not a document, though: `navigate`
(following a Markdown link), `goBack` and `goForward` keep the tab and
swap the file under it, and `collapsedHeaders` travelled with it.

A fold key is a heading slug, unique only within a document, so the
incoming file was rendered with any section whose slug happened to match
already shut. `loadMarkdown` reads the set on the line after the
`navigate` call, so nothing is deferred — the HTML that first appears
carries `is-collapsed`, and the outline hides the same section's children
on the same render, with nothing on screen to explain either.

Measured over 202 real Markdown documents (this repo and its
dependencies' READMEs and changelogs): for 27.9% of ordered document
pairs, at least one heading slug of the first names a heading in the
second. `installation` occurs in 29.7% of them, `usage` in 27.7%.

#447 established that the same three routes must clear the reading
position, and left folds for a separate change. Rather than have each
route remember two resets — the shape #436 and #439 were both about —
those routes now call one `forgetPreviousDocument(tab)`, which calls
#447's `clearReadingPosition` and a new `clearCollapsedHeaders`. The two
helpers stay separate: a stale position moves the viewport, a stale fold
hides text, and each needs its own explanation. What they shared was the
trigger, which had no name until now.

Save As (`updateTabPath`) and rename (`renameTab`) change the path while
the text on screen stays put, so they do not get there. Tests guard both
directions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PathGao added a commit that referenced this pull request Aug 3, 2026
…er one (#448)

#425 gave each document its own heading-fold state by moving the set from
the window onto the tab. A tab is not a document, though: `navigate`
(following a Markdown link), `goBack` and `goForward` keep the tab and
swap the file under it, and `collapsedHeaders` travelled with it.

A fold key is a heading slug, unique only within a document, so the
incoming file was rendered with any section whose slug happened to match
already shut. `loadMarkdown` reads the set on the line after the
`navigate` call, so nothing is deferred — the HTML that first appears
carries `is-collapsed`, and the outline hides the same section's children
on the same render, with nothing on screen to explain either.

Measured over 202 real Markdown documents (this repo and its
dependencies' READMEs and changelogs): for 27.9% of ordered document
pairs, at least one heading slug of the first names a heading in the
second. `installation` occurs in 29.7% of them, `usage` in 27.7%.

#447 established that the same three routes must clear the reading
position, and left folds for a separate change. Rather than have each
route remember two resets — the shape #436 and #439 were both about —
those routes now call one `forgetPreviousDocument(tab)`, which calls
#447's `clearReadingPosition` and a new `clearCollapsedHeaders`. The two
helpers stay separate: a stale position moves the viewport, a stale fold
hides text, and each needs its own explanation. What they shared was the
trigger, which had no name until now.

Save As (`updateTabPath`) and rename (`renameTab`) change the path while
the text on screen stays put, so they do not get there. Tests guard both
directions.

Co-authored-by: PathGao <gaoyanbo@gaoyanbodeMacBook-Air.local>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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