Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

Commit

Permalink
Improved editable aliases and prepared its DOM structure for grid layout
Browse files Browse the repository at this point in the history
the AliasesEditTool doesn't initialize with an empty EditableAliases anymore. Instead we initialize an empty AliasesEditTool if no aliases are set yet, and initialize the EditableAliases instance on demand. This solves problems we had before with the button and solves some minor hackery.
Also prepared DOM structure for grid layout. This requires that aliases label (part of AliasesEditTool) and the value (part of EditableAliases) are structured within one DOM node to have them together as block element which we can give some padding on the right to overlay the edit tools.

- patch-set 2: added QUnit tests for AliasesEditTool and implemented destroy function
- patch-set 5: rebase and fixed conflicts
- patch-set 6: adjusted tests for the new aliases DOM structure

Change-Id: Ie07985986450550d1efa9a27770b2d62bfc01e41
  • Loading branch information
DanweDE committed Aug 23, 2012
1 parent de91e5e commit ac1ac42
Show file tree
Hide file tree
Showing 8 changed files with 140 additions and 26 deletions.
1 change: 1 addition & 0 deletions repo/Wikibase.hooks.php
Expand Up @@ -148,6 +148,7 @@ public static function onResourceLoaderTestModules( array &$testModules, \Resour
'scripts' => array(
'tests/qunit/wikibase.tests.js',
'tests/qunit/wikibase.Site.tests.js',
'tests/qunit/wikibase.ui.AliasesEditTool.tests.js',
'tests/qunit/wikibase.ui.DescriptionEditTool.tests.js',
'tests/qunit/wikibase.ui.LabelEditTool.tests.js',
'tests/qunit/wikibase.ui.SiteLinksEditTool.tests.js',
Expand Down
39 changes: 31 additions & 8 deletions repo/resources/wikibase.ui.AliasesEditTool.js
Expand Up @@ -30,6 +30,13 @@ $.extend( window.wikibase.ui.AliasesEditTool.prototype, {
* @see wikibase.ui.PropertyEditTool._init
*/
_init: function( subject ) {
subject = $( subject );
// we need this additional block element for the grid layout; it will contain the aliases label + editable value
// NOTE: this is just because of the label It would be possible to add the functionality of having a label
// including setter/getter functions into PropertyEditTool directly but it doesn't seem necessary
$( '<div/>', { 'class': 'wb-gridhelper' } ).append( subject.children() ).appendTo( subject );
// .wb-gridhelper will also be returned in _getValuesParent()

// call prototype's _init():
window.wikibase.ui.PropertyEditTool.prototype._init.call( this, subject );
// add class specific to this ui element:
Expand All @@ -39,12 +46,8 @@ $.extend( window.wikibase.ui.AliasesEditTool.prototype, {
this._toolbar.hide(); // hide add button when hitting it since edit mode toolbar will appear
}, this ) );

if ( this.getValues()[0].getValue()[0].length === 0 ) {
/* remove EditableValue by triggering leaving edit mode to get rid of edit button when there are no aliases
yet (using the PropertyEditTool's add button instead) */
$( this.getValues()[0].getToolbar().editGroup.btnCancel ).triggerHandler( 'action' );
} else {
this._toolbar.hide(); // hide add button when there are aliases
if( this._editableValues.length > 0 && this._editableValues[0].getValue().length > 0 ) {
this._toolbar.hide(); // hide add button if there are any aliases
}

/**
Expand All @@ -69,6 +72,27 @@ $.extend( window.wikibase.ui.AliasesEditTool.prototype, {

},

/**
* @see wikibase.ui.PropertyEditTool.destroy
*/
destroy: function() {
// don't forget to remove injected node again:
var gridHelper = this._subject.children( '.wb-gridhelper' );
gridHelper.replaceWith( gridHelper.children() );
window.wikibase.ui.PropertyEditTool.prototype.destroy.call( this );

},

/**
* @see wikibase.ui.PropertyEditTool.EditableValue._getValuesParent
*
* @return jQuery
*/
_getValuesParent: function() {
// grid layout helper constructed in the _init() function
return this._subject.children( '.wb-gridhelper:first' );
},

/**
* @see wikibase.ui.PropertyEditTool._initSingleValue
*
Expand Down Expand Up @@ -125,7 +149,7 @@ $.extend( window.wikibase.ui.AliasesEditTool.prototype, {
* @return jQuery
*/
_getValueElems: function() {
return this._subject.children( '.wb-aliases-container:first' );
return this._subject.find( '.wb-aliases-container:first' );
},

/**
Expand Down Expand Up @@ -169,7 +193,6 @@ window.wikibase.ui.AliasesEditTool.getEmptyStructure = function() {
return $(
'<div class="wb-aliases">' +
'<span class="wb-aliases-label">' + mw.message( 'wikibase-aliases-label' ).escaped() + '</span>' +
'<ul class="wb-aliases-container"></ul>' +
'</div>'
);
};
10 changes: 9 additions & 1 deletion repo/resources/wikibase.ui.PropertyEditTool.js
Expand Up @@ -237,6 +237,14 @@ window.wikibase.ui.PropertyEditTool.prototype = {
return this._subject;
},

/**
* Returns the node the value(s) should be appended to
*
* @return jQuery
*/
_getValuesParent: function() {
return this._subject;
},
/**
* Collects all values represented within the DOM already and initializes EditableValue instances
* for them.
Expand Down Expand Up @@ -388,7 +396,7 @@ window.wikibase.ui.PropertyEditTool.prototype = {
var newValueElem = this._newEmptyValueDOM(); // get DOM for new empty value
newValueElem.addClass( 'wb-pending-value' );

this._subject.append( newValueElem );
this._getValuesParent().append( newValueElem );
var newValue = this._initSingleValue( newValueElem );

if ( !this.allowsFullErase ) { // on allowsFullErase, add button will be hidden when not in use
Expand Down
87 changes: 87 additions & 0 deletions repo/tests/qunit/wikibase.ui.AliasesEditTool.tests.js
@@ -0,0 +1,87 @@
/**
* QUnit tests for aliases edit tool
* @see https://www.mediawiki.org/wiki/Extension:Wikibase
*
* @since 0.1
* @file
* @ingroup Wikibase
*
* @licence GNU GPL v2+
* @author Daniel Werner
*/
( function( mw, wb, $, QUnit ) {
'use strict';
QUnit.module( 'wikibase.ui.AliasesEditTool', QUnit.newWbEnvironment( {
setup: function() {
/**
* Holds the original dom structure the AliasesEditTool was initialized with
* @var jQuery
*/
var initialStructure = wb.ui.AliasesEditTool.getEmptyStructure();
this.initialStructureMembers = initialStructure.children();
this.subject = new wb.ui.AliasesEditTool( initialStructure );

QUnit.assert.ok(
this.subject instanceof wb.ui.AliasesEditTool,
'instantiated AliasesEditTool'
);
},
teardown: function() {
var self = this;
var values = this.subject.getValues();
this.subject.destroy();

// basic check whether initial structure was restored
var initialStructure = true;
this.initialStructureMembers.each( function() {
initialStructure = initialStructure && $( this ).parent().is( self.subject._subject );
} );

QUnit.assert.ok(
initialStructure,
'DOM nodes from initial aliases edit tool are in the right place again'
);

QUnit.assert.equal(
this.initialStructureMembers.length + values.length,
this.subject._subject.children().length,
'No additional DOM nodes left (except those of inserted values)'
);

this.subject = null;
this.initialStructureMembers = null;
}

} ) );

// base for following tests, creates some values
var initAliasesTest = function( assert ) {
var newVal = this.subject.enterNewValue( [ 'alias 1', 'two', 'three' ] );
assert.ok(
newVal instanceof wb.ui.PropertyEditTool.EditableAliases,
'Value entered has instance of EditableAlias'
);
};

// This is the same as the next test, but the next one does additional stuff, so the destroy() in teardown
// might fail independently, depending on the edit tools state.
QUnit.test( 'Test with creating new EditableAliases', initAliasesTest );

QUnit.test( 'Test creating and removing EditableAliases from edit tool', function( assert ) {
initAliasesTest.call( this, assert );

var aliasesValue = this.subject.getValues()[0];

aliasesValue.queryApi = function( deferred, apiAction ) { // override AJAX API call
deferred.resolve().promise();
};
aliasesValue.remove();

assert.ok(
this.subject.getValues.length,
0,
'Empty after removing EditableAliases instance'
);
} );

}( mediaWiki, wikibase, jQuery, QUnit ) );
Expand Up @@ -18,7 +18,7 @@
setup: function() {
var node = $( '<div/>', { id: 'parent' } );
this.propertyEditTool = new window.wikibase.ui.PropertyEditTool( node );
this.editableValue = new window.wikibase.ui.PropertyEditTool.EditableValue;
this.editableValue = new window.wikibase.ui.PropertyEditTool.EditableValue();

var toolbar = this.propertyEditTool._buildSingleValueToolbar( this.editableValue );
this.editableValue._init( node, toolbar );
Expand Down
6 changes: 4 additions & 2 deletions repo/tests/selenium/aliases/aliases_bugs_spec.rb
Expand Up @@ -34,13 +34,15 @@
page.wait_for_aliases_to_load
page.wait_for_item_to_load
page.addAliases
page.addAliasesDivInEditMode_element.style("display").should == "none"
page.addAliases?.should be_false
#page.addAliasesDivInEditMode_element.style("display").should == "none"
page.cancelAliases?.should be_true
page.cancelAliases
page.addAliases?.should be_true
page.cancelAliases?.should be_false
page.addAliases
page.addAliasesDivInEditMode_element.style("display").should == "none"
#page.addAliasesDivInEditMode_element.style("display").should == "none"
page.addAliases?.should be_false
page.cancelAliases?.should be_true
page.cancelAliases
end
Expand Down
2 changes: 1 addition & 1 deletion repo/tests/selenium/aliases/aliases_spec.rb
Expand Up @@ -119,7 +119,7 @@
page.editAliases?.should be_false
page.cancelAliases?.should be_true
page.aliasesTitle?.should be_true
page.aliasesListInEditMode?.should be_true
page.aliasesList?.should be_true
page.aliasesInputEmpty?.should be_true

# check functionality of cancel
Expand Down
19 changes: 6 additions & 13 deletions selenium/lib/pages/aliases_item_page.rb
Expand Up @@ -16,15 +16,14 @@ class AliasesItemPage < SitelinksItemPage
# aliases UI
div(:aliasesDiv, :class => "wb-aliases")
span(:aliasesTitle, :class => "wb-aliases-label")
unordered_list(:aliasesList, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool']/span[2]/div/ul")
unordered_list(:aliasesListInEditMode, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool wb-ui-propertyedittool-ineditmode']/span[2]/div/ul")
unordered_list(:aliasesList, :class => "wb-aliases-container")
div(:addAliasesDiv, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool']/div")
div(:addAliasesDivInEditMode, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool wb-ui-propertyedittool-ineditmode']/div")
link(:addAliases, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool']/div/div/a[text()='add']")
span(:addAliasesDisabled, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool']/div/div/span")
link(:editAliases, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool']/span[2]/span/div/div/div/a[text()='edit']")
link(:saveAliases, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool wb-ui-propertyedittool-ineditmode']/span[2]/span/div/div/div/a[text()='save']")
link(:cancelAliases, :xpath => "//div[@class='wb-aliases wb-ui-propertyedittool wb-ui-aliasesedittool wb-ui-propertyedittool-ineditmode']/span[2]/span/div/div/div/a[text()='cancel']")
link(:editAliases, :xpath => "//div[contains(@class, 'wb-aliases')]/div/span[contains(@class, 'wb-ui-propertyedittool-editablevalue')]/span/div/div/div/a[text()='edit']")
link(:saveAliases, :xpath => "//div[contains(@class, 'wb-aliases')]/div/span[contains(@class, 'wb-ui-propertyedittool-editablevalue')]/span/div/div/div/a[text()='save']")
link(:cancelAliases, :xpath => "//div[contains(@class, 'wb-aliases')]/div/span[contains(@class, 'wb-ui-propertyedittool-editablevalue')]/span/div/div/div/a[text()='cancel']")
text_field(:aliasesInputFirst, :xpath => "//li[@class='tagadata-choice ui-widget-content ui-state-default ui-corner-all wb-aliases-alias']/span/input")
link(:aliasesInputFirstRemove, :xpath => "//li[@class='tagadata-choice ui-widget-content ui-state-default ui-corner-all wb-aliases-alias']/a[@class='tagadata-close']")
text_field(:aliasesInputEmpty, :xpath => "//li[@class='tagadata-choice ui-widget-content ui-state-default ui-corner-all tagadata-choice-empty']/span/input")
Expand All @@ -39,14 +38,8 @@ def wait_for_aliases_to_load

def countExistingAliases
count = 0
if aliasesList_element.exists?
aliasesList_element.each do |aliasElem|
count = count+1
end
else
aliasesListInEditMode_element.each do |aliasElem|
count = count+1
end
aliasesList_element.each do |aliasElem|
count = count+1
end
return count
end
Expand Down

0 comments on commit ac1ac42

Please sign in to comment.