feat: Improve SmartTV scrubbing behavior (#8988)

Author: bzizmo

src/js/control-bar/progress-control/play-progress-bar.js

@@ -56,17 +56,25 @@ class PlayProgressBar extends Component {
    * @param {number} seekBarPoint
    *        A number from 0 to 1, representing a horizontal reference point
    *        from the left edge of the {@link SeekBar}
+   *
+   * @param {Event} [event]
+   *        The `timeupdate` event that caused this function to run.
    */
-  update(seekBarRect, seekBarPoint) {
+  update(seekBarRect, seekBarPoint, event) {
     const timeTooltip = this.getChild('timeTooltip');
 
     if (!timeTooltip) {
       return;
     }
 
-    const time = (this.player_.scrubbing()) ?
-      this.player_.getCache().currentTime :
-      this.player_.currentTime();
+    // Combined logic: if an event with a valid pendingSeekTime getter exists, use it.
+    const time = (event &&
+      event.target &&
+      typeof event.target.pendingSeekTime === 'function') ?
+      event.target.pendingSeekTime() :
+      (this.player_.scrubbing() ?
+        this.player_.getCache().currentTime :
+        this.player_.currentTime());
 
     timeTooltip.updateTime(seekBarRect, seekBarPoint, time);
   }

src/js/control-bar/progress-control/seek-bar.js

@@ -44,16 +44,18 @@ class SeekBar extends Slider {
     // Avoid mutating the prototype's `children` array by creating a copy
     options.children = [...options.children];
 
-    const shouldDisableSeekWhileScrubbingOnMobile = player.options_.disableSeekWhileScrubbingOnMobile && (IS_IOS || IS_ANDROID);
+    const shouldDisableSeekWhileScrubbing =
+      (player.options_.disableSeekWhileScrubbingOnMobile && (IS_IOS || IS_ANDROID)) ||
+      (player.options_.disableSeekWhileScrubbingOnSTV);
 
     // Add the TimeTooltip as a child if we are on desktop, or on mobile with `disableSeekWhileScrubbingOnMobile: true`
-    if ((!IS_IOS && !IS_ANDROID) || shouldDisableSeekWhileScrubbingOnMobile) {
+    if ((!IS_IOS && !IS_ANDROID) || shouldDisableSeekWhileScrubbing) {
       options.children.splice(1, 0, 'mouseTimeDisplay');
     }
 
     super(player, options);
 
-    this.shouldDisableSeekWhileScrubbingOnMobile_ = shouldDisableSeekWhileScrubbingOnMobile;
+    this.shouldDisableSeekWhileScrubbing_ = shouldDisableSeekWhileScrubbing;
     this.pendingSeekTime_ = null;
 
     this.setEventHandlers_();
@@ -196,7 +198,7 @@ class SeekBar extends Slider {
 
       // update the progress bar time tooltip with the current time
       if (this.bar) {
-        this.bar.update(Dom.getBoundingClientRect(this.el()), this.getProgress());
+        this.bar.update(Dom.getBoundingClientRect(this.el()), this.getProgress(), event);
       }
     });
 
@@ -233,6 +235,26 @@ class SeekBar extends Slider {
       this.player_.currentTime();
   }
 
+  /**
+   * Getter and setter for pendingSeekTime.
+   * Ensures the value is clamped between 0 and duration.
+   *
+   * @param {number|null} [time] - Optional. The new pending seek time, can be a number or null.
+   * @return {number|null} - The current pending seek time.
+   */
+  pendingSeekTime(time) {
+    if (time !== undefined) {
+      if (time !== null) {
+        const duration = this.player_.duration();
+
+        this.pendingSeekTime_ = Math.max(0, Math.min(time, duration));
+      } else {
+        this.pendingSeekTime_ = null;
+      }
+    }
+    return this.pendingSeekTime_;
+  }
+
   /**
    * Get the percentage of media played so far.
    *
@@ -242,8 +264,8 @@ class SeekBar extends Slider {
   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();
+    if (this.pendingSeekTime() !== null) {
+      return this.pendingSeekTime() / this.player_.duration();
     }
 
     const currentTime = this.getCurrentTime_();
@@ -284,7 +306,7 @@ class SeekBar extends Slider {
 
     // 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_) {
+    if (!this.shouldDisableSeekWhileScrubbing_) {
       this.player_.pause();
     }
 
@@ -351,8 +373,8 @@ class SeekBar extends Slider {
     }
 
     // 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;
+    if (this.shouldDisableSeekWhileScrubbing_) {
+      this.pendingSeekTime(newTime);
     } else {
       this.userSeek_(newTime);
     }
@@ -402,10 +424,10 @@ 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_);
+    if (this.pendingSeekTime() !== null) {
+      this.userSeek_(this.pendingSeekTime());
 
-      this.pendingSeekTime_ = null;
+      this.pendingSeekTime(null);
     }
 
     /**
@@ -425,18 +447,46 @@ class SeekBar extends Slider {
     }
   }
 
+  /**
+   * Handles pending seek time when `disableSeekWhileScrubbingOnSTV` is enabled.
+   *
+   * @param {number} stepAmount - The number of seconds to step (positive for forward, negative for backward).
+   */
+  handlePendingSeek_(stepAmount) {
+    if (!this.player_.paused()) {
+      this.player_.pause();
+    }
+
+    const currentPos = this.pendingSeekTime() !== null ?
+      this.pendingSeekTime() :
+      this.player_.currentTime();
+
+    this.pendingSeekTime(currentPos + stepAmount);
+    this.player_.trigger({ type: 'timeupdate', target: this, manuallyTriggered: true });
+  }
+
   /**
    * Move more quickly fast forward for keyboard-only users
    */
   stepForward() {
-    this.userSeek_(this.player_.currentTime() + this.options().stepSeconds);
+    // if `disableSeekWhileScrubbingOnSTV: true`, keep track of the desired seek point but we won't initiate the seek
+    if (this.shouldDisableSeekWhileScrubbing_) {
+      this.handlePendingSeek_(this.options().stepSeconds);
+    } else {
+      this.userSeek_(this.player_.currentTime() + this.options().stepSeconds);
+    }
   }
 
   /**
    * Move more quickly rewind for keyboard-only users
    */
   stepBack() {
-    this.userSeek_(this.player_.currentTime() - this.options().stepSeconds);
+    // if `disableSeekWhileScrubbingOnSTV: true`, keep track of the desired seek point but we won't initiate the seek
+    if (this.shouldDisableSeekWhileScrubbing_) {
+      this.handlePendingSeek_(-this.options().stepSeconds);
+    } else {
+      this.userSeek_(this.player_.currentTime() - this.options().stepSeconds);
+    }
   }
 
   /**
@@ -448,6 +498,10 @@ class SeekBar extends Slider {
    *
    */
   handleAction(event) {
+    if (this.pendingSeekTime() !== null) {
+      this.userSeek_(this.pendingSeekTime());
+      this.pendingSeekTime(null);
+    }
     if (this.player_.paused()) {
       this.player_.play();
     } else {

src/js/control-bar/time-controls/current-time-display.js

@@ -35,6 +35,8 @@ class CurrentTimeDisplay extends TimeDisplay {
 
     if (this.player_.ended()) {
       time = this.player_.duration();
+    } else if (event && event.target && typeof event.target.pendingSeekTime === 'function') {
+      time = event.target.pendingSeekTime();
     } else {
       time = (this.player_.scrubbing()) ? this.player_.getCache().currentTime : this.player_.currentTime();
     }

src/js/player.js

@@ -5577,7 +5577,8 @@ Player.prototype.options_ = {
   },
   // Default smooth seeking to false
   enableSmoothSeeking: false,
-  disableSeekWhileScrubbingOnMobile: false
+  disableSeekWhileScrubbingOnMobile: false,
+  disableSeekWhileScrubbingOnSTV: false
 };
 
 TECH_EVENTS_RETRIGGER.forEach(function(event) {

src/js/slider/slider.js

@@ -325,6 +325,10 @@ class Slider extends Component {
         event.stopPropagation();
         this.stepForward();
       } else {
+        if (this.pendingSeekTime()) {
+          this.pendingSeekTime(null);
+          this.userSeek_(this.player_.currentTime());
+        }
         super.handleKeyDown(event);
       }
 

test/unit/controls.test.js

@@ -320,11 +320,11 @@ QUnit.test('Seek bar percent should represent scrub location if we are scrubbing
   const seekBar = player.controlBar.progressControl.seekBar;
 
   player.duration(100);
-  seekBar.pendingSeekTime_ = 20;
+  seekBar.pendingSeekTime(20);
 
   assert.equal(seekBar.getPercent(), 0.2, 'seek bar percent set correctly to pending seek time');
 
-  seekBar.pendingSeekTime_ = 50;
+  seekBar.pendingSeekTime(50);
 
   assert.equal(seekBar.getPercent(), 0.5, 'seek bar percent set correctly to next pending seek time');
 });
@@ -680,3 +680,114 @@ QUnit.test('Remaing time negative sign can be optional', function(assert) {
   rtd2.dispose();
   player.dispose();
 });
+
+QUnit.module('SmartTV UI Updates (Progress Bar & Time Display)', function(hooks) {
+  let player;
+  let seekBar;
+  let currentTimeDisplay;
+
+  hooks.beforeEach(function() {
+    player = TestHelpers.makePlayer({
+      spatialNavigation: { enabled: true },
+      disableSeekWhileScrubbingOnSTV: true,
+      controlBar: {
+        progressControl: {
+          seekBar: {
+            stepSeconds: 5
+          }
+        }
+      }
+    });
+
+    seekBar = player.controlBar.progressControl.seekBar;
+    currentTimeDisplay = player.controlBar.getChild('currentTimeDisplay');
+
+    player.duration(100);
+  });
+
+  hooks.afterEach(function() {
+    player.dispose();
+  });
+
+  QUnit.test('Step forward updates seek bar progress and current-time display', function(assert) {
+    player.currentTime(40);
+    seekBar.stepForward();
+
+    assert.equal(
+      seekBar.pendingSeekTime(),
+      45,
+      'pendingSeekTime should be 45 (40 + 5) after stepForward'
+    );
+
+    assert.equal(
+      seekBar.getPercent(),
+      0.45,
+      'Seek bar progress should reflect 45% progress after stepForward'
+    );
+
+    assert.equal(
+      currentTimeDisplay.formattedTime_,
+      '0:45',
+      'Current-time-display should update to 45s after stepForward'
+    );
+  });
+
+  QUnit.test('Step back updates seek bar progress and current-time display', function(assert) {
+    player.currentTime(40);
+    seekBar.stepBack();
+
+    assert.equal(
+      seekBar.pendingSeekTime(),
+      35,
+      'pendingSeekTime should be 35 (40 - 5) after stepBack'
+    );
+
+    assert.equal(
+      seekBar.getPercent(),
+      0.35,
+      'Seek bar progress should reflect 35% progress after stepBack'
+    );
+
+    assert.equal(
+      currentTimeDisplay.formattedTime_,
+      '0:35',
+      'Current-time-display should update to 35s after stepBack'
+    );
+  });
+
+  QUnit.test('Pressing enter finalizes the seek and updates UI', function(assert) {
+    player.currentTime(40);
+    seekBar.stepForward();
+
+    seekBar.handleAction();
+
+    assert.equal(
+      seekBar.pendingSeekTime(),
+      null,
+      'pendingSeekTime should be reset to null after seeking'
+    );
+
+    assert.equal(
+      seekBar.getPercent(),
+      0.45,
+      'Seek bar progress should remain at 45% after seeking'
+    );
+
+    assert.equal(
+      currentTimeDisplay.formattedTime_,
+      '0:45',
+      'Current-time-display should remain at 45s after seeking'
+    );
+  });
+
+  QUnit.test('Resets pendingSeekTime when SmartTV focus moves away without confirmation', function(assert) {
+    const userSeekSpy = sinon.spy(seekBar, 'userSeek_');
+
+    seekBar.trigger({ type: 'keydown', key: 'ArrowUp' });
+    assert.ok(seekBar.pendingSeekTime() !== null, 'pendingSeekTime should be set after ArrowUp keydown');
+    seekBar.trigger({ type: 'keydown', key: 'ArrowLeft' });
+    assert.equal(seekBar.pendingSeekTime(), null, 'pendingSeekTime should be reset when SeekBar loses focus');
+    assert.ok(userSeekSpy.calledWith(player.currentTime()), 'userSeek_ should be called with current player time');
+    userSeekSpy.restore();
+  });
+});