Fix: update edit button lock style state

When a user is allowed to make edits, show a normal edit button. When a
user cannot make edits, show the locked button.

This patch refactors edit button presentation logic into a new function,
updateEditPageButton(), which consistently updates the UI for both
enabled and disabled states. Additionally, in cases where the old code
only displayed the button via `$caEdit.removeClass( 'hidden' )`, the new
code now updates the state appropriately which is a functional change.
Finally, this patch sprinkles in some TODOs for future minor refactors
that were identified while creating this patch.

Bug: T190834
Change-Id: I083e91f0328cc057541ad42a27aae31b32b3d050
This commit is contained in:
Stephen Niedzielski 2018-04-06 14:52:07 -05:00 committed by Jdlrobson
parent 1eddc3af5e
commit b106ed3219

View file

@ -11,6 +11,11 @@
Button = M.require( 'mobile.startup/Button' ), Button = M.require( 'mobile.startup/Button' ),
Anchor = M.require( 'mobile.startup/Anchor' ), Anchor = M.require( 'mobile.startup/Anchor' ),
skin = M.require( 'skins.minerva.scripts/skin' ), 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( { disabledEditIcon = new Icon( {
name: 'edit', name: 'edit',
glyphPrefix: 'minerva' glyphPrefix: 'minerva'
@ -19,9 +24,12 @@
name: 'edit-enabled', name: 'edit-enabled',
glyphPrefix: 'minerva' glyphPrefix: 'minerva'
} ), } ),
currentPage = M.getCurrentPage(), // TODO: move enabledClass, $caEdit, and disabledClass to locals within
// updateEditPageButton().
enabledClass = enabledEditIcon.getGlyphClassName(), enabledClass = enabledEditIcon.getGlyphClassName(),
disabledClass = disabledEditIcon.getGlyphClassName(), disabledClass = disabledEditIcon.getGlyphClassName(),
// TODO: rename to editPageButton.
$caEdit = $( '#ca-edit' ),
user = M.require( 'mobile.startup/user' ), user = M.require( 'mobile.startup/user' ),
popup = M.require( 'mobile.startup/toast' ), popup = M.require( 'mobile.startup/toast' ),
// FIXME: Disable on IE < 10 for time being // FIXME: Disable on IE < 10 for time being
@ -35,8 +43,7 @@
// FIXME: Should we consider default site options and user prefs? // FIXME: Should we consider default site options and user prefs?
isVisualEditorEnabled = veConfig, isVisualEditorEnabled = veConfig,
CtaDrawer = M.require( 'mobile.startup/CtaDrawer' ), CtaDrawer = M.require( 'mobile.startup/CtaDrawer' ),
drawer, drawer;
$caEdit = $( '#ca-edit' );
if ( user.isAnon() ) { if ( user.isAnon() ) {
blockInfo = false; blockInfo = false;
@ -44,6 +51,9 @@
// for logged in users check if they are blocked from editing this page // for logged in users check if they are blocked from editing this page
isEditable = !blockInfo; isEditable = !blockInfo;
} }
// TODO: 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 * Prepend an edit page button to the container
* Remove any existing links in the container * Remove any existing links in the container
@ -64,6 +74,18 @@
.prependTo( container ); .prependTo( container );
} }
/**
* @param {boolean} enabled
* @return {void}
*/
function updateEditPageButton( enabled ) {
$caEdit
.addClass( enabled ? enabledClass : disabledClass )
.removeClass( enabled ? disabledClass : enabledClass )
// TODO: can hidden be removed from the default state?
.removeClass( 'hidden' );
}
/** /**
* Make an element render a CTA when clicked * Make an element render a CTA when clicked
* @method * @method
@ -229,7 +251,7 @@
return result; return result;
} ); } );
$caEdit.addClass( enabledClass ).removeClass( disabledClass ).removeClass( 'hidden' ); updateEditPageButton( true );
// reveal edit links on user pages // reveal edit links on user pages
page.$( '.edit-link' ).removeClass( 'hidden' ); page.$( '.edit-link' ).removeClass( 'hidden' );
currentPage.getRedLinks().on( 'click', function ( ev ) { currentPage.getRedLinks().on( 'click', function ( ev ) {
@ -290,10 +312,11 @@
*/ */
function init() { function init() {
if ( isEditable ) { if ( isEditable ) {
// Edit button updated in setupEditor.
setupEditor( currentPage ); setupEditor( currentPage );
} else { } else {
updateEditPageButton( false );
if ( blockInfo ) { if ( blockInfo ) {
$caEdit.removeClass( 'hidden' );
$( '#ca-edit' ).on( 'click', function ( ev ) { $( '#ca-edit' ).on( 'click', function ( ev ) {
popup.show( popup.show(
mw.msg( mw.msg(
@ -310,7 +333,6 @@
} ); } );
$( '.edit-page' ).detach(); $( '.edit-page' ).detach();
} else { } else {
$caEdit.removeClass( 'hidden' );
showSorryToast( mw.msg( 'mobile-frontend-editor-disabled' ) ); showSorryToast( mw.msg( 'mobile-frontend-editor-disabled' ) );
} }
} }
@ -325,7 +347,7 @@
// Initialize edit button links (to show Cta) only, if page is editable, // Initialize edit button links (to show Cta) only, if page is editable,
// otherwise show an error toast // otherwise show an error toast
if ( isEditable ) { if ( isEditable ) {
$caEdit.addClass( enabledClass ).removeClass( disabledClass ).removeClass( 'hidden' ); updateEditPageButton( true );
// Init lead section edit button // Init lead section edit button
makeCta( $caEdit, 0 ); makeCta( $caEdit, 0 );
@ -340,7 +362,7 @@
makeCta( $a, section ); makeCta( $a, section );
} ); } );
} else { } else {
$caEdit.removeClass( 'hidden' ); updateEditPageButton( false );
showSorryToast( mw.msg( 'mobile-frontend-editor-disabled' ) ); showSorryToast( mw.msg( 'mobile-frontend-editor-disabled' ) );
} }
} }
@ -364,13 +386,15 @@
return; return;
} else if ( !isEditingSupported ) { } else if ( !isEditingSupported ) {
// Editing is disabled (or browser is blacklisted) // Editing is disabled (or browser is blacklisted)
$caEdit.removeClass( 'hidden' ); updateEditPageButton( false );
showSorryToast( mw.msg( 'mobile-frontend-editor-unavailable' ) ); showSorryToast( mw.msg( 'mobile-frontend-editor-unavailable' ) );
} else if ( isNewFile ) { } else if ( isNewFile ) {
$caEdit.removeClass( 'hidden' ); updateEditPageButton( true );
// Is a new file page (enable upload image only) Bug 58311 // Is a new file page (enable upload image only) Bug 58311
showSorryToast( mw.msg( 'mobile-frontend-editor-uploadenable' ) ); showSorryToast( mw.msg( 'mobile-frontend-editor-uploadenable' ) );
} else { } else {
// Edit button is currently hidden. A call to init() / initCta() will update
// it as needed.
if ( user.isAnon() ) { if ( user.isAnon() ) {
// Cta's will be rendered in EditorOverlay, if anonymous editing is enabled. // Cta's will be rendered in EditorOverlay, if anonymous editing is enabled.
if ( mw.config.get( 'wgMFEditorOptions' ).anonymousEditing ) { if ( mw.config.get( 'wgMFEditorOptions' ).anonymousEditing ) {
@ -382,5 +406,4 @@
init(); init();
} }
} }
}( mw.mobileFrontend, jQuery ) ); }( mw.mobileFrontend, jQuery ) );