Lower tap movement and time thresholds #1830

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
@mmcc
Member

mmcc commented Jan 27, 2015

Right now our tap movement threshold is much higher than most other libs. As @steverandy said, our touch time threshold is also a bit higher than other libs as well, so dropping that to be more inline with those.

This should resolve issues around swipes being confused with taps: #1817

@gkatsev

This comment has been minimized.

Show comment
Hide comment
@gkatsev

gkatsev Jan 27, 2015

Member

LGTM but tests failed.

Member

gkatsev commented Jan 27, 2015

LGTM but tests failed.

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Jan 27, 2015

Member

ha, the tests were passing for me after 30376cd because....I'd never actually committed the threshold change.

Member

mmcc commented Jan 27, 2015

ha, the tests were passing for me after 30376cd because....I'd never actually committed the threshold change.

@heff

This comment has been minimized.

Show comment
Hide comment
@heff

heff Feb 11, 2015

Member

looks good!

Member

heff commented Feb 11, 2015

looks good!

heff added a commit to heff/video.js that referenced this pull request Feb 12, 2015

@mmcc mmcc closed this in 2c14c70 Feb 12, 2015

mmcc added a commit that referenced this pull request Feb 12, 2015

lowered tap movement and tap time thresholds
or, you know, actually change the threshold

updated component test to account for smaller touchmove threshold

closes #1830
@Zae

This comment has been minimized.

Show comment
Hide comment
@Zae

Zae Aug 3, 2015

Is it possible this is still happening on paused videos? I'm seeing that I can't scroll over a paused videojs element without starting the video on ios. This also happens on the video element on the videojs.com site.

Zae commented Aug 3, 2015

Is it possible this is still happening on paused videos? I'm seeing that I can't scroll over a paused videojs element without starting the video on ios. This also happens on the video element on the videojs.com site.

@Zae

This comment has been minimized.

Show comment
Hide comment
@Zae

Zae Aug 3, 2015

By the way, it only happens if I touch the big white play button.

Zae commented Aug 3, 2015

By the way, it only happens if I touch the big white play button.

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Aug 3, 2015

Member

So you're saying you can't scroll only if you're touching the big play button? That seems like a pretty specific edge case to me...

Member

mmcc commented Aug 3, 2015

So you're saying you can't scroll only if you're touching the big play button? That seems like a pretty specific edge case to me...

@Zae

This comment has been minimized.

Show comment
Hide comment
@Zae

Zae Aug 3, 2015

Scrolling works but the video starts to play after you let go. Seems to be an ios thing, because it happens on a 'naked' video element also. Was thinking maybe videojs can do something about it, but it's probably something very low on the prio list 😀

Zae commented Aug 3, 2015

Scrolling works but the video starts to play after you let go. Seems to be an ios thing, because it happens on a 'naked' video element also. Was thinking maybe videojs can do something about it, but it's probably something very low on the prio list 😀

@mmcc

This comment has been minimized.

Show comment
Hide comment
@mmcc

mmcc Aug 3, 2015

Member

ahh iOS...it's the source of a few really bizarre bugs like this. Yep, this is definitely down there on the list of things we need to fix, but if you'd like to dig in we'd gladly accept a PR that helps fix it!

Member

mmcc commented Aug 3, 2015

ahh iOS...it's the source of a few really bizarre bugs like this. Yep, this is definitely down there on the list of things we need to fix, but if you'd like to dig in we'd gladly accept a PR that helps fix it!

@MaxLaumeister

This comment has been minimized.

Show comment
Hide comment
@MaxLaumeister

MaxLaumeister Jan 22, 2016

For me swipes are still being mistaken as taps on Chrome for Android in Video.js 5.4.6. My site's mobile view is very dense with videos in some places, so the user is just about guaranteed to put their finger on one of the videos as they scroll past them. When this happens, the video registers a tap and plays.

Here are steps to reproduce on the Video.js homepage using Chrome for Android: Wait until the page loads fully, then casually swipe up on the video to scroll down the page. Unless you make your swipe deliberately slow, the video will start to play as you scroll past it. A user who is just trying to scroll down the page will have videos start randomly playing, which is not ideal.

Here's a video (gif) of it happening on the Video.js homepage:

ezgif-2913097299

For me swipes are still being mistaken as taps on Chrome for Android in Video.js 5.4.6. My site's mobile view is very dense with videos in some places, so the user is just about guaranteed to put their finger on one of the videos as they scroll past them. When this happens, the video registers a tap and plays.

Here are steps to reproduce on the Video.js homepage using Chrome for Android: Wait until the page loads fully, then casually swipe up on the video to scroll down the page. Unless you make your swipe deliberately slow, the video will start to play as you scroll past it. A user who is just trying to scroll down the page will have videos start randomly playing, which is not ideal.

Here's a video (gif) of it happening on the Video.js homepage:

ezgif-2913097299

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment