diff --git a/src/lib/viewers/media/Settings.js b/src/lib/viewers/media/Settings.js index a4d479774..e1645e751 100644 --- a/src/lib/viewers/media/Settings.js +++ b/src/lib/viewers/media/Settings.js @@ -183,6 +183,11 @@ class Settings extends EventEmitter { this.containerEl.classList.add(CLASS_SETTINGS_SUBTITLES_UNAVAILABLE); this.containerEl.classList.add(CLASS_SETTINGS_AUDIOTRACKS_UNAVAILABLE); + // Allow scrollbars after animations end + this.settingsEl.addEventListener('transitionend', () => { + this.settingsEl.classList.remove('bp-media-settings-in-transition'); + }); + if (Browser.isMobile()) { this.containerEl.classList.add(CLASS_SETTINGS_AUTOPLAY_UNAVAILABLE); } @@ -214,6 +219,7 @@ class Settings extends EventEmitter { destroy() { if (this.settingsEl) { removeActivationListener(this.settingsEl, this.menuEventHandler); + this.settingsEl.removeEventListener('transitionend'); } document.removeEventListener('click', this.blurHandler); } @@ -311,6 +317,7 @@ class Settings extends EventEmitter { showSubMenu(type) { const subMenu = this.settingsEl.querySelector(`.bp-media-settings-menu-${type}`); this.settingsEl.classList.add(`bp-media-settings-show-${type}`); + this.setMenuContainerDimensions(subMenu); // Move focus to the currently selected value const curSelectedOption = this.settingsEl.querySelector( @@ -330,6 +337,9 @@ class Settings extends EventEmitter { const type = menuItem.getAttribute('data-type'); const value = menuItem.getAttribute('data-value'); + // Hide scrollbars while menu animation is happening - this is removed when transition ends + this.settingsEl.classList.add('bp-media-settings-in-transition'); + if (type === 'menu') { // We are in the sub menu and going back to the main menu this.reset(); diff --git a/src/lib/viewers/media/Settings.scss b/src/lib/viewers/media/Settings.scss index 9ef2d8ae4..88005d862 100644 --- a/src/lib/viewers/media/Settings.scss +++ b/src/lib/viewers/media/Settings.scss @@ -32,6 +32,10 @@ $item-hover-color: #f6fafd; } } +.bp-media-settings-in-transition { + overflow: hidden; // hide scrollbars during menu -> submenu transition +} + .bp-media-settings-menu { display: table; } diff --git a/src/lib/viewers/media/__tests__/Settings-test.js b/src/lib/viewers/media/__tests__/Settings-test.js index 3f6cf1a75..190cdcbac 100644 --- a/src/lib/viewers/media/__tests__/Settings-test.js +++ b/src/lib/viewers/media/__tests__/Settings-test.js @@ -186,6 +186,7 @@ describe('lib/viewers/media/Settings', () => { sandbox.stub(settings, 'findParentDataType').returns(document.querySelector('[data-type="menu"]')); settings.menuEventHandler({ type: 'click' }); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.be.called; expect(settings.chooseOption).to.not.be.called; @@ -203,6 +204,7 @@ describe('lib/viewers/media/Settings', () => { }; settings.menuEventHandler(event); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.be.called; expect(settings.chooseOption).to.not.be.called; @@ -223,6 +225,7 @@ describe('lib/viewers/media/Settings', () => { }; settings.menuEventHandler(event); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.be.called; expect(settings.chooseOption).to.not.be.called; @@ -238,6 +241,7 @@ describe('lib/viewers/media/Settings', () => { .stub(settings, 'findParentDataType') .returns(document.querySelector('[data-type="speed"][data-value="2.0"]')); settings.menuEventHandler({ type: 'click' }); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.not.be.called; expect(settings.chooseOption).to.be.calledWith('speed', '2.0'); @@ -256,6 +260,7 @@ describe('lib/viewers/media/Settings', () => { stopPropagation: sandbox.stub() }; settings.menuEventHandler(event); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.not.be.called; expect(settings.chooseOption).to.be.calledWith('speed', '2.0'); @@ -277,6 +282,7 @@ describe('lib/viewers/media/Settings', () => { stopPropagation: sandbox.stub() }; settings.menuEventHandler(event); + settings.settingsEl.classList.remove('bp-media-settings-in-transition'); // simulate transition end expect(settings.reset).to.not.be.called; expect(settings.chooseOption).to.be.calledWith('speed', '2.0'); @@ -592,6 +598,50 @@ describe('lib/viewers/media/Settings', () => { }); }); + describe('menuItemSelect()', () => { + let menuItem; + + beforeEach(() => { + menuItem = document.createElement('div'); + menuItem.setAttribute('data-type', 'someOption'); + menuItem.setAttribute('data-value', 'someValue'); + + sandbox.stub(settings, 'reset'); + settings.firstMenuItem = { + focus: sandbox.stub() + }; + sandbox.stub(settings, 'chooseOption'); + sandbox.stub(settings, 'showSubMenu'); + }); + + it('should add menu settings in transition class', () => { + settings.menuItemSelect(menuItem); + expect(settings.settingsEl).to.have.class('bp-media-settings-in-transition'); + }); + + it('should reset and focus first menu item if selected item is the option to return to menu', () => { + menuItem.setAttribute('data-type', 'menu'); + + settings.menuItemSelect(menuItem); + expect(settings.reset).to.be.called; + expect(settings.firstMenuItem.focus).to.be.called; + }); + + it('should choose option if an menu option with a type and value are selected', () => { + settings.menuItemSelect(menuItem); + expect(settings.chooseOption).to.be.calledWith('someOption', 'someValue'); + }); + + it('should show sub menu if selected option is a sub menu option', () => { + const optionType = 'submenu'; + menuItem.setAttribute('data-type', optionType); + menuItem.setAttribute('data-value', ''); + + settings.menuItemSelect(menuItem); + expect(settings.showSubMenu).to.be.calledWith(optionType); + }); + }); + describe('chooseOption()', () => { it('should reset the menu and focus, cache option, and emit chosen option', () => { sandbox.stub(settings, 'reset');