Repository navigation
fix(curves): make curve endpoints easy to grab and drag - #1853
Merged
Merged
Conversation
- Keep the unsnapped position in the drag accumulator: writing the edge-snapped value back made points at x=0/255 unable to leave the edge with slow mouse moves (regression from the fine adjust mode). - Initialize the pointer and accumulator when dragging a parametric splitter, so it no longer jumps on the first move. - Clicking within 12px of an existing point grabs it instead of adding a new one, and no point is created on an x already used by another point.
Owner
|
LGTM, thanks a lot for the fix! |
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.
Description
The curve endpoints (bottom-left and top-right) are hard to grab and drag in point curve mode: a click often creates a new point next to them, and once grabbed they tend to stay "stuck" on the edge.
There are two separate causes:
1. Points can't leave the left/right edge with slow mouse moves (regression from a683220).
Since the fine adjust mode, dragging is incremental: each move adds its delta to
accumulatedPointRef. The edge snap (x < 5 → 0,x > 250 → 255) was then written back into that accumulator. Moving a point at x = 0 by 1–3 units permousemovegets snapped back to 0 on every event, so it only leaves the edge if a single event moves it more than 5 units (i.e. a fast flick). Vertical movement isn't snapped, so the point slides up and down but feels glued horizontally.The same commit also never initialized
lastPointerRef/accumulatedSplitRefwhen starting to drag a parametric splitter, so the splitter could jump on the first move.2. Narrow misses create a new point.
Point handles are
r="6"circles in a 255-unit viewBox (~12 px wide on a typical panel), and at the corners part of the handle overflows the graph. A click that misses by a couple of pixels lands on the container andhandleContainerStartinserts a new point right next to the endpoint. Clicks in the container padding are clamped to x = 0 / 255, which stacks a second point on the same x as the endpoint; that point is rendered on top and blocks the original one (prevX + 0.01constraint), adding to the "stuck" feeling.Type of Change
Changes Made
handleMove: the accumulator keeps the unsnapped position (only clamped between neighbours); the edge snap is applied to the emitted value only. Snapping near the edges still works, but slow drags now accumulate and leave the edge normally.onMouseDown/onTouchStartnow initializelastPointerRefandaccumulatedSplitReffrom the current pointer and split value.handleContainerStart: before creating a point, look for the nearest existing point in screen pixels; if it is within 12 px (POINT_HIT_RADIUS_PX), grab it viahandlePointStartinstead. Using screen pixels keeps the tolerance constant whatever the panel width.handleContainerStart: never create a point within 1 unit of an existing point's x (MIN_POINT_GAP_X), which prevents the duplicated endpoint from padding clicks.onDragStateChange(true)is only called once a point is actually created or grabbed.Screenshots/Videos
Testing
Tested scenarios:
Test Configuration:
Checklist
Additional Notes
tscpasses and Prettier is clean. ESLint reports no new errors onCurves.tsx(the remaining ones are pre-existingno-explicit-any).AI Disclaimer:
Please state the involvement of AI in this PR: