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

delay option doesn't work #659

Closed
bhecht2 opened this issue Nov 18, 2015 · 26 comments
Closed

delay option doesn't work #659

bhecht2 opened this issue Nov 18, 2015 · 26 comments

Comments

@bhecht2
Copy link

bhecht2 commented Nov 18, 2015

Hi,

I am checking the demo: http://jsbin.com/videlojafi/edit?html,js,output

It doesn't work for me on either firefox, or IE (10).
Once removing the delay line, or changing it to 0, everything starts to work

@ranyauto3p
Copy link

Hi,

I second that, same experience. Feature is required to allow scrolling on touch devices. With no delay list items gets dragged immediately preventing page scroll.

OK, just found out that if I turn 'forceFallback' on delay works (tested on IE 11 and IPad Safari)

@kpost
Copy link

kpost commented Jan 25, 2016

+1. It does work on webkit browsers however, but not on FF.

dragStartFn does get called in the setTimeout, and some stuff is handled, like the classes added to the dragging element. But the actual dragging never happens. Very weird.

@welelay
Copy link

welelay commented Mar 14, 2016

doesn't work on Safari on an iPad either

@skitterm
Copy link

I found the same issue with IE 11 and Firefox. Setting forceFallback: true solved the issue there (even on iPad Safari for me) but made it so drag-drop doesn't work on Mac Safari (and makes the dragged element look blurry).

@skitterm
Copy link

@RubaXa I dug around in the dev code (Sortable.js) and found an issue: When there is a delay for FF/IE11, the dragstart event never fires inside of _triggerDragStart: _on(rootEl, 'dragstart', this._onDragStart);. It looks like the event is attached to the DOM element, but it doesn't fire. I haven't been able to find a fix for it.

Xom9lk added a commit to Xom9lk/Sortable that referenced this issue Jul 15, 2016
emileber pushed a commit to emileber/Sortable that referenced this issue Aug 23, 2016
@elonmallin
Copy link
Contributor

+1.
If the fixes from a few months ago work maybe they can be merged?

@sbayd
Copy link

sbayd commented Nov 24, 2016

+1,

It looks this emileber@20a8058 fixes the bug. Is it possible to be merged?

@emileber
Copy link

emileber commented Nov 25, 2016

@sbayd I just tried @Xom9lk fix with my own fork and it doesn't really work (weird glitch with different browsers) and It'd need some more work to be totally fixed.

While I'm thankful RubaXa made this project, the way he's handling it now is borderline hostile (closing merge requests and issues without explanations, never replying to some issues, overall tone of the communication, etc.) and it seems like a waste of time to try and get non-trivial pull requests merged. This was opened a year ago and never was acknowledged by him. He clearly doesn't have time for this project unfortunately and we'd better use our own fork.

emileber pushed a commit to emileber/Sortable that referenced this issue Dec 16, 2016
@RubaXa
Copy link
Collaborator

RubaXa commented Feb 16, 2017

True, I do not have time, but and from you I have not seen any PR.:

https://github.com/RubaXa/Sortable/pulls?utf8=%E2%9C%93&q=is%3Apr%20author%3Aemileber%20

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 16, 2017

@emileber I tried to use your fix, but it does not work (FF): http://recordit.co/PTeUNSLP7g

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 16, 2017

Now the problem can be solved only by means of forceFallback: true.

@emileber
Copy link

emileber commented Feb 16, 2017

@RubaXa Thanks for taking time to respond, but like I said in my comment above, my fix doesn't work (in fact, not my fix but Xom9lk's) and after seeing the state of the project, I didn't make any PR.

As of now, I stopped using the delay and might look for a workaround on mobile.

@genichm
Copy link

genichm commented Jun 1, 2017

If it mobile environment remove all lines of code where 'pointermove' appears
for example _on(ownerDocument, 'pointermove', _this._disableDelayedDrag);
for some reason it solves delay problem.

@geonanorch
Copy link

In addition to the mobile scrolling use case, the delay option is also required to drag content editables (because enabling draggable on an element disables the contenteditable functionality).
I could only get delay to work by combining with forceFallback --as suggested by @RubaXa ;-)
Of course that is a workaround, but maybe we depend on the HTML5 APIs and browsers maturing a bit...

@webarthur
Copy link

webarthur commented Mar 9, 2018

I have three problems with delay/forceFallback:

  1. [Desktop] If the mouse has a minimal move, drag will not works.
  2. [Desktop] When I drop de element the click event has triggered.
  3. [Mobile] Just not works.

@geonanorch
Copy link

@webarthur, I am not the owner of Sortable, not even did I raise this issue #659, so just quick comments:

  1. That is a design choice I believe, allowing (for instance) to handle selection using the mouse (common action in contenteditables, and the context of my previous comment). At most you could ask for the behavior to be configurable with a flag.
  2. Agree, there is a click when the the cursor is positioned over a sortable element at the time of mouseup. That looks like a bug, you could raise a separate issue for it (assuming it is not already covered in an existing one) or contribute a fix.
  3. What does not work? Your comment is not specific enough --just like the comment you added a few hours later: if you think that some other library can help, show how. Help us help you...

@webarthur
Copy link

The mobile issue occurs on PWA (chrome).

@webarthur
Copy link

All works fine now! =)

I just have replaced the line _on(rootEl, 'dragstart', this._onDragStart); in _triggerDragStart function by _on(dragEl, 'dragend', this); (line 515). Works fine with delay/forceFallback option on both desktop and mobile:

