From f54b4e75db79ed6b818718c24a7509c42bd6e139 Mon Sep 17 00:00:00 2001 From: Stephen Niedzielski Date: Mon, 4 Feb 2019 09:22:09 -0700 Subject: [PATCH] Hygiene: move variable from file to local scope Move allIssues from file scope to local scope. Bug: T212371 Change-Id: I74693925ff0b20a36ec6acd53490cfde7273c984 --- resources/skins.minerva.scripts/pageIssues.js | 54 ++++++++++++------- .../skins.minerva.scripts/pageIssues.test.js | 2 +- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js index 9833d9c..d2bbccd 100644 --- a/resources/skins.minerva.scripts/pageIssues.js +++ b/resources/skins.minerva.scripts/pageIssues.js @@ -2,8 +2,6 @@ /** @typedef {Object.} IssueSummaryMap */ var Page = M.require( 'mobile.startup' ).Page, - /** @type {IssueSummaryMap} */ - allIssues = {}, KEYWORD_ALL_SECTIONS = 'all', config = mw.config, NS_MAIN = 0, @@ -36,7 +34,7 @@ * @param {OverlayManager} overlayManager * @ignore * - * @return {JQuery.Object} + * @return {{ambox: JQuery.Object, issueSummaries: IssueSummary[]}} */ function insertBannersOrNotice( page, labelText, section, inline, overlayManager ) { var @@ -67,8 +65,6 @@ } } } ); - // store it for later - allIssues[ section ] = issueSummaries; if ( inline ) { issueSummaries.forEach( function ( issueSummary, i ) { @@ -89,16 +85,20 @@ pageIssueFormatter.insertPageIssueNotice( labelText, section ); } - return $metadata; + return { + ambox: $metadata, + issueSummaries: issueSummaries + }; } /** * Obtains the list of issues for the current page and provided section + * @param {IssueSummaryMap} allIssues mapping section {number} to {IssueSummary} * @param {number|string} section either KEYWORD_ALL_SECTIONS or a number relating to the * section the issues belong to * @return {jQuery.Object[]} array of all issues. */ - function getIssues( section ) { + function getIssues( allIssues, section ) { if ( section !== KEYWORD_ALL_SECTIONS ) { return allIssues[ section ] || []; } @@ -117,8 +117,8 @@ * Returns an array containing the section of each page issue. * In the case that several page issues are grouped in a 'multiple issues' template, * returns the section of those issues as one item. - * @param {Object} allIssues mapping section {Number} to {IssueSummary} - * @return {array} + * @param {IssueSummaryMap} allIssues mapping section {number} to {IssueSummary} + * @return {number[]} */ function getAllIssuesSections( allIssues ) { return Object.keys( allIssues ).reduce( function ( acc, section ) { @@ -146,7 +146,13 @@ * @param {Page} page */ function initPageIssues( overlayManager, page ) { - var label, + var + section, + /** @type {IssueSummary[]} */ + issueSummaries = [], + /** @type {IssueSummaryMap} */ + allIssues = {}, + label, $lead = page.getLeadSectionElement(), issueOverlayShowAll = CURRENT_NS === NS_CATEGORY || CURRENT_NS === NS_TALK || !$lead, inline = newTreatmentEnabled && CURRENT_NS === 0; @@ -159,16 +165,26 @@ } if ( CURRENT_NS === NS_TALK || CURRENT_NS === NS_CATEGORY ) { + section = KEYWORD_ALL_SECTIONS; // e.g. Template:English variant category; Template:WikiProject - insertBannersOrNotice( page, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), - KEYWORD_ALL_SECTIONS, inline, overlayManager ); + issueSummaries = insertBannersOrNotice( page, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), + section, inline, overlayManager ).issueSummaries; + allIssues[ section ] = issueSummaries; } else if ( CURRENT_NS === NS_MAIN ) { label = mw.msg( 'mobile-frontend-meta-data-issues-header' ); if ( issueOverlayShowAll ) { - insertBannersOrNotice( page, label, KEYWORD_ALL_SECTIONS, inline, overlayManager ); + section = KEYWORD_ALL_SECTIONS; + issueSummaries = insertBannersOrNotice( + page, label, section, inline, overlayManager + ).issueSummaries; + allIssues[ section ] = issueSummaries; } else { // parse lead - insertBannersOrNotice( page, label, '0', inline, overlayManager ); + section = '0'; + issueSummaries = insertBannersOrNotice( + page, label, section, inline, overlayManager + ).issueSummaries; + allIssues[ section ] = issueSummaries; if ( newTreatmentEnabled ) { // parse other sections but only in group B. In treatment A no issues are shown // for sections. @@ -181,9 +197,11 @@ if ( sectionNum ) { // Render banner for sectionNum associated with headingEl inside // Page - insertBannersOrNotice( - page, label, sectionNum.toString(), inline, overlayManager - ); + section = sectionNum.toString(); + issueSummaries = insertBannersOrNotice( + page, label, section, inline, overlayManager + ).issueSummaries; + allIssues[ section ] = issueSummaries; } } ); } @@ -193,7 +211,7 @@ // Setup the overlay route. overlayManager.add( new RegExp( '^/issues/(\\d+|' + KEYWORD_ALL_SECTIONS + ')$' ), function ( section ) { return pageIssuesOverlay( - getIssues( section ), section, CURRENT_NS + getIssues( allIssues, section ), section, CURRENT_NS ); } ); } diff --git a/tests/qunit/skins.minerva.scripts/pageIssues.test.js b/tests/qunit/skins.minerva.scripts/pageIssues.test.js index 6ebfa2b..ae35996 100644 --- a/tests/qunit/skins.minerva.scripts/pageIssues.test.js +++ b/tests/qunit/skins.minerva.scripts/pageIssues.test.js @@ -48,7 +48,7 @@ processedAmbox = insertBannersOrNotice( new Page( { el: $mockContainer } ), labelText, SECTION, inline, overlayManager - ); + ).ambox; QUnit.module( 'Minerva cleanuptemplates' );