fix(sidebar): make the repaint-skip opt-in, not the default - #10
Merged
Conversation
Typing in a wiki page (or a chat, or a note) flashes the sidebar rail. The earlier fix landed in notes.js; this is the same bug in the shared rail. refreshSidebarChildren repaints every expanded category by calling renderCategoryChildren, which opens with host.innerHTML = "" — a hard teardown-and-rebuild, no diffing. And it fires on the sidebarRefresh / notesChanged events that chat.js and notes.js dispatch off their own 500ms autosaves. So an autosave in ANY panel tears down and repaints the wiki rows too, even though the wiki data never changed — a visible flash on roughly every debounce while typing. The wiki save path itself was already clean: its debounced flushSave only touches its own page-list row via renderPageList, and it dispatches notesChanged only on structural events (create/delete/rename), never on body keystrokes. The flash was the shared rail reacting to other panels. Guard the repaint: fetchCategoryChildren returns plain data, so a structural JSON compare against the cached copy is a sound proxy for "would render identically". Update the cache always, repaint only on a difference. Mirrors the notes.js fix. The other renderCategoryChildren call site (category expand into a fresh Loading… list) is a deliberate user action and correctly still repaints. Verified: esprima parses app.js / workspace.js / wiki.js, with untouched app.js as the control. Visual confirmation is the operator on desktop — the flash is a repaint a DOM-shape test cannot see.
CI caught the previous two cuts of this guard swallowing legitimate updates: sidebar-wiki.spec.ts failed on expanding a book, create/delete updating the rail, reparent, and sort. The guard diffed only the category data, but the things that change on those paths live OUTSIDE it — expansion state in sidebarTreeExpanded, page trees in sidebarWikiPagesCache — so a real mutation looked identical and got skipped. Chasing every data source into the fingerprint is the wrong direction; I missed two doing it. Invert instead: refreshSidebarChildren(force = true) repaints by default, and only the autosave-driven sidebarRefresh listener passes force=false to opt into the diff-and-skip. Every mutation caller (notesChanged clearing the cache, caret toggle, drag, page events) keeps its bare call and always repaints. So the flash fix survives — the one path that fires on a 500ms autosave still skips a no-op repaint — while nothing else can be silently suppressed. Safe by default: a new caller repaints unless it explicitly says not to. esprima parses workspace.js. The e2e suite is the real check; it failed on the last cut and is why this exists.
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
1 participant
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.
CI caught the previous two cuts of this guard swallowing legitimate updates:
sidebar-wiki.spec.ts failed on expanding a book, create/delete updating the
rail, reparent, and sort. The guard diffed only the category data, but the
things that change on those paths live OUTSIDE it — expansion state in
sidebarTreeExpanded, page trees in sidebarWikiPagesCache — so a real mutation
looked identical and got skipped.
Chasing every data source into the fingerprint is the wrong direction; I missed
two doing it. Invert instead: refreshSidebarChildren(force = true) repaints by
default, and only the autosave-driven sidebarRefresh listener passes force=false
to opt into the diff-and-skip. Every mutation caller (notesChanged clearing the
cache, caret toggle, drag, page events) keeps its bare call and always repaints.
So the flash fix survives — the one path that fires on a 500ms autosave still
skips a no-op repaint — while nothing else can be silently suppressed. Safe by
default: a new caller repaints unless it explicitly says not to.
esprima parses workspace.js. The e2e suite is the real check; it failed on the
last cut and is why this exists.
Opened automatically after the
tierscheck passed on1795bcd2.The check is attached to this branch head, so this is mergeable now.