Skip to content
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

Control objects do no receive mouseup correctly #7452

Closed
k1w1 opened this issue Oct 25, 2021 · 4 comments · Fixed by #7612
Closed

Control objects do no receive mouseup correctly #7452

k1w1 opened this issue Oct 25, 2021 · 4 comments · Fixed by #7612
Assignees

Comments

@k1w1
Copy link

k1w1 commented Oct 25, 2021

Version

4.5.0

During MouseUp event handling the control corner is looked up again based on the current mouse position https://github.com/aha-app/fabric.js/blob/v4.5.0/src/mixins/canvas_events.mixin.js#L459. This is a problem in any case where the mouse is not over the control when it is released - in those cases the mouseUpHandler function on the control is never called.

Why might the mouse not be over the control? In any situation where the motion of the control is constrained, e.g. with snap to grid, or some other constraint, where the control does not move perfectly in sync with the mouse.

The simple solution appears to be to use the corner that was computed during the mouse down, e.g.

var corner = target.__corner;

@asturur
Copy link
Member

asturur commented Oct 27, 2021

yes i agree this is unoptimal.
I ll have a look at it and see if there are side effect fixing.
An example that comes to my mind is, what if you are having custom controls to use controls as junctions for connections?
At that point you would want to re-check on mouse up on which control you landed on.

@asturur
Copy link
Member

asturur commented Oct 27, 2021

maybe can be first check, if nothing found, assume the old one.
The use cases in which someone needs both would be really rare

@k1w1
Copy link
Author

k1w1 commented Oct 27, 2021

If you keep the existing behavior of calling mouseUp for the control being hovered over, which seems reasonable, I think you should also call mouseUp on the control that received the mouseDown. There are lots of circumstances where it might be necessary to clean up when the drag ends - e.g. to remove guide lines that were drawn during the drag.

@asturur asturur self-assigned this Jan 15, 2022
@asturur
Copy link
Member

asturur commented Jan 15, 2022

i think you are perfectly right.
It sounds correct that both controls should fire a mouseup.
Maybe we can add a property to the event to signal which one is under the corsor and which one is just released

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants