Skip to content

fix: Fix for wrong slider drag when having many touches simultaneous #535

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

Merged

Conversation

daniela-mateescu
Copy link
Contributor

@daniela-mateescu daniela-mateescu commented May 25, 2017

#534
Sample at: http://jsfiddle.net/fq6xkod4/ . This sample requires a device with a touch screen. It illustrates that the possibility to use both sliders simultaneously.

@codecov-io
Copy link

Codecov Report

Merging #535 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #535   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines         943    971   +28     
=====================================
+ Hits          943    971   +28
Impacted Files Coverage Δ
src/rzslider.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5bf3f22...2f762ba. Read the comment docs.

@ValentinH
Copy link
Member

Thanks for this PR. It is really clean and provide full test coverage! Thanks 👍

@ValentinH ValentinH merged commit 9bca8ac into angular-slider:master May 25, 2017
@daniela-mateescu
Copy link
Contributor Author

Hello! Thank you very much for integrating the PR, but we have a little problem with this. I saw that you did a refactor before integration. It is all ok with this refactor, except one little problem. At onStart() method, instead of memorizing the ehEnd handler to be removed later like this: this.ehEndToBeRemovedOnEnd = ehEnd;, you have sent this as a parameters like this: ehEnd = angular.bind(this, this.onEnd, ehMove, ehEnd);. The problem with this code is that the ehEnd variable will be always undefined when passing it to the angular.bind method call.

Thank you in advance.

@ValentinH
Copy link
Member

I modified after merging yesterday because of this issue #536 that was related to this PR. But you are right, I did it a bit too quickly and it's wrong...

It proves that even with 100% test coverage, some tests are still missing because this should have been caught. I'm fixing it right away by keeping the way you were doing it but also by providing the correct event name.

@ValentinH
Copy link
Member

ValentinH commented May 26, 2017

Master should be fixed on http://jsfiddle.net/mejsy5d2/ on and 6.2.2

@daniela-mateescu
Copy link
Contributor Author

Hello,
Thank you very much for the fix. It works great now.

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