Fix: Media player control menus show scrollbars during transition (#494)

Add class to hide overflow during menu transitions and hide after transitions.

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();

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;
 }

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');