-
Notifications
You must be signed in to change notification settings - Fork 526
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
New optional features: Live scrubbing and tap anywhere to toggle play/pause #126
Conversation
…gTimeStep to scrubbing and update readme.
…hat pressing anywhere on seek bar allows seeking. Change onScreenTouch to use a timeout so that single and double taps are completely unambiguous and not not cause multiple play/pause or toggleControl events when double-tapping.
I'm happy to split this into 2 PRs if desired. An alternative implementation of tapAnywhereToPause would be to allow an onTap callback property to be supplied that overrides the default tap behaviour. However, this would need access to enough state information to know whether the controls are currently visible, which is necessary to know whether to toggle the controls or to toggle pause. |
…er so that the initial location is always relative to the correct parent.
I'm sorry I haven't merged this sooner...seems like a really useful addition. If you can update the code to work with the latest releases I'll merge this in. |
# Conflicts: # README.md # VideoPlayer.js
This should be updated now, but unfortunately due to the Covid-19 lockdown I don't have the tools to test it here! Luckily the merge conflicts were very minor changes so it looks like it'll work the same way. |
I’ll give it a shot. Thanks!
|
The 300ms wait is definitely pretty heavy...going to play around with the timing. And live scrubbing doesn't seem to cause as much issues as when we originally tested this....so good so far. Let me play with this a bit.. |
I was thinking this too when I fixed the merge requests this morning, so I made it configurable via a property, doubleTapTime. I've just realised I forgot to add this property to the README, so I've just done that now. Hopefully it takes some of the weight off the 300ms. |
Change the default to 130ms and I'll merge this in. :) |
Done! |
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.
Merci.
Live scrubbing works by pausing the video while the seek bar is held and seeking within the video as the seek bar is moved. The minimum step between seeks (in milliseconds) is controlled by the value of the scrubbing property - I'm not sure if I've described this clearly enough in the README.md.
When tapAnywhereToPause is true, tapping onscreen will first show the controls, then as long as the controls are visible, subsequent single taps will toggle playing/paused instead. In this case the only way to hide the controls is to wait for controlTimeout to expire. Double taps will continue to toggle fullscreen as normal.
Both features are disabled by default so only changes to original behaviour are: