Skip to content

Commit

Permalink
Fix: Enable HD settings only if HD rep is available (#495)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeremy Press authored Dec 5, 2017
1 parent 5c8dffa commit e4302e3
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 12 deletions.
14 changes: 13 additions & 1 deletion src/lib/viewers/media/DashViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ class DashViewer extends VideoBaseViewer {
* @return {void}
*/
handleQuality() {
const quality = this.cache.get('media-quality');
// If there is no HD rep, use the standard definition option
const quality = this.hdVideoId !== -1 ? this.cache.get('media-quality') : 'sd';

switch (quality) {
case 'hd':
Expand Down Expand Up @@ -451,6 +452,17 @@ class DashViewer extends VideoBaseViewer {
this.mediaContainerEl.focus();
}

/**
* @inheritdoc
*/
loadUI() {
super.loadUI();

if (this.hdVideoId !== -1) {
this.mediaControls.enableHDSettings();
}
}

/**
* Loads the film strip
*
Expand Down
9 changes: 9 additions & 0 deletions src/lib/viewers/media/MediaControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,15 @@ class MediaControls extends EventEmitter {
this.settings.addListener('audiotracks', this.handleAudioTrack);
this.settings.loadAlternateAudio(audioLanguages);
}

/**
* Adds the quality options to the settings menu
*
* @return {void}
*/
enableHDSettings() {
this.settings.enableHD();
}
}

export default MediaControls;
54 changes: 45 additions & 9 deletions src/lib/viewers/media/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ const CLASS_SETTINGS_SELECTED = 'bp-media-settings-selected';
const CLASS_SETTINGS_OPEN = 'bp-media-settings-is-open';
const CLASS_SETTINGS_SUBTITLES_UNAVAILABLE = 'bp-media-settings-subtitles-unavailable';
const CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE = 'bp-media-settings-audiotracks-unavailable';
const CLASS_SETTINGS_HD_UNAVAILABLE = 'bp-media-settings-hd-unavailable';
const CLASS_SETTINGS_QUALITY_MENU = 'bp-media-settings-menu-quality';
const CLASS_SETTINGS_AUTOPLAY_UNAVAILABLE = 'bp-media-settings-autoplay-unavailable';
const CLASS_SETTINGS_SUBTITLES_ON = 'bp-media-settings-subtitles-on';
const SELECTOR_SETTINGS_SUB_ITEM = '.bp-media-settings-sub-item';
const SELECTOR_SETTINGS_VALUE = '.bp-media-settings-value';
const MEDIA_SPEEDS = ['0.5', '1.0', '1.25', '1.5', '2.0'];
const MEDIA_QUALITY_SD = 'sd';
const MEDIA_QUALITY_HD = 'hd';
const MEDIA_QUALITY_AUTO = 'auto';

const SETTINGS_TEMPLATE = `<div class="bp-media-settings">
<div class="bp-media-settings-menu-main bp-media-settings-menu" role="menu">
Expand Down Expand Up @@ -86,20 +91,26 @@ const SETTINGS_TEMPLATE = `<div class="bp-media-settings">
<div class="bp-media-settings-value">2.0</div>
</div>
</div>
<div class="bp-media-settings-menu-quality bp-media-settings-menu" role="menu">
<div class="bp-media-settings-menu-quality bp-media-settings-menu bp-media-settings-is-hidden" data-disabled="true" role="menu">
<div class="bp-media-settings-sub-item bp-media-settings-sub-item-quality" data-type="menu" tabindex="0" role="menuitem" aria-haspopup="true">
<div class="bp-media-settings-arrow">${ICON_ARROW_LEFT}</div>
<div class="bp-media-settings-label" aria-label="${__('media_quality')}">${__('media_quality')}</div>
</div>
<div class="bp-media-settings-sub-item" data-type="quality" data-value="sd" tabindex="0" role="menuitemradio">
<div class="bp-media-settings-sub-item" data-type="quality" data-value=${
MEDIA_QUALITY_SD
} tabindex="0" role="menuitemradio">
<div class="bp-media-settings-icon">${ICON_CHECK_MARK}</div>
<div class="bp-media-settings-value">480p</div>
</div>
<div class="bp-media-settings-sub-item" data-type="quality" data-value="hd" tabindex="0" role="menuitemradio">
<div class="bp-media-settings-sub-item" data-type="quality" data-value=${
MEDIA_QUALITY_HD
} tabindex="0" role="menuitemradio">
<div class="bp-media-settings-icon">${ICON_CHECK_MARK}</div>
<div class="bp-media-settings-value">1080p</div>
</div>
<div class="bp-media-settings-sub-item bp-media-settings-selected" data-type="quality" data-value="auto" tabindex="0" role="menuitemradio" aria-checked="true">
<div class="bp-media-settings-sub-item bp-media-settings-selected" data-type="quality" data-value=${
MEDIA_QUALITY_AUTO
} tabindex="0" role="menuitemradio" aria-checked="true">
<div class="bp-media-settings-icon">${ICON_CHECK_MARK}</div>
<div class="bp-media-settings-value">${__('media_quality_auto')}</div>
</div>
Expand Down Expand Up @@ -185,6 +196,7 @@ class Settings extends EventEmitter {
addActivationListener(this.settingsEl, this.menuEventHandler);
this.containerEl.classList.add(CLASS_SETTINGS_SUBTITLES_UNAVAILABLE);
this.containerEl.classList.add(CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE);
this.containerEl.classList.add(CLASS_SETTINGS_HD_UNAVAILABLE);

// Allow scrollbars after animations end
this.settingsEl.addEventListener('transitionend', this.handleTransitionEnd);
Expand All @@ -203,11 +215,11 @@ class Settings extends EventEmitter {
* @return {void}
*/
init() {
const quality = this.cache.get('media-quality') || 'auto';
const speed = this.cache.get('media-speed') || '1.0';
const autoplay = this.cache.get('media-autoplay') || 'Disabled';

this.chooseOption(TYPE_QUALITY, quality);
// We initialize quality with SD because we don't yet know if the video has an HD rep
this.chooseOption(TYPE_QUALITY, MEDIA_QUALITY_SD, false);
this.chooseOption(TYPE_SPEED, speed);
this.chooseOption(TYPE_AUTOPLAY, autoplay);
}
Expand Down Expand Up @@ -317,6 +329,10 @@ class Settings extends EventEmitter {
*/
showSubMenu(type) {
const subMenu = this.settingsEl.querySelector(`.bp-media-settings-menu-${type}`);
if (subMenu.getAttribute('data-disabled')) {
return;
}

this.settingsEl.classList.add(`bp-media-settings-show-${type}`);

this.setMenuContainerDimensions(subMenu);
Expand Down Expand Up @@ -465,11 +481,14 @@ class Settings extends EventEmitter {
*
* @param {string} type - of menu option
* @param {string} value - of menu option
* @param {boolean} setCache - should we set the cache with the given settings value
* @return {void}
*/
chooseOption(type, value) {
// Save the value
this.cache.set(`media-${type}`, value, true);
chooseOption(type, value, setCache = true) {
// Save the value if specified
if (setCache) {
this.cache.set(`media-${type}`, value, true);
}

// Emit to the listener what was chosen
this.emit(type);
Expand Down Expand Up @@ -699,6 +718,23 @@ class Settings extends EventEmitter {
this.containerEl.classList.remove(CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE);
this.reset();
}

/**
* Adds the quality options to the settings menu
*
* @return {void}
*/
enableHD() {
this.containerEl.classList.remove(CLASS_SETTINGS_HD_UNAVAILABLE);
const qualitySubMenu = this.containerEl.querySelector(`.${CLASS_SETTINGS_QUALITY_MENU}`);

// Sub menu should no longer be disabled
qualitySubMenu.setAttribute('data-disabled', '');

// Check our cached settings value now that we know the HD rep is available
const quality = this.cache.get('media-quality') || MEDIA_QUALITY_AUTO;
this.chooseOption(TYPE_QUALITY, quality);
}
}

export default Settings;
16 changes: 16 additions & 0 deletions src/lib/viewers/media/Settings.scss
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,22 @@ $item-hover-color: #f6fafd;
}
}

.bp-media-settings-menu-quality {
.bp-media-settings-hd-unavailable & {
display: none;
}
}

.bp-media-settings-item-quality {
.bp-media-settings-hd-unavailable & {
pointer-events: none;

.bp-media-settings-arrow {
display: none;
}
}
}

.bp-media-settings-label,
.bp-media-settings-value {
display: table-cell;
Expand Down
7 changes: 7 additions & 0 deletions src/lib/viewers/media/VideoBaseViewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ class VideoBaseViewer extends MediaBaseViewer {
this.mediaControls.show();
}

/**
* @inheritdoc
*/
loadUI() {
super.loadUI();
}

/**
* Handler for a pointer event on the media element.
*
Expand Down
51 changes: 50 additions & 1 deletion src/lib/viewers/media/__tests__/DashViewer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,11 +349,20 @@ describe('lib/viewers/media/DashViewer', () => {
describe('handleQuality()', () => {
beforeEach(() => {
dash.hdVideoId = 1;
dash.sdVideoId = 2;
dash.sdVideoId = -1;
stubs.enableVideoId = sandbox.stub(dash, 'enableVideoId');
stubs.adapt = sandbox.stub(dash, 'enableAdaptation');
});

it('should enforce SD if there is no HD video ID', () => {
dash.hdVideoId = -1;
sandbox.stub(dash.cache, 'get').returns('hd');
dash.handleQuality();

expect(stubs.enableVideoId).to.be.calledWith(dash.sdVideoId);
expect(dash.emit).to.be.calledWith('qualitychange', 'sd');
});

it('should enable HD video', () => {
sandbox.stub(dash.cache, 'get').returns('hd');
dash.handleQuality();
Expand Down Expand Up @@ -383,6 +392,14 @@ describe('lib/viewers/media/DashViewer', () => {
expect(stubs.adapt).to.be.calledWith(true); // default to adapt=true
expect(dash.emit).to.not.be.called;
});

it('should enable SD if there is no HD rep available', () => {
dash.hdVideoId = -1;
sandbox.stub(dash.cache, 'get').returns('hd');
dash.handleQuality();
expect(stubs.adapt).to.be.calledWith(false);
expect(dash.emit).to.be.calledWith('qualitychange', 'sd');
});
});

describe('adaptationHandler()', () => {
Expand Down Expand Up @@ -519,6 +536,38 @@ describe('lib/viewers/media/DashViewer', () => {
});
});

describe('loadUI()', () => {
beforeEach(() => {
stubs.loadUI = DashViewer.prototype.loadUI;
dash.mediaControls = {
enableHDSettings: sandbox.stub(),
removeListener: sandbox.stub(),
removeAllListeners: sandbox.stub(),
destroy: sandbox.stub()
};

Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: sandbox.mock() });

});

afterEach(() => {
Object.defineProperty(VideoBaseViewer.prototype, 'loadUI', { value: stubs.loadUI });
});

it('should enable HD settings if an HD rep exists', () => {
dash.hdVideoId = 3;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.be.called;

});

it('should do nothing if there is no HD rep', () => {
dash.hdVideoId = -1;
dash.loadUI();
expect(dash.mediaControls.enableHDSettings).to.not.be.called;
});
});

describe('loadFilmStrip()', () => {
beforeEach(() => {
dash.options = {
Expand Down
9 changes: 9 additions & 0 deletions src/lib/viewers/media/__tests__/MediaControls-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1042,5 +1042,14 @@ describe('lib/viewers/media/MediaControls', () => {
expect(mediaControls.settings.loadAlternateAudio).to.be.calledWith(audios);
});
});

describe('enableHDSettings()', () => {
it('enable HD in the settings menu', () => {
sandbox.stub(mediaControls.settings, 'enableHD');
mediaControls.enableHDSettings();

expect(mediaControls.settings.enableHD).to.be.called;
});
});
});
/* eslint-enable no-unused-expressions */
34 changes: 33 additions & 1 deletion src/lib/viewers/media/__tests__/Settings-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ describe('lib/viewers/media/Settings', () => {
describe('init()', () => {
it('should set the initial quality and speed', () => {
sandbox.stub(settings, 'chooseOption');
const quality = 'HD';
const quality = 'sd';
const speed = '2.0';
const autoplay = 'Enabled'

Expand Down Expand Up @@ -573,12 +573,19 @@ describe('lib/viewers/media/Settings', () => {
});

describe('showSubMenu()', () => {
it('should do nothing if the sub menu is disabled', () => {
sandbox.stub(settings, 'setMenuContainerDimensions');
settings.showSubMenu('quality');
expect(settings.setMenuContainerDimensions).to.not.be.called;
});

it('should show the speed submenu if speed is selected', () => {
settings.showSubMenu('speed');
expect(settings.settingsEl).to.have.class('bp-media-settings-show-speed');
});

it('should show the quality submenu if quality is selected', () => {
settings.enableHD();
settings.showSubMenu('quality');
expect(settings.settingsEl).to.have.class('bp-media-settings-show-quality');
});
Expand Down Expand Up @@ -693,6 +700,16 @@ describe('lib/viewers/media/Settings', () => {

expect(settings.handleSubtitleSelection).to.not.be.called;
});

it('should not not set the cache if setCache is set to false', () => {
sandbox.stub(settings, 'handleSubtitleSelection');
sandbox.stub(settings.cache, 'set');


settings.chooseOption('speed', 0.5, false);

expect(settings.cache.set).to.not.be.called;
});
});

describe('handleSubtitleSelection()', () => {
Expand Down Expand Up @@ -1053,4 +1070,19 @@ describe('lib/viewers/media/Settings', () => {
expect(document.removeEventListener).to.be.calledWith('keydown', settings.blurHandler);
});
});

describe('enableHD()', () => {
it('should remove the unavailable class, enable the sub menu, and choose the cached quality option', () => {
sandbox.stub(settings.cache, 'get').returns('hd');
sandbox.stub(settings, 'chooseOption');
const CLASS_SETTINGS_QUALITY_MENU = 'bp-media-settings-menu-quality';
const qualitySubMenu = settings.containerEl.querySelector(`.${CLASS_SETTINGS_QUALITY_MENU}`)

settings.enableHD();

expect(settings.containerEl.classList.contains('bp-media-settings-hd-unavailable')).to.be.false;
expect(settings.chooseOption).to.be.calledWith('quality', 'hd');
expect(qualitySubMenu.getAttribute('data-disabled')).to.equal('');
});
});
});

0 comments on commit e4302e3

Please sign in to comment.