diff --git a/includes/Hooks.php b/includes/Hooks.php index a1233be..af1605d 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -33,6 +33,50 @@ class Hooks { } } + /** + * Add icon class to an existing navigation item inside a menu hook. + * See self::onSkinTemplateNavigation. + * @param array $item + * @return array + */ + private static function navigationLinkToIcon( array $item ) { + if ( !isset( $item['class'] ) ) { + $item['class'] = ''; + } + $item['class'] = rtrim( 'icon ' . $item['class'], ' ' ); + return $item; + } + + /** + * Upgrades Vector's watch action to a watchstar. + * + * @see https://www.mediawiki.org/wiki/Manual:Hooks/SkinTemplateNavigation + * @param SkinTemplate $sk + * @param array &$content_navigation + */ + public static function onSkinTemplateNavigation( $sk, &$content_navigation ) { + if ( + $sk->getSkinName() === 'vector' && + $sk->getConfig()->get( 'VectorUseIconWatch' ) + ) { + $key = null; + if ( isset( $content_navigation['actions']['watch'] ) ) { + $key = 'watch'; + } + if ( isset( $content_navigation['actions']['unwatch'] ) ) { + $key = 'unwatch'; + } + + // Promote watch link from actions to views and add an icon + if ( $key !== null ) { + $content_navigation['views'][$key] = self::navigationLinkToIcon( + $content_navigation['actions'][$key] + ); + unset( $content_navigation['actions'][$key] ); + } + } + } + /** * Add Vector preferences to the user's Special:Preferences page directly underneath skins. * diff --git a/includes/VectorTemplate.php b/includes/VectorTemplate.php index 0504823..b318766 100644 --- a/includes/VectorTemplate.php +++ b/includes/VectorTemplate.php @@ -22,8 +22,6 @@ * @ingroup Skins */ -use MediaWiki\MediaWikiServices; - /** * QuickTemplate subclass for Vector * @ingroup Skins @@ -122,30 +120,6 @@ class VectorTemplate extends BaseTemplate { $out = $skin->getOutput(); $title = $out->getTitle(); - // Move the watch/unwatch star outside of the collapsed "actions" menu to the main "views" menu - if ( $this->getConfig()->get( 'VectorUseIconWatch' ) ) { - $mode = ( $skin->getRelevantTitle()->isWatchable() && - MediaWikiServices::getInstance()->getPermissionManager()->userHasRight( - $skin->getUser(), - 'viewmywatchlist' - ) && - MediaWikiServices::getInstance()->getWatchedItemStore()->isWatched( - $skin->getUser(), - $skin->getRelevantTitle() - ) - ) ? 'unwatch' : 'watch'; - - $actionUrls = $contentNavigation[ 'actions' ] ?? []; - if ( array_key_exists( $mode, $actionUrls ) ) { - $viewUrls = $contentNavigation[ 'views' ] ?? []; - $viewUrls[ $mode ] = $actionUrls[ $mode ]; - unset( $actionUrls[ $mode ] ); - $contentNavigation['actions'] = $actionUrls; - $contentNavigation['views'] = $viewUrls; - $this->set( 'content_navigation', $contentNavigation ); - } - } - ob_start(); Hooks::run( 'VectorBeforeFooter', [], '1.35' ); $htmlHookVectorBeforeFooter = ob_get_contents(); @@ -401,35 +375,6 @@ class VectorTemplate extends BaseTemplate { ]; } - /** - * @inheritDoc - */ - public function makeListItem( $key, $item, $options = [] ) { - // For fancy styling of watch/unwatch star - if ( - $this->getConfig()->get( 'VectorUseIconWatch' ) - && ( $key === 'watch' || $key === 'unwatch' ) - ) { - if ( !isset( $item['class'] ) ) { - $item['class'] = ''; - } - $item['class'] = rtrim( 'icon ' . $item['class'], ' ' ); - $item['primary'] = true; - } - - // Add CSS class 'collapsible' to links which are not marked as "primary" - if ( - isset( $options['vector-collapsible'] ) && $options['vector-collapsible'] ) { - if ( !isset( $item['class'] ) ) { - $item['class'] = ''; - } - /* @phan-suppress-next-line PhanTypePossiblyInvalidDimOffset */ - $item['class'] = rtrim( 'collapsible ' . $item['class'], ' ' ); - } - - return parent::makeListItem( $key, $item, $options ); - } - /** * @param string $label to be used to derive the id and human readable label of the menu * If the key has an entry in the constant MENU_LABEL_KEYS then that message will be used for the @@ -472,7 +417,16 @@ class VectorTemplate extends BaseTemplate { ]; foreach ( $urls as $key => $item ) { - $props['html-items'] .= $this->makeListItem( $key, $item, $options ); + // 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 ); // Check the class of the item for a `selected` class and if so, propagate the items // label to the main label. @@ -518,7 +472,7 @@ class VectorTemplate extends BaseTemplate { // This code doesn't belong here, it belongs in the UniversalLanguageSelector // It is here to workaround the fact that it wants to be the first item in the personal menus. if ( array_key_exists( 'uls', $personalTools ) ) { - $uls = $this->makeListItem( 'uls', $personalTools[ 'uls' ] ); + $uls = $skin->makeListItem( 'uls', $personalTools[ 'uls' ] ); unset( $personalTools[ 'uls' ] ); } else { $uls = ''; diff --git a/skin.json b/skin.json index e0b0869..6823047 100644 --- a/skin.json +++ b/skin.json @@ -35,6 +35,7 @@ "BeforePageDisplayMobile": "Vector\\Hooks::onBeforePageDisplayMobile", "GetPreferences": "Vector\\Hooks::onGetPreferences", "PreferencesFormPreSave": "Vector\\Hooks::onPreferencesFormPreSave", + "SkinTemplateNavigation": "Vector\\Hooks::onSkinTemplateNavigation", "LocalUserCreated": "Vector\\Hooks::onLocalUserCreated" }, "@note": "When modifying skins.vector.styles definition, make sure the installer still works", diff --git a/tests/phpunit/integration/VectorHooksTest.php b/tests/phpunit/integration/VectorHooksTest.php index ee7ebfb..6d85da1 100644 --- a/tests/phpunit/integration/VectorHooksTest.php +++ b/tests/phpunit/integration/VectorHooksTest.php @@ -271,4 +271,41 @@ class VectorHooksTest extends \MediaWikiTestCase { $isAutoCreated = false; Hooks::onLocalUserCreated( $user, $isAutoCreated ); } + + /** + * @covers ::onSkinTemplateNavigation + */ + public function testOnSkinTemplateNavigation() { + $this->setMwGlobals( [ + 'wgVectorUseIconWatch' => true + ] ); + $skin = new SkinVector(); + $contentNavWatch = [ + 'actions' => [ + 'watch' => [ 'class' => 'watch' ], + ] + ]; + $contentNavUnWatch = [ + 'actions' => [ + 'move' => [ 'class' => 'move' ], + 'unwatch' => [], + ], + ]; + + Hooks::onSkinTemplateNavigation( $skin, $contentNavUnWatch ); + Hooks::onSkinTemplateNavigation( $skin, $contentNavWatch ); + + $this->assertTrue( + strpos( $contentNavWatch['views']['watch']['class'], 'icon' ) !== false, + 'Watch list items require an "icon" class' + ); + $this->assertTrue( + strpos( $contentNavUnWatch['views']['unwatch']['class'], 'icon' ) !== false, + 'Unwatch list items require an "icon" class' + ); + $this->assertFalse( + strpos( $contentNavUnWatch['actions']['move']['class'], 'icon' ) !== false, + 'List item other than watch or unwatch should not have an "icon" class' + ); + } } diff --git a/tests/phpunit/integration/VectorTemplateTest.php b/tests/phpunit/integration/VectorTemplateTest.php index 4d5e11f..20e2819 100644 --- a/tests/phpunit/integration/VectorTemplateTest.php +++ b/tests/phpunit/integration/VectorTemplateTest.php @@ -50,15 +50,32 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase { } /** - * @covers ::makeListItem + * @covers ::getMenuData */ public function testMakeListItemRespectsCollapsibleOption() { - $template = $this->provideVectorTemplateObject(); + $vectorTemplate = $this->provideVectorTemplateObject(); + $template = TestingAccessWrapper::newFromObject( $vectorTemplate ); $listItemClass = 'my_test_class'; $options = [ 'vector-collapsible' => true ]; $item = [ 'class' => $listItemClass ]; - $nonCollapsible = $template->makeListItem( 'key', $item, [] ); - $collapsible = $template->makeListItem( 'key', [], $options ); + $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' ), @@ -74,63 +91,6 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase { ); } - /** - * @covers ::makeListItem - */ - public function testWatcAndUnwatchHasIconClass() { - $template = $this->provideVectorTemplateObject(); - $this->setMwGlobals( [ - 'wgVectorUseIconWatch' => true - ] ); - $listItemClass = 'my_test_class'; - $options = []; - $item = [ 'class' => $listItemClass ]; - - $watchListItem = $template->makeListItem( 'watch', $item, [] ); - $unwatchListItem = $template->makeListItem( 'unwatch', [], $options ); - $regularListItem = $template->makeListItem( 'whatever', $item, $options ); - - $this->assertTrue( - $this->expectNodeAttribute( $watchListItem, 'li', 'class', 'icon' ), - 'Watch list items require an "icon" class' - ); - $this->assertTrue( - $this->expectNodeAttribute( $unwatchListItem, 'li', 'class', 'icon' ), - 'Unwatch list items require an "icon" class' - ); - $this->assertFalse( - $this->expectNodeAttribute( $regularListItem, 'li', 'class', 'icon' ), - 'List item other than watch or unwatch should not have an "icon" class' - ); - $this->assertTrue( - $this->expectNodeAttribute( $watchListItem, 'li', 'class', $listItemClass ), - 'Watch list items require an item class' - ); - } - - /** - * @covers ::makeListItem - */ - public function testWatchAndUnwatchHasIconClassOnlyIfVectorUseIconWatchIsSet() { - $template = $this->provideVectorTemplateObject(); - $this->setMwGlobals( [ - 'wgVectorUseIconWatch' => false - ] ); - $listItemClass = 'my_test_class'; - $item = [ 'class' => $listItemClass ]; - - $watchListItem = $template->makeListItem( 'watch', $item, [] ); - - $this->assertFalse( - $this->expectNodeAttribute( $watchListItem, 'li', 'class', 'icon' ), - 'Watch list should not have an "icon" class when VectorUserIconWatch is disabled' - ); - $this->assertTrue( - $this->expectNodeAttribute( $watchListItem, 'li', 'class', $listItemClass ), - 'Watch list items require an item class' - ); - } - /** * @covers ::getMenuProps */