From 5815a16cc0bfa5d2ea757dc23c245a5428a3dc12 Mon Sep 17 00:00:00 2001 From: Daniel DeMicco Date: Thu, 22 Feb 2018 10:30:49 -0800 Subject: [PATCH 1/2] Fix: filmstrip on previously played video --- src/lib/viewers/media/MediaControls.js | 31 +++++- .../media/__tests__/MediaControls-test.js | 95 ++++++++++++++----- 2 files changed, 99 insertions(+), 27 deletions(-) diff --git a/src/lib/viewers/media/MediaControls.js b/src/lib/viewers/media/MediaControls.js index 1858a6d2e..0204e9a89 100644 --- a/src/lib/viewers/media/MediaControls.js +++ b/src/lib/viewers/media/MediaControls.js @@ -84,6 +84,7 @@ class MediaControls extends EventEmitter { this.handleAutoplay = this.handleAutoplay.bind(this); this.handleSubtitle = this.handleSubtitle.bind(this); this.handleAudioTrack = this.handleAudioTrack.bind(this); + this.timeScrubberHandler = this.timeScrubberHandler.bind(this); this.setDuration(this.mediaEl.duration); this.setupSettings(); @@ -106,10 +107,13 @@ class MediaControls extends EventEmitter { this.removeVolumeScrubberWrapperExpansionHandlers(); } + if (this.timeScrubberEl) { + this.timeScrubberEl.removeEventListener('mousemove', this.timeScrubberElShowHandler); + this.timeScrubberEl.removeEventListener('mouseleave', this.filmstripHideHandler); + } + if (this.timeScrubber) { this.timeScrubber.getHandleEl().removeEventListener('mousedown', this.timeScrubbingStartHandler); - this.timeScrubber.getConvertedEl().removeEventListener('mousemove', this.filmstripShowHandler); - this.timeScrubber.getConvertedEl().removeEventListener('mouseleave', this.filmstripHideHandler); this.timeScrubber.destroy(); this.timeScrubber = undefined; } @@ -170,6 +174,7 @@ class MediaControls extends EventEmitter { this.filmstripContainerEl = undefined; this.filmstripEl = undefined; this.filmstripTimeEl = undefined; + this.timeScrubberElShowHandler = undefined; } /** @@ -723,6 +728,8 @@ class MediaControls extends EventEmitter { initFilmstrip(url, status, aspect, interval) { this.filmstripUrl = url; this.filmstripInterval = interval; + // event delegation for playerEl & convertedEl + this.timeScrubberElShowHandler = this.timeScrubberHandler(this.filmstripShowHandler); this.filmstripContainerEl = this.containerEl.appendChild(document.createElement('div')); this.filmstripContainerEl.className = 'bp-media-filmstrip-container'; @@ -748,8 +755,8 @@ class MediaControls extends EventEmitter { this.timeScrubberEl.addEventListener('touchstart', this.timeScrubbingStartHandler); } - this.timeScrubber.getConvertedEl().addEventListener('mousemove', this.filmstripShowHandler); - this.timeScrubber.getConvertedEl().addEventListener('mouseleave', this.filmstripHideHandler); + this.timeScrubberEl.addEventListener('mousemove', this.timeScrubberElShowHandler); + this.timeScrubberEl.addEventListener('mouseleave', this.filmstripHideHandler); this.filmstripEl.onload = () => { if (this.filmstripContainerEl) { @@ -762,6 +769,22 @@ class MediaControls extends EventEmitter { status.getPromise().then(this.setFilmstrip); } + /** + * Called when user moves mouse over the scrubber + * + * @param {functon} cb the callback function to be executed + * @return {Function} the handler function + */ + timeScrubberHandler(cb) { + return (event) => { + const { target } = event; + const { playedEl, convertedEl } = this.timeScrubber; + if (target === convertedEl || target === playedEl) { + cb(event); + } + }; + } + /** * Called when the user has mouse pressed the scrubbler handler * diff --git a/src/lib/viewers/media/__tests__/MediaControls-test.js b/src/lib/viewers/media/__tests__/MediaControls-test.js index 9cfb803d7..3474e818a 100644 --- a/src/lib/viewers/media/__tests__/MediaControls-test.js +++ b/src/lib/viewers/media/__tests__/MediaControls-test.js @@ -105,6 +105,8 @@ describe('lib/viewers/media/MediaControls', () => { ); stubs.removeEventListener = sandbox.stub(document, 'removeEventListener'); stubs.removeActivationListener = sandbox.stub(util, 'removeActivationListener'); + stubs.timeScrubberElShowHandler = sandbox.stub(); + stubs.genericEl = { getHandleEl: sandbox.stub().returns({ removeEventListener: sandbox.stub() @@ -116,6 +118,10 @@ describe('lib/viewers/media/MediaControls', () => { removeListener: sandbox.stub(), removeEventListener: sandbox.stub() }; + + stubs.timeScrubberEl = { + removeEventListener: sandbox.stub() + }; }); it('should remove all listeners', () => { @@ -129,23 +135,26 @@ describe('lib/viewers/media/MediaControls', () => { it('should remove time scrubber event listeners if it exists', () => { mediaControls.timeScrubber = false; - + mediaControls.timeScrubberEl = null; mediaControls.destroy(); expect(mediaControls.timeScrubber).to.be.false; mediaControls.timeScrubber = stubs.genericEl; + mediaControls.timeScrubberEl = stubs.timeScrubberEl; + mediaControls.timeScrubberElShowHandler = stubs.timeScrubberElShowHandler; + mediaControls.destroy(); expect(stubs.genericEl.getHandleEl().removeEventListener).to.be.calledWith( 'mousedown', mediaControls.timeScrubbingStartHandler ); - expect(stubs.genericEl.getConvertedEl().removeEventListener).to.be.calledWith( + expect(stubs.timeScrubberEl.removeEventListener).to.be.calledWith( 'mousemove', - mediaControls.filmstripShowHandler + stubs.timeScrubberElShowHandler ); - expect(stubs.genericEl.getConvertedEl().removeEventListener).to.be.calledWith( + expect(stubs.timeScrubberEl.removeEventListener).to.be.calledWith( 'mouseleave', mediaControls.filmstripHideHandler ); @@ -245,7 +254,6 @@ describe('lib/viewers/media/MediaControls', () => { expect(settingsStub).to.be.calledWith('quality', mediaControls.handleQuality); expect(settingsStub).to.be.calledWith('speed', mediaControls.handleRate); expect(settingsStub).to.be.calledWith('autoplay', mediaControls.handleAutoplay); - }); }); @@ -697,7 +705,7 @@ describe('lib/viewers/media/MediaControls', () => { describe('isVisible()', () => { beforeEach(() => { - stubs.wrapperParent = document.createElement('div') + stubs.wrapperParent = document.createElement('div'); mediaControls.wrapperEl = stubs.wrapperParent.appendChild(document.createElement('div')); }); @@ -743,14 +751,11 @@ describe('lib/viewers/media/MediaControls', () => { }; mediaControls.setupScrubbers(); stubs.handleElAddEventListener = sandbox.stub(mediaControls.timeScrubber.getHandleEl(), 'addEventListener'); - stubs.getConvertedElAddEventListener = sandbox.stub( - mediaControls.timeScrubber.getConvertedEl(), - 'addEventListener' - ); - - mediaControls.timeScrubberEl = { - addEventListener: sandbox.stub() - } + stubs.timeScrubberEl = { + addEventListener: sandbox.stub(), + removeEventListener: sandbox.stub() + }; + mediaControls.timeScrubberEl = stubs.timeScrubberEl; stubs.setFilmstrip = sandbox.stub(mediaControls, 'setFilmstrip'); }); @@ -771,25 +776,30 @@ describe('lib/viewers/media/MediaControls', () => { }); it('should add the correct eventListeners to the handle and converted time scrubber elements', () => { + const spy = sandbox.spy(mediaControls, 'timeScrubberHandler'); mediaControls.initFilmstrip('url', stubs.status, '380', 1); expect(stubs.handleElAddEventListener).to.be.calledWith( 'mousedown', mediaControls.timeScrubbingStartHandler ); - expect(stubs.getConvertedElAddEventListener).to.be.calledWith( + expect(stubs.timeScrubberEl.addEventListener).to.be.calledWith( 'mousemove', - mediaControls.filmstripShowHandler + mediaControls.timeScrubberElShowHandler ); - expect(stubs.getConvertedElAddEventListener).to.be.calledWith( + expect(stubs.timeScrubberEl.addEventListener).to.be.calledWith( 'mouseleave', mediaControls.filmstripHideHandler ); + expect(spy).to.have.been.called.twice; }); it('should add the touchstart eventListener if touch is detected', () => { mediaControls.hasTouch = true; mediaControls.initFilmstrip('url', stubs.status, '380', 1); - expect(mediaControls.timeScrubberEl.addEventListener).to.be.calledWith('touchstart', mediaControls.timeScrubbingStartHandler) + expect(mediaControls.timeScrubberEl.addEventListener).to.be.calledWith( + 'touchstart', + mediaControls.timeScrubbingStartHandler + ); }); it('should add the onload function to the filmstrip', () => { @@ -1034,12 +1044,9 @@ describe('lib/viewers/media/MediaControls', () => { describe('initAlternateAudio()', () => { it('should load alternate audio', () => { sandbox.stub(mediaControls.settings, 'loadAlternateAudio'); - const audios = [ - { language: 'eng', role: 'audio0' }, - { language: 'rus', role: 'audio1' } - ]; + const audios = [{ language: 'eng', role: 'audio0' }, { language: 'rus', role: 'audio1' }]; mediaControls.initAlternateAudio(audios); - expect(mediaControls.settings.loadAlternateAudio).to.be.calledWith(audios); + expect(mediaControls.settings.loadAlternateAudio).to.be.calledWith(audios); }); }); @@ -1051,5 +1058,47 @@ describe('lib/viewers/media/MediaControls', () => { expect(mediaControls.settings.enableHD).to.be.called; }); }); + + describe('timeScrubberHandler()', () => { + let handler; + beforeEach(() => { + stubs.timeScrubberCallbackSpy = sandbox.spy(); + handler = mediaControls.timeScrubberHandler(stubs.timeScrubberCallbackSpy); + + stubs.playedEl = { + removeEventListener: sandbox.stub() + }; + stubs.convertedEl = { + removeEventListener: sandbox.stub() + }; + + mediaControls.timeScrubber.playedEl = stubs.playedEl; + mediaControls.timeScrubber.convertedEl = stubs.convertedEl; + }); + + it('should execute the callback when target is playedEl', () => { + const eventStub = { + target: stubs.playedEl + }; + handler(eventStub); + expect(stubs.timeScrubberCallbackSpy).to.have.been.called; + }); + + it('should execute the callback when target is convertedEl', () => { + const eventStub = { + target: stubs.convertedEl + }; + handler(eventStub); + expect(stubs.timeScrubberCallbackSpy).to.have.been.called; + }); + + it('should not execute the callback when target is not playedEl or convertedEl', () => { + const eventStub = { + target: sandbox.stub() + }; + handler(eventStub); + expect(stubs.timeScrubberCallbackSpy).to.have.not.been.called; + }); + }); }); /* eslint-enable no-unused-expressions */ From f58d1e3a35c0ac2a9585f8db573644cad8855805 Mon Sep 17 00:00:00 2001 From: Daniel DeMicco Date: Thu, 22 Feb 2018 12:49:35 -0800 Subject: [PATCH 2/2] Chore: fix jsdoc --- src/lib/viewers/media/MediaControls.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/viewers/media/MediaControls.js b/src/lib/viewers/media/MediaControls.js index 0204e9a89..2fbd5b41e 100644 --- a/src/lib/viewers/media/MediaControls.js +++ b/src/lib/viewers/media/MediaControls.js @@ -772,8 +772,8 @@ class MediaControls extends EventEmitter { /** * Called when user moves mouse over the scrubber * - * @param {functon} cb the callback function to be executed - * @return {Function} the handler function + * @param {Function} cb - the callback function to be executed + * @return {Function} - the handler function */ timeScrubberHandler(cb) { return (event) => {