Untangle watchstar icon

The SkinMinerva::createWatchPageAction() was depending upon
$tpl['content_navigation']['actions'] object, which is created by
SkinTemplate::buildContentNavigationUrls(). Although only 'watch'
and 'unwatch' keys were used, moreover, code was using only 'href'
property from both 'watch' and 'unwatch'. All other props are
overrided, not used when rendering. Also the title of unwatch
icon was set to "watch this page" which was incorrect.

This system was bit complex to manage (we should provide dependencies
not pass the whole 'content_navigation']['actions'] array). It would
also make further refactoring more difficult, as watchstar icon
shouldn't depened on quicktemplate stuff. We were using only href
attribute which is super easy to calculate.

Changes:
 - do not depend upon $actions, get href by using $title->getLocalUrl
 - set proper title for unwatch icon
 - simplified SkinMinerva::createWatchPageAction() logic

Bug: T221792
Change-Id: I9609949b60a7e2f2f0a947c05c80c89be32d4cb1
This commit is contained in:
Piotr Miazga 2019-05-23 21:10:51 +02:00
parent 4a54748cfe
commit 82a2edf585

View file

@ -845,11 +845,7 @@ class SkinMinerva extends SkinTemplate {
}
if ( $this->isAllowedPageAction( 'watch' ) ) {
// SkinTemplate#buildContentNavigationUrls creates distinct "watch" and "unwatch" actions.
// Pass these actions in as context for #createWatchPageAction.
$actions = $tpl->data['content_navigation']['actions'];
$toolbar[] = $this->createWatchPageAction( $actions );
$toolbar[] = $this->createWatchPageAction();
}
if ( $this->isAllowedPageAction( 'history' ) ) {
@ -907,42 +903,37 @@ class SkinMinerva extends SkinTemplate {
* add the page to or remove the page from the user's watchlist; or, if the user is logged out,
* will direct the user's UA to Special:Login.
*
* @param array $actions
* @return array A map of HTML attributes and a "text" property to be used with the
* pageActionMenu.mustache template.
*/
protected function createWatchPageAction( $actions ) {
protected function createWatchPageAction() {
$title = $this->getTitle();
$user = $this->getUser();
$ctaUrl = $this->getLoginUrl( [ 'returnto' => $title ] );
if ( $title && $user->isWatched( $title ) ) {
$isWatched = $title && $user->isLoggedIn() && $user->isWatched( $title );
$actionOnClick = $isWatched ? 'unwatch' : 'watch';
$href = $user->isAnon()
? $this->getLoginUrl( [ 'returnto' => $title ] )
: $title->getLocalURL( [ 'action' => $actionOnClick ] );
$additionalClassNames = ' jsonly';
if ( $isWatched ) {
$msg = $this->msg( 'unwatchthispage' );
$icon = 'watched';
$additionalClassNames .= ' watched';
} else {
$msg = $this->msg( 'watchthispage' );
$icon = 'watch';
}
$baseResult = [
return [
'item-id' => 'page-actions-watch',
'id' => 'ca-watch',
// Use blank icon to reserve space for watchstar icon once JS loads
'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' ) . ' jsonly',
'title' => $this->msg( 'watchthispage' ),
'text' => $this->msg( 'watchthispage' )
'class' => MinervaUI::iconClass( $icon, 'element', 'watch-this-article' )
. $additionalClassNames,
'title' => $msg,
'text' => $msg,
'href' => $href
];
if ( isset( $actions['watch'] ) ) {
$result = array_merge( $actions['watch'], $baseResult );
} elseif ( isset( $actions['unwatch'] ) ) {
$result = array_merge( $actions['unwatch'], $baseResult );
$result['class'] .= ' watched';
$result[ 'text' ] = $this->msg( 'unwatchthispage' );
} else {
// placeholder for not logged in
$result = array_merge( $baseResult, [
'href' => $ctaUrl,
] );
}
return $result;
}
/**