jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/345000 )
Change subject: Mobile specific skin changes are enabled inside a hook
......................................................................
Mobile specific skin changes are enabled inside a hook
If a feature is only available in the mobile version of the skin
and not desktop, only enable it when the user enters the mobile skin.
Features that are currently in development in mobile web
beta are now limited to mobile and will not appear on desktop mode.
Bug: T125588
Change-Id: I634bea4b9969e228457b13e01b45a679fa25ed3b
---
M extension.json
A includes/Minerva.hooks.php
A includes/skins/ICustomizableSkin.php
M includes/skins/SkinMinerva.php
M tests/phpunit/skins/SkinMinervaTest.php
5 files changed, 160 insertions(+), 59 deletions(-)
Approvals:
Pmiazga: Looks good to me, but someone else must approve
jenkins-bot: Verified
Phuedx: Looks good to me, approved
diff --git a/extension.json b/extension.json
index f2ceecd..b635549 100644
--- a/extension.json
+++ b/extension.json
@@ -51,6 +51,7 @@
},
"AutoloadClasses": {
"ExtMobileFrontend": "includes/MobileFrontend.body.php",
+ "MinervaHooks": "includes/Minerva.hooks.php",
"MobileFrontendHooks": "includes/MobileFrontend.hooks.php",
"MobileFrontendSkinHooks":
"includes/MobileFrontend.skin.hooks.php",
"MobileFrontend\\BaseDomainExtractorInterface":
"includes/BaseDomainExtractorInterface.php",
@@ -87,6 +88,7 @@
"MFResourceLoaderParsedMessageModule":
"includes/modules/MFResourceLoaderParsedMessageModule.php",
"SkinMinerva": "includes/skins/SkinMinerva.php",
"SkinMinervaBeta": "includes/skins/SkinMinervaBeta.php",
+ "ICustomizableSkin": "includes/skins/ICustomizableSkin.php",
"MobileFrontend\\MenuBuilder": "includes/MenuBuilder.php",
"MobileFrontend\\MenuEntry": "includes/MenuBuilder.php",
"MobileFrontend\\Devices\\DeviceDetector":
"includes/devices/DeviceDetector.php",
@@ -1607,6 +1609,9 @@
"APIGetDescription": [
"ApiParseExtender::onAPIGetDescription"
],
+ "BeforePageDisplayMobile": [
+ "MinervaHooks::onBeforePageDisplayMobile"
+ ],
"RequestContextCreateSkin": [
"MobileFrontendHooks::onRequestContextCreateSkin"
],
diff --git a/includes/Minerva.hooks.php b/includes/Minerva.hooks.php
new file mode 100644
index 0000000..d4d9d4e
--- /dev/null
+++ b/includes/Minerva.hooks.php
@@ -0,0 +1,38 @@
+<?php
+/**
+ * Minerva.hooks.php
+ */
+
+/**
+ * Hook handlers for Minerva skin.
+ * Hooks specific to all skins running in mobile mode should belong in
+ * MobileFrontend.hooks.php
+ *
+ * Hook handler method names should be in the form of:
+ * on<HookName>()
+ */
+class MinervaHooks {
+ /**
+ * BeforePageDisplayMobile hook handler.
+ *
+ * @param OutputPage $out
+ * @param Skin $skin
+ */
+ public static function onBeforePageDisplayMobile( OutputPage $out, Skin
$skin ) {
+ // In mobile mode MobileContext will always be available.
+ $mobileContext = MobileContext::singleton();
+ // setSkinOptions is not available
+ if ( $skin instanceof SkinMinerva ) {
+ $skin->setSkinOptions( [
+ SkinMinerva::OPTION_CATEGORIES
+ => $mobileContext->getConfigVariable(
'MinervaShowCategoriesButton' ),
+ SkinMinerva::OPTION_FONT_CHANGER
+ => $mobileContext->getConfigVariable(
'MinervaEnableFontChanger' ),
+ SkinMinerva::OPTION_BACK_TO_TOP
+ => $mobileContext->getConfigVariable(
'MinervaEnableBackToTop' ),
+ SkinMinerva::OPTION_TOGGLING => true,
+ SkinMinerva::OPTION_MOBILE_OPTIONS => true,
+ ] );
+ }
+ }
+}
diff --git a/includes/skins/ICustomizableSkin.php
b/includes/skins/ICustomizableSkin.php
new file mode 100644
index 0000000..69467bc
--- /dev/null
+++ b/includes/skins/ICustomizableSkin.php
@@ -0,0 +1,16 @@
+<?php
+
+interface ICustomizableSkin {
+ /**
+ * override an existing option or options with new values
+ * @param array $options
+ */
+ public function setSkinOptions( $options );
+
+ /**
+ * Return whether a skin option is truthy
+ * @param string $key
+ * @return boolean
+ */
+ public function getSkinOption( $key );
+}
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index ffae8d4..fedcae6 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -10,9 +10,14 @@
* A skin that works on both desktop and mobile
* @ingroup Skins
*/
-class SkinMinerva extends SkinTemplate {
- /** @var boolean $isMobileMode Describes whether reader is on a mobile
device */
- protected $isMobileMode = false;
+class SkinMinerva extends SkinTemplate implements ICustomizableSkin {
+ /** Set of keys for available skin options. See $skinOptions. */
+ const OPTION_MOBILE_OPTIONS = 'mobileOptionsLink';
+ const OPTION_CATEGORIES = 'categories';
+ const OPTION_FONT_CHANGER = 'fontChanger';
+ const OPTION_BACK_TO_TOP = 'backToTop';
+ const OPTION_TOGGLING = 'toggling';
+
/** @var string $skinname Name of this skin */
public $skinname = 'minerva';
/** @var string $template Name of this used template */
@@ -21,24 +26,56 @@
public $useHeadElement = true;
/** @var string $mode Describes 'stability' of the skin - beta, stable
*/
protected $mode = 'stable';
- /** @var MobileContext $mobileContext Safes an instance of
MobileContext */
- protected $mobileContext;
/** @var bool whether the page is the user's page, i.e. User:Username */
public $isUserPage = false;
/** @var ContentHandler Content handler of page; only access through
getContentHandler */
protected $contentHandler = null;
/** @var bool Whether the page is also available in other languages or
variants */
protected $doesPageHaveLanguages = false;
- /** @var bool Whether to show the categories button on the page */
- protected $shouldShowCategoriesButton = false;
/**
- * Wrapper for MobileContext::getMFConfig()
- * @see MobileContext::getMFConfig()
+ * Return skin specific configuration
+ * Note that while MobileFrontend and Minerva live in the same skin
this will also return
+ * MobileFrontend config.
* @return Config
*/
public function getMFConfig() {
- return $this->mobileContext->getMFConfig();
+ // FIXME: When Minerva lives in its own skin this will make
sense and be Minerva.Config
+ return MediaWikiServices::getInstance()->getService(
'MobileFrontend.Config' );
+ }
+
+ /** @var array skin specific options */
+ protected $skinOptions = [
+ /**
+ * Whether the main menu should include a link to
+ * Special:Preferences of Special:MobileOptions
+ */
+ self::OPTION_MOBILE_OPTIONS => false,
+ /** Whether a categories button should appear at the bottom of
the skin. */
+ self::OPTION_CATEGORIES => false,
+ /** Whether an option to change font size appears in
Special:MobileOptions */
+ self::OPTION_FONT_CHANGER => false,
+ /** Whether a back to top button appears at the bottom of the
view page */
+ self::OPTION_BACK_TO_TOP => false,
+ /** Whether sections can be collapsed (requires MobileFrontend
and MobileFormatter) */
+ self::OPTION_TOGGLING => false,
+ ];
+
+ /**
+ * override an existing option or options with new values
+ * @param array $options
+ */
+ public function setSkinOptions( $options ) {
+ $this->skinOptions = array_merge( $this->skinOptions, $options
);
+ }
+
+ /**
+ * Return whether a skin option is truthy
+ * @param string $key
+ * @return boolean
+ */
+ public function getSkinOption( $key ) {
+ return $this->skinOptions[$key];
}
/**
@@ -248,11 +285,15 @@
}
/**
- * Whether the output page contains category links
+ * Whether the output page contains category links and the category
feature is enabled.
* @return bool
*/
private function hasCategoryLinks() {
+ if ( !$this->getSkinOption( self::OPTION_CATEGORIES ) ) {
+ return false;
+ }
$categoryLinks = $this->getOutput()->getCategoryLinks();
+
if ( !count( $categoryLinks ) ) {
return false;
}
@@ -263,8 +304,6 @@
* Initiate class
*/
public function __construct() {
- $this->mobileContext = MobileContext::singleton();
- $this->isMobileMode =
$this->mobileContext->shouldDisplayMobileView();
$title = $this->getTitle();
if ( $title->inNamespace( NS_USER ) && !$title->isSubpage() ) {
$pageUserId = User::idFromName( $title->getText() );
@@ -273,8 +312,6 @@
$this->isUserPage = true;
}
}
- $this->shouldShowCategoriesButton =
$this->mobileContext->getConfigVariable(
- 'MinervaShowCategoriesButton' ) &&
$this->hasCategoryLinks();
}
/**
@@ -438,7 +475,7 @@
$returnToTitle = $this->getTitle()->getPrefixedText();
// Links specifically for mobile mode
- if ( $this->isMobileMode ) {
+ if ( $this->getSkinOption( self::OPTION_MOBILE_OPTIONS ) ) {
// Settings link
$menu->insert( 'settings' )
@@ -939,7 +976,7 @@
$buttons['language'] = $this->getLanguageButton();
}
- if ( $this->shouldShowCategoriesButton ) {
+ if ( $this->hasCategoryLinks() ) {
$buttons['categories'] = $this->getCategoryButton();
}
@@ -1213,15 +1250,15 @@
$modules[] = 'skins.minerva.talk';
}
- if ( $this->shouldShowCategoriesButton ) {
+ if ( $this->hasCategoryLinks() ) {
$modules[] = 'skins.minerva.categories';
}
- if ( $this->mobileContext->getConfigVariable(
'MinervaEnableFontChanger' ) ) {
+ if ( $this->getSkinOption( self::OPTION_FONT_CHANGER ) ) {
$modules[] = 'skins.minerva.fontchanger';
}
- if ( $this->mobileContext->getConfigVariable(
'MinervaEnableBackToTop' ) ) {
+ if ( $this->getSkinOption( self::OPTION_BACK_TO_TOP ) ) {
$modules[] = 'skins.minerva.backtotop';
}
@@ -1248,7 +1285,7 @@
$modules['context'] = $this->getContextSpecificModules();
- if ( $this->isMobileMode ) {
+ if ( $this->getSkinOption( self::OPTION_TOGGLING ) ) {
$modules['toggling'] = [ 'skins.minerva.toggling' ];
}
$modules['site'] = 'mobile.site';
diff --git a/tests/phpunit/skins/SkinMinervaTest.php
b/tests/phpunit/skins/SkinMinervaTest.php
index 504646b..a692862 100644
--- a/tests/phpunit/skins/SkinMinervaTest.php
+++ b/tests/phpunit/skins/SkinMinervaTest.php
@@ -3,25 +3,11 @@
namespace Tests\MobileFrontend\Skins;
use MediaWikiTestCase;
-use MobileContext;
use OutputPage;
use SkinMinerva;
use TestingAccessWrapper;
use Title;
-
-class TestSkinMinerva extends SkinMinerva {
-
- /**
- * The Minimum Viable Constructor for SkinMinerva.
- *
- * @FIXME Why doesn't SkinMinerva have its dependencies injected?
- *
- * @param MobileContext $mobileContext
- */
- public function __construct( MobileContext $mobileContext ) {
- $this->mobileContext = $mobileContext;
- }
-}
+use RequestContext;
/**
* @covers SkinMinerva
@@ -45,21 +31,38 @@
private function addToBodyAttributes(
$bodyClassName
) {
- $context = MobileContext::singleton();
+ $context = RequestContext::getMain();
$outputPage = $context->getOutput();
$outputPage->setProperty( 'bodyClassName', $bodyClassName );
$bodyAttrs = [ 'class' => '' ];
- $this->factorySkin( $context )
- ->addToBodyAttributes( $outputPage, $bodyAttrs );
+ $skin = new SkinMinerva();
+ $skin->addToBodyAttributes( $outputPage, $bodyAttrs );
return explode( ' ', $bodyAttrs[ 'class' ] );
}
- private function factorySkin( MobileContext $context ) {
- return new TestSkinMinerva( $context );
+ public function testHasCategoryLinksWhenOptionIsOff() {
+ $outputPage = $this->getMockBuilder( OutputPage::class )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $outputPage->expects( $this->never() )
+ ->method( 'getCategoryLinks' );
+
+ $context = $this->getMockBuilder( 'IContextSource' )->getMock();
+ $context->expects( $this->any() )
+ ->method( 'getOutput' )
+ ->willReturn( $outputPage );
+
+ $skin = new SkinMinerva();
+ $skin->setContext( $context );
+ $skin->setSkinOptions( [ SkinMinerva::OPTION_CATEGORIES =>
false ] );
+
+ $skin = TestingAccessWrapper::newFromObject( $skin );
+
+ $this->assertEquals( $skin->hasCategoryLinks(), false );
}
/**
@@ -75,14 +78,17 @@
->method( 'getCategoryLinks' )
->will( $this->returnValue( $categoryLinks ) );
- $skin = TestingAccessWrapper::newFromObject(
- $this->getMockBuilder( SkinMinerva::class )
- ->disableOriginalConstructor()
- ->getMock()
- );
- $skin->expects( $this->once() )
+ $context = $this->getMockBuilder( 'IContextSource' )->getMock();
+ $context->expects( $this->any() )
->method( 'getOutput' )
- ->will( $this->returnValue( $outputPage ) );
+ ->willReturn( $outputPage );
+
+ $skin = new SkinMinerva();
+ $skin->setContext( $context );
+ $skin->setSkinOptions( [ SkinMinerva::OPTION_CATEGORIES => true
] );
+
+ $skin = TestingAccessWrapper::newFromObject( $skin );
+
$this->assertEquals( $skin->hasCategoryLinks(), $expected );
}
@@ -122,12 +128,12 @@
*
* @covers SkinMinerva::getContextSpecificModules
* @dataProvider provideGetContextSpecificModules
- * @param string $configName Config name that needs to be set
- * @param mixed $configValue Config value that is assigned to
$configName
+ * @param string $fontchangerValue whether font changer feature is
enabled
+ * @param mixed $backToTopValue whether back to top feature is enabled
* @param string $moduleName Module name that is being tested
* @param bool $expected Whether the module is expected to be returned
by the function being tested
*/
- public function testGetContextSpecificModules( $configName,
$configValue,
+ public function testGetContextSpecificModules( $fontchangerValue,
$backToTopValue,
$moduleName, $expected ) {
$skin = TestingAccessWrapper::newFromObject(
$this->getMockBuilder( SkinMinerva::class )
@@ -135,15 +141,14 @@
->setMethods( [ 'getTitle' ] )
->getMock()
);
- $skin->mobileContext = MobileContext::singleton();
- $skin->isMobileMode =
$skin->mobileContext->shouldDisplayMobileView();
$title = Title::newFromText( 'Test' );
$skin->expects( $this->any() )
->method( 'getTitle' )
->will( $this->returnValue( $title ) );
- $this->setMwGlobals( $configName, [
- 'base' => $configValue
+ $skin->setSkinOptions( [
+ 'fontChanger' => $fontchangerValue,
+ 'backToTop' => $backToTopValue,
] );
if ( $expected ) {
@@ -155,10 +160,10 @@
public function provideGetContextSpecificModules() {
return [
- [ 'wgMinervaEnableFontChanger', true,
'skins.minerva.fontchanger', true ],
- [ 'wgMinervaEnableFontChanger', false,
'skins.minerva.fontchanger', false ],
- [ 'wgMinervaEnableBackToTop', true,
'skins.minerva.backtotop', true ],
- [ 'wgMinervaEnableBackToTop', false,
'skins.minerva.backtotop', false ],
+ [ true, false, 'skins.minerva.fontchanger', true ],
+ [ false, true, 'skins.minerva.fontchanger', false ],
+ [ false, true, 'skins.minerva.backtotop', true ],
+ [ false, false, 'skins.minerva.backtotop', false ],
];
}
}
--
To view, visit https://gerrit.wikimedia.org/r/345000
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I634bea4b9969e228457b13e01b45a679fa25ed3b
Gerrit-PatchSet: 21
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Bmansurov <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Kaldari <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: Pmiazga <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
