Skip to content

Commit

Permalink
+ supportPointer (low level api)
Browse files Browse the repository at this point in the history
  • Loading branch information
@RubaXa
RubaXa committed Oct 28, 2017
1 parent 811e448 commit d1e6026
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
1 change: 1 addition & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ mock.png
.*.sw*
.build*
jquery.fn.*
.idea/
16 changes: 10 additions & 6 deletions Sortable.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,8 @@
fallbackClass: 'sortable-fallback',
fallbackOnBody: false,
fallbackTolerance: 0,
fallbackOffset: {x: 0, y: 0}
fallbackOffset: {x: 0, y: 0},
supportPointer: Sortable.supportPointer !== false,
};


Expand All @@ -299,7 +300,7 @@
// Bind events
_on(el, 'mousedown', this._onTapStart);
_on(el, 'touchstart', this._onTapStart);
_on(el, 'pointerdown', this._onTapStart);
options.supportPointer && _on(el, 'pointerdown', this._onTapStart);

if (this.nativeDraggable) {
_on(el, 'dragover', this);
Expand Down Expand Up @@ -440,8 +441,8 @@
_on(ownerDocument, 'mouseup', _this._onDrop);
_on(ownerDocument, 'touchend', _this._onDrop);
_on(ownerDocument, 'touchcancel', _this._onDrop);
_on(ownerDocument, 'pointercancel', _this._onDrop);
_on(ownerDocument, 'selectstart', _this);
options.supportPointer && _on(ownerDocument, 'pointercancel', _this._onDrop);

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Oct 30, 2017

@RubaXa, do you happen to remember why this line is even here in the first place? What scenario were you trying to address to call onDrop when getting pointercancel/touchcancel?

This comment has been minimized.

Sign in to view

Copy link
@RubaXa

RubaXa Oct 30, 2017

Author Collaborator

Yes, I do. This need for cancel dnd.

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Oct 30, 2017

What browser in which scenario sends you a touchcancel/pointercancel if user wants to cancel dnd? I mean what should user do in the normal scenarios (ignoring the change we made in Chrome 62) for this to happen?

This comment has been minimized.

Sign in to view

Copy link
@RubaXa

RubaXa Oct 30, 2017

Author Collaborator

For example, too many touch points are created.

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Oct 30, 2017

Can you elaborate a little more? To activate the touch drag and drop on Chrome you need to long press touch to get the drag to start. Now what does it have to do with multiple touch points? Can you give a more explicit scenario or maybe an example page if you have it handy?

This comment has been minimized.

Sign in to view

Copy link
@RubaXa

RubaXa Oct 30, 2017

Author Collaborator

http://rubaxa.github.io/Sortable/ — example
Drag one item, and during a move, tap a screen another finger. (to be dnd is canceled)

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Nov 1, 2017

I see. But in that scenario the page is receiving a touchend/pointerup for the second finger. Not touchcancel/pointercancel. when do you expect touchcancel/pointercancel?

This comment has been minimized.

Sign in to view

Copy link
@owen-m1

owen-m1 Jun 10, 2019 •
edited
Loading

Member

@NavidZ I know this is very old, but there are so many issues with pointer events that the events are going to be removed from Sortable. If you work on Chrome I am just letting you know that they are pretty much broken, and it seems to be an issue with Chrome in particular.

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Jun 10, 2019

@owen-m1 I'm sad to hear that.
But I would love to know what is causing the problem so we can be on top of it and fix it. Particularly because FF has shipped pointer events already and Safari is in the process of doing the same knowing the issues earlier and having the tests and fixes for them on the browser side is a must. Do you mind filing bugs in crbug.com (for Chromium) whenever you see an unexpected behavior in Chromium or against pointerevents spec if you see some functionality is missing maybe from the pointerevents in principle. When filing bugs for Chromium please add "blink>input" component to it so it comes directly to my team.

This comment has been minimized.

Sign in to view

Copy link
@owen-m1

owen-m1 Jun 11, 2019

Member

I think most of the issues are on Chrome Android.
Pointer move is fired right on mouse down w/ no touch move (even touchmove event is not fired). #1517
Pointerup is fired when the mouse is moved from one element to another while the pointer is down, even if the touch is not lifted. #1469

And of course pointercancel issue #1199

This comment has been minimized.

Sign in to view

Copy link
@NavidZ

NavidZ Jun 11, 2019 •
edited
Loading

I took a quick look at those issues. None of them seemed to say what they expect from the browser and what they get (I guess because mainly there are filed by the users of those libraries and not the authors). If you can file a more accurate bugs with a reproduction and the actions to do and the expected and actual behavior I can help you much faster.

However, taking a quick look at the sortable code I don't know why the code is handling both touchmove/mousemove as well as pointermoves. Technically there should be a feature check and only handle pointermoves (which covers both mouse and touch moves) or the other two but not both. I wonder handling them all somehow interfere with the dragging logic and whatnot. I have commented on the issue. Feel free to cc me on any issue that you think is relevant.

if (options.delay) {
// If the user moves the pointer or let go the click or touch
Expand All @@ -452,7 +453,7 @@
_on(ownerDocument, 'touchcancel', _this._disableDelayedDrag);
_on(ownerDocument, 'mousemove', _this._disableDelayedDrag);
_on(ownerDocument, 'touchmove', _this._disableDelayedDrag);
_on(ownerDocument, 'pointermove', _this._disableDelayedDrag);
options.supportPointer && _on(ownerDocument, 'pointermove', _this._disableDelayedDrag);

_this._dragStartTimer = setTimeout(dragStartFn, options.delay);
} else {
Expand Down Expand Up @@ -674,8 +675,11 @@
_on(document, 'touchmove', _this._onTouchMove);
_on(document, 'touchend', _this._onDrop);
_on(document, 'touchcancel', _this._onDrop);
_on(document, 'pointermove', _this._onTouchMove);
_on(document, 'pointerup', _this._onDrop);

if (options.supportPointer) {
_on(document, 'pointermove', _this._onTouchMove);
_on(document, 'pointerup', _this._onDrop);
}
} else {
// Old brwoser
_on(document, 'mousemove', _this._onTouchMove);
Expand Down

0 comments on commit d1e6026

Please sign in to comment.