[...]
		_triggerDragStart: function (/** Event */evt, /** Touch */touch) {
			touch = touch || (evt.pointerType == 'touch' ? evt : null);

			if (touch) {
				// Touch device support
				tapEvt = {
					target: dragEl,
					clientX: touch.clientX,
					clientY: touch.clientY
				};

				this._onDragStart(tapEvt, 'touch');
			}
			else if (!this.nativeDraggable) {
				this._onDragStart(tapEvt, true);
			}
			else {
				_on(dragEl, 'dragend', this);
				// _on(rootEl, 'dragstart', this._onDragStart);
				this._onDragStart(evt);
			}

			try {
				if (document.selection) {
					// Timeout neccessary for IE9
					_nextTick(function () {
						document.selection.empty();
					});
				} else {
					window.getSelection().removeAllRanges();
				}
			} catch (err) {
			}
		},
[...]

The click bug, the evt.preventDefault() is not working. I put a flag in callback to check it on custom click function, example:

        var preventDragClick = false
        Sortable.create(catList, {
          delay: 500,
          forceFallback: true,
          onUpdate: function () {
            preventDragClick = true
          }
        })

$(el).click(function () {
  if (preventDragClick) {
    return preventDragClick = false
  }
  [...]
})

@geonanorch thanks for helping. =D

@hakoiko
Copy link

hakoiko commented Apr 5, 2018

the troublemaker is Android device.
the only way to solve this problem is using 'window.requestAnimationFrame' instead of 'setTimeout'
see this and this

@Czeran
Copy link

Czeran commented Apr 24, 2018

Hello,

I don't know if my solution is correct but for my reasons work perfectly. I found that on Android, line clearTimeout(this._dragStartTimer) in function _disableDelayedDrag causes this issue, so I disable this line for android, so my whole function _disableDelayedDrag looks like this:

_disableDelayedDrag: function () {
      const userAgent = navigator.userAgent || navigator.vendor;
      var ownerDocument = this.el.ownerDocument;
      if (!/android/i.test(userAgent)) {
        clearTimeout(this._dragStartTimer);
      }
      _off(ownerDocument, 'mouseup', this._disableDelayedDrag);
      _off(ownerDocument, 'touchend', this._disableDelayedDrag);
      _off(ownerDocument, 'touchcancel', this._disableDelayedDrag);
      _off(ownerDocument, 'mousemove', this._disableDelayedDrag);
      _off(ownerDocument, 'touchmove', this._disableDelayedDrag);
      _off(ownerDocument, 'pointermove', this._disableDelayedDrag);
    }

DomenZajc added a commit to forksmealplanner/Sortable that referenced this issue Nov 9, 2018
@owen-m1
Copy link
Member

owen-m1 commented Jan 7, 2019

Try master branch please

@geonanorch
Copy link

@owen-m1 I just did (using Firefox 61.0.2), no luck but I think that I have a lead.

Imho the delay parameter is very useful to:

  • handle contenteditables (e.g. to allow selecting within a draggable contenteditable)-
  • or even plain clicks (e.g. short touch to activate, long touch to initiate drag).

Something like:

    Sortable.create(el, {
      draggable: '.zone_out',
      delay: 200,
      touchStartThreshold: 5,
      onChoose: () => console.log('sortable onChoose'),
      onUnchoose: () => console.log('sortable onUnchoose'),
      onStart: () => console.log('sortable onStart'),
      onEnd: () => console.log('sortable onEnd'),
    });

will not work, but commenting out delay will make it work, More observations when delay is set:

  • the sortable-chosen class is set
  • onStart is not called
  • the dragging class is not set

Debugging my particular case further, going up the call stack:

  • _dragStarted is not called
  • _onDragStart is not called
  • _triggerDragStart is called with: nativeDraggable=true and touch=null, resulting in event handler dragstart being set. Somehow the problem lies here, that event does not fire.
  • _onTouchMove is not called (as expected based on the previous point)

I then looked at _prepareDragStart to try and pinpoint the issue. Somehow it makes a difference whether dragStartFn is called synchronously or delayed. Don't ask me why, but the lines:

        // Make the element draggable
        dragEl.draggable = _this.nativeDraggable;

should be moved outside of dragStartFn (so executed whether delayed or not) for the dragstart event to be fired in delayed mode. I am not claiming that it is the fix because I do not fully understand why it works, but here is a patch that works for me (based on the current master commit):

$ diff -e Sortable.js Sortable.js.org
819,820d
808a
                                        // Make the element draggable
                                        dragEl.draggable = _this.nativeDraggable;

.

@owen-m1
Copy link
Member

owen-m1 commented Mar 2, 2019

@geonanorch I used your fix in the above commit. Please try master branch.

@geonanorch
Copy link

@owen-m1 just gave it a go, works for me, thanks.
Would be nice to have confirmation from other folks who contributed to the thread though, just to be sure...

I guess that some browsers initiate dragging on the mousedown/touchstart event (seems natural) and that draggable must be set by then at the latest. Not sure what the spec really says.

@owen-m1
Copy link
Member

owen-m1 commented Mar 3, 2019

@geonanorch IE and Edge seemed to have that paticular restraint as well. However, IE and Edge will also not cancel a dragstart if the draggable property is removed after the mousedown, making a delay feature impossible on these browsers.

@owen-m1 owen-m1 closed this as completed Mar 4, 2019
@emileber
Copy link

emileber commented Mar 4, 2019

While I was still receiving notifications for this issue, it's been more than 2 years since I haven't touched this lib, which was in a Backbone project 2 job hops ago.

Though I'm glad that the development of Sortable was taken over by someone! Good job 🎉

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

No branches or pull requests