Merge "Refactor: Replace PHP complexity with JS simplicity"

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"
 	}
 ]

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',

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

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
 	 */