From 93f930ce3e6d6166df4f29ea3dad5639553604d4 Mon Sep 17 00:00:00 2001 From: Stephen Niedzielski Date: Wed, 31 Jul 2019 11:36:19 -0600 Subject: [PATCH] [UI] [menu] remove menu height resizing Remove the page action overflow and user menu height sizing. Previously, a maximum menu height was set so that the menu itself would scroll. A minimum height was also used so that the menu couldn't be shrunk down to a silly size. Both the minimum height LESS and maximum height JS are now removed. Bug: T225959 Change-Id: I201374ab8b249272ee5dbb1401b844ffe034ea66 --- components/ToggleList/ToggleList.js | 39 ++----------------- .../skins.minerva.amc.styles/userMenu.less | 2 - .../pageactions.less | 3 -- resources/skins.minerva.scripts/Toolbar.js | 12 ++---- resources/skins.minerva.scripts/init.js | 5 +-- 5 files changed, 8 insertions(+), 53 deletions(-) diff --git a/components/ToggleList/ToggleList.js b/components/ToggleList/ToggleList.js index af763ed..c2eaad8 100644 --- a/components/ToggleList/ToggleList.js +++ b/components/ToggleList/ToggleList.js @@ -5,25 +5,21 @@ /** The visible label icon associated with the checkbox. */ toggleSelector = '.toggle-list__toggle', /** The underlying hidden checkbox that controls list visibility. */ - checkboxSelector = '.toggle-list__checkbox', - listSelector = '.toggle-list__list'; + checkboxSelector = '.toggle-list__checkbox'; /** * Automatically dismiss the list when clicking or focusing elsewhere and update the * aria-expanded attribute based on list visibility. * @param {Window} window * @param {HTMLElement} component - * @param {OO.EventEmitter} eventBus - * @param {boolean} [resize] If true, resize the menu on scroll and window resize. * @return {void} */ - function bind( window, component, eventBus, resize ) { + function bind( window, component ) { var toggle = component.querySelector( toggleSelector ), checkbox = /** @type {HTMLInputElement} */ ( component.querySelector( checkboxSelector ) - ), - list = component.querySelector( listSelector ); + ); window.addEventListener( 'click', function ( event ) { if ( event.target !== toggle && event.target !== checkbox ) { @@ -42,23 +38,6 @@ }, true ); checkbox.addEventListener( 'change', _updateAriaExpanded.bind( undefined, checkbox ) ); - - if ( resize ) { - eventBus.on( 'scroll:throttled', _resize.bind( undefined, list ) ); - eventBus.on( 'resize:throttled', _resize.bind( undefined, list ) ); - } - } - - /** - * @param {HTMLElement} component - * @param {boolean} [resize] If true, resize the menu to fit within the window. - * @return {void} - */ - function render( component, resize ) { - var list = /** @type {HTMLElement} */ ( component.querySelector( listSelector ) ); - if ( resize ) { - _resize( list ); - } } /** @@ -71,17 +50,6 @@ _updateAriaExpanded( checkbox ); } - /** - * @param {HTMLElement} list - * @return {void} - */ - function _resize( list ) { - var rect = list.getClientRects()[ 0 ]; - if ( rect ) { - list.style.maxHeight = window.document.documentElement.clientHeight - rect.top + 'px'; - } - } - /** * Revise the aria-expanded state to match the checked state. * @param {HTMLInputElement} checkbox @@ -93,7 +61,6 @@ module.exports = Object.freeze( { selector: selector, - render: render, bind: bind } ); }() ); diff --git a/resources/skins.minerva.amc.styles/userMenu.less b/resources/skins.minerva.amc.styles/userMenu.less index 1d9333b..c950a4b 100644 --- a/resources/skins.minerva.amc.styles/userMenu.less +++ b/resources/skins.minerva.amc.styles/userMenu.less @@ -6,6 +6,4 @@ right: 0; margin-right: @contentMargin / 2; min-width: 200px; - // A variable max-height is set in JavaScript so a minimum height is needed. - min-height: 200px; } diff --git a/resources/skins.minerva.base.styles/pageactions.less b/resources/skins.minerva.base.styles/pageactions.less index 671b0d6..a1880ed 100644 --- a/resources/skins.minerva.base.styles/pageactions.less +++ b/resources/skins.minerva.base.styles/pageactions.less @@ -93,9 +93,6 @@ // The top of the menu is flush with the bottom of the page actions toolbar. top: 100%; right: 0; - // - // A variable max-height is set in JavaScript so a minimum height is needed. - min-height: 200px; } // overriding common.less `display:inherit` (which causes `display: flex;` in this instance). diff --git a/resources/skins.minerva.scripts/Toolbar.js b/resources/skins.minerva.scripts/Toolbar.js index 8cd0c1b..429bdb5 100644 --- a/resources/skins.minerva.scripts/Toolbar.js +++ b/resources/skins.minerva.scripts/Toolbar.js @@ -14,13 +14,12 @@ /** * @param {Window} window * @param {Element} toolbar - * @param {OO.EventEmitter} eventBus * @return {void} */ - function bind( window, toolbar, eventBus ) { + function bind( window, toolbar ) { var overflowSubmenu = toolbar.querySelector( overflowSubmenuSelector ); if ( overflowSubmenu ) { - ToggleList.bind( window, overflowSubmenu, eventBus, true ); + ToggleList.bind( window, overflowSubmenu ); } } @@ -30,14 +29,9 @@ * @return {void} */ function render( window, toolbar ) { - var - overflowSubmenu = toolbar.querySelector( overflowSubmenuSelector ), - overflowList = toolbar.querySelector( overflowListSelector ); + var overflowList = toolbar.querySelector( overflowListSelector ); renderEditButton(); renderDownloadButton( window, overflowList ); - if ( overflowSubmenu ) { - ToggleList.render( overflowSubmenu, true ); - } } /** diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index 71055b5..632b3ce 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -326,12 +326,11 @@ // eslint-disable-next-line no-jquery/no-global-selector initHistoryLink( $( '.last-modifier-tagline a' ) ); if ( toolbarElement ) { - Toolbar.bind( window, toolbarElement, eventBus ); + Toolbar.bind( window, toolbarElement ); Toolbar.render( window, toolbarElement ); } if ( userMenu ) { - ToggleList.bind( window, userMenu, eventBus, true ); - ToggleList.render( userMenu, true ); + ToggleList.bind( window, userMenu ); } initRedlinksCta(); initUserRedLinks();