From ed7fd252cd05ef3b8b354b82343f7261531d3e9a Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Tue, 21 Jul 2020 15:58:33 -0700 Subject: [PATCH] Refactor: Replace PHP complexity with JS simplicity In PHP we add collapsible classes to all elements except watchstar so that certain tabs can be collapsed under the more menu in JS. This adds unnecessary complexity to our codebase and is not used if JS is disabled. To simplify this and bring Vector's PHP consistency with core this logic is moved to JavaScript. Bug: T259372 Change-Id: I2acbf7089198118626368ee8a37615d2de062f83 --- bundlesize.config.json | 4 +- includes/SkinVector.php | 20 ++------- .../skins.vector.legacy.js/collapsibleTabs.js | 10 +++++ tests/phpunit/integration/SkinVectorTest.php | 42 ------------------- 4 files changed, 15 insertions(+), 61 deletions(-) diff --git a/bundlesize.config.json b/bundlesize.config.json index bef7f6d..fcc3f71 100644 --- a/bundlesize.config.json +++ b/bundlesize.config.json @@ -1,7 +1,7 @@ [ { "resourceModule": "skins.vector.styles.legacy", - "maxSize": "7.8 kB" + "maxSize": "7.9 kB" }, { "resourceModule": "skins.vector.styles", @@ -17,6 +17,6 @@ }, { "resourceModule": "skins.vector.legacy.js", - "maxSize": "1.7 kB" + "maxSize": "1.8 kB" } ] diff --git a/includes/SkinVector.php b/includes/SkinVector.php index dac629c..d6a738f 100644 --- a/includes/SkinVector.php +++ b/includes/SkinVector.php @@ -355,7 +355,6 @@ class SkinVector extends SkinMustache { * @param array $urls to convert to list items stored as string in html-items key * @param int $type of menu (optional) - a plain list (MENU_TYPE_DEFAULT), * a tab (MENU_TYPE_TABS) or a dropdown (MENU_TYPE_DROPDOWN) - * @param array $options (optional) to be passed to makeListItem * @param bool $setLabelToSelected (optional) the menu label will take the value of the * selected item if found. * @return array @@ -364,7 +363,6 @@ class SkinVector extends SkinMustache { string $label, array $urls = [], int $type = self::MENU_TYPE_DEFAULT, - array $options = [], bool $setLabelToSelected = false ) : array { $skin = $this->getSkin(); @@ -397,17 +395,7 @@ class SkinVector extends SkinMustache { ]; foreach ( $urls as $key => $item ) { - // Add CSS class 'collapsible' to all links EXCEPT watchstar. - if ( - $key !== 'watch' && $key !== 'unwatch' && - isset( $options['vector-collapsible'] ) && $options['vector-collapsible'] ) { - if ( !isset( $item['class'] ) ) { - $item['class'] = ''; - } - $item['class'] = rtrim( 'collapsible ' . $item['class'], ' ' ); - } - $props['html-items'] .= $this->getSkin()->makeListItem( $key, $item, $options ); - + $props['html-items'] .= $this->getSkin()->makeListItem( $key, $item ); // Check the class of the item for a `selected` class and if so, propagate the items // label to the main label. if ( $setLabelToSelected ) { @@ -491,14 +479,12 @@ class SkinVector extends SkinMustache { 'variants', $contentNavigation[ 'variants' ] ?? [], self::MENU_TYPE_DROPDOWN, - [], true + true ), 'data-page-actions' => $this->getMenuData( 'views', $contentNavigation[ 'views' ] ?? [], - self::MENU_TYPE_TABS, [ - 'vector-collapsible' => true, - ] + self::MENU_TYPE_TABS ), 'data-page-actions-more' => $this->getMenuData( 'cactions', diff --git a/resources/skins.vector.legacy.js/collapsibleTabs.js b/resources/skins.vector.legacy.js/collapsibleTabs.js index 323ad27..c52151e 100644 --- a/resources/skins.vector.legacy.js/collapsibleTabs.js +++ b/resources/skins.vector.legacy.js/collapsibleTabs.js @@ -1,9 +1,19 @@ +/** + * This adds behaviour to Vector's tabs in the bottom right so that at smaller + * displays they collapse under the more menu. + */ + /** @interface CollapsibleTabsOptions */ function init() { /** @type {boolean|undefined} */ var boundEvent; var isRTL = document.documentElement.dir === 'rtl'; var rAF = window.requestAnimationFrame || setTimeout; + // Mark the tabs which can be collapsed under the more menu + // eslint-disable-next-line no-jquery/no-global-selector + $( '#p-views li' ) + .not( '#ca-watch, #ca-unwatch' ).addClass( 'collapsible' ); + $.fn.collapsibleTabs = function ( options ) { // Merge options into the defaults var settings = $.extend( {}, $.collapsibleTabs.defaults, options ); diff --git a/tests/phpunit/integration/SkinVectorTest.php b/tests/phpunit/integration/SkinVectorTest.php index 6080e58..1c498d0 100644 --- a/tests/phpunit/integration/SkinVectorTest.php +++ b/tests/phpunit/integration/SkinVectorTest.php @@ -44,48 +44,6 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase { return in_array( $search, $values ); } - /** - * @covers ::getMenuData - */ - public function testMakeListItemRespectsCollapsibleOption() { - $vectorTemplate = $this->provideVectorTemplateObject(); - $template = TestingAccessWrapper::newFromObject( $vectorTemplate ); - $listItemClass = 'my_test_class'; - $options = [ 'vector-collapsible' => true ]; - $item = [ 'class' => $listItemClass ]; - $propsCollapsible = $template->getMenuData( - 'foo', - [ - 'bar' => $item, - ], - 0, - $options - ); - $propsNonCollapsible = $template->getMenuData( - 'foo', - [ - 'bar' => $item, - ], - 0, - [] - ); - $nonCollapsible = $propsNonCollapsible['html-items']; - $collapsible = $propsCollapsible['html-items']; - - $this->assertTrue( - $this->expectNodeAttribute( $collapsible, 'li', 'class', 'collapsible' ), - 'The collapsible element has to have `collapsible` class' - ); - $this->assertFalse( - $this->expectNodeAttribute( $nonCollapsible, 'li', 'class', 'collapsible' ), - 'The non-collapsible element should not have `collapsible` class' - ); - $this->assertTrue( - $this->expectNodeAttribute( $nonCollapsible, 'li', 'class', $listItemClass ), - 'The non-collapsible element should preserve item class' - ); - } - /** * @covers ::getMenuProps */