fix: align list scroll speed with tree view and stabilize rename position#54
Merged
Merged
Conversation
…ization The virtualized list rebuilt its visible-row DOM on every scroll event, which made flat list scrolling move noticeably more content per wheel input than the non-virtualized tree view (perceived as 'too fast'). Raise VIRTUAL_THRESHOLD out of reach so the list renders all rows directly, matching the tree view path and its scroll behavior. The VirtualList code is retained for a future fix that re-enables virtualization without the speed regression.
Entering or exiting inline rename (and background vault-event rebuilds)
called renderList, which emptied the container and rebuilt the DOM. Two
issues caused the list to jump:
1. scrollTop was restored while the container was still empty, so the
browser clamped it to 0 — the list flashed to the top / selected row.
2. input.focus() scrolls the focused row into view by default, fighting
the restored position asynchronously.
Split renderList into renderListContent + a try/finally wrapper that
restores scrollTop only after the new DOM exists (via VirtualList.scrollTo
or a direct assignment). Pass { preventScroll: true } to focus so it no
longer scrolls. Remove the now-redundant scrollTop save/restore in the
manual-reorder handlers, which renderList now owns.
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 two issues in the flat list view.
1. List view scrolls faster than tree view
The virtualized list rebuilt its visible-row DOM on every scroll event. Each scroll frame emptied and re-appended the visible rows, so the content appeared to move more than the wheel input warranted — perceived as "scrolling too fast" relative to the non-virtualized tree view.
Fix: raise
VIRTUAL_THRESHOLDso the flat list renders all rows directly, matching the tree view rendering path and its scroll behavior.Trade-off: this effectively disables virtual scrolling, so very large vaults (thousands of files) will render more DOM nodes. The
VirtualListcode is retained for a future fix that re-enables virtualization without the speed regression (e.g. DOM node reuse instead of empty-and-rebuild).2. List position jumps when entering or exiting inline rename
Entering or exiting inline rename (also background vault-event rebuilds) called
renderList, which emptied the container and rebuilt the DOM. Two causes made the list jump:scrollTopwas restored while the container was still empty, so the browser clamped it to 0 — the list flashed to the top / selected row.input.focus()scrolls the focused row into view by default, fighting the restored position asynchronously.Fix:
renderListintorenderListContent+ atry/finallywrapper that restoresscrollToponly after the new DOM exists (viaVirtualList.scrollTofor the virtual path, direct assignment otherwise). Covers every return path.focus({ preventScroll: true })so focusing the input no longer scrolls.renderListnow owns.Verification
npm run build(tsc + esbuild) ✅npm run lint(eslint) ✅npm test— 112 tests, 18 suites ✅