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

(fix) Bind context to setTimeout #1322

Merged
merged 2 commits into from
Nov 9, 2018
Merged

Conversation

p-herbert
Copy link
Contributor

Description

There are reported issues with the option delay not working, specifically on mobile (see GitHub Issues 659, 981, 989, 1130, 1136).

Cause

A dragStartFn call loses its context because it is invoked by setTimeout.

Solution

Bind the context.

Note

I do not believe this is necessary for other setTimeout or setInterval calls because they meet one of the following criteria:

  1. Their function calls do not require a context.
  2. The correct context is already in scope by the variable _this.
  3. Private functions already have their context bounded to them.

@kimemin
Copy link

kimemin commented Oct 25, 2018

this commit saves my life.


it works but make errors like this.

k(...).bind is not a function

so I change like this and it works.

_this._dragStartTimer = setTimeout(dragStartFn.bind(_this), options.delay);

@owen-m1
Copy link
Member

owen-m1 commented Nov 8, 2018

Please make the necessary changes @p-herbert and then this can be merged

@p-herbert
Copy link
Contributor Author

@Flame2057 the edit has been made.

Thanks for the fix @kimemin!

@owen-m1 owen-m1 merged commit 4ef871c into SortableJS:master Nov 9, 2018
Runi-c pushed a commit to Tupperbox/Sortable that referenced this pull request Nov 10, 2023
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.

3 participants