Use native HTML5 Drag'n'Drop for text. #1599

danyaPostfactum · 2013-09-10T11:45:50Z

Sorry, I donno how to rebase branch, so this is the new version of #1431 .
All issues I wanted are fixed.

This needs test and review.

Issues I would like to fix but need help:

Wrong selection on undo of drag operation.
Autoscroll while dragging should not scroll if there is no line (scrolled past last row) or char (scrolled past last char) at cursor.
Editor should not scroll selection into view if we leave (through negligence) editor rect while autoscrolling.

Issues we can not resolve:

Wrong ui feedback (drag cursor) in IE9+
Safari 5 does not support dnd

Issues I do not know reason of:

Opera blinks drag cursor (cursor icon) while dragging.

Issues I don't like or not sure:

Sequence click interval is handled by browsers differently.

Also, in Firefox, dragenter event fires twice, so dragleave event does not get caret back. Can anybody reproduce?

Great news! All bugs I (or someone else) found in IE and reported to MS are marked as Won't Fix )))
They say those are "by design", "not reproducible" etc :)

danyaPostfactum · 2013-09-11T16:56:08Z

Fixed IE issues (reverted captureMouse logic).
Improved autoscroll behavior.

danyaPostfactum · 2013-09-11T17:25:32Z

Selection is correct after undo if text was moved below (or right). It's wrong if you move text to the left or to the top.
It's ok if I remove this line: https://github.com/ajaxorg/ace/blob/master/lib/ace/edit_session.js#L1248

nightwing · 2013-09-11T19:03:11Z

This is absolutely awesome!
Only issue i noticed is that on old Opera setting drag image doesn't work anymore.

For 'wrong selection on undo' issues, i am going to change it to keep and restore actual selection instead of reconstructing from change ranges.
Will do that and more detailed review this weekend.

danyaPostfactum · 2013-09-12T04:17:37Z

What is your version of Opera? 12.14 (win7) works fine. Can you check CodeMirror? It uses similar workaround (in fact, I stole it :) )
Can you reproduce mentioned Firefox issue?

nightwing · 2013-09-12T08:15:08Z

I was testing on 12.14 too. Seems like it happens only when splitview is active.
I couldn't reproduce firefox issue on ff 26

danyaPostfactum · 2013-09-12T09:20:42Z

It seems image must be rendered on screen. Is it ok if image always present (in Opera) at screen with such styles?

element.style { 
    position: fixed;
    height: 1px;
    width: 1px;
    top: 0px;
    left: 0px;
    opacity: 0;
    visibility: hidden;
    z-index: 2147483647;
}

visibility is toggled right on dragstart event for a while. display:none can't be used...

nightwing · 2013-09-12T09:25:03Z

I think it's ok.

danyaPostfactum · 2013-09-12T13:55:00Z

Opera drag image is fixed.
Bth, I found why cursor flashes. It is caused by element dimension access. This includes getBoundingClientRect() and offset*** properties. As an option, we could disable font size change polling, and cache scroller client rect (updating on page resizing (zoom) and scrolling (browser autoscroll) ) while dragging. But I think this is not necessary. May be later.

nightwing · 2013-09-12T15:12:16Z

lib/ace/css/editor.css

@@ -13,6 +16,7 @@
    top: 0;
    bottom: 0;
    background-color: inherit;
+    z-index: 8;


why is this needed?

Scrollbar elements are a bit wider, than their visible part. This is needed to overlap invisible part of scrollbar, so autoscroll triggers at very edge of scroller.

This won't work on mac since scroller will completely cover scrollbar elements. Can we add drag listeners to the container instead?

Hm..Changing event target seems working, but I don't like that scrollbars turned into drop zones..

Why, in textarea scrollbars are drop zones, in notepad++ both gutter and scrollbars are drop zones, i think it's much better.

danyaPostfactum · 2013-09-14T16:52:43Z

lib/ace/mouse/dragdrop_handler.js

