From 53a80e1bcc20e87f01ef87d44098baba065f9b25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Fri, 21 Sep 2018 02:32:49 +0200 Subject: [PATCH] Move edit link enabling/disabling out of skins.minerva.editor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These CSS classes are specific to the Minerva skin and we want to move this file to MobileFrontend. Now that this code is all in one place, I noticed that it seems to not do much at all… Everything in this function looks like it could just be done in the PHP code. If PHP does it all, then we could remove all of it for a free performance improvement. Bug: T198765 Change-Id: I6487c2fc520e14e0856b2e9a6f9dfa5066205817 --- resources/skins.minerva.editor/init.js | 47 ++----------------------- resources/skins.minerva.scripts/init.js | 42 ++++++++++++++++++++++ 2 files changed, 45 insertions(+), 44 deletions(-) diff --git a/resources/skins.minerva.editor/init.js b/resources/skins.minerva.editor/init.js index a346e411d..362278062 100644 --- a/resources/skins.minerva.editor/init.js +++ b/resources/skins.minerva.editor/init.js @@ -4,32 +4,14 @@ // see: https://www.mediawiki.org/wiki/Manual:Interface/JavaScript#Page-specific isReadOnly = mw.config.get( 'wgMinervaReadOnly' ), isEditable = !isReadOnly && mw.config.get( 'wgIsProbablyEditable' ), - blockInfo = mw.config.get( 'wgMinervaUserBlockInfo', false ), router = require( 'mediawiki.router' ), issues = M.require( 'skins.minerva.scripts/pageIssues' ), overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ), loader = M.require( 'mobile.startup/rlModuleLoader' ), - Icon = M.require( 'mobile.startup/Icon' ), skin = M.require( 'skins.minerva.scripts/skin' ), currentPage = M.getCurrentPage(), - // TODO: create a utility method to generate class names instead of - // constructing temporary objects. This affects disabledEditIcon, - // enabledEditIcon, enabledEditIcon, and disabledClass and - // a number of other places in the code base. - disabledEditIcon = new Icon( { - name: 'edit', - glyphPrefix: 'minerva' - } ), - enabledEditIcon = new Icon( { - name: 'edit-enabled', - glyphPrefix: 'minerva' - } ), editErrorMessage = isReadOnly ? mw.msg( 'apierror-readonly' ) : mw.msg( 'mobile-frontend-editor-disabled' ), - // TODO: move enabledClass, $caEdit, and disabledClass to locals within - // updateEditPageButton(). - enabledClass = enabledEditIcon.getGlyphClassName(), - disabledClass = disabledEditIcon.getGlyphClassName(), - // TODO: rename to editPageButton. + // FIXME: rename to editPageButton. $caEdit = $( '#ca-edit' ), user = M.require( 'mobile.startup/user' ), popup = M.require( 'mobile.startup/toast' ), @@ -46,10 +28,6 @@ CtaDrawer = M.require( 'mobile.startup/CtaDrawer' ), drawer; - if ( user.isAnon() ) { - blockInfo = false; - } - /** * Event handler for edit link clicks. Will prevent default link * behaviour and will not allow propagation @@ -65,7 +43,7 @@ return false; } - // TODO: rename addEditSectionButton and evaluate whether the page edit button + // FIXME: rename addEditSectionButton and evaluate whether the page edit button // can leverage the same code. Also: change the CSS class name to use // the word "section" instead of "page". /** @@ -86,16 +64,6 @@ .prependTo( container ); } - /** - * @param {boolean} enabled - * @return {void} - */ - function updateEditPageButton( enabled ) { - $caEdit - .addClass( enabled ? enabledClass : disabledClass ) - .removeClass( enabled ? disabledClass : enabledClass ); - } - /** * Make an element render a CTA when clicked * @method @@ -164,7 +132,7 @@ isNewPage = page.options.id === 0, leadSection = page.getLeadSectionElement(); - page.$( '.edit-page, .edit-link' ).removeClass( disabledClass ) + page.$( '.edit-page, .edit-link' ) .on( 'click', onEditLinkClick ); overlayManager.add( /^\/editor\/(\d+|all)$/, function ( sectionId ) { var @@ -260,7 +228,6 @@ return loadSourceEditor(); } } ); - updateEditPageButton( true ); // Make sure we never create two edit links by accident // FIXME: split the selector and cache it @@ -300,10 +267,6 @@ router.navigate( fragment ); } } - - if ( blockInfo ) { - updateEditPageButton( false ); - } } /** @@ -326,7 +289,6 @@ // Edit button updated in setupEditor. setupEditor( currentPage ); } else { - updateEditPageButton( false ); hideSectionEditIcons(); showSorryToast( editErrorMessage ); } @@ -341,7 +303,6 @@ // Initialize edit button links (to show Cta) only, if page is editable, // otherwise show an error toast if ( isEditable ) { - updateEditPageButton( true ); // Init lead section edit button makeCta( $caEdit, 0 ); @@ -356,7 +317,6 @@ makeCta( $a, section ); } ); } else { - updateEditPageButton( false ); showSorryToast( editErrorMessage ); } } @@ -391,7 +351,6 @@ } if ( isNewFile ) { - updateEditPageButton( true ); // Is a new file page (enable upload image only) Bug 58311 showSorryToast( mw.msg( 'mobile-frontend-editor-uploadenable' ) ); } else { diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index 13f409526..251dcbdfa 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -2,6 +2,7 @@ var toast = M.require( 'mobile.startup/toast' ), time = M.require( 'mobile.startup/time' ), + user = M.require( 'mobile.startup/user' ), skin = M.require( 'mobile.init/skin' ), issues = M.require( 'skins.minerva.scripts/pageIssues' ), DownloadIcon = M.require( 'skins.minerva.scripts/DownloadIcon' ), @@ -10,6 +11,7 @@ router = require( 'mediawiki.router' ), OverlayManager = M.require( 'mobile.startup/OverlayManager' ), CtaDrawer = M.require( 'mobile.startup/CtaDrawer' ), + Icon = M.require( 'mobile.startup/Icon' ), Button = M.require( 'mobile.startup/Button' ), Anchor = M.require( 'mobile.startup/Anchor' ), overlayManager = new OverlayManager( require( 'mediawiki.router' ) ), @@ -312,6 +314,45 @@ } ); } + /** + * Initialize page edit action link (#ca-edit) + * + * Mark the edit link as disabled if the user is not actually able to edit the page for some + * reason (e.g. page is protected or user is blocked). + * + * Note that the link is still clickable, but clicking it will probably open a view-source + * form or display an error message, rather than open an edit form. + * + * FIXME: Review this code as part of T206262 + * + * @ignore + */ + function initEditLink() { + var + // FIXME: create a utility method to generate class names instead of + // constructing temporary objects. This affects disabledEditIcon, + // enabledEditIcon, enabledEditIcon, and disabledClass and + // a number of other places in the code base. + disabledEditIcon = new Icon( { + name: 'edit', + glyphPrefix: 'minerva' + } ), + enabledEditIcon = new Icon( { + name: 'edit-enabled', + glyphPrefix: 'minerva' + } ), + enabledClass = enabledEditIcon.getGlyphClassName(), + disabledClass = disabledEditIcon.getGlyphClassName(), + isReadOnly = mw.config.get( 'wgMinervaReadOnly' ), + isEditable = mw.config.get( 'wgIsProbablyEditable' ), + blockInfo = user.isAnon() ? false : mw.config.get( 'wgMinervaUserBlockInfo', false ), + canEdit = !isReadOnly && isEditable && !blockInfo; + + $( '#ca-edit' ) + .addClass( canEdit ? enabledClass : disabledClass ) + .removeClass( canEdit ? disabledClass : enabledClass ); + } + $( function () { // Update anything else that needs enhancing (e.g. watchlist) initModifiedInfo(); @@ -321,6 +362,7 @@ loadTabletModules(); appendDownloadButton(); initRedlinksCta(); + initEditLink(); // Setup the issues banner on the page // Pages which dont exist (id 0) cannot have issues if ( !page.isMissing ) {