fix(er-diagram): zoom the ER and query plan diagrams on Cmd+scroll - #2840
Merged
Merged
Conversation
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
This was referenced Sep 14, 2026
Merged
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.
Problem
Holding Command and scrolling over the ER diagram or the EXPLAIN plan diagram scrolled the canvas instead of zooming it. The docs (
docs/features/er-diagram.mdx,docs/features/explain-visualization.mdx) and the changelog both promise Cmd+scroll zoom.Root cause
Both diagrams sit in
MagnifiableCanvasView, a plainNSScrollViewwithallowsMagnification. That property wiresmagnify(with:)andsmartMagnify(with:)only. Measured with a swiftc probe on macOS 27: a scroll-wheel event carrying Command, Option or Control, in line or pixel units, only scrolls. The view's header comment said Cmd+scroll came from AppKit.#2117 moved both diagrams onto that component and deleted
ERDiagramCanvasContainer, thescrollWheel(with:)override that used to turn a Command scroll into zoom (#1810). Nothing replaced it.Every other surface was checked: the result map uses MapKit's own controls, and the cell image preview, charts, SQL editor, data grid and the iOS app have no canvas zoom, so none of them has this bug.
Fix
DiagramScrollView, anNSScrollViewsubclass the canvas now creates, overridesscrollWheel(with:). The scroll view already owns magnification, so it owns this zoom too.DiagramScrollZoomholds the decision as a pure type:exp(points * 0.01): one wheel notch (10 points,NSScrollView's line scroll) is about 10.5%, a notch in and a notch out cancel exactly, and one event is capped at 30 points so an accelerated wheel spin cannot jump across the whole range.isDirectionInvertedFromDevice).setMagnification(_:centeredAt:), taking the point in clip-view space asNSScrollView.hspecifies.scrollWheel(with:)turns off; every scroll it does not zoom still goes tosuper.Tests
DiagramScrollZoomTests: factor per notch and per point, natural scrolling, cancellation, the per-event cap, every modifier chord, the gesture latch, momentum, degenerate deltas.DiagramScrollViewTests: realNSEvent(cgEvent:)wheel events into a real scroll view: Command zooms, other chords do not, zooming back returns to the start, the document point under the pointer stays within 0.5pt at 50%, 100% and 200%, the bounds hold, responsive scrolling stays on.DiagramZoomUITests: opens the ER diagram on the sample database and checks a Command-modified scroll changes the zoom level.Verification
DiagramScrollZoomTests,DiagramScrollViewTests,DiagramViewportControllerTests,DiagramZoomTests,DiagramPaintCoverageTests: 61 cases, all pass locally.--stricton every changed file: 0 violations.DiagramZoomUITestsdid not run locally: macOS held an unanswered "Enable UI Automation" prompt, so the runner timed out enabling automation mode with zero cases executed. CI runs it./code-review high, which found no defects.scrollWheel(with:)override with responsive scrolling on. Synthesized events cannot exercise a real trackpad gesture.Before / After
This is an input change with no static visual difference: before, the diagram scrolled under Cmd+scroll; after, it zooms around the pointer. The UI test covers it in the running app.
Part of a stack: the next PR moves pointer input in both diagrams to AppKit, because SwiftUI reports clicks and drags at the wrong position once a diagram is zoomed.
https://claude.ai/code/session_015m6g84UEwS6v3m2twr9Taw