Skip to content

Commit

Permalink
feat: Add option to disable seeking while scrubbing on mobile (#8903)
Browse files Browse the repository at this point in the history
## Description
On desktop, a user can hover over the progress bar while content plays,
which makes it possible to seek to a relatively precise location without
disrupting playback. On mobile there is no hovering, so in order to seek
during inline playback the user can only tap a location on the progress
bar (very hard to do precisely on a small screen) or scrub to try to
hone in on a specific location (can be very clunky because seeks are
constantly being executed). This PR adds a feature to treat scrubbing on
mobile more like hovering on desktop-- while scrubbing, seeks are
disabled and playback continues, only when the user finishes scrubbing
is a single seek executed to the desired location.

One key use-case for this feature is thumbnail seeking integrations on
mobile, where the user can scrub through different thumbnail images
until they find their desired seek location.

## Specific Changes proposed
This behavior is similar to the existing `enableSmoothSeeking` behavior
in that the `PlayProgressBar` slider visibly updates with the scrubbing
movements, but differs in a few ways:
- Playback continues while scrubbing, no seeks are executed until
`touchend`.
- The seek bar's `TimeTooltip` component displays the target seek time
while scrubbing, rather than the `CurrentTimeDisplay` (which continues
to show the current time of the playing content).
  • Loading branch information
alex-barstow authored Nov 25, 2024
1 parent 62f3844 commit 57d6ab6
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 13 deletions.
10 changes: 8 additions & 2 deletions src/css/components/_progress.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@

// This increases the size of the progress holder so there is an increased
// hit area for clicks/touches.
.video-js .vjs-progress-control:hover .vjs-progress-holder {
.video-js .vjs-progress-control:hover .vjs-progress-holder,
.video-js.vjs-scrubbing.vjs-touch-enabled .vjs-progress-control .vjs-progress-holder {
font-size: 1.666666666666666666em;
}

Expand Down Expand Up @@ -143,7 +144,8 @@
}

.video-js .vjs-progress-control:hover .vjs-time-tooltip,
.video-js .vjs-progress-control:hover .vjs-progress-holder:focus .vjs-time-tooltip {
.video-js .vjs-progress-control:hover .vjs-progress-holder:focus .vjs-time-tooltip,
.video-js.vjs-scrubbing.vjs-touch-enabled .vjs-progress-control .vjs-time-tooltip {
display: block;

// Ensure that we maintain a font-size of ~10px.
Expand Down Expand Up @@ -172,6 +174,10 @@
display: block;
}

.video-js.vjs-scrubbing.vjs-touch-enabled .vjs-progress-control .vjs-mouse-display {
display: block;
}

.video-js.vjs-user-inactive .vjs-progress-control .vjs-mouse-display {
visibility: hidden;
opacity: 0;
Expand Down
4 changes: 2 additions & 2 deletions src/js/control-bar/progress-control/progress-control.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class ProgressControl extends Component {
}

this.off(['mousedown', 'touchstart'], this.handleMouseDownHandler_);
this.off(this.el_, 'mousemove', this.handleMouseMove);
this.off(this.el_, ['mousemove', 'touchmove'], this.handleMouseMove);

this.removeListenersAddedOnMousedownAndTouchstart();

Expand Down Expand Up @@ -172,7 +172,7 @@ class ProgressControl extends Component {
}

this.on(['mousedown', 'touchstart'], this.handleMouseDownHandler_);
this.on(this.el_, 'mousemove', this.handleMouseMove);
this.on(this.el_, ['mousemove', 'touchmove'], this.handleMouseMove);
this.removeClass('disabled');

this.enabled_ = true;
Expand Down
50 changes: 42 additions & 8 deletions src/js/control-bar/progress-control/seek-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as Dom from '../../utils/dom.js';
import * as Fn from '../../utils/fn.js';
import {formatTime} from '../../utils/time.js';
import {silencePromise} from '../../utils/promise';
import {merge} from '../../utils/obj';
import document from 'global/document';

/** @import Player from '../../player' */
Expand Down Expand Up @@ -40,7 +41,23 @@ class SeekBar extends Slider {
* The key/value store of player options.
*/
constructor(player, options) {
options = merge(SeekBar.prototype.options_, options);

// Avoid mutating the prototype's `children` array by creating a copy
options.children = [...options.children];

const shouldDisableSeekWhileScrubbingOnMobile = player.options_.disableSeekWhileScrubbingOnMobile && (IS_IOS || IS_ANDROID);

// Add the TimeTooltip as a child if we are on desktop, or on mobile with `disableSeekWhileScrubbingOnMobile: true`
if ((!IS_IOS && !IS_ANDROID) || shouldDisableSeekWhileScrubbingOnMobile) {
options.children.splice(1, 0, 'mouseTimeDisplay');
}

super(player, options);

this.shouldDisableSeekWhileScrubbingOnMobile_ = shouldDisableSeekWhileScrubbingOnMobile;
this.pendingSeekTime_ = null;

this.setEventHandlers_();
}

Expand Down Expand Up @@ -225,6 +242,12 @@ class SeekBar extends Slider {
* The percentage of media played so far (0 to 1).
*/
getPercent() {
// If we have a pending seek time, we are scrubbing on mobile and should set the slider percent
// to reflect the current scrub location.
if (this.pendingSeekTime_) {
return this.pendingSeekTime_ / this.player_.duration();
}

const currentTime = this.getCurrentTime_();
let percent;
const liveTracker = this.player_.liveTracker;
Expand Down Expand Up @@ -260,7 +283,12 @@ class SeekBar extends Slider {
event.stopPropagation();

this.videoWasPlaying = !this.player_.paused();
this.player_.pause();

// Don't pause if we are on mobile and `disableSeekWhileScrubbingOnMobile: true`.
// In that case, playback should continue while the player scrubs to a new location.
if (!this.shouldDisableSeekWhileScrubbingOnMobile_) {
this.player_.pause();
}

super.handleMouseDown(event);
}
Expand Down Expand Up @@ -324,8 +352,12 @@ class SeekBar extends Slider {
}
}

// Set new time (tell player to seek to new time)
this.userSeek_(newTime);
// if on mobile and `disableSeekWhileScrubbingOnMobile: true`, keep track of the desired seek point but we won't initiate the seek until 'touchend'
if (this.shouldDisableSeekWhileScrubbingOnMobile_) {
this.pendingSeekTime_ = newTime;
} else {
this.userSeek_(newTime);
}

if (this.player_.options_.enableSmoothSeeking) {
this.update();
Expand Down Expand Up @@ -371,6 +403,13 @@ class SeekBar extends Slider {
}
this.player_.scrubbing(false);

// If we have a pending seek time, then we have finished scrubbing on mobile and should initiate a seek.
if (this.pendingSeekTime_) {
this.userSeek_(this.pendingSeekTime_);

this.pendingSeekTime_ = null;
}

/**
* Trigger timeupdate because we're done seeking and the time has changed.
* This is particularly useful for if the player is paused to time the time displays.
Expand Down Expand Up @@ -513,10 +552,5 @@ SeekBar.prototype.options_ = {
barName: 'playProgressBar'
};

// MouseTimeDisplay tooltips should not be added to a player on mobile devices
if (!IS_IOS && !IS_ANDROID) {
SeekBar.prototype.options_.children.splice(1, 0, 'mouseTimeDisplay');
}

Component.registerComponent('SeekBar', SeekBar);
export default SeekBar;
3 changes: 2 additions & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -5567,7 +5567,8 @@ Player.prototype.options_ = {
horizontalSeek: false
},
// Default smooth seeking to false
enableSmoothSeeking: false
enableSmoothSeeking: false,
disableSeekWhileScrubbingOnMobile: false
};

TECH_EVENTS_RETRIGGER.forEach(function(event) {
Expand Down
14 changes: 14 additions & 0 deletions test/unit/controls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,20 @@ QUnit.test('SeekBar should be filled on 100% when the video/audio ends', functio
window.cancelAnimationFrame = oldCAF;
});

QUnit.test('Seek bar percent should represent scrub location if we are scrubbing on mobile and have a pending seek time', function(assert) {
const player = TestHelpers.makePlayer();
const seekBar = player.controlBar.progressControl.seekBar;

player.duration(100);
seekBar.pendingSeekTime_ = 20;

assert.equal(seekBar.getPercent(), 0.2, 'seek bar percent set correctly to pending seek time');

seekBar.pendingSeekTime_ = 50;

assert.equal(seekBar.getPercent(), 0.5, 'seek bar percent set correctly to next pending seek time');
});

QUnit.test('playback rate button is hidden by default', function(assert) {
assert.expect(1);

Expand Down
148 changes: 148 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3611,6 +3611,154 @@ QUnit.test('smooth seeking set to true should update the display time components
player.dispose();
});

QUnit.test('mouseTimeDisplay should be added as child when disableSeekWhileScrubbingOnMobile is true on mobile', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: true });
const seekBar = player.controlBar.progressControl.seekBar;
const mouseTimeDisplay = seekBar.getChild('mouseTimeDisplay');

assert.ok(mouseTimeDisplay, 'mouseTimeDisplay added as a child');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('mouseTimeDisplay should not be added as child on mobile when disableSeekWhileScrubbingOnMobile is false', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: false });
const seekBar = player.controlBar.progressControl.seekBar;
const mouseTimeDisplay = seekBar.getChild('mouseTimeDisplay');

assert.notOk(mouseTimeDisplay, 'mouseTimeDisplay not added as a child');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('Seeking should occur while scrubbing on mobile when disableSeekWhileScrubbingOnMobile is false', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: false });
const seekBar = player.controlBar.progressControl.seekBar;
const userSeekSpy = sinon.spy(seekBar, 'userSeek_');

// Simulate a source loaded
player.duration(10);

// Simulate scrub
seekBar.handleMouseMove({ pageX: 200 });

assert.ok(userSeekSpy.calledOnce, 'Seek initiated while scrubbing');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('Seeking should not occur while scrubbing on mobile when disableSeekWhileScrubbingOnMobile is true', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: true });
const seekBar = player.controlBar.progressControl.seekBar;
const userSeekSpy = sinon.spy(seekBar, 'userSeek_');

// Simulate a source loaded
player.duration(10);

// Simulate scrub
seekBar.handleMouseMove({ pageX: 200 });

assert.ok(userSeekSpy.notCalled, 'Seek not initiated while scrubbing');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('Seek should occur when scrubbing completes on mobile when disableSeekWhileScrubbingOnMobile is true', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: true });
const seekBar = player.controlBar.progressControl.seekBar;
const userSeekSpy = sinon.spy(seekBar, 'userSeek_');
const targetSeekTime = 5;

// Simulate a source loaded
player.duration(10);

seekBar.pendingSeekTime_ = targetSeekTime;

// Simulate scrubbing completion
seekBar.handleMouseUp();

assert.ok(userSeekSpy.calledWith(targetSeekTime), 'Seeks to correct location when scrubbing completes');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('Player should pause while scrubbing on mobile when disableSeekWhileScrubbingOnMobile is false', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: false });
const seekBar = player.controlBar.progressControl.seekBar;
const pauseSpy = sinon.spy(player, 'pause');

// Simulate start playing
player.play();

const mockMouseDownEvent = {
pageX: 200,
stopPropagation: () => {}
};

// Simulate scrubbing start
seekBar.handleMouseDown(mockMouseDownEvent);

assert.ok(pauseSpy.calledOnce, 'Player paused');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('Player should not pause while scrubbing on mobile when disableSeekWhileScrubbingOnMobile is true', function(assert) {
const originalIsIos = browser.IS_IOS;

browser.stub_IS_IOS(true);

const player = TestHelpers.makePlayer({ disableSeekWhileScrubbingOnMobile: true });
const seekBar = player.controlBar.progressControl.seekBar;
const pauseSpy = sinon.spy(player, 'pause');

// Simulate start playing
player.play();

const mockMouseDownEvent = {
pageX: 200,
stopPropagation: () => { }
};

// Simulate scrubbing start
seekBar.handleMouseDown(mockMouseDownEvent);

assert.ok(pauseSpy.notCalled, 'Player not paused');

player.dispose();
browser.stub_IS_IOS(originalIsIos);
});

QUnit.test('addSourceElement calls tech method with correct args', function(assert) {
const player = TestHelpers.makePlayer();
const addSourceElementSpy = sinon.spy(player.tech_, 'addSourceElement');
Expand Down

3 comments on commit 57d6ab6

@phloxic
Copy link
Contributor

@phloxic phloxic commented on 57d6ab6 Dec 9, 2024

Choose a reason for hiding this comment

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

@alex-barstow this feat currently doesn't appear in the changelog.

@phloxic
Copy link
Contributor

Choose a reason for hiding this comment

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

@alex-barstow I think this needs something like this:

diff --git a/src/css/components/_progress.scss b/src/css/components/_progress.scss
index 362901fc..5334c655 100644
--- a/src/css/components/_progress.scss
+++ b/src/css/components/_progress.scss
@@ -178,7 +178,8 @@
   display: block;
 }

-.video-js.vjs-user-inactive .vjs-progress-control .vjs-mouse-display {
+.video-js.vjs-user-inactive .vjs-progress-control .vjs-mouse-display,
+.video-js.vjs-touch-enabled:not(.vjs-scrubbing) .vjs-progress-control .vjs-mouse-display {
   visibility: hidden;
   opacity: 0;
   $trans: visibility 1.0s, opacity 1.0s;

As it stands, the tooltip is shown on its last seek target position whenever you tap the screen. That is quite confusing.

@phloxic
Copy link
Contributor

Choose a reason for hiding this comment

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

Please sign in to comment.