Skip to content

fix(er-diagram): replace app-wide scroll monitor with responder chain scroll handling#1810

Merged
datlechin merged 1 commit into
mainfrom
fix/er-scroll-monitor-1805
Jul 3, 2026
Merged

fix(er-diagram): replace app-wide scroll monitor with responder chain scroll handling#1810
datlechin merged 1 commit into
mainfrom
fix/er-scroll-monitor-1805

Conversation

@datlechin

Copy link
Copy Markdown
Member

Fixes #1805.

Root cause

Two-finger scrolling could go dead across the whole app. The ER diagram installed an app-wide NSEvent local monitor for scroll events, gated on an isMouseOverCanvas flag fed by onContinuousHover. Tabs are native window tabs, so a backgrounded ER diagram tab keeps its view tree mounted: onDisappear never fires and hover never reports .ended when the window is hidden without mouse movement (keyboard tab switch, Cmd+T). The flag froze at true, the monitor stayed installed, and it swallowed every scroll event in every window. Scrolling stayed dead until the user moved the mouse over the diagram again. The reporter confirmed both the trigger and this exact recovery path on the issue.

Fix

Delete the monitor and the hover flag. The diagram content now sits inside an NSView container (ERDiagramCanvasContainer) that overrides scrollWheel(with:). Scroll events reach the diagram only through AppKit hit testing and the responder chain, scoped to the view under the pointer, so there is no global state left to go stale. Pan and Cmd-scroll zoom behave exactly as before, including momentum panning. The delta math moved to ERDiagramScrollTranslator, a pure type.

A container behind the hosted content does not intercept mouse events, so node dragging, tap select, pinch zoom, and the hover cursor keep working. This avoids the drag blocking that the earlier overlay approach caused (686649b), which is why the monitor existed in the first place (0b6bd13).

Testing

  • ERDiagramScrollTranslatorTests: pan multipliers for precise vs line deltas, zoom delta, sign handling, zero deltas.
  • ERDiagramCanvasContainerViewTests: drives scrollWheel(with:) with synthesized CGEvent-backed scroll events and asserts canvasOffset and magnification through the view model, including zoom clamping.
  • swiftlint --strict clean on all changed files.
  • No UI automation: XCUITest cannot synthesize trackpad phased scroll gestures.

@datlechin datlechin merged commit 9a96533 into main Jul 3, 2026
3 checks passed
@datlechin datlechin deleted the fix/er-scroll-monitor-1805 branch July 3, 2026 18:07
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.

Scrolling broken

1 participant