+                }
+                this.setState("dragWait");
+            } else {
+                this.startDrag();


This must not be called on double/triple/quad click. How can we pass click count here?

Maybe set clickCount on event in multimousedown listener and add getClickCount to mouseEvent?

nightwing · 2013-09-14T18:50:11Z

Good to know what causes flickering on opera, i agree that fixing it is better left for the future.

nightwing · 2013-09-14T18:58:34Z

In old version dragging was disabled for multiselect, but here it only drags text from the last selection.
Should we disable it or set drag text to the value of getSelectedText() ?

Also on IE when ctrl+clicking on selected text there is a quick flash of native selection.

nightwing · 2013-09-15T11:06:15Z

Editor should not scroll selection into view if we leave (through negligence) editor rect while autoscrolling.

also it shouldn't loose it's scroll position if text is dragged over it to another editor

danyaPostfactum · 2013-09-15T13:55:33Z

Phhhhhh...Many bad words to webkit developers... Chrome dispatches fake mousemove events sometimes.. This breaks drag ability. I will try to use the same workaround: https://github.com/ajaxorg/ace/pull/1599/files#L7R340

danyaPostfactum · 2013-09-15T15:11:37Z

also it shouldn't loose it's scroll position if text is dragged over it to another editor

Ok. if we just pass by, editor should stay still. If we stop moving and delay for 250ms over inactive editor - editor come to live.
If we leave editor area - editor restores it's scroll position after some delay.

Do you like this, @nightwing ?

danyaPostfactum · 2013-09-15T16:24:32Z

It could be done differently: set delay for autoscrolling and do not restore previous scroll position on dragleave. I like this more.

nightwing · 2013-09-15T16:28:20Z

Was going to make the same comment:) Yes delay for autoscrolling is better.

Add overlay (css ::before generated content) while dragging to prevent IE to trigger `drop` event while autoscrolling

danyaPostfactum · 2013-09-17T09:11:21Z

Ok, now all issues are fixed (I would like to separate multiselect issue from this).

nightwing · 2013-09-17T09:20:09Z

Ok, looks good to me.
@lennartcl can you test this on mac?

danyaPostfactum · 2013-09-17T09:27:12Z

lib/ace/mouse/dragdrop_handler.js

+    var dragOperation;
+    var autoScrollStartTime;
+    var cursorMovedTime;
+    var cursorPointOnCaretMoved;


Sorry for bad variables naming, I have no idea how to name them better.

danyaPostfactum · 2013-09-18T04:02:20Z

Unfortunately, new bug. On dblclick (quadclick, and every even click), Firefox does not capture mouse outside window. So, when you doubleclick an editor to select by words and move your mouse outside a window and release mouse there - editor will not receive mouseup event.
I am not sure, but I guess this bug hardly depends on clickcount, so there is no workaround.
I think this bug is ugly, but not significant, and can be fixed in future versions of FF.

nightwing · 2013-09-18T06:07:13Z

FF bug isn't caused by this pull request it is present on master too.
We can check for event.buttons !== 0 in mousemove https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent, but let's not do it in this pull request.

danyaPostfactum · 2013-09-18T08:04:22Z

Is associated Firefox bug filed on bugzilla? Can't find. If no, I'll file it.

lennartcl · 2013-09-18T15:45:53Z

Alright, I tested this on OSX with Chrome, Firefox, Safari. Seems to work! One thing that is different compared to master is that it now shows an arrow cursor instead of a "move" cursor. Combined with #1609 that doesn't make it very intuitive to use, as you get no feedback that you're dragging text: http://screencast.com/t/Aqx8fXhC.

(Note that #1609 is something that also existed in master, it's not necessary to address that as part of this PR.)

lennartcl · 2013-09-19T16:06:34Z

lib/ace/css/editor.css

@@ -23,6 +27,25 @@
    cursor: text;
 }

