fix(stats): refresh word/char/reading-time on every render - #243
Conversation
updateDocumentStats() only ran from inside the preview post-processing path, which is skipped whenever renderMarkdown() detects the preview HTML is already up to date for the active tab/content. Closing the only open tab reset the stats to zero via clearActiveDocument(), and reopening that same tab hit the cache-hit early return, leaving the footer stuck at 0 Min Read / 0 Words / 0 Chars until a full page reload forced a fresh render. Call updateDocumentStats() unconditionally at the top of renderMarkdown() so it stays in sync with editor content regardless of preview caching decisions.
|
@crazy-explore-r is attempting to deploy a commit to the ThisIs-Developer Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Fixes stale footer/mobile document statistics (word/char/reading-time) when a document is closed and reopened without a full page reload by ensuring stats are refreshed even when preview HTML rendering is skipped due to the preview cache.
Changes:
- Recalculates document stats during
renderMarkdown()calls to prevent stale counters when cached preview HTML short-circuits rendering. - Adds a Playwright e2e regression test covering close-tab → reopen-from-Files and verifying stats are restored.
- Regenerates the bundled desktop copy of
script.jsto keep desktop behavior consistent with web.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/e2e/tab-split-sidebar-update.spec.js | Adds an e2e regression test ensuring stats restore correctly after closing/reopening the only tab. |
| script.js | Updates renderMarkdown() to refresh document stats when render is invoked. |
| desktop-app/resources/js/script.js | Mirrors the script.js change for the desktop build output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Stats must reflect the current editor content on every render call, even when | ||
| // the preview HTML itself is skipped below because it's already up to date | ||
| // (e.g. switching back to a tab whose content didn't change). | ||
| updateDocumentStats(); |
| // Stats must reflect the current editor content on every render call, even when | ||
| // the preview HTML itself is skipped below because it's already up to date | ||
| // (e.g. switching back to a tab whose content didn't change). | ||
| updateDocumentStats(); |
|
@crazy-explore-r Thank you for this fix. The main approach looks correct and the regression test properly covers the stale statistics issue. I noticed that Could you please check whether the old Once this is clarified or updated, the PR should be ready to merge. |
ThisIs-Developer
left a comment
ThisIs-Developer
left a comment
There was a problem hiding this comment.
Check and remove the duplicate calculation if unnecessary.
updateDocumentStats() now runs unconditionally at the top of renderMarkdown(), so the copy inside postProcessPreview() was double-counting words/chars on every non-cached render. The guard in executeMainThreadRender/executeWorkerRender already skips postProcessPreview when editor content has moved on since the render started, and a fresh renderMarkdown() call follows any such edit anyway, so dropping the duplicate call doesn't leave stats stale.
|
Good catch — you're right, it wasn't needed. Removed the postProcessPreview() call in the latest commit. It's safe to drop: both executeMainThreadRender and executeWorkerRender already bail out before reaching postProcessPreview() if markdownEditor.value has changed since the render started, and any such edit triggers its own renderMarkdown() call (which now always computes stats up front). So there's no path left where the postProcessPreview() call was doing anything the top-of-function call hadn't already covered — just the double scan you flagged. Regenerated desktop-app/resources/js/script.js and reran the full suite (npm run check:static + playwright --project=chromium), 110/110 passing. |
|
@crazy-explore-r This PR has now been merged. Thank you for your contribution and for addressing the review feedback! |
The word/char/reading-time counters in the footer (and mobile menu) go stale once you close a document and reopen it from Files, without a page reload — they get stuck showing 0 even though the editor has content.
The cause:
updateDocumentStats()only ran as part of the preview post-processing step, andrenderMarkdown()skips that step whenever it thinks the preview HTML is already up to date for the active tab (a caching optimization). Closing the only open tab resets the stats to zero. Reopening the same tab hits that cache and returns early before the stats ever get recalculated, so they stay at zero until something forces a full re-render (like reloading the page).Fix is small: call
updateDocumentStats()unconditionally at the top ofrenderMarkdown(), so the counters always reflect the current editor content whether or not the preview HTML itself needs regenerating.Added an e2e test that reproduces it directly — type some content, close the tab, reopen it from Files, and check the counters come back instead of staying at 0. Confirmed it fails on the old code and passes with the fix.
Also ran
npm run check:staticand the full Playwright suite (--project=chromium): 109/110 passed. The one failure was an unrelated clipboard-paste test for the secret workspace flow, which is flaky under full parallel load but passes consistently on its own — not something this change touches.Regenerated
desktop-app/resources/js/script.jsvianode desktop-app/prepare.jssince it bundles a copy ofscript.js.