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

Remove duplicate "down" events #1029

Closed
wants to merge 1 commit into from
Closed

Conversation

AStoker
Copy link

@AStoker AStoker commented Jan 30, 2017

Resolves: #972
Related: #1000
Removes duplicate “down” events and preventDefault operation on click
events on “filter” items. This allows selection of things like text
boxes and input elements that are inside a sortable element.
Repro of bug: http://jsbin.com/cerumanoma/edit?html,js,output

Resolves: SortableJS#972
Related: SortableJS#1000
Removes duplicate “down” events and `preventDefault` operation on click
events on “filter” items. This allows selection of things like text
boxes and input elements that are inside a sortable element.
Repro of bug: http://jsbin.com/cerumanoma/edit?html,js,output
@fourpixels
Copy link

Why isn't this accepted?

@RubaXa
Copy link
Collaborator

RubaXa commented Feb 15, 2017

Removes duplicate “down” events

Your solution is incorrect, you can not suscribe on one of the event, should always subscribe to both, because the user can be simultaneously be two input devices: mouse and touch-screen.

So need a different solution.

Removes preventDefault operation on click events on “filter” items.

That means you should not use the "filter". Now this method is implemented exactly as designed.

@RubaXa RubaXa closed this Feb 15, 2017
RubaXa added a commit that referenced this pull request Feb 15, 2017
@RubaXa
Copy link
Collaborator

RubaXa commented Feb 15, 2017

Added preventOnFilter: 1aa0121

RubaXa added a commit that referenced this pull request Feb 15, 2017
@RubaXa
Copy link
Collaborator

RubaXa commented Feb 15, 2017

Removes duplicate “down”

Try master: a7cddbc

@geonanorch
Copy link

I was about to open a new issue when I found this one.
I am not 100% sure that it is the same problem, but there is definitely a problem of duplicate filter event:

the test which would filter duplicates (and in particular reading of lastDownEl):

if (lastDownEl === target) {
	// Ignoring duplicate `down`
	return;
}

comes before the dispatch of the filter event, which causes processing to abort with a return:

_dispatchEvent(_this, criteria, 'filter', target, el, el, startIndex);
return true;

But setting of lastDownEl only takes place later in the normal case, not when filtering was triggered:

_prepareDragStart: function (/** Event */evt, /** Touch */touch, /** HTMLElement */target, /** Number */startIndex) {
  . . . . . . .
lastDownEl = target;

Imho the assignment of lastDownEl should take place right after the duplicate check:

if (lastDownEl === target) {
	// Ignoring duplicate `down`
	return;
}
lastDownEl = target;

But I do not know the code well enough to be sure that there are no side-effects.
Still, I hope this helps,

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

Successfully merging this pull request may close these issues.

4 participants