-
-
Notifications
You must be signed in to change notification settings - Fork 59
Improve scroll synchronization between editor and preview panes #201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ExactDoug
wants to merge
14
commits into
mb21:master
Choose a base branch
from
ExactDoug:claude/analyze-split-pane-scroll-0151jxfoZo18YWfEW5xZFBjn
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Improve scroll synchronization between editor and preview panes #201
ExactDoug
wants to merge
14
commits into
mb21:master
from
ExactDoug:claude/analyze-split-pane-scroll-0151jxfoZo18YWfEW5xZFBjn
Conversation
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
Document root causes of split pane scroll synchronization issues on Windows, including O(n) reverse lookup, 30ms throttle jitter with Windows timer resolution, and DPI rounding inconsistencies. Includes recommended fixes prioritized by impact.
- Replace O(n) linear search with O(log n) binary search for reverse scroll map lookups, fixing severe performance issues with large docs - Switch from 30ms fixed throttle to requestAnimationFrame for frame-coupled updates, eliminating Windows timer jitter - Normalize rounding to Math.round() everywhere for consistency, fixing cumulative DPI drift from mixed Math.ceil/round These changes address the primary root causes of scroll sync issues on Windows, particularly with DPI scaling and large documents.
Add entry for editorPos=0, previewPos=0 to reverseScrollMapEntries. The loop started at i=1, causing the binary search to fail for positions near the top of the document.
Instead of returning just the lower bound from binary search, interpolate between the two nearest entries to calculate a more accurate editor position. This should eliminate the ~1/2 page offset that occurred when positions fell between known entries.
Temporary instrumentation to diagnose scroll sync issues: - Log scroll map build stats (sizes, sample entries) - Log editor→preview sync (input/output positions) - Log preview→editor sync (input/output positions) - Log interpolation details (entries, factor, result) This commit should be reverted after debugging is complete.
- Add scroll lock mechanism to prevent feedback loops where scrolling
one pane triggers the other pane to scroll back
- Track scroll source ('editor' or 'preview') and ignore events from
the pane that didn't initiate the scroll
- Clear lock after 50ms to allow normal scrolling to resume
- Clamp editor scroll position to valid scrollMap range to prevent
undefined mappings at document end
Replace absolute pixel position mapping with scroll percentage approach: - Calculate scroll percentage as: scrollTop / scrollableRange - Apply same percentage to the other pane's scrollable range - Both panes now reach their bottoms at the same time This fixes the issue where preview would reach bottom while editor was only 1/3 through the document, caused by different content heights and viewport sizes between the panes.
Combines best of both approaches: 1. Uses scroll map for line-based correlation, so headings (H1/H2/H3) and other elements with varying heights align properly 2. Scales the scroll map values to fit actual scrollable ranges, ensuring both panes reach their bottoms at the same time The scroll map provides content-aware positioning (e.g., a heading in the editor maps to the same heading in the preview), while the scaling ensures the overall scroll ranges match up.
Fix scaling instability caused by scroll map being rebuilt with different values. Instead of using maxPreviewInMap (which changes each rebuild), use the actual content heights: - previewScrollTo = (scrollMapValue / previewContentHeight) * scrollableRange - editorScrollTo = (editorPosInMap / editorContentHeight) * scrollableRange This provides a stable basis for scaling that doesn't change when the scroll map is rebuilt during scrolling.
The previous change to use previewContentHeight caused worse sync because scrollMapValue can exceed previewContentHeight (they come from different sources - getBoundingClientRect vs scrollHeight). Revert to using the scroll map's own maximum value for consistent scaling, as the scroll map values are internally consistent even if they don't match the actual content height.
Clean up console.log statements used during development and testing. The scroll synchronization improvements are complete for this iteration. Key improvements in this branch: - O(log n) binary search with interpolation (vs O(n) linear) - requestAnimationFrame throttling (vs 30ms fixed interval) - Scroll feedback loop prevention - Hybrid range normalization for consistent scrolling
The Preview→Editor direction was incorrectly scaling editorPosInMap by (editorPosInMap / maxEditorInMap) * editorScrollableRange, but the Editor→Preview direction uses editorScrollTop directly as an index without scaling. This asymmetry caused ~5% drift (e.g., 1800 → 1715 instead of 1800). Now both directions are symmetric: - Editor→Preview: input direct, output scaled - Preview→Editor: input scaled, output direct Also re-added debug logging for testing.
The editor scrollable range (17,422px) exceeded the scroll map size (16,615 entries), causing the preview to stay stuck at the bottom when scrolling up from the end of the document. Now both directions scale between actual scroll ranges and scroll map coordinates: Editor→Preview: - Scale editorScrollTop to scroll map index - Scale scrollMapValue to preview range Preview→Editor: - Scale previewScrollY to scroll map coordinates - Scale editorPosInMap back to editor range This ensures the full scroll range is utilized in both directions with perfect round-trip accuracy.
Final cleanup after implementing symmetric scaling fix. All scroll sync improvements are complete for this iteration.
Owner
|
Thanks for the PR. Did you test it yourself as well? And don't you think we still need some kind of throttle? |
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.
Summary
This PR significantly improves the scroll synchronization between the markdown editor (left pane) and the rendered preview (right pane), addressing longstanding issues particularly noticeable on Windows.
Key Improvements
Technical Details
The scroll map correlates editor line positions to preview element positions. Previously:
Now both directions use symmetric transformations:
Performance Impact
Testing
Tested on Windows with:
Known Limitations
The scroll map is line-based and cannot perfectly account for varying content heights (H1/H2/H3 headers, tables, images render at different sizes in editor vs preview). This is a fundamental limitation that would require a different approach to fully resolve.
Test Plan