-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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(Canvas): control down/up pointer #9463
base: master
Are you sure you want to change the base?
Conversation
Build Stats
|
Eventually it would be the same thing we send to the actionHandler for consistency. Why do you think this is a bug? which is the backstory? |
It is another control confusion
Also a confusion. Actually a patch/hack I did to fix controls under group without a lot of refactoring. But it makes no sense. All should be in the viewport as controls are |
yeah i don't know. your patch/hack (as you call it), could be less hacky by moving that plane shift inside the action handler with a wrapper, called 'support nested object' Going back to controls, so for you since those works with oCoords they should work viewport coords. Since we don't have any custom existing mousup/down coordinates why we don't offer a wrapper for actions like the withFixedPosition, and we call it withViewportCoordinates? so that we can change what the custom action see and who writes the custom action can pick up a wrapper or not. One example is that i want to check where i m dropping a control compared to other objects and since all geometry function are getting the viewport version removed, i would then need the version with the scene pointer. if i want to know if i m overing another control, for example to connect an arrow between 2 controls of 2 objects, the viewport one seems more usable. |
That is why I promote |
Indeed go away |
In fact this the control API has ambiguous values for x, y passed args. |
Motivation
Description
The pointer for mouseDownHandler/mouseUpHandler should be from the viewport
https://github.com/fabricjs/fabric.js/pull/8519/files#diff-72332330d8e00b2fda6e57d000a93e4e8dd8a46fdc8bcbdb3320c23b1509f8b5L748-L752
the bug exists on v5:
https://github.com/fabricjs/fabric.js/blob/5.x/src/mixins/canvas_events.mixin.js#L468-L472
Changes
Gist
In Action