-
-
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
feat(IText): Draggable text #7802
Conversation
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Code Coverage Summary
|
Tried to reproduce on windows Chrome without success. |
I've simplified my test and the tiny bounding box was actually an app issue. The issue of the object being stuck in edit mode with no bounding box remains though. Here's my dataTransfer data: Also, firefox seems render the hover image in the wrong position/size on highdpi displays (my pixel ratio is 1.5) |
could you upload the test into a sandbox? |
The positioning issue is strange. |
@melchiar test on http://fabricjs.com/test/misc/itext.html (with the local server) |
border: 'none' | ||
}); | ||
fabric.document.body.appendChild(dragImage); | ||
e.dataTransfer.setDragImage(dragImage, offset.x, offset.y); |
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.
@melchiar try manipulating the offset value
Perhaps firefox accounts for retina scaling?
can you make sure all drag/drop events are fired in the correct order? It might be that the mouse up handler isn't called correctly at the drag end operation |
Code Coverage Summary
|
There are a bunch of things i want to experiment with before merging.
I want to try and test the code and see if i have alternatives for those problems, i don't have direct comment to lines that i can ask to change, is more a generic thing. But again, all those informations shouldn't be digged in code review, for a large pr and features |
Code Coverage Summary
|
|
@@ -299,6 +299,17 @@ | |||
*/ | |||
onSelect: function() { | |||
// implemented by sub-classes, as needed. | |||
} | |||
}, | |||
|
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.
I guess we should/could add onDragStart: () => false
here, as onDragStart
and canDrop
are public methods for overriding
Only for better readability
Co-authored-by: Shachar <34343793+ShaMan123@users.noreply.github.com> Co-authored-by: Shachar <34343793+ShaMan123@users.noreply.github.com> Co-authored-by: Shachar <34343793+ShaMan123@users.noreply.github.com>
Code Coverage Summary
|
@@ -512,6 +506,7 @@ | |||
e.dataTransfer.effectAllowed = 'copyMove'; | |||
this.setDragImage(e, data); | |||
} | |||
this.abortCursorAnimation(); |
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.
for future follow up
this line might be causing selection to be cleared on drag start
Co-authored-by: Shachar <34343793+ShaMan123@users.noreply.github.com> Co-authored-by: Andrea Bogazzi <andreabogazzi79@gmail.com>
General
Adds support for native like text dragging! Out of the box!
This PR aims to support text dragging, making fabric's text objects feel/behave like a HTML textarea.
It is an expected interaction from the user's point of view.
closes #7800
Changes
onDragStart
andcanDrop
are public methods for overriding that control drag and drop respectivelydragStart
logic cycle - imitates spec, invokes target'sonDragStart
method, if result istrue
drag starts. Behaves likeonSelect
.onDragStart
dragEnd
calls_onMouseUp
synthetically - the browser doesn't do it and fabric needs it to finalize interaction logicThoughts
I think we ought to change the
selectWord
method to align with the behavior of html textarea which selects the word and all the trailing whitespace (it's important for drag and drop).TO DO
DragEvent.dataTransfer