+.ace_dragging, .ace_dragging * {
+    cursor: default !important;


This used to be the "move" cursor, would it be okay to put it back? (seems to work as desired if it's "move")

Still, "move" cursor looks wrong and does not match OS system Drag'n'Drop behavior (as far as I know).
I would like to use "default" cursor...

nightwing · 2013-09-20T09:24:48Z

@lennartcl let's merge this since main part of changes are good, and we need to tweak cursor anyway e.g see #303.
Also see #1613

lennartcl · 2013-09-20T13:11:33Z

@nightwing @danyaPostfactum Alright, merging as is.

Thanks for your contribution @danyaPostfactum!

Use native HTML5 Drag'n'Drop for text.

danyaPostfactum · 2013-09-21T16:14:46Z

Just played with setDragImage: https://rawgithub.com/danyaPostfactum/ace/dragimage/kitchen-sink.html
This demo works in Webkit only, but it is not too hard to implement in all browsers.

danyaPostfactum · 2013-09-26T02:45:33Z

Anybody knows how to disable drag image scaling in Firefox? It scales drag image with height > screen height / 2 (it seems)

nightwing · 2013-09-26T20:06:37Z

This is really nice!

It scales drag image with height > screen height / 2 (it seems)

And same for width, but It seems to be impossible to change this behavior.
Maybe it would be better to show small portion of text with ...?

danyaPostfactum · 2013-09-30T17:21:52Z

We could enable dnd in Safari 5 (Windows) by covering editor with blank textarea.
Additionally, this trick would fix IE dropEffect issue.

nightwing · 2013-10-01T17:32:39Z

I don't think it is worth the effort, almost no one uses safari for windows, and it is not updated anymore.
IE dropEffect issues might be worth fixing if it is broken on ie11, and can be done with very small amount of additional code.

mlajtos · 2015-05-28T14:05:09Z

Why this branch was not merged? It gives much better feeback while dragging the selected text than current solution. Or is dragging broken?

danyaPostfactum added 4 commits September 10, 2013 21:41

Use native HTML5 Drag'n'Drop for text.

8936861

Set this context of event handlers for old IE.

2993ae8

Fix IE native drag'n'drop issues

b7daccf

Improve autoscroll behavior for text dragging

ddf5d1a

Fix Opera drag image issue.

b6dab9e

nightwing reviewed Sep 12, 2013
View reviewed changes

danyaPostfactum added 2 commits September 15, 2013 01:13

Make editor container to be drop zone

624eb7c

Add onMouseDrag method to the DragdropHandler

165cec5

danyaPostfactum reviewed Sep 14, 2013
View reviewed changes

danyaPostfactum added 2 commits September 15, 2013 20:41

Improve native selection prevention on dnd

cb7a0d5

Fix dbl/triple click issues of native dnd

b1ba675

danyaPostfactum added 2 commits September 17, 2013 19:02

Fix webkit fake mousemove event (breaking dnd) issue

ad4e57e

Fix IE autoscroll drop triggering issue

e1ff382

Add overlay (css ::before generated content) while dragging to prevent IE to trigger `drop` event while autoscrolling

Add autoscroll delay

8ebf34a

danyaPostfactum reviewed Sep 17, 2013
View reviewed changes

Capture mouse on quadclick

8a841df

lennartcl mentioned this pull request Sep 18, 2013

Drag and drop text cursor behaves strangely #1609

Closed

lennartcl reviewed Sep 19, 2013
View reviewed changes

lennartcl added a commit that referenced this pull request Sep 20, 2013

Merge pull request #1599 from danyaPostfactum/nativednd

ee95161

Use native HTML5 Drag'n'Drop for text.

lennartcl merged commit ee95161 into ajaxorg:master Sep 20, 2013

nightwing mentioned this pull request Sep 20, 2013

Exposing internal functions of the dragDrop object - 'dragEnter', 'dragOver', 'drop' - to the $mouseHandler object. #1613

Closed

Use native HTML5 Drag'n'Drop for text. #1599

Use native HTML5 Drag'n'Drop for text. #1599

Conversation

danyaPostfactum commented Sep 10, 2013

danyaPostfactum commented Sep 11, 2013

danyaPostfactum commented Sep 11, 2013

nightwing commented Sep 11, 2013

danyaPostfactum commented Sep 12, 2013

nightwing commented Sep 12, 2013

danyaPostfactum commented Sep 12, 2013

nightwing commented Sep 12, 2013

danyaPostfactum commented Sep 12, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nightwing commented Sep 14, 2013

nightwing commented Sep 14, 2013

nightwing commented Sep 15, 2013

danyaPostfactum commented Sep 15, 2013

danyaPostfactum commented Sep 15, 2013

danyaPostfactum commented Sep 15, 2013

nightwing commented Sep 15, 2013

danyaPostfactum commented Sep 17, 2013

nightwing commented Sep 17, 2013

Choose a reason for hiding this comment

danyaPostfactum commented Sep 18, 2013

nightwing commented Sep 18, 2013

danyaPostfactum commented Sep 18, 2013

lennartcl commented Sep 18, 2013

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nightwing commented Sep 20, 2013

lennartcl commented Sep 20, 2013

danyaPostfactum commented Sep 21, 2013

danyaPostfactum commented Sep 26, 2013

nightwing commented Sep 26, 2013

danyaPostfactum commented Sep 30, 2013

nightwing commented Oct 1, 2013

mlajtos commented May 28, 2015