Henning Snater has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/101198


Change subject: Toolbar edit group widget optimization
......................................................................

Toolbar edit group widget optimization

The change applies some logic and performance improvement to the toolbar 
editgroup widget.
The edit group buttons' characteristics are moved to the options object. Except 
for the
edit button which is displayed initially, the other buttons will be created not 
before
they are needed for interaction. Additionally, the event handlers on each 
button have
been merged to a single event handler applied to the edit group node itself.

Change-Id: I80a5f17c24b20c230062630193b3d3360de77697
---
M lib/resources/jquery.wikibase/toolbar/edittoolbar.js
M lib/resources/jquery.wikibase/toolbar/toolbareditgroup.js
M lib/resources/wikibase.ui.PropertyEditTool.EditableAliases.js
M lib/resources/wikibase.ui.PropertyEditTool.EditableValue.js
M lib/tests/qunit/jquery.wikibase/toolbar/toolbareditgroup.tests.js
5 files changed, 275 insertions(+), 104 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase 
refs/changes/98/101198/1

diff --git a/lib/resources/jquery.wikibase/toolbar/edittoolbar.js 
b/lib/resources/jquery.wikibase/toolbar/edittoolbar.js
index 69d355f..408976e 100644
--- a/lib/resources/jquery.wikibase/toolbar/edittoolbar.js
+++ b/lib/resources/jquery.wikibase/toolbar/edittoolbar.js
@@ -199,9 +199,9 @@
                                        && 
