fix(er-diagram): replace app-wide scroll monitor with responder chain scroll handling#1810
Merged
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
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 #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