jenkins-bot has submitted this change and it was merged.
Change subject: Remove most itemremoved events
......................................................................
Remove most itemremoved events
Change-Id: Id1e93bcdc176af663e70aafb3aed91809a951806
---
M view/resources/jquery/wikibase/jquery.wikibase.listview.js
M view/resources/jquery/wikibase/jquery.wikibase.referenceview.js
M view/resources/jquery/wikibase/jquery.wikibase.sitelinklistview.js
M view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
M view/resources/jquery/wikibase/jquery.wikibase.statementview.js
M view/resources/wikibase/view/ViewFactory.js
M view/tests/qunit/jquery/wikibase/jquery.wikibase.referenceview.tests.js
M view/tests/qunit/jquery/wikibase/jquery.wikibase.statementview.tests.js
M view/tests/qunit/wikibase/view/ViewFactory.tests.js
9 files changed, 121 insertions(+), 131 deletions(-)
Approvals:
Thiemo Mättig (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.listview.js
b/view/resources/jquery/wikibase/jquery.wikibase.listview.js
index 86f5c52..3396705 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.listview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.listview.js
@@ -25,13 +25,6 @@
* Node name of the base node of new list items.
*/
/**
- * @event itemremoved
- * Triggered after a list got removed from the list.
- * @param {jQuery.Event} event
- * @param {*|null} value The value of the list item which will be removed.
`null` for empty value.
- * @param {jQuery} $li The list item's DOM node that was removed.
- */
-/**
* @event destroy
* Triggered when the widget has been destroyed.
* @param {jQuery.Event} event
@@ -285,11 +278,10 @@
throw new Error( 'The given node is not an element in
this list' );
}
- var liValue = this._lia.liInstance( $li ).value();
-
this._removeItem( $li );
- this._trigger( 'itemremoved', null, [liValue, $li] );
+ // FIXME: Remove all itemremoved events, see
https://gerrit.wikimedia.org/r/298766.
+ this._trigger( 'itemremoved', null, [null, $li] );
},
_removeItem: function( $li ) {
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.referenceview.js
b/view/resources/jquery/wikibase/jquery.wikibase.referenceview.js
index 4025cca..d407806 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.referenceview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.referenceview.js
@@ -16,7 +16,7 @@
*
* @param {Object} options
* @param {wikibase.datamodel.Reference|null} options.value
- * @param {jQuery.wikibase.listview.ListItemAdapter} options.listItemAdapter
+ * @param {Function} options.getListItemAdapter
*/
/**
* @event afterstartediting
@@ -55,7 +55,7 @@
$listview: '.wikibase-referenceview-listview'
},
value: null,
- listItemAdapter: null
+ getListItemAdapter: null
},
/**
@@ -72,16 +72,20 @@
* @throws {Error} if a required option is not specified properly.
*/
_create: function() {
- if ( !this.options.listItemAdapter ) {
+ if ( !this.options.getListItemAdapter ) {
throw new Error( 'Required option not specified
properly' );
}
PARENT.prototype._create.call( this );
+ var listview;
this.$listview.listview( {
- listItemAdapter: this.options.listItemAdapter,
+ listItemAdapter: this.options.getListItemAdapter(
function( snaklistview ) {
+ listview.removeItem( snaklistview.element );
+ } ),
value: this.options.value ?
this.options.value.getSnaks().getGroupedSnakLists() : []
} );
+ listview = this.$listview.data( 'listview' );
this._updateReferenceHashClass( this.value() );
@@ -101,22 +105,12 @@
var changeEvents = [
'snakviewchange.' + this.widgetName,
lia.prefixedEvent( 'change.' + this.widgetName ),
+ // FIXME: Remove all itemremoved events, see
https://gerrit.wikimedia.org/r/298766.
'listviewitemremoved.' + this.widgetName
];
this.$listview
.on( changeEvents.join( ' ' ), function( event ) {
- if ( event.type === 'listviewitemremoved' ) {
- // Check if last snaklistview item (snakview)
has been removed and remove the
- // listview item (the snaklistview itself) if
so:
- var $snaklistview = $( event.target ).closest(
':wikibase-snaklistview' ),
- snaklistview = lia.liInstance(
$snaklistview );
-
- if ( snaklistview &&
!snaklistview.value().length ) {
- listview.removeItem( $snaklistview );
- }
- }
-
// Propagate "change" event.
self._trigger( 'change' );
} );
@@ -131,7 +125,6 @@
var lia = this.$listview.data( 'listview' ).listItemAdapter(),
events = [
'snakviewchange.' + this.widgetName,
- 'listviewitemremoved.' + this.widgetName,
lia.prefixedEvent( 'change.' + this.widgetName )
];
this.$listview.off( events.join( ' ' ) );
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.sitelinklistview.js
b/view/resources/jquery/wikibase/jquery.wikibase.sitelinklistview.js
index cf8b033..36b8955 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.sitelinklistview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.sitelinklistview.js
@@ -132,6 +132,8 @@
},
function( sitelinkview ) {
self.$listview.data( 'listview'
).removeItem( sitelinkview.element );
+ self._refreshCounter();
+ self._trigger( 'change' );
}
);
@@ -173,16 +175,6 @@
].join( ' ' ),
function( event ) {
event.stopPropagation();
- }
- )
- .on(
- 'listviewitemremoved.' + this.widgetName,
- function( event, sitelinkview ) {
- self._refreshCounter();
- if ( sitelinkview ) {
- // Do not trigger "change" event when
handling empty elements.
- self._trigger( 'change' );
- }
}
);
},
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
b/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
index 7faf6f8..adb0a50 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
@@ -19,10 +19,11 @@
* @param {Object} options
* @param {wikibase.datamodel.SnakList} [value=new
wikibase.datamodel.SnakList()]
* The `SnakList` to be displayed by this view.
- * @param {jQuery.wikibase.listview.ListItemAdapter} options.listItemAdapter
+ * @param {Function} options.getListItemAdapter
* @param {boolean} [singleProperty=true]
* If `true`, it is assumed that the widget is filled with `Snak`s
featuring a single common
* property.
+ * @param {Function} removeCallback A function that removes this snaklistview
*/
/**
* @event afterstartediting
@@ -56,7 +57,8 @@
},
value: null,
singleProperty: false,
- listItemAdapter: null
+ getListItemAdapter: null,
+ removeCallback: null
},
/**
@@ -125,6 +127,11 @@
this.$listview.listview( {
listItemAdapter: this.options.getListItemAdapter(
function( snakview ) {
self._listview.removeItem( snakview.element );
+ if ( self.value().length === 0 ) {
+ self.options.removeCallback();
+ } else {
+ self._trigger( 'change' );
+ }
} ),
value: this.options.value.toArray()
} );
@@ -140,7 +147,8 @@
this.$listview
.off( '.' + this.widgetName )
.on( this._lia.prefixedEvent( 'change.' ) + this.widgetName
- + ' listviewitemremoved.' + this.widgetName, function(
event ) {
+ // FIXME: Remove all itemremoved events, see
https://gerrit.wikimedia.org/r/298766.
+ + ' listviewitemremoved.' + this.widgetName,
function( event ) {
// Forward the "change" event to external
components (e.g. the edit toolbar).
self._trigger( 'change' );
}
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.statementview.js
b/view/resources/jquery/wikibase/jquery.wikibase.statementview.js
index fdd9fe1..eafc65f 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.statementview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.statementview.js
@@ -41,7 +41,7 @@
* Allows to predefine certain aspects of the `Statement` to be created
from the view. If
* this option is omitted, an empty view is created. A common use-case
is adding a value to a
* property existing already by specifying, for example: `{
mainSnak.property: 'P1' }`.
- * @param {jQuery.wikibase.listview.ListItemAdapter}
options.qualifiersListItemAdapter
+ * @param {Function} options.getQualifiersListItemAdapter
* @param {Object} [options.locked={ mainSnak: false }]
* Elements that shall be locked and may not be changed by user
interaction.
* @param {string} [options.helpMessage=mw.msg(
'wikibase-claimview-snak-new-tooltip' )]
@@ -118,12 +118,6 @@
_referencesListview: null,
/**
- * @property {boolean}
- * @private
- */
- _ignoreReferencesListviewChanges: false,
-
- /**
* Reference to the `listview` widget managing the qualifier
`snaklistview`s.
* @property {jQuery.wikibase.listview|null}
* @private
@@ -154,7 +148,7 @@
|| !this.options.buildSnakView
|| !this.options.entityIdPlainFormatter
|| !this.options.guidGenerator
- || !this.options.qualifiersListItemAdapter
+ || !this.options.getQualifiersListItemAdapter
) {
throw new Error( 'Required option not specified
properly' );
}
@@ -276,7 +270,9 @@
$qualifiers = $( '<div/>' ).prependTo( this.$qualifiers
);
}
$qualifiers.listview( {
- listItemAdapter: this.options.qualifiersListItemAdapter,
+ listItemAdapter:
this.options.getQualifiersListItemAdapter( function( snaklistview ) {
+ self._qualifiers.removeItem(
snaklistview.element );
+ } ),
value: groupedQualifierSnaks
} )
.on( 'snaklistviewchange.' + this.widgetName,
@@ -284,22 +280,7 @@
event.stopPropagation();
self._trigger( 'change' );
}
- )
- .on( 'listviewitemremoved.' + this.widgetName, function( event,
value, $itemNode ) {
- if ( event.target === self._qualifiers.element.get( 0 )
) {
- self._trigger( 'change' );
- return;
- }
-
- // Check if last snaklistview of a qualifier listview
item has been removed and
- // remove the listview item if so:
- var $snaklistview = $( event.target ).closest(
':wikibase-snaklistview' ),
- snaklistview = $snaklistview.data(
'snaklistview' );
-
- if ( !snaklistview.value().length ) {
- self._qualifiers.removeItem(
snaklistview.element );
- }
- } );
+ );
this._qualifiers = $qualifiers.data( 'listview' );
},
@@ -322,6 +303,8 @@
var lia = this.options.getReferenceListItemAdapter(
function( referenceview ) {
self._referencesListview.removeItem(
referenceview.element );
+ self._drawReferencesCounter();
+ self._trigger( 'change' );
}
);
@@ -333,17 +316,9 @@
this._referencesListview = $listview.data( 'listview' );
$listview
- .on( 'listviewitemremoved', function( event, value, $li ) {
- if ( self._ignoreReferencesListviewChanges ) {
- return;
- }
- if ( event.target === $listview[0] ) {
- self._drawReferencesCounter();
- }
- self._trigger( 'change' );
- } )
.on( lia.prefixedEvent( 'change.' + this.widgetName ),
function( event ) {
event.stopPropagation();
+ self._drawReferencesCounter();
self._trigger( 'change' );
} );
@@ -687,6 +662,7 @@
this._stopEditingQualifiers( dropValue );
this._rankSelector.stopEditing( dropValue );
+ // FIXME: Should not be necessary if _setOption would do the
right thing for values
this._recreateReferences();
return PARENT.prototype._afterStopEditing.call( this, dropValue
);
@@ -696,11 +672,8 @@
* @protected
*/
_recreateReferences: function() {
- // Normally, statementview would trigger a change event when
references are removed and added
- this._ignoreReferencesListviewChanges = true;
this._referencesListview.option( 'value', this.options.value
? this.options.value.getReferences().toArray()
: [] );
- this._ignoreReferencesListviewChanges = false;
this._drawReferencesCounter();
},
@@ -765,6 +738,7 @@
}
if ( key === 'value' ) {
this.element.toggleClass( 'wb-new', value === null );
+ // FIXME: set the value!
}
return response;
diff --git a/view/resources/wikibase/view/ViewFactory.js
b/view/resources/wikibase/view/ViewFactory.js
index 6b293b6..568e1c8 100644
--- a/view/resources/wikibase/view/ViewFactory.js
+++ b/view/resources/wikibase/view/ViewFactory.js
@@ -451,9 +451,9 @@
),
entityIdPlainFormatter:
this._entityIdPlainFormatter,
getAdder: this._getAdderWithStartEditing(
startEditingCallback ),
+ getQualifiersListItemAdapter:
this.getListItemAdapterForSnakListView.bind( this, startEditingCallback ),
getReferenceListItemAdapter:
this.getListItemAdapterForReferenceView.bind( this, startEditingCallback ),
- guidGenerator: new
wb.utilities.ClaimGuidGenerator( entityId ),
- qualifiersListItemAdapter:
this.getListItemAdapterForSnakListView( startEditingCallback )
+ guidGenerator: new
wb.utilities.ClaimGuidGenerator( entityId )
}
);
return view;
@@ -480,8 +480,8 @@
$dom,
{
value: value || null,
- listItemAdapter:
this.getListItemAdapterForSnakListView( startEditingCallback ),
getAdder: this._getAdderWithStartEditing(
startEditingCallback ),
+ getListItemAdapter:
this.getListItemAdapterForSnakListView.bind( this, startEditingCallback ),
getReferenceRemover: function( $dom ) {
return
structureEditorFactory.getRemover( function() {
return
startEditingCallback().then( function() { return removeCallback( view ); } );
@@ -497,20 +497,37 @@
*
* @return {jQuery.wikibase.listview.ListItemAdapter} The constructed
ListItemAdapter
*/
- SELF.prototype.getListItemAdapterForSnakListView = function(
startEditingCallback ) {
+ SELF.prototype.getListItemAdapterForSnakListView = function(
startEditingCallback, removeCallback ) {
return new $.wikibase.listview.ListItemAdapter( {
listItemWidget: $.wikibase.snaklistview,
- newItemOptionsFn: $.proxy( function( value ) {
- return {
- getListItemAdapter:
this.getListItemAdapterForSnakView.bind( this, startEditingCallback ),
- value: value || undefined,
- singleProperty: true
- };
+ getNewItem: $.proxy( function( value, dom ) {
+ return this.getSnakListView(
startEditingCallback, removeCallback, $( dom ), value );
}, this )
} );
};
/**
+ * Construct a `snaklistview`
+ *
+ * @return {jQuery.wikibase.snaklistview} The constructed snaklistview
+ */
+ SELF.prototype.getSnakListView = function( startEditingCallback,
removeCallback, $dom, value ) {
+ var view = this._getView(
+ 'snaklistview',
+ $dom,
+ {
+ value: value || undefined,
+ singleProperty: true,
+ removeCallback: function() {
+ removeCallback( view );
+ },
+ getListItemAdapter:
this.getListItemAdapterForSnakView.bind( this, startEditingCallback )
+ }
+ );
+ return view;
+ };
+
+ /**
* Construct a `ListItemAdapter` for `snakview`s
*
* @return {jQuery.wikibase.listview.ListItemAdapter} The constructed
ListItemAdapter
diff --git
a/view/tests/qunit/jquery/wikibase/jquery.wikibase.referenceview.tests.js
b/view/tests/qunit/jquery/wikibase/jquery.wikibase.referenceview.tests.js
index 669548e..db8c673 100644
--- a/view/tests/qunit/jquery/wikibase/jquery.wikibase.referenceview.tests.js
+++ b/view/tests/qunit/jquery/wikibase/jquery.wikibase.referenceview.tests.js
@@ -5,6 +5,26 @@
( function( $, wb, QUnit ) {
'use strict';
+ var listItemAdapter = wb.tests.getMockListItemAdapter(
+ 'snaklistview',
+ function() {
+ this.enterNewItem = function() {
+ return $.Deferred().resolve( {
+ data: function() {
+ return { focus: function() {} };
+ }
+ } ).promise();
+ };
+ this.isValid = function() {
+ return false;
+ };
+ this.stopEditing = function() {};
+ this.value = function() {
+ return this.options.value;
+ };
+ }
+ );
+
/**
* Generates a referenceview widget suitable for testing.
*
@@ -23,25 +43,9 @@
destroy: function() {}
};
},
- listItemAdapter: wb.tests.getMockListItemAdapter(
- 'snaklistview',
- function() {
- this.enterNewItem = function() {
- return $.Deferred().resolve( {
- data: function() {
- return { focus:
function() {} };
- }
- } ).promise();
- };
- this.isValid = function() {
- return false;
- };
- this.stopEditing = function() {};
- this.value = function() {
- return this.options.value;
- };
- }
- )
+ getListItemAdapter: function() {
+ return listItemAdapter;
+ }
}, options );
return $( '<div/>' )
diff --git
a/view/tests/qunit/jquery/wikibase/jquery.wikibase.statementview.tests.js
b/view/tests/qunit/jquery/wikibase/jquery.wikibase.statementview.tests.js
index c4074ef..a6d4c7e 100644
--- a/view/tests/qunit/jquery/wikibase/jquery.wikibase.statementview.tests.js
+++ b/view/tests/qunit/jquery/wikibase/jquery.wikibase.statementview.tests.js
@@ -68,14 +68,16 @@
guidGenerator: 'I am a ClaimGuidGenerator',
locked: 'I am a',
predefined: 'I am a',
- qualifiersListItemAdapter: wb.tests.getMockListItemAdapter(
- 'mytestqualifiersview',
- function() {
- this.value = function() {
- return this.options.value;
- };
- }
- )
+ getQualifiersListItemAdapter: function() {
+ return wb.tests.getMockListItemAdapter(
+ 'mytestqualifiersview',
+ function() {
+ this.value = function() {
+ return this.options.value;
+ };
+ }
+ );
+ }
}, options || {} );
$node = $node || $( '<div/>' ).appendTo( 'body' );
diff --git a/view/tests/qunit/wikibase/view/ViewFactory.tests.js
b/view/tests/qunit/wikibase/view/ViewFactory.tests.js
index 2e2d60d..fc4b148 100644
--- a/view/tests/qunit/wikibase/view/ViewFactory.tests.js
+++ b/view/tests/qunit/wikibase/view/ViewFactory.tests.js
@@ -274,11 +274,11 @@
}
},
+ getQualifiersListItemAdapter:
sinon.match.instanceOf( Function ),
getReferenceListItemAdapter:
sinon.match.instanceOf( Function ),
buildSnakView: sinon.match.instanceOf( Function
),
entityIdPlainFormatter: entityIdPlainFormatter,
- guidGenerator: sinon.match.instanceOf(
wb.utilities.ClaimGuidGenerator ),
- qualifiersListItemAdapter:
sinon.match.instanceOf( ListItemAdapter )
+ guidGenerator: sinon.match.instanceOf(
wb.utilities.ClaimGuidGenerator )
} )
);
@@ -378,7 +378,7 @@
referenceview,
sinon.match( {
value: value || null,
- listItemAdapter: sinon.match.instanceOf(
$.wikibase.listview.ListItemAdapter ),
+ getListItemAdapter: sinon.match.func,
getReferenceRemover: sinon.match.func
} )
);
@@ -387,9 +387,8 @@
} );
QUnit.test( 'getListItemAdapterForSnakListView passes correct options
to ListItemAdapter', function( assert ) {
- assert.expect( 3 );
- var value = null,
- viewFactory = new ViewFactory(),
+ assert.expect( 1 );
+ var viewFactory = new ViewFactory(),
ListItemAdapter = sinon.spy( $.wikibase.listview,
'ListItemAdapter' );
viewFactory.getListItemAdapterForSnakListView();
@@ -398,24 +397,33 @@
ListItemAdapter,
sinon.match( {
listItemWidget: $.wikibase.snaklistview,
- newItemOptionsFn: sinon.match.func
+ getNewItem: sinon.match.func
} )
);
- var result = ListItemAdapter.args[0][0].newItemOptionsFn( value
);
+ ListItemAdapter.restore();
+ } );
- assert.deepEqual(
- result,
- {
- getListItemAdapter: result.getListItemAdapter,
// Hack
+ QUnit.test( 'getSnakListView passes correct options to view', function(
assert ) {
+ assert.expect( 1 );
+
+ var value = null,
+ viewFactory = new ViewFactory(),
+ $dom = $( '<div/>' ),
+ stub = sinon.stub( $dom, 'snaklistview' );
+
+ viewFactory.getSnakListView( {}, null, $dom, value );
+
+ sinon.assert.calledWith(
+ stub,
+ sinon.match( {
+ getListItemAdapter: sinon.match.func,
singleProperty: true,
value: value || undefined
- }
- );
+ } )
+ ) ;
- assert.ok( result.getListItemAdapter instanceof Function );
-
- $.wikibase.listview.ListItemAdapter.restore();
+ stub.restore();
} );
QUnit.test( 'getListItemAdapterForSnakView passes correct options to
ListItemAdapter', function( assert ) {
@@ -436,7 +444,7 @@
$.wikibase.listview.ListItemAdapter.restore();
} );
- QUnit.test( 'getSnakView passes correct options to ListItemAdapter',
function( assert ) {
+ QUnit.test( 'getSnakView passes correct options to view', function(
assert ) {
assert.expect( 2 );
var contentLanguages = {},
value = null,
--
To view, visit https://gerrit.wikimedia.org/r/298766
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Id1e93bcdc176af663e70aafb3aed91809a951806
Gerrit-PatchSet: 13
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Adrian Heine <m...@adrianheine.de>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
