Fix: Seek time doesn't match filmstrip timecode (#131)

Previously, hovering over a time in the scrubber to seek to that point
would display a timecode that doesn't match the actual time that the
seek would take you to. This was more apparent in longer videos. For
example, hovering over 10:00 in a 1hr video, and clicking at that point
would actually take you to 9:30. This was due to two different ways the
timecodes (the filmstrip timecode versus the seeked timecode) were
calculated.

src/lib/viewers/media/MediaControls.js

@@ -721,7 +721,7 @@ class MediaControls extends EventEmitter {
      * @return {Object}
      */
     computeFilmstripPositions(pageX, rectLeft, rectWidth, filmstripWidth) {
-        const time = ((pageX - rectLeft) * this.mediaEl.duration) / rectWidth; // given the mouse X position, get the relative time
+        const time = this.timeScrubber.computeScrubberPosition(pageX) * this.mediaEl.duration; // given the mouse X position, get the relative time
         const frame = Math.floor(time / this.filmstripInterval); // get the frame number to show
         let frameWidth = filmstripWidth / FILMSTRIP_FRAMES_PER_ROW; // calculate the frame width based on the filmstrip width
         let left = -1 * (frame % FILMSTRIP_FRAMES_PER_ROW) * frameWidth; // get the frame position in a given row

src/lib/viewers/media/Scrubber.js

@@ -162,19 +162,28 @@ class Scrubber extends EventEmitter {
         this.convertedEl.style.width = `${this.convertedValue * 100}%`;
     }
 
+    /**
+     * Returns the relative X position of mouse on scrubber
+     *
+     * @param {number} pageX - Mouse X position
+     * @return {number} A float between 0 and 1, for the relative position of pageX
+     */
+    computeScrubberPosition(pageX) {
+        const rect = this.scrubberEl.getBoundingClientRect();
+        const relPos = (pageX - rect.left) / rect.width;
+        return Math.max(Math.min(relPos, MAX_VALUE), MIN_VALUE);
+    }
+
     /**
      * Calculates the position of the scrubber handle based on mouse action
      *
      * @private
-     * @param {Event} event - the instance of the class
+     * @param {Event} event - The mouse event that this handler is subscribed to
      * @return {void}
      */
     scrubbingHandler(event) {
-        const rect = this.scrubberEl.getBoundingClientRect();
         const pageX = event.pageX;
-        let newValue = (pageX - rect.left) / rect.width;
-
-        newValue = Math.max(Math.min(newValue, MAX_VALUE), MIN_VALUE);
+        const newValue = this.computeScrubberPosition(pageX);
 
         this.setValue(newValue);
         this.emit('valuechange');

src/lib/viewers/media/__tests__/MediaControls-test.js

@@ -752,10 +752,11 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 100
             };
             mediaControls.filmstripInterval = 1;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.25);
 
             const positions = mediaControls.computeFilmstripPositions(400, 200, 1000, null);
 
-            expect(positions.time).to.equal(20);
+            expect(positions.time).to.equal(25);
             expect(positions.left).to.equal(0);
             expect(positions.top).to.equal(0);
             expect(positions.containerLeft).to.equal(120);
@@ -766,6 +767,7 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 100
             };
             mediaControls.filmstripInterval = 1;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.2);
 
             const positions = mediaControls.computeFilmstripPositions(400, 200, 1000, 16000);
 
@@ -780,6 +782,7 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 1100
             };
             mediaControls.filmstripInterval = 1;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.2);
 
             const positions = mediaControls.computeFilmstripPositions(400, 200, 1000, 16000);
 
@@ -794,6 +797,7 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 2000
             };
             mediaControls.filmstripInterval = 5;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.6);
 
             const positions = mediaControls.computeFilmstripPositions(800, 200, 1000, 16000);
 
@@ -808,6 +812,7 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 100
             };
             mediaControls.filmstripInterval = 1;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.01);
 
             const positions = mediaControls.computeFilmstripPositions(210, 200, 1000, 16000);
 
@@ -822,6 +827,7 @@ describe('lib/viewers/media/MediaControls', () => {
                 duration: 100
             };
             mediaControls.filmstripInterval = 1;
+            sandbox.stub(mediaControls.timeScrubber, 'computeScrubberPosition').returns(0.99);
 
             const positions = mediaControls.computeFilmstripPositions(1190, 200, 1000, 16000);
 

src/lib/viewers/media/__tests__/Scrubber-test.js

@@ -122,14 +122,51 @@ describe('lib/viewers/media/Scrubber', () => {
         it('should adjust the scrubber value to the current scrubber handle position value in the video', () => {
             stubs.setValue = sandbox.stub(scrubber, 'setValue');
             stubs.emit = sandbox.stub(scrubber, 'emit');
-            scrubber.scrubberEl.style.width = '25px';
-            scrubber.scrubbingHandler({ pageX: 100 });
+            stubs.scrubberPosition = sandbox.stub(scrubber, 'computeScrubberPosition').returns(0.5);
 
-            expect(stubs.setValue).to.be.calledWith(1);
+            scrubber.scrubbingHandler({ pageX: 50 });
+
+            expect(stubs.scrubberPosition).to.be.calledWith(50);
+            expect(stubs.setValue).to.be.calledWith(0.5);
             expect(stubs.emit).to.be.calledWith('valuechange');
         });
     });
 
+    describe('computeScrubberPosition()', () => {
+        it('should compute correct scrubber position', () => {
+            sandbox.stub(scrubber.scrubberEl, 'getBoundingClientRect').returns({
+                left: 20,
+                width: 100
+            });
+
+            const position = scrubber.computeScrubberPosition(30);
+
+            expect(position).to.equal(0.1);
+        });
+
+        it('should cap the scrubber position to 1', () => {
+            sandbox.stub(scrubber.scrubberEl, 'getBoundingClientRect').returns({
+                left: 20,
+                width: 100
+            });
+
+            const position = scrubber.computeScrubberPosition(130);
+
+            expect(position).to.equal(1);
+        });
+
+        it('should floor the scrubber position to 0', () => {
+            sandbox.stub(scrubber.scrubberEl, 'getBoundingClientRect').returns({
+                left: 20,
+                width: 100
+            });
+
+            const position = scrubber.computeScrubberPosition(10);
+
+            expect(position).to.equal(0);
+        });
+    });
+
     describe('mouseDownHandler()', () => {
         beforeEach(() => {
             stubs.scrub = sandbox.stub(scrubber, 'scrubbingHandler');