Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Use native drag'n'drop to drag text #973

Closed
danyaPostfactum opened this Issue Sep 25, 2012 · 19 comments

Comments

Projects
None yet
2 participants
Contributor

danyaPostfactum commented Sep 25, 2012

http://caniuse.com/#feat=dragndrop

As I see, drag'n'drop api are supported by almost all desktop browsers

This will give ability to move text out of editor, visualize ctrl key state while dragging, have true drag'n'drop cursor.

Contributor

danyaPostfactum commented Oct 1, 2012

I've tried to implement it. During this I have found next problems:

  • IE 9(8) does not handle d'n'd of div's by default. We have to give kick to it by calling element.dragDrop() (ie non-standart native method)
  • IE 9 does not allow to dinamically change dropEffect (visualization of action using cursor - such as copy or move), depended on Ctrl key state, for example .
  • IE 9(8) has no setDragImage() method (this method is usefull for clearing default drag image (semitransparent copy of element), but IE by default has no this image, so this is not the really problem).
  • FF document.setCapture() method disables dragging possibility. (and it seems IE can also break dragging process)

I added draggable attribut to ace_content and added handlers to dragstart/dragend/drop events - it forks fine in Chrome, but need to be fixed in IE and FF. I have not tested it in Safari.

I would like to hear any thougths of the developers about all it. I will add my tries to my fork later.

Member

nightwing commented Oct 1, 2012

Using native drag'n'drop would be very good.
Only we need to keep dragDelay setting,
so draggable attribute should be added only after clicking and holding mouse.

btw, I have tried to do this about 2 years ago nightwing/ace@574bf9e
but back then it was horribly slow on firefox.

Contributor

danyaPostfactum commented Oct 1, 2012

I am confusing by dragDelay usage. Why this for ? I have not seen such strange setting in any text editor.

I did not think about dinamic draggable property - looks interesting, but I am not sure this works in all browsers.

But we can also cancel the drag process on dragstart event to support dragDelay feature.

I see to ways to keep inner drag state of editor (to differ external dragging from internal):

  1. set state on dragstart and clear it on dragend (but I have read that in some old browsers the dragend event is fired before the drop event)
  2. check event.srcElement on dragover / drop.
Member

nightwing commented Oct 2, 2012

using dragDelay was suggested in #134 (comment). Seems to be the standard way on mac. At first i didn't like it too, but now think it is better.

But we can also cancel the drag process on dragstart event to support dragDelay feature.

i remember that on firefox canceling dragstart was canceling mousemove events until mouseup

I see to ways to keep inner drag state of editor (to differ external dragging from internal):

don't know which one is better, maybe try both, or have a look at codemirror or orion.

Contributor

danyaPostfactum commented Oct 20, 2012

Seems to be the standard way on mac

So, let it behave on mac like on mac, and on windows like on windows (the same with linux).

First time i was using ACE, I thought it did not support text dragging. Drag delay makes windows user experience bad. If we want native drag, so let it have native drag delay (zero in most cases).

Contributor

danyaPostfactum commented Oct 20, 2012

Opera does not support setDragImage (tested on 12.02) and shows default generated drag image of whole editor ( .ace_content is draggable). Same with CodeMirror.

I think this could be avoided if we create new transparent layer and handle dragging of it. Does we need it?

Member

nightwing commented Oct 21, 2012

So, let it behave on mac like on mac, and on windows like on windows (the same with linux).

Setting dragdelay to 0 for non mac might be good,
but mac uses dragdelay only with selection, and draggable behaves the same as on other platforms.
So we must keep the setting.

Opera not handling drag image isn't important, it will be fixed sometime, but we can try to reuse almost empty printMargin for that.

Contributor

danyaPostfactum commented Oct 21, 2012

I have almost finished d'n'd implementation on Windows (Linux seems also)

There are serious problem with IE10 Preview - native d'n'd is broken. This browser does not update event.clientX/Y coordinates during ondragover event. I hope they will fix it. It would be better to check this bug in the buglist, but I can not find their bugtracker unfortunatly.

IE 9 has little bug - it does not respect event.dropEffect, so it always is copy.
IE 8-9 does not support draggable attribute, so the process is triggered manually on mousemove event with custom drag offset (5px)

Opera creates semitransparent bitmap even for transparent element. I used some trick - just created draggable div with 1px width/height.

Since IE and Opera does not fully support pointer-events, ones need extra mask layer to prevent multiple dragenter/dragleave events (I am not sure wether is this nesessary).

Contributor

danyaPostfactum commented Oct 25, 2012

Where should be placed drag event listeners ? In dragdrop.js ? Or default_handlers.js ?

Member

nightwing commented Nov 6, 2012

