fix: review unsaved tabs one at a time on window close; restore window state only#211
Open
PathGao wants to merge 7 commits into
Open
fix: review unsaved tabs one at a time on window close; restore window state only#211PathGao wants to merge 7 commits into
PathGao wants to merge 7 commits into
Conversation
Replace the aggregate "you have N unsaved files" modal with a per-tab walk (issue alecdotdev#189): activate each dirty tab and run the same localized unsaved-changes dialog a single tab close shows (canCloseTab), then close the tab. Cancel stops the walk and keeps the window open with the remaining tabs; the window closes only after every dirty tab is resolved. The auto-save fast path and the restore-on-reopen branch are untouched original behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The restore snapshot previously carried every tab's full document content (rawContent + originalContent + history), doubling as an unreliable unsaved-content store and letting restored tabs show stale bytes when the file changed on disk while the app was closed. Separate the concerns. serializeState (v2) records window state only: open file paths, active tab, edit mode, split, scroll. On startup each restored tab reads its file from disk; unreadable files drop their tab. The close flow resolves dirty tabs FIRST through the per-tab dialogs — regardless of the restore setting — then writes the snapshot: Save persists to disk, Don't Save reverts the tab to its last saved content, Cancel keeps the window open. Untitled tabs are never persisted; the auto-save fast path still saves titled tabs silently. Legacy snapshots restore through the same path (window-state fields only). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The red button is a native control, so the in-app dialog overlay does not block it: a second click while the walk had a dialog up re-entered the handler and started a competing walk whose setActive calls fought the first one — with two untitled tabs the highlight and the dialog visibly disagreed. Guard the walk with a re-entrancy flag (released in finally, including the cancel path), and start each round from the tab the user is already looking at so the highlight only jumps when it has to. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A legacy build restoring a v2 window-state snapshot reconstructs tabs whose rawContent is undefined (it expects full-tab snapshots). Its editor then fails to swap buffers on tab switch and attributes the previous tab's still-visible content to the newly active tab — and auto-save writes that misattributed content to disk. Observed live: after a v2-format close, an older build showed 4.md's content under the 3.md tab; one edit event away from corrupting 3.md on disk. Write v2 snapshots under their own key (savedTabsDataV2) and remove the legacy key on every write, so an older build sharing the same storage container never sees a format it cannot restore. Startup reads the v2 key first and falls back to the legacy key once for migration; explicit exit clears both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
With two untitled tabs both called "Untitled", the per-tab
unsaved-changes dialog at window close could not tell the user which
tab it was asking about. Give untitled tabs numbered titles
("Untitled 1", "Untitled 2", reusing the smallest free number), so the
tab strip and every dialog naming a tab become unambiguous. A legacy
unnumbered title counts as slot 1; localized bases work unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The close walk preferred the active tab, then wrapped around the strip — starting on the third of three tabs produced the sequence 3, 1, 2, which reads as random. Walk strictly left to right instead: numbered untitled titles already keep the dialog unambiguous, so predictability wins over avoiding one highlight jump. Also prefill the untitled Save As dialog with the numbered tab title so the save panel itself says which tab is being saved. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…dow-close # Conflicts: # src/lib/MarkdownViewer.svelte
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.
Fixes #189. Independent of #210 — either can merge first.
What changed
Closing a window with unsaved tabs now reviews them one at a time: each dirty tab is activated and gets the same localized unsaved-changes dialog a single tab close shows (Save / Don't Save / Cancel). Cancel stops the walk and keeps the window open. The window closes only after every dirty tab is resolved.
Alongside that, session restore ("Restore State on Reopen") was reworked to persist window state only, and untitled tabs got numbered titles so the dialogs can name them.
Design rationale
1. Reuse the existing per-tab dialog instead of adding new UI. The previous behavior showed one aggregate "You have N unsaved files" modal whose Save-all/Discard-all semantics were all-or-nothing (and Discard silently cleared every dirty flag). Walking the tabs through the existing
canCloseTabflow keeps the UI native, localized, and familiar — one decision per document, in strict tab-strip order, with the tab in question activated so the user sees exactly what they are deciding about.2. Separate unsaved content from window restore. The restore snapshot previously carried every tab's full document content (
rawContent+originalContent+ history) and doubled as an implicit unsaved-content store. That was unreliable by construction: the snapshot was written withlocalStorage.setItemduringonCloseRequested, and WebKit flushes storage asynchronously in a separate process — the write races window teardown. We instrumented the storage layer and confirmed both outcomes live: sometimes the snapshot landed (unsaved content "restored"), sometimes it died with the process (content silently lost, window restored from a stale earlier snapshot). A safety feature that works probabilistically is worse than none, because users learn to trust it.The v2 snapshot therefore stores window state only — open file paths, active tab, edit/split mode, scroll — a few hundred bytes instead of potentially megabytes. Unsaved content is handled exclusively by the close dialogs; on restore, content is always read from disk, so restored tabs show the file's real current bytes (the old snapshot showed stale content when the file changed on disk while the app was closed). Untitled tabs are never persisted: they are resolved at close (Save gives them a path; Don't Save reverts them).
3. Keep the v2 format invisible to older builds. The v2 snapshot lives under a new key (
savedTabsDataV2) and the legacy key is removed on write. This is deliberate downgrade protection: an older build restoring a v2 snapshot it cannot understand reconstructs tabs withundefinedcontent, and its editor then mis-attributes the previous tab's still-visible buffer to the newly active tab — one auto-save later, the wrong content is written to the wrong file. We reproduced this corruption live while testing old and new builds side by side (details in the commit message of the key-separation commit). With separate keys, a rollback simply starts clean. New builds still read the legacy key once for migration.4. Numbered untitled tabs ("Untitled 1", "Untitled 2"). With two untitled tabs both called "Untitled", the per-tab dialog could not communicate which tab it was asking about. Numbering (smallest free number, like most editors) makes the tab strip, the close dialog, and the prefilled Save As filename all agree on a name. Numbers are session-local by construction, since untitled tabs never enter the snapshot.
Edge cases covered
Testing
node --test,scripts/*.test.ts): a pure state walk + contract tests over the close handler, serializer v2 (format, untitled exclusion, legacy migration), discard revert, key separation, and untitled numbering.svelte-checkclean.🤖 Generated with Claude Code
Closes #146