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

New optional features: Live scrubbing and tap anywhere to toggle play/pause #126

Merged
merged 13 commits into from
May 18, 2020

Conversation

EdTorbett
Copy link
Contributor

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:

  • 300ms delay before controls toggle (waiting to determine if tap or double-tap),
  • Seeking can be initiated by pressing anywhere on the seek bar rather than having to accurately select the seek bar handle.

…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.
@EdTorbett
Copy link
Contributor Author

EdTorbett commented Jan 10, 2019

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.

@kylemilloy
Copy link
Contributor

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.

@EdTorbett
Copy link
Contributor Author

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.

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.

@kylemilloy
Copy link
Contributor

kylemilloy commented May 18, 2020 via email

@kylemilloy
Copy link
Contributor

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..

@EdTorbett
Copy link
Contributor Author

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.

@kylemilloy
Copy link
Contributor

Change the default to 130ms and I'll merge this in. :)

@EdTorbett
Copy link
Contributor Author

Done!

Copy link
Contributor

@kylemilloy kylemilloy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci.

@kylemilloy kylemilloy merged commit 01818fc into itsnubix:master May 18, 2020
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.

2 participants