From 0ca42ee64e4fc518a4cba3b20a63d67b97085cef Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Mon, 30 Jul 2018 22:45:44 +0800 Subject: [PATCH] Hygiene: Separate cleanups library from initialisation In order to write tests, we'd like to separate code without side effects from code that executes it as part of setup. This shuffles dependencies and makes page and overlayManager parameters to the init function (injected dependencies) Change-Id: I96808541d48be7869fed3bc30babb80866e139ec --- .../skins.minerva.scripts/cleanuptemplates.js | 28 +++++++++---------- resources/skins.minerva.scripts/init.js | 6 ++++ skin.json | 8 +++--- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/resources/skins.minerva.scripts/cleanuptemplates.js b/resources/skins.minerva.scripts/cleanuptemplates.js index 08bedda..f2bbc6c 100644 --- a/resources/skins.minerva.scripts/cleanuptemplates.js +++ b/resources/skins.minerva.scripts/cleanuptemplates.js @@ -21,9 +21,7 @@ } } ).getBucket() === 'B', Icon = M.require( 'mobile.startup/Icon' ), - page = M.getCurrentPage(), pageIssueParser = M.require( 'skins.minerva.scripts/pageIssueParser' ), - overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ), CleanupOverlay = M.require( 'mobile.issues/CleanupOverlay' ); /** @@ -88,9 +86,10 @@ * If string KEYWORD_ALL_SECTIONS banner should apply to entire page. * @param {boolean} inline - if true the first ambox in the section will become the entry point for the issues overlay * and if false, a link will be rendered under the heading. + * @param {OverlayManager} overlayManager * @ignore */ - function createBanner( $container, labelText, section, inline ) { + function createBanner( $container, labelText, section, inline, overlayManager ) { var $learnMore, issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section, selector = 'table.ambox, table.tmbox, table.cmbox, table.fmbox', @@ -195,8 +194,10 @@ * Scan an element for any known cleanup templates and replace them with a button * that opens them in a mobile friendly overlay. * @ignore + * @param {OverlayManager} overlayManager + * @param {Page} page */ - function initPageIssues() { + function initPageIssues( overlayManager, page ) { var ns = mw.config.get( 'wgNamespaceNumber' ), label, headingText = ACTION_EDIT ? mw.msg( 'edithelp' ) : getNamespaceHeadingText( ns ), @@ -211,17 +212,18 @@ if ( ACTION_EDIT ) { // Editor uses different parent element $container = $( '#mw-content-text' ); - createBanner( $container, mw.msg( 'edithelp' ), KEYWORD_ALL_SECTIONS, inline ); + createBanner( $container, mw.msg( 'edithelp' ), KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else if ( ns === NS_TALK || ns === NS_CATEGORY ) { // e.g. Template:English variant category; Template:WikiProject - createBanner( $container, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), KEYWORD_ALL_SECTIONS, inline ); + createBanner( $container, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), + KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else if ( ns === NS_MAIN ) { label = mw.msg( 'mobile-frontend-meta-data-issues-header' ); if ( issueOverlayShowAll ) { - createBanner( $container, label, KEYWORD_ALL_SECTIONS, inline ); + createBanner( $container, label, KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else { // parse lead - createBanner( $lead, label, 0, inline ); + createBanner( $lead, label, 0, inline, overlayManager ); if ( isInGroupB ) { // parse other sections but only in group B. In treatment A no issues are shown for sections. $lead.nextAll( 'h1,h2,h3,h4,h5,h6' ).each( function ( i, headingEl ) { @@ -229,7 +231,7 @@ $section = $headingEl.next(), sectionNum = $headingEl.find( '.edit-page' ).data( 'section' ); - createBanner( $section, label, sectionNum, inline ); + createBanner( $section, label, sectionNum, inline, overlayManager ); } ); } } @@ -247,10 +249,8 @@ } ); } - // Setup the issues banner on the page - // Pages which dont exist (id 0) cannot have issues - if ( !page.isMissing ) { - $( initPageIssues ); - } + M.define( 'skins.minerva.scripts/cleanuptemplates', { + init: initPageIssues + } ); }( mw.mobileFrontend, jQuery ) ); diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index e588b5e..0d9f039 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -3,6 +3,7 @@ toast = M.require( 'mobile.startup/toast' ), time = M.require( 'mobile.startup/time' ), skin = M.require( 'mobile.init/skin' ), + issues = M.require( 'skins.minerva.scripts/cleanuptemplates' ), DownloadIcon = M.require( 'skins.minerva.scripts/DownloadIcon' ), browser = M.require( 'mobile.startup/Browser' ).getSingleton(), loader = M.require( 'mobile.startup/rlModuleLoader' ), @@ -268,6 +269,11 @@ M.on( 'resize', loadTabletModules ); loadTabletModules(); appendDownloadButton(); + // Setup the issues banner on the page + // Pages which dont exist (id 0) cannot have issues + if ( !page.isMissing ) { + issues.init( overlayManager ); + } } ); M.define( 'skins.minerva.scripts/overlayManager', overlayManager ); diff --git a/skin.json b/skin.json index 377bef1..cf31e9c 100644 --- a/skin.json +++ b/skin.json @@ -415,14 +415,14 @@ "scripts": [ "resources/skins.minerva.scripts/preInit.js", "resources/skins.minerva.scripts/DownloadIcon.js", + "resources/skins.minerva.scripts/pageIssueParser.js", + "resources/skins.minerva.scripts/AB.js", + "resources/skins.minerva.scripts/cleanuptemplates.js", "resources/skins.minerva.scripts/init.js", "resources/skins.minerva.scripts/initLogging.js", "resources/skins.minerva.scripts/mobileRedirect.js", "resources/skins.minerva.scripts/search.js", - "resources/skins.minerva.scripts/references.js", - "resources/skins.minerva.scripts/pageIssueParser.js", - "resources/skins.minerva.scripts/AB.js", - "resources/skins.minerva.scripts/cleanuptemplates.js" + "resources/skins.minerva.scripts/references.js" ] }, "skins.minerva.scripts.top": {