From ac1ac427638ff2a9db53b1f071dc65be19f07563 Mon Sep 17 00:00:00 2001 From: Daniel Werner Date: Tue, 21 Aug 2012 11:26:05 +0200 Subject: [PATCH] Improved editable aliases and prepared its DOM structure for grid layout 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 --- repo/Wikibase.hooks.php | 1 + repo/resources/wikibase.ui.AliasesEditTool.js | 39 +++++++-- .../resources/wikibase.ui.PropertyEditTool.js | 10 ++- .../wikibase.ui.AliasesEditTool.tests.js | 87 +++++++++++++++++++ ...ui.PropertyEditTool.EditableValue.tests.js | 2 +- .../selenium/aliases/aliases_bugs_spec.rb | 6 +- repo/tests/selenium/aliases/aliases_spec.rb | 2 +- selenium/lib/pages/aliases_item_page.rb | 19 ++-- 8 files changed, 140 insertions(+), 26 deletions(-) create mode 100644 repo/tests/qunit/wikibase.ui.AliasesEditTool.tests.js diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php index df9b4548b7..efed83a2f3 100644 --- a/repo/Wikibase.hooks.php +++ b/repo/Wikibase.hooks.php @@ -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', diff --git a/repo/resources/wikibase.ui.AliasesEditTool.js b/repo/resources/wikibase.ui.AliasesEditTool.js index 5233635044..6fa960c012 100644 --- a/repo/resources/wikibase.ui.AliasesEditTool.js +++ b/repo/resources/wikibase.ui.AliasesEditTool.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 + $( '
', { '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 } /** @@ -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 * @@ -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 $( '
' + '' + mw.message( 'wikibase-aliases-label' ).escaped() + '' + - '
    ' + '
    ' ); }; \ No newline at end of file diff --git a/repo/resources/wikibase.ui.PropertyEditTool.js b/repo/resources/wikibase.ui.PropertyEditTool.js index 1180ce65a4..a80c1d4602 100644 --- a/repo/resources/wikibase.ui.PropertyEditTool.js +++ b/repo/resources/wikibase.ui.PropertyEditTool.js @@ -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. @@ -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 diff --git a/repo/tests/qunit/wikibase.ui.AliasesEditTool.tests.js b/repo/tests/qunit/wikibase.ui.AliasesEditTool.tests.js new file mode 100644 index 0000000000..6fa2bc9157 --- /dev/null +++ b/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 ) ); diff --git a/repo/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.tests.js b/repo/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.tests.js index 8ccfdac321..7c6c72c9b9 100644 --- a/repo/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.tests.js +++ b/repo/tests/qunit/wikibase.ui.PropertyEditTool.EditableValue.tests.js @@ -18,7 +18,7 @@ setup: function() { var node = $( '
    ', { 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 ); diff --git a/repo/tests/selenium/aliases/aliases_bugs_spec.rb b/repo/tests/selenium/aliases/aliases_bugs_spec.rb index 6bd08a613a..16decbf57c 100644 --- a/repo/tests/selenium/aliases/aliases_bugs_spec.rb +++ b/repo/tests/selenium/aliases/aliases_bugs_spec.rb @@ -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 diff --git a/repo/tests/selenium/aliases/aliases_spec.rb b/repo/tests/selenium/aliases/aliases_spec.rb index 9fad04792d..3b21c70ba2 100644 --- a/repo/tests/selenium/aliases/aliases_spec.rb +++ b/repo/tests/selenium/aliases/aliases_spec.rb @@ -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 diff --git a/selenium/lib/pages/aliases_item_page.rb b/selenium/lib/pages/aliases_item_page.rb index 9bc8e05c78..18199bed44 100644 --- a/selenium/lib/pages/aliases_item_page.rb +++ b/selenium/lib/pages/aliases_item_page.rb @@ -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