self._interactionWidget.isInitialValue();
 
                                if ( self._interactionWidget.isValid() && 
!isEmpty && !isInitial ) {
-                                       editGroup.$btnSave.data( 
'toolbarbutton' ).enable();
+                                       editGroup.enableButton( '$btnSave' );
                                } else {
-                                       editGroup.$btnSave.data( 
'toolbarbutton' ).disable();
+                                       editGroup.disableButton( '$btnSave' );
                                }
                        } )
                        .on( prefix + 'toggleerror', function( event, error ) {
diff --git a/lib/resources/jquery.wikibase/toolbar/toolbareditgroup.js 
b/lib/resources/jquery.wikibase/toolbar/toolbareditgroup.js
index cb4d831..e2bd735 100644
--- a/lib/resources/jquery.wikibase/toolbar/toolbareditgroup.js
+++ b/lib/resources/jquery.wikibase/toolbar/toolbareditgroup.js
@@ -1,10 +1,10 @@
 /**
- * @file
- * @ingroup WikibaseLib
- *
  * @licence GNU GPL v2+
  * @author H. Snater < [email protected] >
  * @author Daniel Werner
+ *
+ * @option {Object} buttonCharacteristics The characteristics of the edit 
group's buttons.
+ *         Default: Object containing basic information to initialize the 
buttons.
  *
  * @option {boolean} displayRemoveButton Whether to display a "remove" button.
  *         Default: false
@@ -47,6 +47,32 @@
         * @type {Object}
         */
        options: {
+               buttonCharacteristics: {
+                       $btnEdit: {
+                               label: mw.msg( 'wikibase-edit' ),
+                               href: 'javascript:void(0);',
+                               cssClassExt: 'editbutton',
+                               disabled: false
+                       },
+                       $btnSave: {
+                               label: mw.msg( 'wikibase-save' ),
+                               href: 'javascript:void(0);',
+                               cssClassExt: 'savebutton',
+                               disabled: false
+                       },
+                       $btnRemove: {
+                               label: mw.msg( 'wikibase-remove' ),
+                               href: 'javascript:void(0);',
+                               cssClassExt: 'removebutton',
+                               disabled: false
+                       },
+                       $btnCancel: {
+                               label: mw.msg( 'wikibase-cancel' ),
+                               href: 'javascript:void(0);',
+                               cssClassExt: 'cancelbutton',
+                               disabled: false
+                       }
+               },
                displayRemoveButton: false
        },
 
@@ -89,8 +115,21 @@
 
        /**
         * @see jQuery.Widget._create
+        *
+        * @throws {Error} if any button characteristic is not defined.
         */
        _create: function() {
+               var buttonCharacteristics = this.options.buttonCharacteristics;
+
+               if(
+                       !buttonCharacteristics.$btnEdit
+                       || !buttonCharacteristics.$btnSave
+                       || !buttonCharacteristics.$btnRemove
+                       || !buttonCharacteristics.$btnCancel
+               ) {
+                       throw new Error( 'Incomplete button characteristics' );
+               }
+
                PARENT.prototype._create.call( this );
                this._initToolbar();
        },
@@ -156,40 +195,7 @@
                // Tooltip anchor has no disabled/enabled behavior.
                .toolbarlabel( { stateChangeable: false } );
 
-               // Now, create the buttons we need for basic editing:
-               this.$btnEdit = mw.template(
-                       'wikibase-toolbarbutton',
-                       mw.msg( 'wikibase-edit' ),
-                       'javascript:void(0);'
-               )
-               .toolbarbutton();
-               this.$btnEdit.addClass( this.widgetBaseClass + '-editbutton' );
-
-               this.$btnCancel = mw.template(
-                       'wikibase-toolbarbutton',
-                       mw.msg( 'wikibase-cancel' ),
-                       'javascript:void(0);'
-               )
-               .toolbarbutton();
-               this.$btnCancel.addClass( this.widgetBaseClass + 
'-cancelbutton' );
-
-               this.$btnSave = mw.template(
-                       'wikibase-toolbarbutton',
-                       mw.msg( 'wikibase-save' ),
-                       'javascript:void(0);'
-               )
-               .toolbarbutton();
-               this.$btnSave.addClass( this.widgetBaseClass + '-savebutton' );
-
-               this.$btnRemove = mw.template(
-                       'wikibase-toolbarbutton',
-                       mw.msg( 'wikibase-remove' ),
-                       'javascript:void(0);'
-               )
-               .toolbarbutton();
-               this.$btnRemove.addClass( this.widgetBaseClass + 
'-removebutton' );
-
-               this._attachEventHandlers();
+               this._attachEventHandler();
 
                this.toNonEditMode(); // Initialize the toolbar.
        },
@@ -198,34 +204,35 @@
         * Attaches event handlers regarding the toolbar edit group.
         * @since 0.4
         */
-       _attachEventHandlers: function() {
+       _attachEventHandler: function() {
                var self = this;
 
-               this.$btnEdit.on( 'toolbarbuttonaction.' + this.widgetName, 
function( event ) {
-                       self._trigger( 'edit', null, [ function() { 
self.toEditMode(); } ] );
-               } );
-
-               this.$btnCancel.on( 'toolbarbuttonaction.' + this.widgetName, 
function( event ) {
-                       self._trigger( 'cancel', null, [ function() {
-                               if( self.element.data( 'toolbareditgroup' ) ) {
-                                       self.toNonEditMode();
-                               }
-                       } ] );
-               } );
-
-               this.$btnSave.on( 'toolbarbuttonaction.' + this.widgetName, 
function( event ) {
-                       self._trigger( 'save', null, [ function() {
-                               if( self.element.data( 'toolbareditgroup' ) && 
!self.isDisabled() ) {
-                                       // Only toggle EditGroup as long as it 
still exists.
-                                       // If edit group is disabled, the 
interaction target appears to be invalid and
-                                       // edit mode is supposed to persist.
-                                       self.toNonEditMode();
-                               }
-                       } ] );
-               } );
-
-               this.$btnRemove.on( 'toolbarbuttonaction.' + this.widgetName, 
function( event ) {
-                       self._trigger( 'remove' );
+               this.element.on( 'toolbarbuttonaction.' + this.widgetName, 
function( event ) {
+                       switch( event.target ) {
+                               case self.$btnEdit.get( 0 ):
+                                       self._trigger( 'edit', null, [ 
function() { self.toEditMode(); } ] );
+                                       break;
+                               case self.$btnSave.get( 0 ):
+                                       self._trigger( 'save', null, [ 
function() {
+                                               if( self.element.data( 
'toolbareditgroup' ) && !self.isDisabled() ) {
+                                                       // Only toggle 
EditGroup as long as it still exists.
+                                                       // If edit group is 
disabled, the interaction target appears to be invalid and
+                                                       // edit mode is 
supposed to persist.
+                                                       self.toNonEditMode();
+                                               }
+                                       } ] );
+                                       break;
+                               case self.$btnRemove.get( 0 ):
+                                       self._trigger( 'remove' );
+                                       break;
+                               case self.$btnCancel.get( 0 ):
+                                       self._trigger( 'cancel', null, [ 
function() {
+                                               if( self.element.data( 
'toolbareditgroup' ) ) {
+                                                       self.toNonEditMode();
+                                               }
+                                       } ] );
+                                       break;
+                       }
                } );
        },
 
@@ -236,12 +243,18 @@
         * @since 0.4
         */
        toEditMode: function() {
-               this.innerGroup.removeElement( this.$btnEdit );
-               this.innerGroup.addElement( this.$btnSave );
-               if ( this.options.displayRemoveButton ) {
-                       this.innerGroup.addElement( this.$btnRemove );
+               if( this.$btnEdit ) {
+                       this.innerGroup.removeElement( this.$btnEdit );
                }
-               this.innerGroup.addElement( this.$btnCancel );
+
+               this.innerGroup.addElement( this._getButton( '$btnSave' ) );
+
+               if ( this.options.displayRemoveButton ) {
+                       this.innerGroup.addElement( this._getButton( 
'$btnRemove' ) );
+               }
+
+               this.innerGroup.addElement( this._getButton( '$btnCancel' ) );
+
                this.addElement( this.$tooltipAnchor, 1 ); // Insert tooltip 
after interaction links.
 
                this.element
@@ -259,18 +272,78 @@
        toNonEditMode: function() {
                // Remove buttons for editing and display buttons for switching 
back to edit mode:
                this.removeElement( this.$tooltipAnchor );
-               this.innerGroup.removeElement( this.$btnSave );
-               if ( this.options.displayRemoveButton ) {
+
+               if( this.$btnSave ) {
+                       this.innerGroup.removeElement( this.$btnSave );
+               }
+
+               if ( this.options.displayRemoveButton && this.$btnRemove ) {
                        this.innerGroup.removeElement( this.$btnRemove );
                }
-               this.innerGroup.removeElement( this.$btnCancel );
-               this.innerGroup.addElement( this.$btnEdit );
+
+               if( this.$btnCancel ) {
+                       this.innerGroup.removeElement( this.$btnCancel );
+               }
+
+               this.innerGroup.addElement( this._getButton( '$btnEdit' ) );
 
                this.element
                .removeClass( this.widgetBaseClass + '-ineditmode' )
                .addClass( this.widgetBaseClass + '-innoneditmode' );
 
                this.draw();
+       },
+
+       /**
+        * Returns a button by its name creating the button if it has not yet 
been created.
+        *
+        * @param {string} buttonName
+        * @return {jQuery}
+        */
+       _getButton: function( buttonName ) {
+               if( !this[buttonName] ) {
+                       var buttonCharacteristics = 
this.options.buttonCharacteristics[buttonName];
+
+                       this[buttonName] = mw.template(
+                               'wikibase-toolbarbutton',
+                               buttonCharacteristics.label,
+                               buttonCharacteristics.href
+                       )
+                       .toolbarbutton()
+                       .addClass( this.widgetBaseClass + '-' + 
buttonCharacteristics.cssClassExt );
+
+                       if( buttonCharacteristics.disabled ) {
+                               this[buttonName].data( 'toolbarbutton' 
).disable();
+                       }
+               }
+
+               return this[buttonName];
+       },
+
+       /**
+        * Disables a particular edit group button.
+        * @since 0.5
+        *
+        * @param {string} buttonName
+        */
+       disableButton: function( buttonName ) {
+               this.options.buttonCharacteristics[buttonName].disabled = true;
+               if( this[buttonName] ) {
+                       this[buttonName].data( 'toolbarbutton' ).disable();
+               }
+       },
+
+       /**
+        * Enables a particular edit group button.
+        * @since 0.5
+        *
+        * @param {string} buttonName
+        */
+       enableButton: function( buttonName ) {
+               this.options.buttonCharacteristics[buttonName].disabled = false;
+               if( this[buttonName] ) {
+                       this[buttonName].data( 'toolbarbutton' ).enable();
+               }
        },
 
        /**
@@ -319,13 +392,13 @@
                } ) );
 
                // Clone buttons:
-               clone.$btnEdit = this.$btnEdit.data( 'toolbarbutton' ).clone();
-               clone.$btnCancel = this.$btnCancel.data( 'toolbarbutton' 
).clone();
-               clone.$btnSave = this.$btnSave.data( 'toolbarbutton' ).clone();
-               clone.$btnRemove = this.$btnRemove.data( 'toolbarbutton' 
).clone();
+               clone.$btnEdit = this.$btnEdit ? this.$btnEdit.data( 
'toolbarbutton' ).clone() : null;
+               clone.$btnCancel = this.$btnCancel ? this.$btnCancel.data( 
'toolbarbutton' ).clone() : null;
+               clone.$btnSave = this.$btnSave ? this.$btnSave.data( 
'toolbarbutton' ).clone() : null;
+               clone.$btnRemove = this.$btnRemove ? this.$btnRemove.data( 
'toolbarbutton' ).clone() : null;
 
                // Re-attach event handlers:
-               clone._attachEventHandlers();
+               clone._attachEventHandler();
 
                // Re-init the toolbar contents' visibility:
                clone.toNonEditMode();
diff --git a/lib/resources/wikibase.ui.PropertyEditTool.EditableAliases.js 
b/lib/resources/wikibase.ui.PropertyEditTool.EditableAliases.js
index e72460a..212b5ac 100644
--- a/lib/resources/wikibase.ui.PropertyEditTool.EditableAliases.js
+++ b/lib/resources/wikibase.ui.PropertyEditTool.EditableAliases.js
@@ -62,8 +62,7 @@
                        
PARENT.prototype._interfaceHandler_onInputRegistered.call( this, 
relatedInterface );
 
                        // Always enable cancel button since it is alright to 
have an empty value:
-                       var editGroup = this._toolbar.$editGroup.data( 
'toolbareditgroup' );
-                       editGroup.$btnCancel.data( 'toolbarbutton' ).enable();
+                       this._toolbar.$editGroup.data( 'toolbareditgroup' 
).enableButton( '$btnCancel' );
                }
        },
 
diff --git a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.js 
b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.js
index 285967c..031fbce 100644
--- a/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.js
+++ b/lib/resources/wikibase.ui.PropertyEditTool.EditableValue.js
@@ -526,7 +526,7 @@
                } );
 
                var editGroup = this._toolbar.$editGroup.data( 
'toolbareditgroup' );
-               if( editGroup.$btnSave.data( 'wbtooltip' ) ) {
+               if( editGroup.$btnSave && editGroup.$btnSave.data( 'wbtooltip' 
) ) {
                        editGroup.$btnSave.data( 'wbtooltip' ).degrade();
                }
                editGroup.$tooltipAnchor.data( 'wbtooltip' ).hide();
@@ -937,8 +937,8 @@
                        !this.isPending() && this.isEmpty() && this.isNew();
 
                var editGroup = this._toolbar.$editGroup.data( 
'toolbareditgroup' );
-               editGroup.$btnSave.data( 'toolbarbutton' )[ disableSave ? 
'disable' : 'enable' ]();
-               editGroup.$btnCancel.data( 'toolbarbutton' )[ disableCancel ? 
'disable' : 'enable' ]();
+               editGroup[ disableSave ? 'disableButton' : 'enableButton' ]( 
'$btnSave' );
+               editGroup[ disableCancel ? 'disableButton' : 'enableButton' ]( 
'$btnCancel' );
 
                /**
                 * Propagate stopping of edit mode (enabling other actions) 
when all editable value actions
diff --git a/lib/tests/qunit/jquery.wikibase/toolbar/toolbareditgroup.tests.js 
b/lib/tests/qunit/jquery.wikibase/toolbar/toolbareditgroup.tests.js
index 08e11f3..8ea9a2c 100644
--- a/lib/tests/qunit/jquery.wikibase/toolbar/toolbareditgroup.tests.js
+++ b/lib/tests/qunit/jquery.wikibase/toolbar/toolbareditgroup.tests.js
@@ -1,8 +1,4 @@
 /**
- * @since 0.1
- * @file
- * @ingroup WikibaseLib
- *
  * @licence GNU GPL v2+
  * @author H. Snater < [email protected] >
  */
@@ -12,18 +8,18 @@
 
        QUnit.module( 'jQuery.wikibase.toolbareditgroup', 
QUnit.newWbEnvironment() );
 
-       QUnit.test( 'init check', function( assert ) {
+       QUnit.test( 'Create & Destroy', function( assert ) {
                var $editGroup = $( '<span/>' ).toolbareditgroup(),
                        editGroup = $editGroup.data( 'toolbareditgroup' );
 
                assert.ok(
                        editGroup.innerGroup instanceof jQuery.wikibase.toolbar,
-                       'Initiated inner group.'
+                       'Created inner group.'
                );
 
                assert.ok(
                        editGroup.$tooltipAnchor instanceof $,
-                       'Initiated tooltip.'
+                       'Created tooltip.'
                );
 
                assert.equal(
@@ -34,22 +30,25 @@
 
                assert.ok(
                        editGroup.$btnEdit.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
-                       'Initiated edit button.'
+                       'Created edit button.'
                );
 
-               assert.ok(
-                       editGroup.$btnCancel.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
-                       'Initiated cancel button.'
+               assert.strictEqual(
+                       editGroup.$btnCancel,
+                       null,
+                       'Cancel button not yet initialized.'
                );
 
-               assert.ok(
-                       editGroup.$btnSave.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
-                       'Initiated save button.'
+               assert.strictEqual(
+                       editGroup.$btnSave,
+                       null,
+                       'Save button not yet initialized.'
                );
 
-               assert.ok(
-                       editGroup.$btnRemove.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
-                       'Initiated remove button.'
+               assert.strictEqual(
+                       editGroup.$btnRemove,
+                       null,
+                       'Remove button not yet initialized.'
                );
 
                assert.equal(
@@ -93,19 +92,119 @@
                assert.equal(
                        editGroup.$btnCancel,
                        null,
-                       'Destroyed cancel button.'
+                       'Cancel button does not exist.'
                );
 
                assert.equal(
                        editGroup.$btnSave,
                        null,
-                       'Destroyed save button.'
+                       'Save button does not exist.'
                );
 
                assert.equal(
                        editGroup.$btnRemove,
                        null,
-                       'Destroyed remove button.'
+                       'Remove button does not exist.'
+               );
+
+       } );
+
+       QUnit.test( 'toEditMode() & toNonEditMode()', function( assert ) {
+               var $editGroup = $( '<span/>' ).toolbareditgroup( {
+                               displayRemoveButton: false
+                       } ),
+                       editGroup = $editGroup.data( 'toolbareditgroup' );
+
+               editGroup.toEditMode();
+
+               assert.ok(
+                       editGroup.$btnEdit.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
+                       'Created edit button.'
+               );
+
+               assert.ok(
+                       editGroup.$btnCancel.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
+                       'Created cancel button.'
+               );
+
+               assert.ok(
+                       editGroup.$btnSave.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
+                       'Created save button.'
+               );
+
+               assert.strictEqual(
+                       editGroup.$btnRemove,
+                       null,
+                       'Remove button is not initialized.'
+               );
+
+               assert.ok(
+                       !editGroup.innerGroup.hasElement( editGroup.$btnEdit ),
+                       'Edit button is not visible.'
+               );
+
+               assert.ok(
+                       editGroup.innerGroup.hasElement( editGroup.$btnSave ),
+                       'Save button is visible.'
+               );
+
+               assert.ok(
+                       editGroup.innerGroup.hasElement( editGroup.$btnCancel ),
+                       'Cancel button is visible.'
+               );
+
+               assert.ok(
+                       editGroup.hasElement( editGroup.$tooltipAnchor ),
+                       'Tooltip anchor is visible.'
+               );
+
+               editGroup.toNonEditMode();
+
+               assert.ok(
+                       editGroup.innerGroup.hasElement( editGroup.$btnEdit ),
+                       'Edit button is visible.'
+               );
+
+               assert.ok(
+                       !editGroup.innerGroup.hasElement( editGroup.$btnSave ),
+                       'Save button is not visible.'
+               );
+
+               assert.ok(
+                       !editGroup.innerGroup.hasElement( editGroup.$btnCancel 
),
+                       'Cancel button is not visible.'
+               );
+
+               assert.ok(
+                       !editGroup.hasElement( editGroup.$tooltipAnchor ),
+                       'Tooltip anchor is not visible.'
+               );
+
+       } );
+
+       QUnit.test( 'Remove button handling on toEditMode() & toNonEditMode()', 
function( assert ) {
+               var $editGroup = $( '<span/>' ).toolbareditgroup( {
+                               displayRemoveButton: true
+                       } ),
+                       editGroup = $editGroup.data( 'toolbareditgroup' );
+
+               editGroup.toEditMode();
+
+               assert.ok(
+                       editGroup.$btnRemove.data( 'toolbarbutton' ) instanceof 
$.wikibase.toolbarbutton,
+                       'Created remove button.'
+               );
+
+               assert.ok(
+                       editGroup.innerGroup.hasElement( editGroup.$btnRemove ),
+                       'Remove button is visible.'
+               );
+
+               editGroup.toNonEditMode();
+
+               assert.ok(
+                       !editGroup.innerGroup.hasElement( editGroup.$btnRemove 
),
+                       'Remove button is not visible.'
                );
 
        } );

-- 
To view, visit https://gerrit.wikimedia.org/r/101198
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I80a5f17c24b20c230062630193b3d3360de77697
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Henning Snater <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to