-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Set pointer capture on the right target #1255
base: master
Are you sure you want to change the base?
Set pointer capture on the right target #1255
Conversation
Set the pointer capture on the slider itself only when we are in the swiping mode. The ensures not only clicks would work on elements such as links within the slider if user hasn't moved the pointer, but also disable activating and navigating to the links within the slider if the user has started the slider move on an embedded link within the slider.
src/js/jquery.bxslider.js
Outdated
// record the starting touch x, y coordinates | ||
slider.touch.start.x = touchPoints[0].pageX; | ||
slider.touch.start.y = touchPoints[0].pageY; | ||
|
||
if (slider.viewport.get(0).setPointerCapture) { | ||
// This captures the event stream to the target to make sure even if the pointer leaves | ||
// the slider bounday in the very next move it still keeps the events. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo. Meant boundary
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
@@ -30,6 +30,7 @@ | |||
responsive: true, | |||
slideZIndex: 50, | |||
wrapperClass: 'bx-wrapper', | |||
sliderSlopSize: 5, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I see, all current configuration options have some description in respective README section.
Could you add a short note/confirm what the "slop" value controls (move threshold to discern between slide/click) and what dimensions (pixels) it's measured in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. I missed it. I didn't know. I added one, Does that sound good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Hoping that any of project's maintainers can now race ahead with PR merge and new release 🤞
Looking forward for this fix |
Can we release it already? :D |
Yes, please release. We're already using this fix because tablet users (Android & Chrome browser) had difficulty clicking items in the slider area. |
I'm glad the fix works for you. But this library doesn't seem to be maintained by the owner anymore and hence why this change and others are just sitting with no merge. I suggest you find another library that is actively being maintained. |
Set the pointer capture on the slider itself only
when we are in the swiping mode. This ensures not only
clicks would work on elements such as links within the
slider if user hasn't moved the pointer, but also
disable activating and navigating to the links within
the slider if the user has started the slider move on
an embedded link within the slider.
Fixes #1240