-
Notifications
You must be signed in to change notification settings - Fork 38
fix drag mouse #79
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix drag mouse #79
Conversation
Did some testing on this site: https://master--5fc05e08a4a65d0021ae0bf2.chromatic.com/?path=/story/core-draggable-hooks-usedraggable--basic-setup discovered that the number of steps and the delay between them as you drag the mouse is important to finesse found success with the defaults in this PR, but making them configurable in case users want to deviate
Mesa DescriptionDid some testing on this site: https://master--5fc05e08a4a65d0021ae0bf2.chromatic.com/?path=/story/core-draggable-hooks-usedraggable--basic-setup This PR improves mouse dragging reliability by introducing configurable steps and delays. Testing revealed that breaking a single drag action into multiple smaller movements with a slight delay is crucial for success.
Checklist
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Performed full review of 4ef116e...9c427f3
Analysis
While the PR introduces smooth, interpolated dragging with a mathematically sound approach, potential architectural issues include:
-
The reliance on relative mouse movements may introduce compatibility issues with systems expecting absolute positioning.
-
The added configuration parameters (steps_per_segment and step_delay_ms) could increase API complexity and create maintenance challenges if default values need future adjustments.
-
Precomputing total steps optimizes performance but may increase memory usage for complex drag operations with many segments.
-
The interpolation algorithm, while mathematically precise, might introduce subtle behavioral differences that could affect existing client code dependent on the previous implementation.
Tip
⚡ Quick Actions
This review was generated by Mesa.
Actions:
Slash Commands:
/review- Request a full code review/review latest- Review only changes since the last review/describe- Generate PR description. This will update the PR body or issue comment depending on your configuration/help- Get help with Mesa commands and configuration options
4 files reviewed | 0 comments | Review on Mesa | Edit Reviewer Settings
|
Just curious Should the num steps dynamically change by the distance of each segment? For example, I kinda find (0, 0) -> (1, 1) and (0, 0) -> (1000, 1000) having the same # of steps to be weird. |
|
at least in my testing i only saw dragging fail when the delay was too short, so i don't think the number of steps matters as much as the delay between steps |
Did some testing on this site: https://master--5fc05e08a4a65d0021ae0bf2.chromatic.com/?path=/story/core-draggable-hooks-usedraggable--basic-setup
discovered that the number of steps and the delay between them as you drag the mouse is important to finesse
found success with the defaults in this PR, but making them configurable in case users want to deviate
Checklist
Note
Implements smooth drag using relative step movements with configurable per-segment step count and per-step delay, updates OpenAPI, and adds tests.
pathviamousemove_relativesteps with small sleeps for smoothing; derive steps usinggenerateRelativeSteps; supportsteps_per_segment(default10) andstep_delay_ms(default50).generateRelativeSteps(dx, dy, steps)helper to distribute integer relative moves.generateRelativeSteps.DragMouseRequest: addsteps_per_segmentandstep_delay_ms(with defaults/min constraints) inserver/openapi.yamland generatedserver/lib/oapi/oapi.go.PressKeyRequest.keysdoc: note items may be combos likeCtrl+t.Written by Cursor Bugbot for commit 9c427f3. This will update automatically on new commits. Configure here.