diff --git a/includes/MinervaHooks.php b/includes/MinervaHooks.php index 42b2536..183574a 100644 --- a/includes/MinervaHooks.php +++ b/includes/MinervaHooks.php @@ -111,6 +111,9 @@ class MinervaHooks { 'resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js', 'resources/skins.minerva.scripts/page-issues/overlay/IssueList.js', 'resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js', + "resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js", + "resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js", + "resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js", 'resources/skins.minerva.scripts/pageIssues.js', // test files 'tests/qunit/skins.minerva.scripts/downloadPageAction.test.js', diff --git a/resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js b/resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js new file mode 100644 index 0000000..72fc07c --- /dev/null +++ b/resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js @@ -0,0 +1,14 @@ +( function ( M ) { + /** + * Creates a "read more" button with given text. + * @param {string} msg + * @return {JQuery} + */ + function newPageIssueLearnMoreLink( msg ) { + return $( '' ) + .addClass( 'ambox-learn-more' ) + .text( msg ); + } + + M.define( 'skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink', newPageIssueLearnMoreLink ); +}( mw.mobileFrontend ) ); diff --git a/resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js b/resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js new file mode 100644 index 0000000..679ba8f --- /dev/null +++ b/resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js @@ -0,0 +1,13 @@ +( function ( M ) { + /** + * Create a link element that opens the issues overlay. + * + * @param {string} labelText The text value of the element + * @return {JQuery} + */ + function newPageIssueLink( labelText ) { + return $( '' ).addClass( 'cleanup mw-mf-cleanup' ).text( labelText ); + } + + M.define( 'skins.minerva.scripts/page-issues/page/PageIssueLink', newPageIssueLink ); +}( mw.mobileFrontend ) ); diff --git a/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js b/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js new file mode 100644 index 0000000..0173bb5 --- /dev/null +++ b/resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js @@ -0,0 +1,51 @@ +( function ( M ) { + var + newPageIssueLink = M.require( 'skins.minerva.scripts/page-issues/page/PageIssueLink' ), + newPageIssueLearnMoreLink = M.require( 'skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink' ); + + /** + * Modifies the `issue` DOM to create a banner designed for single / multiple issue templates, + * and handles event-binding for that issues overlay. + * + * @param {IssueSummary} issue + * @param {string} msg + * @param {string} overlayUrl + * @param {Object} overlayManager + * @param {boolean} [multiple] + */ + function insertPageIssueBanner( issue, msg, overlayUrl, overlayManager, multiple ) { + var $learnMoreEl = newPageIssueLearnMoreLink( msg ), + $issueContainer = multiple ? + issue.$el.parents( '.mbox-text-span, .mbox-text-div' ) : + issue.$el.find( '.mbox-text' ), + $clickContainer = multiple ? issue.$el.parents( '.mbox-text' ) : issue.$el; + + $issueContainer.prepend( issue.iconString ); + $issueContainer.prepend( $learnMoreEl ); + + $clickContainer.on( 'click', function () { + overlayManager.router.navigate( overlayUrl ); + return false; + } ); + } + + /** + * Modifies the page DOM to insert a page-issue notice below the title of the page, + * containing a link with a message like "this page has issues". + * Used on talk & category namespaces, or when page-issue banners have been disabled.s + * + * @param {string} labelText + * @param {string} section + */ + function insertPageIssueNotice( labelText, section ) { + var $link = newPageIssueLink( labelText ); + $link.attr( 'href', '#/issues/' + section ); + // eslint-disable-next-line jquery/no-global-selector + $link.insertAfter( $( 'h1#section_0' ) ); + } + + M.define( 'skins.minerva.scripts/page-issues/page/pageIssueFormatter', { + insertPageIssueBanner: insertPageIssueBanner, + insertPageIssueNotice: insertPageIssueNotice + } ); +}( mw.mobileFrontend ) ); diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js index 0fc7f34..f2559bf 100644 --- a/resources/skins.minerva.scripts/pageIssues.js +++ b/resources/skins.minerva.scripts/pageIssues.js @@ -10,94 +10,12 @@ features = mw.config.get( 'wgMinervaFeatures', {} ), pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ), pageIssuesOverlay = M.require( 'skins.minerva.scripts/pageIssuesOverlay' ), + pageIssueFormatter = M.require( 'skins.minerva.scripts/page-issues/page/pageIssueFormatter' ), // When the query string flag is set force on new treatment. // When wgMinervaPageIssuesNewTreatment is the default this line can be removed. QUERY_STRING_FLAG = mw.util.getParamValue( 'minerva-issues' ), newTreatmentEnabled = features.pageIssues || QUERY_STRING_FLAG; - /** - * Create a link element that opens the issues overlay. - * - * @ignore - * - * @param {string} labelText The text value of the element - * @return {JQuery} - */ - function createLinkElement( labelText ) { - return $( '' ).addClass( 'cleanup mw-mf-cleanup' ).text( labelText ); - } - - /** - * Creates a "read more" button with given text. - * @param {string} msg - * @return {JQuery} - */ - function createLearnMoreLink( msg ) { - return $( '' ) - .addClass( 'ambox-learn-more' ) - .text( msg ); - } - - /** - * Modifies the `issue` DOM to create a banner designed for - * single issue templates, and handles event-binding for that issues overlay. - * - * @param {IssueSummary} issue - * @param {string} msg - * @param {string} overlayUrl - * @param {Object} overlayManager - */ - function createPageIssueBanner( issue, msg, overlayUrl, overlayManager ) { - var $learnMoreEl = createLearnMoreLink( msg ), - $issueContainer = issue.$el.find( '.mbox-text' ); - - $issueContainer.prepend( issue.iconString ); - $issueContainer.prepend( $learnMoreEl ); - - issue.$el.on( 'click', function () { - overlayManager.router.navigate( overlayUrl ); - return false; - } ); - } - - /** - * Modifies the `issue` DOM to create a banner designed for - * multiple issue templates, and handles event-binding for that issues overlay. - * - * @param {IssueSummary} issue - * @param {string} msg - * @param {string} overlayUrl - * @param {Object} overlayManager - */ - function createPageIssueBannerMultiple( issue, msg, overlayUrl, overlayManager ) { - var $learnMoreEl = createLearnMoreLink( msg ), - $parentContentContainer = issue.$el.parents( '.mbox-text-span, .mbox-text-div' ), - $parentContainer = issue.$el.parents( '.mbox-text' ); - - $parentContentContainer.prepend( issue.iconString ); - $parentContentContainer.prepend( $learnMoreEl ); - - $parentContainer.on( 'click', function () { - overlayManager.router.navigate( overlayUrl ); - return false; - } ); - } - - /** - * Modifies the page DOM to insert a page-issue notice below the title of the page, - * containing a link with a message like "this page has issues". - * Used on talk & category namespaces, or when page-issue banners have been disabled.s - * - * @param {string} labelText - * @param {string} section - */ - function createPageIssueNotice( labelText, section ) { - var $link = createLinkElement( labelText ); - $link.attr( 'href', '#/issues/' + section ); - // eslint-disable-next-line jquery/no-global-selector - $link.insertAfter( $( 'h1#section_0' ) ); - } - /** * Render a banner in a containing element. * if in group B, a learn more link will be append to any amboxes inside $container @@ -117,7 +35,7 @@ * * @return {JQuery.Object} */ - function createBanner( page, labelText, section, inline, overlayManager ) { + function insertBannersOrNotice( page, labelText, section, inline, overlayManager ) { var $metadata, issueUrl = section === KEYWORD_ALL_SECTIONS ? '#/issues/' + KEYWORD_ALL_SECTIONS : '#/issues/' + section, @@ -153,16 +71,13 @@ issueSummaries.forEach( function ( issueSummary, i ) { var isGrouped = issueSummary.issue.grouped, lastIssueIsGrouped = issueSummaries[ i - 1 ] && - issueSummaries[ i - 1 ].issue.grouped; + issueSummaries[ i - 1 ].issue.grouped, + multiple = isGrouped && !lastIssueIsGrouped; // only render the first grouped issue of each group - if ( isGrouped && !lastIssueIsGrouped ) { - createPageIssueBannerMultiple( issueSummary, mw.msg( 'skin-minerva-issue-learn-more' ), issueUrl, overlayManager ); - } else { - createPageIssueBanner( issueSummary, mw.msg( 'skin-minerva-issue-learn-more' ), issueUrl, overlayManager ); - } + pageIssueFormatter.insertPageIssueBanner( issueSummary, mw.msg( 'skin-minerva-issue-learn-more' ), issueUrl, overlayManager, multiple ); } ); } else if ( issueSummaries.length ) { - createPageIssueNotice( labelText, section ); + pageIssueFormatter.insertPageIssueNotice( labelText, section ); } return $metadata; @@ -236,15 +151,15 @@ if ( CURRENT_NS === NS_TALK || CURRENT_NS === NS_CATEGORY ) { // e.g. Template:English variant category; Template:WikiProject - createBanner( page, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), + insertBannersOrNotice( page, mw.msg( 'mobile-frontend-meta-data-issues-header-talk' ), KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else if ( CURRENT_NS === NS_MAIN ) { label = mw.msg( 'mobile-frontend-meta-data-issues-header' ); if ( issueOverlayShowAll ) { - createBanner( page, label, KEYWORD_ALL_SECTIONS, inline, overlayManager ); + insertBannersOrNotice( page, label, KEYWORD_ALL_SECTIONS, inline, overlayManager ); } else { // parse lead - createBanner( page, label, '0', inline, overlayManager ); + insertBannersOrNotice( page, label, '0', inline, overlayManager ); if ( newTreatmentEnabled ) { // parse other sections but only in group B. In treatment A no issues are shown // for sections. @@ -257,7 +172,7 @@ if ( sectionNum ) { // Render banner for sectionNum associated with headingEl inside // Page - createBanner( + insertBannersOrNotice( page, label, sectionNum.toString(), inline, overlayManager ); } @@ -278,7 +193,7 @@ init: initPageIssues, test: { getAllIssuesSections: getAllIssuesSections, - createBanner: createBanner + insertBannersOrNotice: insertBannersOrNotice } } ); diff --git a/skin.json b/skin.json index 1c70258..844fbcb 100644 --- a/skin.json +++ b/skin.json @@ -418,6 +418,9 @@ "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js", "resources/skins.minerva.scripts/page-issues/overlay/IssueList.js", "resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js", + "resources/skins.minerva.scripts/page-issues/page/PageIssueLearnMoreLink.js", + "resources/skins.minerva.scripts/page-issues/page/PageIssueLink.js", + "resources/skins.minerva.scripts/page-issues/page/pageIssueFormatter.js", "resources/skins.minerva.scripts/pageIssues.js", "resources/skins.minerva.scripts/init.js", "resources/skins.minerva.scripts/initLogging.js", diff --git a/tests/qunit/skins.minerva.scripts/pageIssues.test.js b/tests/qunit/skins.minerva.scripts/pageIssues.test.js index 8e60ed0..6ebfa2b 100644 --- a/tests/qunit/skins.minerva.scripts/pageIssues.test.js +++ b/tests/qunit/skins.minerva.scripts/pageIssues.test.js @@ -3,7 +3,7 @@ pageIssues = M.require( 'skins.minerva.scripts/pageIssues' ), mobile = M.require( 'mobile.startup' ), util = mobile.util, - createBanner = pageIssues.test.createBanner, + insertBannersOrNotice = pageIssues.test.insertBannersOrNotice, icon = {}, MEDIUM_ISSUE = { issue: { @@ -45,21 +45,21 @@ labelText = 'label text', inline = true, SECTION = '0', - processedAmbox = createBanner( + processedAmbox = insertBannersOrNotice( new Page( { el: $mockContainer } ), labelText, SECTION, inline, overlayManager ); QUnit.module( 'Minerva cleanuptemplates' ); - QUnit.test( 'createBanner() should add a "learn more" message', function ( assert ) { + QUnit.test( 'insertBannersOrNotice() should add a "learn more" message', function ( assert ) { assert.strictEqual( /⧼skin-minerva-issue-learn-more⧽/.test( processedAmbox.html() ), true ); } ); - QUnit.test( 'createBanner() should add an icon', function ( assert ) { + QUnit.test( 'insertBannersOrNotice() should add an icon', function ( assert ) { assert.strictEqual( /mw-ui-icon/.test( processedAmbox.html() ), true ); } ); - QUnit.test( 'clicking on the product of createBanner() should trigger a URL change', function ( assert ) { + QUnit.test( 'clicking on the product of insertBannersOrNotice() should trigger a URL change', function ( assert ) { processedAmbox.click(); assert.strictEqual( window.location.hash, '#/issues/' + SECTION ); } );