not sure, probably can leave in dragdrop.js if it in dragdrop.js for now.
Is the code on github, i couldn't find it in your fork?

Contributor

danyaPostfactum commented Nov 12, 2012

I have spent lots of time while d'n'd implementation. Now I have following issues:

  1. dnd doesn't work with Safari 5 (tested on Win)
  2. dnd doesn't work with IE10 preview ( I hope it is fixed in IE 10 release )
  3. IE 9 always shows cursor for copy dropEffect.
  4. Opera drag cursor blinks during dragging ( codemirror has no this issue).
  5. In linux Chrome, pressing Ctrl key does not trigger dropEffect change to copy immediatly - user need to move the cursor a bit to update drag operation (dropEffect)

It seems 1 and 3 can not be fixed. 1 and 4 are minor ui bugs, i suppose these can be left. 2 need for test.

By the way, dragDelay should be 150ms by default ( as on Mac OS). dragWait waits for 1px moving for start to select, because of this movement disables drag ability.

Tested in IE9,Chrome, FF, Opera on Win. Chrome, FF on Linux. Safari 6 on Mac OS

Contributor

danyaPostfactum commented Nov 24, 2012

https://github.com/danyaPostfactum/ace/compare/nativednd

Here is almost complete version of my work.

This works fine with FF, almost fine with Chrome and Safari 6. Chrome sometimes doesn't start dragging.

In IE the cursor goes crazy (blink interval is decreasing). In Opera the dragging cursor (pointer) blinks due to unknown reason.

dragDelay is set to 150ms (approx. as in MacOS)
dragOffset is equal to 0

Member

nightwing commented Dec 11, 2012

sorry i didn't have time to review this earlier.
Overly i like this a lot, and only see some minor issues

  • would be good to keep setCapture, without that cursor changes when moving mouse out of editor during selection
  • not sure about using private variables for storing selection state 1) they are shared between all editor instances 2) makes it harder for extensions (like multiselect) to modify default behavior
  • default handlers became too big, i'd say keeping all of the drag behavior in dragdrop.js would be cleaner. What do you think?
  • dragging over selection should display editor caret inside the selection, and dropping should clear selection, this would improve situation on chrome as well, since there will be some feedback when drag starts
  • would be nice to scroll if cursor enters first/last line (but this might be tricky and not important)
  • dargging text from the end of the line to it's start removes wrong text
  • it might be better to move code which creats overlay div to dragdrop
Contributor

danyaPostfactum commented Dec 15, 2012

  • setCapture disables changing draggable on the fly.
  • Don't know :). I think you are right.
  • Agree, but how to share a state and possibly some variables?
  • For now, the dataTransfer.dropEffect is used to define drop action. So, when it's value is 'none', there is no ondrop effect occured. dragOperation variable is used for IE and Chrome to avoid their bugs. But we can use it for internal usage, to keep the actual dropEffect (set dropEffect to move, while dragOperation to none). Or something like that.
  • Yes, but I have not mastered it, and leave to someone else :)
  • Indeed. This is because the old selection is cleared on dragend event, after drop event (and text insertion). I think we have to differ the logic of internal drag/drop from external one.
  • What? I can't understand...

There are the issues to be fixed:

  • Pointer blinking in Opera. At least to figure out the cause of it.
  • Crazy caret in IE.
  • Wrong selection restoring on undo.
  • As was said, wrong range clearing.

Also, I did not tested it with IE10.

Contributor

danyaPostfactum commented Jan 27, 2013

I give up. This is all I can do: https://github.com/danyaPostfactum/ace/tree/nativednd

  • Opera blinks cursor due to cyclic font measuring + complex dragover handler
  • I fixed IE crazy caret. But it works very slow (don't know why)
  • Selection is not restored on undo
  • Chrome sometimes does not start dragging.

@nightwing if you like, you can create new branch to pull my work (if you think this is not ready for master branch)

Contributor

danyaPostfactum commented Jan 27, 2013

The issue in Chrome is related with multiple clicks. Double-click on a word and press mouse button again within a 200ms - webkit interprets this action as triple-click and prevents dragging. I don't know how to fix it ((

Contributor

danyaPostfactum commented Jan 27, 2013

This Chrome issue is depended on #978. Browser recognizes triple click (wich default action is line selection, not dragging), but ace not. I can't understand why Firefox has no this problem.
I think this issue can be solved by modifying event.addMultiMouseDownListener

Contributor

danyaPostfactum commented Jan 30, 2013

We can setCapture() on startSelect function in Firefox. With click count patch native d'n'd works fine on FF and WebKit (except sleeping cursor). I'm sure it is possible to get it work fast on IE.

Contributor

danyaPostfactum commented Jun 16, 2013

Proposed patch: #1431

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment