Skip to content

Fix preview pane scrolling to bottom on checkbox click#134

Closed
SLatz18 wants to merge 2 commits into
MarkEdit-app:mainfrom
SLatz18:fix/preview-checkbox-scroll-jump
Closed

Fix preview pane scrolling to bottom on checkbox click#134
SLatz18 wants to merge 2 commits into
MarkEdit-app:mainfrom
SLatz18:fix/preview-checkbox-scroll-jump

Conversation

@SLatz18

@SLatz18 SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown

Problem

Clicking a task-list checkbox in preview mode (overlay) caused the preview pane to jump to the bottom of the document.

Root cause: handleTaskItemToggle dispatches a silent edit to the editor to sync the checkbox state. That dispatch can cause the editor's scrollDOM to emit a scroll/scrollend event, which triggers syncScrollProgress. The editor's scrollTop reflects wherever the cursor was during the last edit session (often the bottom of the document), so the preview pane is incorrectly snapped to that position.

Fix

Save previewPane.scrollTop immediately before the dispatch and restore it both synchronously and via requestAnimationFrame (to cover async scroll triggers from WKWebView or CodeMirror's measure cycle).

The checkbox toggle and source-sync behavior are unchanged.

Testing

  • Open a file with task list items in preview mode (overlay)
  • Scroll the preview to any position
  • Click a checkbox — preview pane stays in place
  • Checkbox state and underlying source both update correctly

🤖 Generated with Claude Code

When clicking a task-list checkbox in preview mode, dispatching the
source edit could trigger scroll-sync via the editor's scrollDOM and
jump the preview to the editor's cursor position (often the bottom of
the document). Save and restore the preview pane's scroll position
around the dispatch to prevent this.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 8, 2026 13:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Preserves the preview pane scroll position when toggling task items, preventing editor dispatch side-effects from shifting the preview unexpectedly.

Changes:

  • Save previewPane.scrollTop before dispatching editor changes.
  • Restore the saved scroll position immediately and again on the next animation frame.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/view.ts Outdated
Comment thread src/view.ts Outdated
Comment thread src/view.ts Outdated
When clicking a task-list checkbox in preview mode, the editor's
scrollDOM could emit a scrollend event that triggered syncScrollProgress
and jumped the preview to the editor's cursor position (often the bottom
of the document).

Instead of saving/restoring scrollTop (which risks overriding user
momentum scrolling from a delayed rAF callback), suppress the scroll-sync
listener for the duration of the dispatch and re-enable it on the next
animation frame. The preview position is never touched, so user scroll
state is fully preserved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@SLatz18

SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Addressed Copilot's feedback — revised approach in the latest commit:

Problem with the original fix: saving/restoring scrollTop via requestAnimationFrame risked overriding legitimate user scrolling (e.g. trackpad momentum) that could happen in the ~16ms window between the click and the rAF callback. Setting scrollTop twice also caused unnecessary layout/paint work.

Revised approach: instead of touching scrollTop at all, the scroll-sync listener in scroll.ts is now suppressed for the duration of the dispatch via suppressScrollSync() / resumeScrollSync(). When the editor's scrollDOM emits a scrollend event as a side-effect of the dispatch, the listener simply skips the sync. Re-enabled on the next animation frame. The preview pane's scroll position is never written to, so user scroll state is fully preserved.

@SLatz18

SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Addressing all three Copilot flags inline:

Threads 1 & 2 — rAF can override user scroll

Why it was a problem: The requestAnimationFrame callback fires ~16ms after the click. In that window, trackpad momentum or a deliberate scroll could have moved previewPane.scrollTop. The callback would then unconditionally overwrite it with the pre-click value, silently undoing the user's scroll.

How it's fixed: Both scrollTop writes are gone. suppressScrollSync() now gates the scroll-sync listener in scroll.ts before the dispatch. If the editor's scrollDOM emits a scrollend as a side-effect of the dispatch, syncScrollProgress simply skips. resumeScrollSync() is called in a single requestAnimationFrame so async events within the current paint are also covered. The preview pane's position is never written to.

