fix(minimap, controls): keep a double-click on the canvas chrome off the canvas - #84
Open
webard wants to merge 1 commit into
Open
fix(minimap, controls): keep a double-click on the canvas chrome off the canvas#84webard wants to merge 1 commit into
webard wants to merge 1 commit into
Conversation
…the canvas Double-click zoom is bound to the container, and both the minimap and the controls panel are inside it — so a double-click on either bubbled up and zoomed the canvas as well as doing whatever the panel does. Two clicks on the minimap are two pans; the canvas jumping to another scale on top of them answers a gesture nobody aimed at it. And two quick presses of zoom-in are a double-click as far as the container is concerned, so pressing the button twice in a hurry left the canvas at the double-click level instead of one step further in under the cursor — the control fighting the canvas over the same two clicks. The controls panel already swallowed mousedown, pointerdown and wheel for exactly this reason; `dblclick` was the one it let through. The minimap now does the same for its own panel. Stopping it at the panel fixes both `zoomOnDoubleClick` modes at once — d3's stepped handler and the toggle both listen on the container, and neither sees an event that never leaves the panel. The buttons' own handlers are untouched: the event is stopped on the way out of the panel, not on the way in.
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.
What
A double-click on the minimap or on the controls panel no longer zooms the canvas underneath.
Why
dblclickzoom is bound to the container — d3's stepped handler in the default mode, AlpineFlow's own in'toggle'— and both panels are children of that container, so the event bubbled to it.Two symptoms, one cause:
zoom: 0.31tozoom: 1(the toggle level) on top of the pan it had just done.+twice in a hurry left the canvas at the double-click level instead of one step further in under the cursor — the control and the canvas fighting over the same two clicks.controls-panel.tsalready swallowsmousedown,pointerdownandwheelfor exactly this reason ("Prevent pan/zoom from triggering when interacting with buttons");dblclickwas the one that got missed. This adds it, and gives the minimap the same treatment for its own panel.Shape
One
stopPropagationper panel, on the panel wrapper. Stopping it there fixes bothzoomOnDoubleClickmodes at once — neither handler sees an event that never leaves the panel — and it does not touch the buttons: the event is stopped on the way out of the panel, not on the way in.The minimap's listener is removed in
destroy()alongside its siblings, for symmetry with the rest of the file.Tests
minimap.test.ts— a double-click on the minimap does not reach the container, and one anywhere else still does.controls-panel-fullscreen.test.ts—mousedown/pointerdown/wheel/dblclickall stay inside the panel (the first three pinning the behaviour that was already there), plus a case proving the button's ownonZoomInstill fires twice across a double-click, so the fix cannot be "swallow everything".Both new cases fail against
devwithout the source change.npm run test— 187 files, 3039 tests, all passing.Not included
dist/is not rebuilt — source, tests and docs only. No version bump, noCHANGELOG.mdentry, no new dependencies.One thing I noticed while in
minimap.tsand did not change, in case it is deliberate:onWheelcallspreventDefault()but notstopPropagation(), so a wheel over the minimap runs the minimap's own zoom and reaches the canvas's wheel zoom. Happy to fix in a follow-up if it is not intended.Independent of #82 and #83; all three branch off
devand touch different files.