feat: save and restore reading progress per file#10
Conversation
Implements silent save/restore of scroll position using data-source-line anchoring. Includes debounced scroll save, visibilitychange + beforeunload flush, LRU-capped localStorage persistence, and scrollend-based restore guard with timeout fallback. Closes vaibhavuk-dev#7
vaibhavuk-dev
left a comment
There was a problem hiding this comment.
Thanks @pushkarchoudhari — nice work on this. The schema versioning, LRU eviction, isRestoring / scrollend guard, and defensive paste:// checks all match what I was hoping to see. A few things to address before this can land:
1. Merge conflict. Branch was opened before #8 merged; both PRs touch the imports + onMount listener wiring in +page.svelte. A rebase against main should sort it.
2. Tab close doesn't save progress. handleCloseTab calls tabStore.closeTab(id) directly with no save first. If a user scrolls and immediately closes within the 500ms debounce window, the position is lost. Could you call saveProgressNow() (or inline equivalent) for the closing tab before tabStore.closeTab(id)?
3. Tab-switch save: timing concern on the DOM read. In the tab-leave block, getCurrentSourceLine("viewer") queries the current article DOM. Depending on $effect ordering, the article HTML may already have been swapped to the new tab's content by the time this runs, which would mean we're computing the line from the new tab's elements at the old tab's scroll position. Two safer options: (a) use the outgoing tab's captured scroll position from the tab store (tabs.ts already calls saveScrollPosition on switch) and convert that to a line, or (b) confirm via logging on your end that the DOM read happens before the swap. Worth verifying either way.
4. File-shrunk comment doesn't match the code. In restoreProgress the comment says "closest valid line ≤ savedLine" but the loop finds the first element with elLine >= savedLine and falls back to the last element. The behavior is actually correct in all cases the design doc enumerates, but the comment will confuse readers — could you reword to something like "find the saved line if it still exists, else fall back to the last element (handles file-shrunk case)"?
Out of scope, will file as a follow-up: the close-on-ESC path (quit_app Tauri command, added in #4) calls AppHandle::exit(0) and bypasses beforeunload, so the final position is lost when the user quits via ESC. Separate from this PR — fix is to saveProgressNow() from the close-on-ESC handler before invoke("quit_app").
CI hasn't run yet — once you push the rebase, I'll approve the workflow. Thanks again for picking this one up!
Summary
readingProgressstore (src/lib/stores/readingProgress.ts) withsave/get/clear/clearAllAPI, LRU eviction at 500 entries, schema-versioned localStorage persistence, and safe JSON parsebeforeunload+visibilitychangein+page.sveltedata-source-lineanchoring so saved positions survive font/width/reflow changes between sessionspaste://skip,url://key normalization,isRestoringguard withscrollend+ 1s timeout fallbackCloses #7
Test plan
paste://— no progress tracking