Thread 3 — double scrollTop set causes jank

Why it was a problem: Two back-to-back writes to scrollTop (one synchronous, one via rAF) forced the browser to recalculate layout and paint twice, which could produce visible jank on WKWebView when clicking a checkbox.

How it's fixed: Both assignments are removed entirely — the suppress approach means we never touch previewPane.scrollTop, so there is zero extra layout work.

@cyanzhong

cyanzhong commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

I need a repro before reviewing changes. Just tried clicking on checkboxes in overlay mode, no repro.

Can you please share more details? For example, your system version, app version, app settings/customizations, other extensions.

@SLatz18

SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Thanks for checking — here are the details and the exact repro path.

Environment

  • macOS 26.5.1 (build 25F80)
  • MarkEdit 1.33.0
  • markedit-preview 1.8.0 (full build)
  • Other extensions loaded: markedit-ai-writer, markedit-mte, markedit-enhanced-paste, markedit-highlight, markedit-proofreading, markedit-wikilinks, hitl-review, ai-rainbow

Repro steps

The bug only surfaces when the editor's scroll position differs from the preview's — so a short document that fits on one screen won't show it.

  1. Open a markdown file long enough to require scrolling — task list items near the top, plenty of content below. A quick test file:
- [ ] Task one
- [ ] Task two
- [ ] Task three

[paste ~60 blank lines here, or any filler content]

End of document.
  1. In edit mode, scroll all the way to the bottom (or place the cursor there).
  2. Switch to preview mode (overlay).
  3. Scroll the preview back up to where the checkboxes are.
  4. Click a checkbox.

Before fix: the preview pane jumps back to the bottom (matching the editor's last scroll position).
After fix: the preview stays in place, checkbox toggles normally.

If the editor cursor is already at the top when you switch to preview, the two scroll positions match and there's no visible jump — which is why it's easy to miss in a quick test.

@cyanzhong

Copy link
Copy Markdown
Contributor

Thanks, I can reproduce the issue following your steps.

However, I am still having trouble understanding:

Why is only overlay mode affected? Doing the same in side-by-side mode works fine.

@SLatz18

SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Good question — the difference comes down to whether the editor and preview scroll positions can get out of sync.

Side-by-side mode: the user scrolls the editor and the preview follows via syncScrollProgress. The two panes stay correlated. So if a checkbox click incidentally triggers a scrollend on the editor and sync fires, it snaps the preview to a position it was already near — no visible jump.

Overlay/preview mode: the editor is hidden behind the overlay. The user scrolls the preview independently, completely decoupled from the editor. The editor's scrollTop stays wherever it was when the user was last editing (e.g. the bottom of the document). When a checkbox click triggers scrollend on the editor and sync fires, it snaps the preview from wherever the user scrolled to all the way to the editor's last position — a dramatic, visible jump.

In short: side-by-side keeps the two in sync so the jump is invisible. Overlay lets them diverge, so the sync triggered by the dispatch is always destructive.

@SLatz18

SLatz18 commented Jun 8, 2026

Copy link
Copy Markdown
Author

Correction to my previous comment — the issue does occur in side-by-side mode too under the same conditions (editor and preview scrolled to different positions). I was wrong to say side-by-side was unaffected.

The root cause is the same regardless of mode: clicking a checkbox dispatches a silent edit to the editor, which can trigger a scrollend event on the editor's scrollDOM, causing syncScrollProgress to snap the preview to the editor's current scroll position. If those two positions differ, the preview jumps.

The fix (suppressScrollSync / resumeScrollSync) addresses both modes — the sync listener is simply gated for the duration of the dispatch and the following frame, regardless of which view mode is active.

@cyanzhong

Copy link
Copy Markdown
Contributor

Interesting, while I still don't have a repro in side-by-side, I think it can theoretically happen.

I checked your change and believe it works. However I think blocking scroll temporarily is a bit strange, and a rAF may be a concern.

Can you please check if changes in #135 fixes the issue? It's simpler and self-contained.

@cyanzhong cyanzhong closed this Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants