Permalink
Browse files

Merge "Improved editable aliases and prepared its DOM structure for g…

…rid layout"
  • Loading branch information...
2 parents 7a5c2a5 + ac1ac42 commit cec56cce659397df5f239e394f6eeb3455448eb8 @tobijat tobijat committed with Gerrit Code Review Aug 23, 2012
View
@@ -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',
@@ -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:
@@ -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
}
/**
@@ -70,6 +73,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
*
* @return wikibase.ui.PropertyEditTool.EditableValue
@@ -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' );
},
/**
@@ -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>'
);
};
@@ -238,6 +238,14 @@ window.wikibase.ui.PropertyEditTool.prototype = {
},
/**
+ * 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.
*/
@@ -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
@@ -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 ) );
@@ -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 );
@@ -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
@@ -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
@@ -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")
@@ -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

0 comments on commit cec56cc

Please sign in to comment.