golem
/
VectorGOLEM
9
0
Fork 0
Code Issues 1 Pull Requests Releases Wiki Activity

BaseTemplate:makeListItem is deprecated 

Use SkinTemplateNavigation hook instead and copy the collapsible
behavior to the menu function

The code inside getSkinData that checks VectorUseIconWatch is
redundant as it duplicates checks already inside
SkinTemplate::buildContentNavigationUrls
It is enough to simplify check whether watch or unwatch is
present in the array.

Bug: T251212
Change-Id: If6b10b0ddcbd4b21dd13a2813e60b604c3a23415
This commit is contained in:
jdlrobson 2020-05-11 15:09:21 -07:00
parent 8eb631b05f
commit 5b31c49e15
5 changed files with 114 additions and 118 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

44
includes/Hooks.php
View File

@ -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.
*

68
includes/VectorTemplate.php
View File

@ -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 = '';

1
skin.json
View File

@ -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",

37
tests/phpunit/integration/VectorHooksTest.php
View File

@ -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'
);
}
}

82
tests/phpunit/integration/VectorTemplateTest.php
View File

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