From 6b0ce8641076416fb1c955696e8c75e074daf1db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Fri, 21 Sep 2018 03:38:10 +0200 Subject: [PATCH] Make Minerva section editing more like other skins Goal: Make skins.minerva.editor not rely on Minerva-specific markup. SkinMinerva.php: * Add `class="mw-editsection"` to section edit links in SkinMinerva. This is the default behavior in SkinTemplate. * Tweak the page "Edit" link generated in PHP to be the same as the link we were generating in JS: add class="edit-page" and change the message for the text. * (Fix an unrelated code comment that was incorrect.) skins.minerva.content.styles/hacks.less: * Remove a hack that was hiding .mw-editsection, since we now use it. skins.minerva.editor/init.js: * Stop using the `data-section` attribute on links to decide which page section to open in the editor. Instead, use the `href` attribute and extract the `section` URL parameter from it. * Stop using the `edit-page` class to find section edit links. Instead, use the `mw-editsection` class. * Remove super weird code that removed the original "Edit" link from the page and generated an identical one to replace it, instead of just adding event handlers to the existing one. * Centralize event handling for all types of edit links. Bug: T198765 Change-Id: I79639c738ff1c3ec4b48ee2e462d23060151a21b --- includes/skins/SkinMinerva.php | 10 ++- .../skins.minerva.content.styles/hacks.less | 3 +- resources/skins.minerva.editor/init.js | 82 ++++++------------- skin.json | 1 - 4 files changed, 32 insertions(+), 64 deletions(-) diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 0dc8a08..a22df2c 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -256,7 +256,7 @@ class SkinMinerva extends SkinTemplate { public function doEditSectionLink( Title $nt, $section, $tooltip = null, $lang = false ) { if ( $this->isAllowedPageAction( 'edit' ) ) { $message = $this->msg( 'mobile-frontend-editor-edit' )->inLanguage( $lang )->text(); - $html = Html::openElement( 'span' ); + $html = Html::openElement( 'span', [ 'class' => 'mw-editsection' ] ); $html .= Html::element( 'a', [ 'href' => $this->getTitle()->getLocalUrl( [ 'action' => 'edit', 'section' => $section ] ), 'title' => $this->msg( 'editsectionhint', $tooltip )->inLanguage( $lang )->text(), @@ -1136,7 +1136,7 @@ class SkinMinerva extends SkinTemplate { $editArgs = [ 'action' => 'edit' ]; if ( $title->isWikitextPage() ) { // If the content model is wikitext we'll default to editing the lead section. - // Full wikitext editing is not possible via the api and hard on mobile devices. + // Full wikitext editing is hard on mobile devices. $editArgs['section'] = self::LEAD_SECTION_NUMBER; } $userCanEdit = $title->quickUserCan( 'edit', $this->getUser() ); @@ -1147,10 +1147,12 @@ class SkinMinerva extends SkinTemplate { 'class' => MinervaUI::iconClass( $userCanEdit ? 'edit-enabled' : 'edit', 'element' ), 'links' => [ 'edit' => [ - 'href' => $title->getLocalURL( $editArgs ) + 'href' => $title->getLocalURL( $editArgs ), + 'msg' => 'mobile-frontend-editor-edit', + 'class' => 'edit-page', ], ], - 'is_js_only' => false + 'is_js_only' => false, ]; } diff --git a/resources/skins.minerva.content.styles/hacks.less b/resources/skins.minerva.content.styles/hacks.less index 65921e8..4b2f6dd 100644 --- a/resources/skins.minerva.content.styles/hacks.less +++ b/resources/skins.minerva.content.styles/hacks.less @@ -102,8 +102,7 @@ FIXME: Review all of these hacks to see if they still apply. } // FIXME: Remove when filetoc is stripped from file pages a la table of contents (toc) -#filetoc, -.mw-editsection { +#filetoc { display: none; } diff --git a/resources/skins.minerva.editor/init.js b/resources/skins.minerva.editor/init.js index 3622780..9515df3 100644 --- a/resources/skins.minerva.editor/init.js +++ b/resources/skins.minerva.editor/init.js @@ -11,8 +11,9 @@ skin = M.require( 'skins.minerva.scripts/skin' ), currentPage = M.getCurrentPage(), editErrorMessage = isReadOnly ? mw.msg( 'apierror-readonly' ) : mw.msg( 'mobile-frontend-editor-disabled' ), - // FIXME: rename to editPageButton. - $caEdit = $( '#ca-edit' ), + // #ca-edit, .mw-editsection are standard MediaWiki elements + // .edit-link comes from MobileFrontend user page creation CTA + $allEditLinks = $( '#ca-edit a, .mw-editsection a, .edit-link' ), user = M.require( 'mobile.startup/user' ), popup = M.require( 'mobile.startup/toast' ), // FIXME: Disable on IE < 10 for time being @@ -36,34 +37,13 @@ * @return {boolean} */ function onEditLinkClick() { + var section = ( new mw.Uri( this.href ) ).query.section || 'all'; issues.log( { action: 'editClicked' } ); - - router.navigate( '#/editor/' + $( this ).data( 'section' ) ); + router.navigate( '#/editor/' + section ); // prevent folding section when clicking Edit by stopping propagation return false; } - // 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". - /** - * Prepend an edit page button to the container - * Remove any existing links in the container - * @method - * @ignore - * @param {number} section number - * @param {string} container CSS selector of the container - * @return {JQuery.Object} newly created edit page button - */ - function addEditButton( section, container ) { - $( container ).find( 'a' ).remove(); - return $( '' ) - .data( 'section', section ) - .on( 'click', onEditLinkClick ) - .text( mw.msg( 'mobile-frontend-editor-edit' ) ) - .prependTo( container ); - } - /** * Make an element render a CTA when clicked * @method @@ -77,7 +57,7 @@ ev.preventDefault(); // prevent folding section when clicking Edit ev.stopPropagation(); - // need to use toggle() because we do ev.stopPropagation() (in addEditButton()) + // need to use toggle() because we do ev.stopPropagation() (in onEditLinkClick()) if ( !drawer ) { drawer = new CtaDrawer( { queryParams: { @@ -128,12 +108,11 @@ * @param {Page} page The page to edit. */ function setupEditor( page ) { - var uri, fragment, editorOverride, + var uri, fragment, editorOverride, section, isNewPage = page.options.id === 0, leadSection = page.getLeadSectionElement(); - page.$( '.edit-page, .edit-link' ) - .on( 'click', onEditLinkClick ); + $allEditLinks.on( 'click', onEditLinkClick ); overlayManager.add( /^\/editor\/(\d+|all)$/, function ( sectionId ) { var $content = $( '#mw-content-text' ), @@ -229,22 +208,19 @@ } } ); - // Make sure we never create two edit links by accident - // FIXME: split the selector and cache it - if ( $caEdit.find( '.edit-page' ).length === 0 ) { - if ( isNewPage || - ( leadSection && leadSection.text() ) || page.getSections().length === 0 ) { - // if lead section is not empty, open editor with lead section - // In some namespaces (controlled by MFNamespacesWithoutCollapsibleSections) - // sections are not marked. Use the lead section for such cases. - addEditButton( 0, '#ca-edit' ); - } else if ( leadSection !== null ) { - // if lead section is empty open editor with first section - // be careful not to do this when leadSection is null as this means MobileFormatter - // has not been run and thus we could not identify the lead - addEditButton( 1, '#ca-edit' ); - } + // By default the editor opens section 0 (lead section). If lead section is empty, and + // there are sections on the page, open editor with section 1 instead. + // (Be careful not to do this when leadSection is null, as this means MobileFormatter + // has not been run and thus we could not identify the lead.) + section = 0; + if ( leadSection && !leadSection.text() && !isNewPage && page.getSections().length !== 0 ) { + section = 1; } + $( '#ca-edit a' ).prop( 'href', function ( i, href ) { + var uri = new mw.Uri( href ); + uri.query.section = section; + return uri.toString(); + } ); if ( !router.getPath() && ( mw.util.getParamValue( 'veaction' ) || mw.util.getParamValue( 'action' ) === 'edit' ) ) { if ( mw.util.getParamValue( 'veaction' ) === 'edit' ) { @@ -276,7 +252,7 @@ * @ignore */ function hideSectionEditIcons() { - currentPage.$( '.edit-page' ).hide(); + currentPage.$( '.mw-editsection' ).hide(); } /** @@ -303,18 +279,10 @@ // Initialize edit button links (to show Cta) only, if page is editable, // otherwise show an error toast if ( isEditable ) { - // Init lead section edit button - makeCta( $caEdit, 0 ); - // Init all edit links (including lead section, if anonymous editing is enabled) - $( '.edit-page' ).each( function () { - var $a = $( this ), - section = 0; - - if ( $( this ).data( 'section' ) !== undefined ) { - section = $( this ).data( 'section' ); - } - makeCta( $a, section ); + $allEditLinks.each( function () { + var section = ( new mw.Uri( this.href ) ).query.section || ''; + makeCta( $( this ), section ); } ); } else { showSorryToast( editErrorMessage ); @@ -328,7 +296,7 @@ * @param {string} msg Message for sorry message */ function showSorryToast( msg ) { - $( '#ca-edit, .edit-page' ).on( 'click', function ( ev ) { + $allEditLinks.on( 'click', function ( ev ) { popup.show( msg ); ev.preventDefault(); } ); diff --git a/skin.json b/skin.json index d58ab52..d0f8380 100644 --- a/skin.json +++ b/skin.json @@ -535,7 +535,6 @@ "mobile-frontend-editor-disabled", "mobile-frontend-editor-uploadenable", "mobile-frontend-editor-cta", - "mobile-frontend-editor-edit", "apierror-readonly" ], "scripts": [