fix(tabs): forget a document's folds when the tab is pointed at another one - #448
Merged
Conversation
…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
force-pushed
the
fix/navigate-leaves-stale-folds
branch
from
August 3, 2026 13:10
35631c7 to
4153c79
Compare
This was referenced Aug 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fold
## Introduction, click a link to another file that also has an## Introduction, and it opens with that section already shut — in a document you have never folded anything in.This is the third instance of one shape. #425 found it between documents in different tabs and moved
collapsedHeadersfrom the window onto theTab. #447 found it in the scroll cascade, wherenavigate/goBack/goForwardrepoint a tab at a different document and carried the reader's position over; its "Not covered" section names this one and scopes it out. A tab is not a document, and the routes that change which document a tab holds are where the difference shows.Rebased onto
df80968(#447).npm test592/592,npm run check638 files / 0 errors,npm run buildclean, all measured on the rebased branch. No Rust touched.Mechanism
Three routes keep a tab and point it at a different file:
navigate()(following a Markdown link),goBack()andgoForward(). None of them touchedcollapsedHeaders, andloadMarkdownrenders the incoming document with the tab's set on the line straight after:foldsForTabreturnstab.collapsedHeaders— which at that instant still holds the keys of the document being left.Nothing is deferred. Unlike #447, this needs no restore effect:
processMarkdownHtmlwritesis-collapsedonto the heading and its wrapper while it builds the HTML, so the very first frame of the new document has the section shut. The outline reads the same field through the viewer's$derived, so it hides that section's children on the same render.The key collides because it is a slug.
h.id || h.textContent?.trim()is unique within a document and nowhere else. Measured over the 202 real Markdown files in this checkout with ≥3 headings (the repo's own, plus its dependencies' READMEs and changelogs):licenseinstallationusageSo a reader who folds
## Installationin one README and follows a link has roughly a 1-in-3 chance of the next one arriving with its Installation section hidden.Measured in the running frontend
Driven through the real Svelte frontend in a browser against a stubbed Tauri bridge (
render_markdownemitting comrak's shape, two in-memory documents that share## Introductionwith a### Detailunder it). The number is the fold wrapper's measured height — 0px is a genuinely hidden section, not just a class. Both columns re-measured after the rebase, againstdf80968and against this branch.df80968)## Introductionina.mdb.mdb.mdopens with the section shutb.md's chevron oncea.mda.mdis gone, spent onb.mdb.mdA second run, folding only in
b.mdand then pressing Back, lands ona.mdat 0px — the same leak in the other direction, this time with the folded tab's own document nowhere on screen.Both symptoms in the "before" column come from one
Set: because the two documents share the key, a toggle in either one moves the fold for both, so the leak does not only hide text the reader did not hide — it also loses folds they did make. #447 changed nothing here; the two tables measured before and after that merge are identical.The fix, and the shape
A private
clearCollapsedHeaders(tab)that replaces the set. The interesting decision is where it is called from.The mechanical rebase onto #447 gave each of
navigate/goBack/goForwardtwo calls —clearReadingPositionandclearCollapsedHeaders. That is the shape #447 set out to remove: three routes that must each remember two things, and a fourth route later that must remember both. This repo has been bitten by it twice recently (#436: two of five explicit saves remembered to cancel the auto-save timer; #439: six sites cleared one flag while two cleared the other).So the three routes now call one
forgetPreviousDocument(tab), and that calls both helpers:The helpers are not merged. A stale reading position moves the viewport; a stale fold hides text. They invalidate for different reasons, they fail differently, and each earns its own doc comment. What they share is the trigger, and the tell that the trigger was an unnamed concept is that both helpers' comments opened by restating it in slightly different words.
forgetPreviousDocumentis that name, and it is now the single place stating which routes reach it and which deliberately do not.Replaces rather than empties.
Tab.collapsedHeadersis documented as replace-only: the viewer holds the Set through a$derived, and Svelte cannot see a.clear()of a Set it is already holding — the outline would keep hiding the section. Tested.Save As and rename must not get there.
updateTabPathandrenameTabchange the tab's path while the text on screen stays put, so the reader has not moved and the folds they put in that text still describe it. Every path-changing site inTabManagerwas checked; those two are the only others.Cross-window transfer is unchanged.
tabTransfer.tsalready excludescollapsedHeadersdeliberately (#425) — the destination re-renders from source with everything open, which is already the cleared state.Is that now the whole set?
With folds added,
forgetPreviousDocumentclearseditorViewState,anchorLine,scrollPercentage,scrollTop,collapsedHeaders. Every otherTabfield was classified:path,pathKey,title,isDirty,history,historyIndexcontent,rawContent,originalContent,isTruncated(setTabRawContent),hasReplacementChars(setTabDecodedLossy, called on both load branches and documented as clearing)isEditing,isSplit,splitRatio,isScrollSyncedNo sixth field of the same shape. One near-miss, checked and left alone:
_lastRenderedRawContent, an ad-hoc property compared againstrawContentto skip a redundant render. It does hold the previous document's text after a navigate, but a stale value can only skip a render when the two documents are byte-identical — in which case the render would be identical anyway.Tests
Eight tests added to
scripts/foldStatePerDocument.test.ts— the file that owns this rule — driving the realTabManager, the realprocessMarkdownHtml, the realtoggleFoldout ofMarkdownViewer.svelte, the realvisibleItemsout ofToc.svelte, and now the realfoldsForTabplucked out ofdocumentSession.svelte.ts, which is the reader that decides what the incoming HTML arrives with. No file is asserted on as text.navigategoBackclears nothinggoForwardclears nothing.clear()the$derivedcannot seetab.path === pathearly returnRed on
df80968: 5 fail / 10 pass, with #447's owntabReadingPosition.test.tsat 8/8 on the same source. The three of mine that pass on master are the guards — they must be green on both sides or they are not guarding anything.One draft of the back/forward tests passed on master for the wrong reason: the setup folded in
a.mdfirst, sotoggleFoldinb.mdfound the carried key and removed it, leaving an empty set that satisfied the assertion. They now fold only in the document on screen and assert the precondition.Mutation check
Each mutation applied alone on top of the fix, on
df80968, run against both suites — including the two new ways a single entry point can break, which is dropping a call from inside it:clearReadingPositiondropped insideforgetPreviousDocumentclearCollapsedHeadersdropped insideforgetPreviousDocumentnavigategoBackgoForwardupdateTabPath(over-reach)renameTab(over-reach)The first two rows are the point of the table: each helper is covered by exactly one suite, so the entry point cannot quietly lose either half. The rest fail in both, which is what a shared entry point should do.
Not covered
Forward no longer restores a fold made before you went Back. In the table above,
Forward to b.mdused to land at 0px because the key was still in the tab's set; it now opens flush. That is the same call the fix makes everywhere else — the tab's set describes the document it is leaving, not the one being restored — and it matches #447, which clears the reading position on back/forward too. Per-document fold history would need folds keyed by document, which #425 argued against for persistence for the same reason: a fold key describes a heading in a particular revision of a file.The outline was not measured in the browser. The overlay renders at zero height in the headless viewport, so the live ToC could not be read. Its evidence is the real
visibleItemsin the test, plus #425 having established it reads the same field. The preview measurements above are real.Stale keys still accumulate in a tab that stays on one document while its headings are renamed — pre-existing, listed in #425's own "Not covered", untouched here.
No
cargo test. No Rust in the diff.Severity. A hidden section is quieter than a lost scroll position: the text is simply not there, the chevron looks the same as any other collapsed heading, and the first click on it opens a section instead of closing one — which is the moment the reader learns their fold in the previous document is also gone.
🤖 Generated with Claude Code