Skip to content

Commit

Permalink
Fix: update autoplay behavior (#461)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press committed Oct 31, 2017
1 parent bdbb75a commit c4778f8
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 35 deletions.
3 changes: 3 additions & 0 deletions src/lib/Preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -905,6 +905,9 @@ class Preview extends EventEmitter {
case 'notificationhide':
this.ui.hideNotification();
break;
case 'mediaendautoplay':
this.navigateRight();
break;
default:
// This includes 'notification', 'preload' and others
this.emit(data.event, data.data);
Expand Down
8 changes: 8 additions & 0 deletions src/lib/__tests__/Preview-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,14 @@ describe('lib/Preview', () => {
expect(preview.ui.hideNotification).to.be.called;
});

it('should navigate right on mediaendautoplay event', () => {
sandbox.stub(preview, 'navigateRight');
const data = { event: 'mediaendautoplay' };

preview.handleViewerEvents(data);
expect(preview.navigateRight).to.be.called;
});

it('should emit viewerevent when event does not match', () => {
sandbox.stub(preview, 'emit');
const data = {
Expand Down
5 changes: 4 additions & 1 deletion src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,10 @@ class DashViewer extends VideoBaseViewer {
return;
}

this.checkAutoplay();
if (this.isAutoplayEnabled()) {
this.autoplay();
}

this.calculateVideoDimensions();
this.loadUI();
this.loadFilmStrip();
Expand Down
37 changes: 29 additions & 8 deletions src/lib/viewers/media/MediaBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ class MediaBaseViewer extends BaseViewer {
.getPromise()
.then(() => {
this.mediaEl.src = this.mediaUrl;
this.checkAutoplay();
if (this.isAutoplayEnabled()) {
this.autoplay();
}
})
.catch(this.handleAssetError);
}
Expand Down Expand Up @@ -221,8 +223,7 @@ class MediaBaseViewer extends BaseViewer {
* @return {void}
*/
handleAutoplay() {
const isAutoplayEnabled = this.cache.get(MEDIA_AUTOPLAY_CACHE_KEY) === 'Enabled';
this.emit('autoplay', isAutoplayEnabled);
this.emit('autoplay', this.isAutoplayEnabled());
}

/**
Expand All @@ -232,11 +233,7 @@ class MediaBaseViewer extends BaseViewer {
* @emits volume
* @return {void}
*/
checkAutoplay() {
if (this.cache.get(MEDIA_AUTOPLAY_CACHE_KEY) !== 'Enabled') {
return;
}

autoplay() {
// Play may return a promise depening on browser support. This promise
// will resolve when playback starts. If it fails, pause UI should be shown.
// https://webkit.org/blog/7734/auto-play-policy-changes-for-macos/
Expand All @@ -257,6 +254,16 @@ class MediaBaseViewer extends BaseViewer {
}
}

/**
* Determines if autoplay is enabled
*
* @private
* @return {boolean} Indicates if autoplay is enabled
*/
isAutoplayEnabled() {
return this.cache.get(MEDIA_AUTOPLAY_CACHE_KEY) === 'Enabled';
}

/**
* Resize handler
*
Expand Down Expand Up @@ -403,6 +410,19 @@ class MediaBaseViewer extends BaseViewer {
this.debouncedEmit('seeked', this.mediaEl.currentTime);
}

/**
* Emits the previewnextfile event if autoplay is enabled.
*
* @private
* @emits previewnextfile
* @return {void}
*/
mediaendHandler() {
if (this.isAutoplayEnabled()) {
this.emit('mediaendautoplay');
}
}

/**
* Shows the play button in media content.
*
Expand Down Expand Up @@ -582,6 +602,7 @@ class MediaBaseViewer extends BaseViewer {
this.mediaEl.addEventListener('pause', this.pauseHandler);
this.mediaEl.addEventListener('ended', this.resetPlayIcon);
this.mediaEl.addEventListener('seeked', this.seekHandler);
this.mediaEl.addEventListener('ended', this.mediaendHandler);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/lib/viewers/media/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class Settings extends EventEmitter {
this.containerEl.classList.add(CLASS_SETTINGS_SUBTITLES_UNAVAILABLE);
this.containerEl.classList.add(CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE);

if (Browser.isIOS()) {
if (Browser.isMobile()) {
this.containerEl.classList.add(CLASS_SETTINGS_AUTOPLAY_UNAVAILABLE);
}

Expand Down
11 changes: 7 additions & 4 deletions src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,9 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash, 'loadDashPlayer');
sandbox.stub(dash, 'resetLoadTimeout');
sandbox.stub(dash, 'loadAssets');
sandbox.stub(dash, 'checkAutoplay');
sandbox.stub(dash, 'isAutoplayEnabled').returns(true);
sandbox.stub(dash, 'autoplay');

sandbox.stub(dash, 'getRepStatus').returns({ getPromise: () => Promise.resolve() });
sandbox.stub(Promise, 'all').returns(stubs.promise);

Expand All @@ -147,7 +149,7 @@ describe('lib/viewers/media/DashViewer', () => {
expect(dash.setup).to.be.called;
expect(dash.loadDashPlayer).to.be.called;
expect(dash.resetLoadTimeout).to.be.called;
expect(dash.checkAutoplay).to.be.called;
expect(dash.autoplay).to.be.called;
})
.catch(() => {});
});
Expand Down Expand Up @@ -493,7 +495,8 @@ describe('lib/viewers/media/DashViewer', () => {
it('should load the meta data for the media element, show the media/play button, load subs, check for autoplay, and set focus', () => {
sandbox.stub(dash, 'isDestroyed').returns(false);
sandbox.stub(dash, 'showMedia');
sandbox.stub(dash, 'checkAutoplay');
sandbox.stub(dash, 'isAutoplayEnabled').returns(true);
sandbox.stub(dash, 'autoplay');
sandbox.stub(dash, 'calculateVideoDimensions');
sandbox.stub(dash, 'loadUI');
sandbox.stub(dash, 'loadFilmStrip');
Expand All @@ -506,7 +509,7 @@ describe('lib/viewers/media/DashViewer', () => {
sandbox.stub(dash, 'showPlayButton');

dash.loadeddataHandler();
expect(dash.checkAutoplay).to.be.called;
expect(dash.autoplay).to.be.called;
expect(dash.showMedia).to.be.called;
expect(dash.showPlayButton).to.be.called;
expect(dash.loadSubtitles).to.be.called;
Expand Down
54 changes: 35 additions & 19 deletions src/lib/viewers/media/__tests__/MediaBaseViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
beforeEach(() => {
media.mediaEl = document.createElement('video');
media.mediaEl.addEventListener = sandbox.stub();
sandbox.stub(media, 'isAutoplayEnabled').returns(false);
sandbox.stub(media, 'autoplay');
});

it('should load mediaUrl in the media element', () => {
Expand All @@ -135,6 +137,16 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
expect(media.mediaEl.autoplay).to.be.true;
})
});

it('should autoplay if enabled', () => {
media.isAutoplayEnabled.returns(true);
sandbox.stub(media, 'getRepStatus').returns({ getPromise: () => Promise.resolve() });
media.mediaEl = document.createElement('video');

return media.load().then(() => {
expect(media.autoplay).to.be.called;
})
});
});

describe('loadeddataHandler()', () => {
Expand Down Expand Up @@ -218,50 +230,38 @@ describe('lib/viewers/media/MediaBaseViewer', () => {

describe('handleAutoplay()', () => {
it('should emit the new autoplay value', () => {
sandbox.stub(media.cache, 'has').returns(true);
sandbox.stub(media.cache, 'get').returns('Disbled');
sandbox.stub(media, 'isAutoplayEnabled').returns(false);
sandbox.stub(media, 'emit');

media.handleAutoplay();
expect(media.emit).to.be.calledWith('autoplay', false);

media.cache.get.returns('Enabled');
media.isAutoplayEnabled.returns(true);

media.handleAutoplay();
expect(media.emit).to.be.calledWith('autoplay', true);
});
});

describe('checkAutoplay()', () => {
describe('autoplay()', () => {
beforeEach(() => {
media.mediaEl = {
play: sandbox.stub().returns(Promise.resolve())
};

sandbox.stub(media.cache, 'get').returns('Enabled');
sandbox.stub(media, 'isAutoplayEnabled').returns(true);
sandbox.stub(Browser, 'isIOS').returns(false);
});

it('should do nothing the settings value is absent or disabled', () => {
media.cache.get.returns(false);
media.checkAutoplay();
expect(media.mediaEl.play).to.not.be.called;

media.cache.get.returns(true);
media.cache.get.returns('Disabled');
media.checkAutoplay();
expect(media.mediaEl.play).to.not.be.called;
});

it('should set autoplay if setting is enabled and handle the promise if it is a valid promise', () => {
media.checkAutoplay();
media.autoplay();
expect(media.mediaEl.play).to.be.called;
expect(media.mediaEl.autoplay).to.be.undefined;
});

it('should set autoplay to true if play does not return a promise', () => {
media.mediaEl.play.returns(undefined);
media.checkAutoplay();
media.autoplay();
expect(media.mediaEl.autoplay).to.be.true;
});
});
Expand Down Expand Up @@ -395,6 +395,22 @@ describe('lib/viewers/media/MediaBaseViewer', () => {
});
});

describe('mediaendHandler()', () => {
it('emit the mediaendautoplay event if autoplay is enabled', () => {
sandbox.stub(media, 'isAutoplayEnabled').returns(false);
sandbox.stub(media, 'emit');

media.mediaendHandler();
expect(media.isAutoplayEnabled).to.be.called;
expect(media.emit).to.not.be.called;

media.isAutoplayEnabled.returns(true);

media.mediaendHandler();
expect(media.emit).to.be.calledWith('mediaendautoplay');
});
});

describe('showPlayButton', () => {
it('should show the play button if it exists', () => {
media.playButtonEl = document.createElement('button');
Expand Down Expand Up @@ -649,7 +665,7 @@ describe('lib/viewers/media/MediaBaseViewer', () => {

media.addEventListenersForMediaElement();

expect(media.mediaEl.addEventListener.callCount).to.equal(7);
expect(media.mediaEl.addEventListener.callCount).to.equal(8);
});
});

Expand Down
4 changes: 2 additions & 2 deletions src/lib/viewers/media/__tests__/Settings-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe('lib/viewers/media/Settings', () => {
beforeEach(() => {
fixture.load('viewers/media/__tests__/Settings-test.html');
const containerEl = document.querySelector('.container');
sandbox.stub(Browser, 'isIOS').returns(true);
sandbox.stub(Browser, 'isMobile').returns(true);
settings = new Settings(containerEl, {
set: () => {},
has: () => {},
Expand Down Expand Up @@ -44,7 +44,7 @@ describe('lib/viewers/media/Settings', () => {
expect(settings.containerEl).to.have.class('bp-media-settings-subtitles-unavailable');
});

it('should hide the autoplay option if on iOS', () => {
it('should hide the autoplay option if on mobile', () => {
expect(settings.containerEl.classList.contains('bp-media-settings-autoplay-unavailable')).to.be.true;
});
});
Expand Down

0 comments on commit c4778f8

Please sign in to comment.