From cbad0bd36398a2755e30d1673c53a2503d8c424a Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Wed, 26 Sep 2018 16:03:20 -0700 Subject: [PATCH] Merge skin option modules into a single ResourceLoader module and move enabled logic to client It's presumed that skin options will eventually become the default or be removed from the skin. While they are not the default, it would be helpful to package them in one single module - as ResourceLoader modules are costly bloating the dependency graph in the MediaWiki startup module. In T167713 we talked about grouping our entry points by page rather than feature, which this seems consistent with. A page with special options enabled is different from a page without. Change-Id: Id948f913d4743532ba3442d2059a03c122419ff2 --- includes/skins/SkinMinerva.php | 23 +++++++++---- .../BackToTopOverlay.hogan | 0 .../BackToTopOverlay.js | 4 +-- .../backtotop.js} | 5 +-- .../backtotop.less | 0 .../categories.js} | 6 ++++ skin.json | 33 +++++++------------ tests/phpunit/skins/SkinMinervaTest.php | 5 +-- 8 files changed, 42 insertions(+), 34 deletions(-) rename resources/{skins.minerva.backtotop => skins.minerva.options}/BackToTopOverlay.hogan (100%) rename resources/{skins.minerva.backtotop => skins.minerva.options}/BackToTopOverlay.js (87%) rename resources/{skins.minerva.backtotop/init.js => skins.minerva.options/backtotop.js} (71%) rename resources/{skins.minerva.backtotop => skins.minerva.options}/backtotop.less (100%) rename resources/{skins.minerva.categories/init.js => skins.minerva.options/categories.js} (89%) diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 87f9110..a5e8fc0 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -120,6 +120,19 @@ class SkinMinerva extends SkinTemplate { return $this->skinOptions[$key]; } + /** + * Return whether any of the skin options have been set + * @return bool + */ + public function hasSkinOptions() { + foreach ( $this->skinOptions as $key => $val ) { + if ( $val ) { + return true; + } + } + return false; + } + /** * initialize various variables and generate the template * @return QuickTemplate @@ -1245,6 +1258,7 @@ class SkinMinerva extends SkinTemplate { return $data; } + /** * Returns array of config variables that should be added only to this skin * for use in JavaScript. @@ -1256,6 +1270,7 @@ class SkinMinerva extends SkinTemplate { $out = $this->getOutput(); $vars = [ + 'wgMinervaFeatures' => $this->skinOptions, 'wgMinervaDownloadNamespaces' => $this->getConfig()->get( 'MinervaDownloadNamespaces' ), 'wgMinervaMenuData' => $this->getMenuData(), 'wgMFDescription' => $out->getProperty( 'wgMFDescription' ), @@ -1326,12 +1341,8 @@ class SkinMinerva extends SkinTemplate { $modules[] = 'skins.minerva.talk'; } - if ( $this->hasCategoryLinks() ) { - $modules[] = 'skins.minerva.categories'; - } - - if ( $this->getSkinOption( self::OPTION_BACK_TO_TOP ) ) { - $modules[] = 'skins.minerva.backtotop'; + if ( $this->hasSkinOptions() ) { + $modules[] = 'skins.minerva.options'; } return $modules; diff --git a/resources/skins.minerva.backtotop/BackToTopOverlay.hogan b/resources/skins.minerva.options/BackToTopOverlay.hogan similarity index 100% rename from resources/skins.minerva.backtotop/BackToTopOverlay.hogan rename to resources/skins.minerva.options/BackToTopOverlay.hogan diff --git a/resources/skins.minerva.backtotop/BackToTopOverlay.js b/resources/skins.minerva.options/BackToTopOverlay.js similarity index 87% rename from resources/skins.minerva.backtotop/BackToTopOverlay.js rename to resources/skins.minerva.options/BackToTopOverlay.js index 9d2cd6e..5d6a78a 100644 --- a/resources/skins.minerva.backtotop/BackToTopOverlay.js +++ b/resources/skins.minerva.options/BackToTopOverlay.js @@ -13,7 +13,7 @@ OO.mfExtend( BackToTopOverlay, View, { className: 'backtotop', - template: mw.template.get( 'skins.minerva.backtotop', 'BackToTopOverlay.hogan' ), + template: mw.template.get( 'skins.minerva.options', 'BackToTopOverlay.hogan' ), events: $.extend( {}, View.prototype.events, { click: 'onBackToTopClick' } ), @@ -47,6 +47,6 @@ } } ); - M.define( 'skins.minerva.backtotop/BackToTopOverlay', BackToTopOverlay ); + M.define( 'skins.minerva.options/BackToTopOverlay', BackToTopOverlay ); }( mw.mobileFrontend, jQuery ) ); diff --git a/resources/skins.minerva.backtotop/init.js b/resources/skins.minerva.options/backtotop.js similarity index 71% rename from resources/skins.minerva.backtotop/init.js rename to resources/skins.minerva.options/backtotop.js index f6553a6..23a4caf 100644 --- a/resources/skins.minerva.backtotop/init.js +++ b/resources/skins.minerva.options/backtotop.js @@ -1,10 +1,11 @@ ( function ( M, $ ) { - var BackToTopOverlay = M.require( 'skins.minerva.backtotop/BackToTopOverlay' ), + var BackToTopOverlay = M.require( 'skins.minerva.options/BackToTopOverlay' ), backtotop = new BackToTopOverlay(), + features = mw.config.get( 'wgMinervaFeatures', {} ), browser = M.require( 'mobile.startup/Browser' ).getSingleton(); // check if browser user agent is iOS (T141598) - if ( browser.isIos() ) { + if ( browser.isIos() || !features.backToTop ) { return; } diff --git a/resources/skins.minerva.backtotop/backtotop.less b/resources/skins.minerva.options/backtotop.less similarity index 100% rename from resources/skins.minerva.backtotop/backtotop.less rename to resources/skins.minerva.options/backtotop.less diff --git a/resources/skins.minerva.categories/init.js b/resources/skins.minerva.options/categories.js similarity index 89% rename from resources/skins.minerva.categories/init.js rename to resources/skins.minerva.options/categories.js index 65000ad..24c4f34 100644 --- a/resources/skins.minerva.categories/init.js +++ b/resources/skins.minerva.options/categories.js @@ -1,9 +1,15 @@ ( function ( M, $ ) { var loader = M.require( 'mobile.startup/rlModuleLoader' ), + features = mw.config.get( 'wgMinervaFeatures', {} ), overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ), user = M.require( 'mobile.startup/user' ); + // check the categories feature has been turned on + if ( !features.categories ) { + return; + } + // categories overlay overlayManager.add( /^\/categories$/, function () { return loader.loadModule( 'mobile.categories.overlays', true ).then( function ( loadingOverlay ) { diff --git a/skin.json b/skin.json index 7c3e0ea..2cc0373 100644 --- a/skin.json +++ b/skin.json @@ -516,37 +516,26 @@ "resources/skins.minerva.editor/init.js" ] }, - "skins.minerva.backtotop": { - "targets": [ - "mobile", - "desktop" - ], - "dependencies": [ - "mobile.toggle", - "mobile.startup" - ], - "templates": { - "BackToTopOverlay.hogan": "resources/skins.minerva.backtotop/BackToTopOverlay.hogan" - }, - "styles": [ - "resources/skins.minerva.backtotop/backtotop.less" - ], - "scripts": [ - "resources/skins.minerva.backtotop/BackToTopOverlay.js", - "resources/skins.minerva.backtotop/init.js" - ] - }, - "skins.minerva.categories": { + "skins.minerva.options": { "targets": [ "mobile", "desktop" ], "dependencies": [ "skins.minerva.scripts", + "mobile.toggle", "mobile.startup" ], + "templates": { + "BackToTopOverlay.hogan": "resources/skins.minerva.options/BackToTopOverlay.hogan" + }, + "styles": [ + "resources/skins.minerva.options/backtotop.less" + ], "scripts": [ - "resources/skins.minerva.categories/init.js" + "resources/skins.minerva.options/BackToTopOverlay.js", + "resources/skins.minerva.options/backtotop.js", + "resources/skins.minerva.options/categories.js" ] }, "skins.minerva.talk": { diff --git a/tests/phpunit/skins/SkinMinervaTest.php b/tests/phpunit/skins/SkinMinervaTest.php index 140e206..8e47a48 100644 --- a/tests/phpunit/skins/SkinMinervaTest.php +++ b/tests/phpunit/skins/SkinMinervaTest.php @@ -39,6 +39,7 @@ class EchoNotifUser { * @group MinervaNeue */ class SkinMinervaTest extends MediaWikiTestCase { + const OPTIONS_MODULE = 'skins.minerva.options'; /** * @covers ::addToBodyAttributes @@ -190,8 +191,8 @@ class SkinMinervaTest extends MediaWikiTestCase { public function provideGetContextSpecificModules() { return [ - [ true, 'skins.minerva.backtotop', true ], - [ false, 'skins.minerva.backtotop', false ], + [ true, self::OPTIONS_MODULE, true ], + [ false, self::OPTIONS_MODULE, false ], ]; }