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

[dev] Replace menu entry inheritance with functions 

Break up Menu/DefaultBuilder into functions that are reusable without
inheritance. The functions do not need much state to produce their
outputs and a weighty inheritance hierarchy makes the code difficult to
reason about. The functions are used in a following patch for the user
menu. They're now simple, independent, static functions in BuilderUtil
that are easy to reason about and compose.

Also, ban inheritance via `final` in a few places nearby. Inheritance
has not worked well in MobileFrontend and enabling it should be a
special deliberate case, not a default. E.g., in the user menu, the
changes could have been to the base class' getPersonalTools() method
such that the client passes a parameter for the advanced config or maybe
just override it in the subclass. In either case, it makes the whole
hierarchy nuanced and harder to reason about for something that should
be dead simple.

Bug: T214540
Change-Id: I6e9a2b36a1bff387eb3b33ea65b0a6806962810a
This commit is contained in:
Stephen Niedzielski 2019-07-22 14:58:34 -06:00
parent f068dc7af5
commit 933dc0e370
4 changed files with 168 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
includes/menu/Definitions.php
View File

@ -76,7 +76,6 @@ final class Definitions {
'contributions',
$this->context->msg( 'mobile-frontend-main-menu-contributions' )->escaped(),
SpecialPage::getTitleFor( 'Contributions', $this->user->getName() )->getLocalURL()
) );
}
@ -110,12 +109,12 @@ final class Definitions {
}
/**
* Creates a login or logout button
* Creates a login or logout button with a profile button.
*
* @param Group $group
* @throws MWException
*/
public function insertLogInOutMenuItem( Group $group ) {
public function insertAuthMenuItem( Group $group ) {
$group->insertEntry( new AuthMenuEntry(
$this->user,
$this->context,

69
includes/menu/Main/AdvancedBuilder.php
View File

@ -23,6 +23,8 @@ namespace MediaWiki\Minerva\Menu\Main;
use FatalError;
use Hooks;
use MWException;
use User;
use MediaWiki\Minerva\Menu\Definitions;
use MediaWiki\Minerva\Menu\Group;
/**
@ -32,29 +34,88 @@ use MediaWiki\Minerva\Menu\Group;
*
* @package MediaWiki\Minerva\Menu\Main
*/
class AdvancedBuilder extends DefaultBuilder {
final class AdvancedBuilder implements IBuilder {
/**
* @var bool
*/
private $showMobileOptions;
/**
* Currently logged in user
* @var User
*/
private $user;
/**
* @var Definitions
*/
private $definitions;
/**
* Initialize the Default Main Menu builder
*
* @param bool $showMobileOptions Show MobileOptions instead of Preferences
* @param User $user The current user
* @param Definitions $definitions A menu items definitions set
*/
public function __construct( $showMobileOptions, User $user, Definitions $definitions ) {
$this->showMobileOptions = $showMobileOptions;
$this->user = $user;
$this->definitions = $definitions;
}
/**
* @inheritDoc
* @return Group[]
* @throws FatalError
* @throws MWException
*/
public function getGroups(): array {
return [
$this->getDiscoveryTools(),
BuilderUtil::getDiscoveryTools( $this->definitions ),
$this->getPersonalTools(),
$this->getSiteTools(),
$this->getConfigurationTools(),
BuilderUtil::getConfigurationTools( $this->definitions, $this->showMobileOptions ),
];
}
/**
* @inheritDoc
* @throws FatalError
* @throws MWException
*/
public function getSiteLinks(): Group {
return BuilderUtil::getSiteLinks( $this->definitions );
}
/**
* Builds the personal tools menu item group.
* @return Group
* @throws FatalError
* @throws MWException
*/
private function getPersonalTools(): Group {
$group = new Group();
$this->definitions->insertAuthMenuItem( $group );
if ( $this->user->isLoggedIn() ) {
$this->definitions->insertWatchlistMenuItem( $group );
$this->definitions->insertContributionsMenuItem( $group );
}
// Allow other extensions to add or override tools
Hooks::run( 'MobileMenu', [ 'personal', &$group ] );
return $group;
}
/**
* Prepares a list of links that have the purpose of discovery in the main navigation menu
* @return Group
* @throws FatalError
* @throws MWException
*/
public function getSiteTools(): Group {
private function getSiteTools(): Group {
$group = new Group();
$this->definitions->insertSpecialPages( $group );

90
includes/menu/Main/BuilderUtil.php Normal file
View File

@ -0,0 +1,90 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
*/
namespace MediaWiki\Minerva\Menu\Main;
use FatalError;
use Hooks;
use MWException;
use MediaWiki\Minerva\Menu\Definitions;
use MediaWiki\Minerva\Menu\Group;
/**
* Group generators shared between menu builders.
*
* @package MediaWiki\Minerva\Menu\Main
*/
final class BuilderUtil {
/**
* Prepares a list of links that have the purpose of discovery in the main navigation menu
* @param Definitions $definitions A menu items definitions set
* @return Group
* @throws FatalError
* @throws MWException
*/
public static function getDiscoveryTools( Definitions $definitions ): Group {
$group = new Group();
$definitions->insertHomeItem( $group );
$definitions->insertRandomItem( $group );
$definitions->insertNearbyIfSupported( $group );
// Allow other extensions to add or override tools
Hooks::run( 'MobileMenu', [ 'discovery', &$group ] );
return $group;
}
/**
* Like <code>SkinMinerva#getDiscoveryTools</code> and <code>#getPersonalTools</code>, create
* a group of configuration-related menu items. Currently, only the Settings menu item is in the
* group.
* @param Definitions $definitions A menu items definitions set
* @param bool $showMobileOptions Show MobileOptions instead of Preferences
* @return Group
* @throws MWException
*/
public static function getConfigurationTools(
Definitions $definitions, $showMobileOptions
): Group {
$group = new Group();
$showMobileOptions ?
$definitions->insertMobileOptionsItem( $group ) :
$definitions->insertPreferencesItem( $group );
return $group;
}
/**
* Returns an array of sitelinks to add into the main menu footer.
* @param Definitions $definitions A menu items definitions set
* @return Group Collection of site links
* @throws MWException
*/
public static function getSiteLinks( Definitions $definitions ): Group {
$group = new Group();
$definitions->insertAboutItem( $group );
$definitions->insertDisclaimersItem( $group );
// Allow other extensions to add or override tools
Hooks::run( 'MobileMenu', [ 'sitelinks', &$group ] );
return $group;
}
}

66
includes/menu/Main/DefaultBuilder.php
View File

@ -30,7 +30,7 @@ use MediaWiki\Minerva\Menu\Group;
/**
* Used to build default (available for everyone by default) main menu
*/
class DefaultBuilder implements IBuilder {
final class DefaultBuilder implements IBuilder {
/**
* @var bool
@ -41,12 +41,12 @@ class DefaultBuilder implements IBuilder {
* Currently logged in user
* @var User
*/
protected $user;
private $user;
/**
* @var Definitions
*/
protected $definitions;
private $definitions;
/**
* Initialize the Default Main Menu builder
@ -62,34 +62,24 @@ class DefaultBuilder implements IBuilder {
}
/**
* @return Group[]
* @inheritDoc
* @throws FatalError
* @throws MWException
*/
public function getGroups(): array {
return [
$this->getDiscoveryTools(),
BuilderUtil::getDiscoveryTools( $this->definitions ),
$this->getPersonalTools(),
$this->getConfigurationTools(),
BuilderUtil::getConfigurationTools( $this->definitions, $this->showMobileOptions ),
];
}
/**
* Prepares a list of links that have the purpose of discovery in the main navigation menu
* @return Group
* @throws FatalError
* @inheritDoc
* @throws MWException
*/
protected function getDiscoveryTools(): Group {
$group = new Group();
$this->definitions->insertHomeItem( $group );
$this->definitions->insertRandomItem( $group );
$this->definitions->insertNearbyIfSupported( $group );
// Allow other extensions to add or override tools
Hooks::run( 'MobileMenu', [ 'discovery', &$group ] );
return $group;
public function getSiteLinks(): Group {
return BuilderUtil::getSiteLinks( $this->definitions );
}
/**
@ -101,10 +91,10 @@ class DefaultBuilder implements IBuilder {
* @throws FatalError
* @throws MWException
*/
protected function getPersonalTools(): Group {
private function getPersonalTools(): Group {
$group = new Group();
$this->definitions->insertLogInOutMenuItem( $group );
$this->definitions->insertAuthMenuItem( $group );
if ( $this->user->isLoggedIn() ) {
$this->definitions->insertWatchlistMenuItem( $group );
@ -115,38 +105,4 @@ class DefaultBuilder implements IBuilder {
Hooks::run( 'MobileMenu', [ 'personal', &$group ] );
return $group;
}
/**
* Like <code>SkinMinerva#getDiscoveryTools</code> and <code>#getPersonalTools</code>, create
* a group of configuration-related menu items. Currently, only the Settings menu item is in the
* group.
*
* @return Group
* @throws MWException
*/
protected function getConfigurationTools(): Group {
$group = new Group();
$this->showMobileOptions ?
$this->definitions->insertMobileOptionsItem( $group ) :
$this->definitions->insertPreferencesItem( $group );
return $group;
}
/**
* Returns an array of sitelinks to add into the main menu footer.
* @return Group Collection of site links
* @throws MWException
*/
public function getSiteLinks(): Group {
$group = new Group();
$this->definitions->insertAboutItem( $group );
$this->definitions->insertDisclaimersItem( $group );
// Allow other extensions to add or override tools
Hooks::run( 'MobileMenu', [ 'sitelinks', &$group ] );
return $group;
}
}