From ad04f31441c0f044480bc31ee2368e070aaa161f Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Thu, 25 Jul 2019 12:34:50 +0200 Subject: [PATCH] Follow-up: User menu improvements Changes: - We should limit the interfaces we pass around, AdvancedUserMenuBuilder doesn't need whole IContextSource as it uses only msg() method. It's better to define that this methods needs only MessageLocalizer - move UserMenuDirector into ServiceWiring to be consistent with other Directors/Builders - pass PersonalTools as a dependency to UserMenuDirector, which will pass to each Builder. The personalTools is set of links that can/should be used when rendering user menu (which in the fact has almost same subset of tools as the personal toolbox) Bug: T214540 Change-Id: I7f744651b0665452a5a9d1ce661f20547e80812d --- includes/ServiceWiring.php | 26 ++++++++++++++- .../menu/User/AdvancedUserMenuBuilder.php | 33 +++++++++---------- includes/menu/User/DefaultUserMenuBuilder.php | 2 +- includes/menu/User/IUserMenuBuilder.php | 4 ++- includes/menu/User/UserMenuDirector.php | 5 +-- includes/skins/SkinMinerva.php | 24 +++----------- 6 files changed, 51 insertions(+), 43 deletions(-) diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index dd40895..631cdc6 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -24,6 +24,9 @@ use MediaWiki\Minerva\LanguagesHelper; use MediaWiki\Minerva\Menu\Definitions; use MediaWiki\Minerva\Menu\Main as MainMenu; use MediaWiki\Minerva\Menu\PageActions as PageActionsMenu; +use MediaWiki\Minerva\Menu\User\AdvancedUserMenuBuilder; +use MediaWiki\Minerva\Menu\User\DefaultUserMenuBuilder; +use MediaWiki\Minerva\Menu\User\UserMenuDirector; use MediaWiki\Minerva\Permissions\IMinervaPagePermissions; use MediaWiki\Minerva\Permissions\MinervaPagePermissions; use MediaWiki\Minerva\Permissions\MinervaNoPagePermissions; @@ -31,13 +34,34 @@ use MediaWiki\Minerva\SkinOptions; use MediaWiki\Minerva\SkinUserPageHelper; return [ + 'Minerva.Menu.Definitions' => function ( MediaWikiServices $services ): Definitions { + return new Definitions( RequestContext::getMain(), $services->getSpecialPageFactory() ); + }, + 'Minerva.Menu.UserMenuDirector' => function ( MediaWikiServices $services ): UserMenuDirector { + $options = $services->getService( 'Minerva.SkinOptions' ); + $definitions = $services->getService( 'Minerva.Menu.Definitions' ); + + $context = RequestContext::getMain(); + $builder = $options->get( SkinOptions::OPTION_AMC ) ? + new AdvancedUserMenuBuilder( + $context, + $context->getUser(), + $definitions + ) : + new DefaultUserMenuBuilder(); + + return new UserMenuDirector( + $builder, + $context->getSkin() + ); + }, 'Minerva.Menu.MainDirector' => function ( MediaWikiServices $services ): MainMenu\Director { $context = RequestContext::getMain(); /** @var SkinOptions $options */ $options = $services->getService( 'Minerva.SkinOptions' ); + $definitions = $services->getService( 'Minerva.Menu.Definitions' ); $showMobileOptions = $options->get( SkinOptions::OPTION_MOBILE_OPTIONS ); $user = $context->getUser(); - $definitions = new Definitions( $context, $services->getSpecialPageFactory() ); $builder = $options->get( SkinOptions::OPTION_AMC ) ? new MainMenu\AdvancedBuilder( $showMobileOptions, $user, $definitions ) : new MainMenu\DefaultBuilder( $showMobileOptions, $user, $definitions ); diff --git a/includes/menu/User/AdvancedUserMenuBuilder.php b/includes/menu/User/AdvancedUserMenuBuilder.php index eb6c834..3ef3792 100644 --- a/includes/menu/User/AdvancedUserMenuBuilder.php +++ b/includes/menu/User/AdvancedUserMenuBuilder.php @@ -20,11 +20,11 @@ namespace MediaWiki\Minerva\Menu\User; use Hooks; -use IContextSource; use MediaWiki\Minerva\Menu\Definitions; use MediaWiki\Minerva\Menu\Entries\ProfileMenuEntry; use MediaWiki\Minerva\Menu\Entries\SingleMenuEntry; use MediaWiki\Minerva\Menu\Group; +use MessageLocalizer; use User; /** @@ -32,9 +32,9 @@ use User; */ final class AdvancedUserMenuBuilder implements IUserMenuBuilder { /** - * @var IContextSource + * @var MessageLocalizer */ - private $context; + private $messageLocalizer; /** * @var User @@ -47,46 +47,43 @@ final class AdvancedUserMenuBuilder implements IUserMenuBuilder { private $definitions; /** - * @var array|null - */ - private $sandbox; - - /** - * @param IContextSource $context + * @param MessageLocalizer $messageLocalizer * @param User $user * @param Definitions $definitions A menu items definitions set - * @param array|null $sandbox */ public function __construct( - IContextSource $context, User $user, Definitions $definitions, $sandbox + MessageLocalizer $messageLocalizer, User $user, Definitions $definitions ) { - $this->context = $context; + $this->messageLocalizer = $messageLocalizer; $this->user = $user; $this->definitions = $definitions; - $this->sandbox = $sandbox; } /** * @inheritDoc + * @param array $personalTools list of personal tools generated by + * SkinTemplate::getPersonalTools * @return Group */ - public function getGroup(): Group { + public function getGroup( array $personalTools ): Group { $group = new Group(); $group->insertEntry( new ProfileMenuEntry( $this->user ) ); $group->insertEntry( new SingleMenuEntry( 'userTalk', - $this->context->msg( 'mobile-frontend-user-page-talk' )->escaped(), + $this->messageLocalizer->msg( 'mobile-frontend-user-page-talk' )->escaped(), $this->user->getUserPage()->getTalkPage()->getLocalURL(), true, null, 'before', 'wikimedia-ui-userTalk-base20' ) ); - if ( $this->sandbox ) { + $sandbox = $personalTools['sandbox']['links'][0] ?? false; + + if ( $sandbox ) { $group->insertEntry( new SingleMenuEntry( 'userSandbox', - $this->sandbox['text'], - $this->sandbox['href'] + $sandbox['text'], + $sandbox['href'] ) ); } $this->definitions->insertWatchlistMenuItem( $group ); diff --git a/includes/menu/User/DefaultUserMenuBuilder.php b/includes/menu/User/DefaultUserMenuBuilder.php index afe687a..821e058 100644 --- a/includes/menu/User/DefaultUserMenuBuilder.php +++ b/includes/menu/User/DefaultUserMenuBuilder.php @@ -29,7 +29,7 @@ final class DefaultUserMenuBuilder implements IUserMenuBuilder { * @inheritDoc * @return Group */ - public function getGroup(): Group { + public function getGroup( array $personalTools ): Group { return new Group(); } } diff --git a/includes/menu/User/IUserMenuBuilder.php b/includes/menu/User/IUserMenuBuilder.php index 7dafc5b..88d6620 100644 --- a/includes/menu/User/IUserMenuBuilder.php +++ b/includes/menu/User/IUserMenuBuilder.php @@ -24,7 +24,9 @@ use MediaWiki\Minerva\Menu\Group; interface IUserMenuBuilder { /** + * @param array $personalTools list of personal tools generated by + * SkinTemplate::getPersonalTools * @return Group */ - public function getGroup(): Group; + public function getGroup( array $personalTools ): Group; } diff --git a/includes/menu/User/UserMenuDirector.php b/includes/menu/User/UserMenuDirector.php index 4fb64c9..2192993 100644 --- a/includes/menu/User/UserMenuDirector.php +++ b/includes/menu/User/UserMenuDirector.php @@ -48,10 +48,11 @@ final class UserMenuDirector { /** * Build the menu data array that can be passed to views/javascript + * @param array $personalTools Personal tools list generated by BaseTemplate::getPersonalTools * @return string|null */ - public function renderMenuData() { - $entries = $this->builder->getGroup()->getEntries(); + public function renderMenuData( array $personalTools ) { + $entries = $this->builder->getGroup( $personalTools )->getEntries(); foreach ( $entries as &$entry ) { foreach ( $entry['components'] as &$component ) { diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 45afae0..5172d86 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -23,10 +23,6 @@ use MediaWiki\Minerva\Menu\Main\Director as MainMenuDirector; use MediaWiki\Minerva\Permissions\IMinervaPagePermissions; use MediaWiki\Minerva\SkinOptions; use MediaWiki\Minerva\SkinUserPageHelper; -use MediaWiki\Minerva\Menu\User\UserMenuDirector; -use MediaWiki\Minerva\Menu\User\AdvancedUserMenuBuilder; -use MediaWiki\Minerva\Menu\User\DefaultUserMenuBuilder; -use MediaWiki\Minerva\Menu\Definitions; /** * Minerva: Born from the godhead of Jupiter with weapons! @@ -98,22 +94,10 @@ class SkinMinerva extends SkinTemplate { * @return string|null */ private function getUserMenuHTML( BaseTemplate $tpl ) { - $services = MediaWikiServices::getInstance(); - $options = $services->getService( 'Minerva.SkinOptions' ); - $context = RequestContext::getMain(); - $definitions = new Definitions( $context, $services->getSpecialPageFactory() ); - $director = new UserMenuDirector( - $options->get( SkinOptions::OPTION_AMC ) - ? new AdvancedUserMenuBuilder( - $context, - $this->getUser(), - $definitions, - $tpl->getPersonalTools()['sandbox']['links'][0] ?? null - ) - : new DefaultUserMenuBuilder(), - $this - ); - return $director->renderMenuData(); + /** @var \MediaWiki\Minerva\Menu\User\UserMenuDirector $director */ + $director = MediaWikiServices::getInstance() + ->getService( 'Minerva.Menu.UserMenuDirector' ); + return $director->renderMenuData( $tpl->getPersonalTools() ); } /**