Fix settings menu title cut off by scrollbar (#599)

COXP-4461
box · Jan 24, 2018 · cdb627e · cdb627e
1 parent a2397ce
commit cdb627e
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 6 deletions.
diff --git a/src/lib/viewers/media/Settings.js b/src/lib/viewers/media/Settings.js
@@ -22,6 +22,9 @@ 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_MENU_PADDING = 18;
+const SETTINGS_MENU_PADDING_SCROLL = 32;
+const SETTINGS_MENU_MAX_HEIGHT = 210;
 
 const SETTINGS_TEMPLATE = `<div class="bp-media-settings">
     <div class="bp-media-settings-menu-main bp-media-settings-menu" role="menu">
@@ -292,12 +295,13 @@ class Settings extends EventEmitter {
      * @return {void}
      */
     setMenuContainerDimensions(menu) {
-        // NOTE: need to explicitly set the dimensions in order to get css transitions. width=auto doesn't work with css transitions
-        this.settingsEl.style.width = `${menu.offsetWidth + 18}px`;
-        // height = n * $item-height + 2 * $padding (see Settings.scss) + 2 * border (see Settings.scss)
-        // where n is the number of displayed items in the menu
-        const sumHeight = [].reduce.call(menu.children, (sum, child) => sum + child.offsetHeight, 0);
-        this.settingsEl.style.height = `${sumHeight + 18}px`;
+        const paddedHeight = menu.offsetHeight + SETTINGS_MENU_PADDING;
+        this.settingsEl.style.height = `${paddedHeight}px`;
+
+        // If the menu grows tall enough to require scrolling, take into account scroll bar width.
+        const scrollPadding =
+            paddedHeight >= SETTINGS_MENU_MAX_HEIGHT ? SETTINGS_MENU_PADDING_SCROLL : SETTINGS_MENU_PADDING;
+        this.settingsEl.style.width = `${menu.offsetWidth + scrollPadding}px`;
     }
 
     /**

diff --git a/src/lib/viewers/media/__tests__/Settings-test.js b/src/lib/viewers/media/__tests__/Settings-test.js
@@ -101,6 +101,37 @@ describe('lib/viewers/media/Settings', () => {
         });
     });
 
+    describe('setMenuContainerDimensions', () => {
+        it('should add padding to settingsEl based on menu contents and additional padding', () => {
+            const menuEl = document.createElement('div');
+            menuEl.appendChild(document.createElement('span'));
+            settings.setMenuContainerDimensions(menuEl);
+
+            expect(settings.settingsEl.style.width).to.equal('18px');
+        });
+
+        it('should add extra padding to settingsEl based on menu contents that require scroll bar', () => {
+            const menuEl = {
+                offsetWidth: 0,
+                offsetHeight: 500 // 210 is max height of settings menu
+            };
+            settings.setMenuContainerDimensions(menuEl);
+
+            expect(settings.settingsEl.style.width).to.equal('32px');
+        });
+
+        it('should grow the height of the settingsEl to that of the sub-menu, with padding', () => {
+            const menuEl = {
+                offsetWidth: 0,
+                offsetHeight: 18
+            };
+            settings.setMenuContainerDimensions(menuEl);
+
+            // Adds 18px to the offsetHeight of sum of child element's heights
+            expect(settings.settingsEl.style.height).to.equal('36px');
+        });
+    });
+
     describe('destroy()', () => {
         it('should remove event listeners on settings element and document', () => {
             sandbox.stub(settings.settingsEl, 'removeEventListener');