jenkins-bot has submitted this change and it was merged.
Change subject: Use ViewFactory for building snakviews in snaklistview
......................................................................
Use ViewFactory for building snakviews in snaklistview
This change introduces a `ViewFactory` method for building `ListItemAdapter`s
for `snakview` widgets. This `ListItemAdapter` is passed to the `snaklistview`
so that it can create the widgets without having to pass them all their
dependencies.
This change leaves `listview` as an implementation detail of `snaklistview`.
This implementation detail is publicly represented by having to pass in a
`listview.ListItemAdapter`.
It improves separation between the `snaklistview` and `snakview`, since the
`snaklistview` doesn't have to know how to construct a `snakview` anymore. It
also allows to inject a different implementation, for example in tests.
`snakview::propertyLabelIsVisible` is removed, since it was only used in the
`snaklistview` tests. I also removed a duplicate `change` event binding.
Bug: T75380
Change-Id: I3f89b733139d7ced99b9044305298b723f4e9e80
---
M view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
M view/resources/jquery/wikibase/resources.php
M view/resources/jquery/wikibase/snakview/snakview.js
M view/resources/wikibase/view/ViewFactory.js
M view/tests/qunit/jquery/wikibase/jquery.wikibase.snaklistview.tests.js
M view/tests/qunit/jquery/wikibase/resources.php
M view/tests/qunit/wikibase/view/ViewFactory.tests.js
7 files changed, 126 insertions(+), 131 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
b/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
index 013c893..762487b 100644
--- a/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
+++ b/view/resources/jquery/wikibase/jquery.wikibase.snaklistview.js
@@ -19,21 +19,10 @@
* @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 {boolean} [singleProperty=true]
* If `true`, it is assumed that the widget is filled with `Snak`s
featuring a single common
* property.
- * @param {wikibase.entityIdFormatter.EntityIdHtmlFormatter}
options.entityIdHtmlFormatter
- * Required for dynamically rendering links to `Entity`s.
- * @param {wikibase.entityIdFormatter.EntityIdPlainFormatter}
options.entityIdPlainFormatter
- * Required for dynamically rendering plain text references to
`Entity`s.
- * @param {wikibase.store.EntityStore} options.entityStore
- * Required for dynamically gathering `Entity`/`Property` information.
- * @param {wikibase.ValueViewBuilder} options.valueViewBuilder
- * Required by the `snakview` interfacing a `snakview` "value"
`Variation` to
- * `jQuery.valueview`.
- * @param {dataTypes.DataTypeStore} options.dataTypeStore
- * Required by the `snakview` for retrieving and evaluating a proper
`dataTypes.DataType`
- * object when interacting on a "value" `Variation`.
* @param {string} [optionshelpMessage=mw.msg(
'wikibase-claimview-snak-new-tooltip' )]
* End-user message explaining how to use the `snaklistview` widget.
The message is most
* likely to be used inside the tooltip of the toolbar corresponding to
the `snaklistview`.
@@ -77,12 +66,8 @@
},
value: null,
singleProperty: false,
- helpMessage: mw.msg( 'wikibase-claimview-snak-new-tooltip' ),
- entityIdHtmlFormatter: null,
- entityIdPlainFormatter: null,
- entityStore: null,
- valueViewBuilder: null,
- dataTypeStore: null
+ listItemAdapter: null,
+ helpMessage: mw.msg( 'wikibase-claimview-snak-new-tooltip' )
},
/**
@@ -116,11 +101,7 @@
_create: function() {
this.options.value = this.options.value || new
wb.datamodel.SnakList();
- if ( !this.options.entityStore
- || !this.options.valueViewBuilder
- || !this.options.dataTypeStore
- || !( this.options.value instanceof
wb.datamodel.SnakList )
- ) {
+ if ( !this.options.listItemAdapter || !( this.options.value
instanceof wb.datamodel.SnakList ) ) {
throw new Error( 'Required option not specified
properly' );
}
@@ -152,26 +133,7 @@
}
this.$listview.listview( {
- listItemAdapter: new
$.wikibase.listview.ListItemAdapter( {
- listItemWidget: $.wikibase.snakview,
- newItemOptionsFn: function( value ) {
- return {
- value: value || {
- property: null,
- snaktype:
wb.datamodel.PropertyValueSnak.TYPE
- },
- locked: {
- // Do not allow
changing the property when editing existing an snak.
- property: !!value
- },
- dataTypeStore: self.option(
'dataTypeStore' ),
- entityIdHtmlFormatter:
self.options.entityIdHtmlFormatter,
- entityIdPlainFormatter:
self.options.entityIdPlainFormatter,
- entityStore: self.option(
'entityStore' ),
- valueViewBuilder: self.option(
'valueViewBuilder' )
- };
- }
- } ),
+ listItemAdapter: this.options.listItemAdapter,
value: this.options.value.toArray()
} );
@@ -185,14 +147,6 @@
this.$listview
.off( '.' + this.widgetName )
- .on( 'listviewitemadded.' + this.widgetName, function( event,
value, $newLi ) {
- // Listen to all the snakview "change" events to be
able to determine whether the
- // snaklistview itself is valid.
- $newLi.on( self._lia.prefixedEvent( 'change' ),
function( event ) {
- // Forward the "change" event to external
components (e.g. the edit toolbar).
- self._trigger( 'change' );
- } );
- } )
.on( this._lia.prefixedEvent( 'change.' ) + this.widgetName
+ ' listviewafteritemmove.' + this.widgetName
+ ' listviewitemremoved.' + this.widgetName, function(
event ) {
diff --git a/view/resources/jquery/wikibase/resources.php
b/view/resources/jquery/wikibase/resources.php
index dbe4fc4..a52543f 100644
--- a/view/resources/jquery/wikibase/resources.php
+++ b/view/resources/jquery/wikibase/resources.php
@@ -392,10 +392,9 @@
),
'dependencies' => array(
'jquery.ui.TemplatedWidget',
- 'jquery.ui.widget',
'jquery.wikibase.listview',
- 'jquery.wikibase.snakview',
- 'wikibase.datamodel',
+ 'wikibase.datamodel.Snak',
+ 'wikibase.datamodel.SnakList',
),
'messages' => array(
'wikibase-claimview-snak-tooltip',
diff --git a/view/resources/jquery/wikibase/snakview/snakview.js
b/view/resources/jquery/wikibase/snakview/snakview.js
index 8865f66..4dc08b0 100644
--- a/view/resources/jquery/wikibase/snakview/snakview.js
+++ b/view/resources/jquery/wikibase/snakview/snakview.js
@@ -918,15 +918,8 @@
*/
showPropertyLabel: function() {
this.$property.show();
- },
-
- /**
- * Returns whether the property label currently is visible.
- * @since 0.5
- */
- propertyLabelIsVisible: function() {
- return this.$property.is( ':visible' );
}
+
} );
$.extend( $.wikibase.snakview, existingSnakview );
diff --git a/view/resources/wikibase/view/ViewFactory.js
b/view/resources/wikibase/view/ViewFactory.js
index d69cf3c..18ddf59 100644
--- a/view/resources/wikibase/view/ViewFactory.js
+++ b/view/resources/wikibase/view/ViewFactory.js
@@ -350,12 +350,34 @@
return {
value: value || undefined,
singleProperty: true,
- dataTypeStore: this._dataTypeStore,
- entityIdHtmlFormatter:
this._entityIdHtmlFormatter,
- entityIdPlainFormatter:
this._entityIdPlainFormatter,
- entityStore: this._entityStore,
- valueViewBuilder:
this._getValueViewBuilder()
+ listItemAdapter:
this.getListItemAdapterForSnakView()
};
+ }, this )
+ } );
+ };
+
+ /**
+ * Construct a `ListItemAdapter` for `snakview`s
+ *
+ * @return {jQuery.wikibase.listview.ListItemAdapter} The constructed
ListItemAdapter
+ */
+ SELF.prototype.getListItemAdapterForSnakView = function() {
+ return new $.wikibase.listview.ListItemAdapter( {
+ listItemWidget: $.wikibase.snakview,
+ newItemOptionsFn: $.proxy( function( value ) {
+ return this._getSnakViewOptions(
+ null,
+ {
+ locked: {
+ // Do not allow
changing the property when editing existing an snak.
+ property: Boolean(
value )
+ }
+ },
+ value || {
+ property: null,
+ snaktype:
wb.datamodel.PropertyValueSnak.TYPE
+ }
+ );
}, this )
} );
};
@@ -373,21 +395,30 @@
return this._getView(
'snakview',
$dom,
- {
- value: snak || undefined,
- locked: options.locked,
- autoStartEditing: options.autoStartEditing,
- dataTypeStore: this._dataTypeStore,
- entityIdHtmlFormatter:
this._entityIdHtmlFormatter,
- entityIdPlainFormatter:
this._entityIdPlainFormatter,
- entityStore: this._entityStore,
- valueViewBuilder: this._getValueViewBuilder(),
- encapsulatedBy: encapsulatedBy
- }
+ this._getSnakViewOptions( encapsulatedBy, options, snak
)
);
};
/**
+ * @param {string|null} encapsulatedBy A jQuery selector for getting
the encapsulating view
+ * @param {Object} options An object with keys `locked` and
`autoStartEditing`
+ * @param {wikibase.datamodel.Snak|null} snak
+ */
+ SELF.prototype._getSnakViewOptions = function( encapsulatedBy, options,
snak ) {
+ return {
+ value: snak || undefined,
+ locked: options.locked,
+ autoStartEditing: options.autoStartEditing,
+ dataTypeStore: this._dataTypeStore,
+ entityIdHtmlFormatter: this._entityIdHtmlFormatter,
+ entityIdPlainFormatter: this._entityIdPlainFormatter,
+ entityStore: this._entityStore,
+ valueViewBuilder: this._getValueViewBuilder(),
+ encapsulatedBy: encapsulatedBy
+ };
+ };
+
+ /**
* @private
* @return {wikibase.ValueViewBuilder}
**/
diff --git
a/view/tests/qunit/jquery/wikibase/jquery.wikibase.snaklistview.tests.js
b/view/tests/qunit/jquery/wikibase/jquery.wikibase.snaklistview.tests.js
index 88ba36b..8b856f1 100644
--- a/view/tests/qunit/jquery/wikibase/jquery.wikibase.snaklistview.tests.js
+++ b/view/tests/qunit/jquery/wikibase/jquery.wikibase.snaklistview.tests.js
@@ -2,7 +2,7 @@
* @licence GNU GPL v2+
* @author H. Snater < [email protected] >
*/
-( function( mw, $, wb, dv, vf, vv, QUnit ) {
+( function( $, wb, dv, QUnit ) {
'use strict';
var snakLists = [
@@ -26,27 +26,29 @@
new wb.datamodel.PropertyValueSnak( 'p4', new dv.StringValue(
'g' ) )
];
- // We need a filled entity store for the instances of
jQuery.wikibase.snakview.variations.Value
- // and jQuery.wikibase.snakview created by jQuery.wikibase.snaklistview.
- var entities = {
- p1: new wb.datamodel.Property( 'P1', 'string' ),
- p2: new wb.datamodel.Property( 'P2', 'string' ),
- p3: new wb.datamodel.Property( 'P3', 'string' ),
- p4: new wb.datamodel.Property( 'P4', 'string' )
- };
+ var listItemAdapter = wb.tests.getMockListItemAdapter( 'snakview',
function() {
+ var _value = this.options.value;
+ this.options.locked = this.options.locked || {};
+ this.snak = function( value ) {
+ if ( arguments.length ) {
+ _value = value;
+ }
+ return _value;
+ };
+ this.isValid = function() {};
+ this.startEditing = function() {
+ this._trigger( 'change' );
+ this._trigger( 'afterstartediting' );
+ };
+ this.stopEditing = function() {};
- var entityStore = {
- get: function( entityId ) {
- return $.Deferred().resolve( entities[entityId] );
- }
- };
-
- var valueViewBuilder = new wb.ValueViewBuilder(
- new vv.ExpertStore(),
- new vf.ValueFormatterStore( vf.NullFormatter ),
- 'I am a ParserStore',
- 'I am a language code'
- );
+ this.showPropertyLabel = function() {
+ this._propertyLabelVisible = true;
+ };
+ this.hidePropertyLabel = function() {
+ this._propertyLabelVisible = false;
+ };
+ } );
/**
* Generates a snaklistview widget suitable for testing.
@@ -58,21 +60,7 @@
function createSnaklistview( value, additionalOptions ) {
var options = $.extend( additionalOptions, {
value: value || undefined,
- dataTypeStore: {
- getDataType: function() {}
- },
- entityIdHtmlFormatter: {
- format: function() {
- return $.Deferred().resolve( 'P1'
).promise();
- }
- },
- entityIdPlainFormatter: {
- format: function( entityId ) {
- return $.Deferred().resolve( entityId
).promise();
- }
- },
- entityStore: entityStore,
- valueViewBuilder: valueViewBuilder
+ listItemAdapter: listItemAdapter
} );
return $( '<div/>' )
@@ -867,9 +855,6 @@
var $node = createSnaklistview( snakLists[0], { singleProperty:
true } ),
snaklistview = $node.data( 'snaklistview' );
- // Append node to body in order to correctly detect visibility:
- $node.appendTo( 'body' );
-
assert.ok(
snaklistview._listview.items().length > 0,
'Initialized snaklistview with more than one item.'
@@ -882,12 +867,12 @@
if ( i === 0 ) {
assert.ok(
-
snakview.propertyLabelIsVisible(),
+ snakview._propertyLabelVisible,
'Topmost snakview\'s property
label is visible.'
);
} else {
assert.ok(
-
!snakview.propertyLabelIsVisible(),
+ !snakview._propertyLabelVisible,
'Property label of snakview
that is not on top of the snaklistview is not '
+ 'visible.'
);
@@ -907,4 +892,4 @@
testPropertyLabelVisibility( assert, snaklistview );
} );
-} )( mediaWiki, jQuery, wikibase, dataValues, valueFormatters,
jQuery.valueview, QUnit );
+} )( jQuery, wikibase, dataValues, QUnit );
diff --git a/view/tests/qunit/jquery/wikibase/resources.php
b/view/tests/qunit/jquery/wikibase/resources.php
index a5fbf1f..faf896d 100644
--- a/view/tests/qunit/jquery/wikibase/resources.php
+++ b/view/tests/qunit/jquery/wikibase/resources.php
@@ -262,11 +262,8 @@
),
'dependencies' => array(
'jquery.wikibase.snaklistview',
- 'wikibase.ValueViewBuilder',
'wikibase.datamodel',
- 'mediawiki.Title',
- 'jquery.valueview',
- 'valueFormatters'
+ 'wikibase.tests.getMockListItemAdapter',
),
),
diff --git a/view/tests/qunit/wikibase/view/ViewFactory.tests.js
b/view/tests/qunit/wikibase/view/ViewFactory.tests.js
index 9afd8c2..b909c9a 100644
--- a/view/tests/qunit/wikibase/view/ViewFactory.tests.js
+++ b/view/tests/qunit/wikibase/view/ViewFactory.tests.js
@@ -324,14 +324,43 @@
} );
QUnit.test( 'getListItemAdapterForSnakListView passes correct options
to ListItemAdapter', function( assert ) {
+ var value = null,
+ viewFactory = new ViewFactory(),
+ ListItemAdapter = sinon.spy( $.wikibase.listview,
'ListItemAdapter' );
+
+ viewFactory.getListItemAdapterForSnakListView();
+
+ sinon.assert.calledWith(
+ ListItemAdapter,
+ sinon.match( {
+ listItemWidget: $.wikibase.snaklistview,
+ newItemOptionsFn: sinon.match.func
+ } )
+ );
+
+ var result = ListItemAdapter.args[0][0].newItemOptionsFn( value
);
+
+ assert.deepEqual(
+ result,
+ {
+ value: value || undefined,
+ singleProperty: true,
+ listItemAdapter: result.listItemAdapter // Hack
+ }
+ );
+
+ assert.ok( result.listItemAdapter instanceof
$.wikibase.listview.ListItemAdapter );
+
+ $.wikibase.listview.ListItemAdapter.restore();
+ } );
+
+ QUnit.test( 'getListItemAdapterForSnakView passes correct options to
ListItemAdapter', function( assert ) {
var contentLanguages = {},
value = null,
dataTypeStore = {},
claimsChanger = {},
- referencesChanger = {},
entityChangersFactory = {
- getClaimsChanger: function() { return
claimsChanger; },
- getReferencesChanger: function() { return
referencesChanger; }
+ getClaimsChanger: function() { return
claimsChanger; }
},
entityIdHtmlFormatter = {},
entityIdPlainFormatter = {},
@@ -356,12 +385,12 @@
),
ListItemAdapter = sinon.spy( $.wikibase.listview,
'ListItemAdapter' );
- viewFactory.getListItemAdapterForSnakListView();
+ viewFactory.getListItemAdapterForSnakView();
sinon.assert.calledWith(
ListItemAdapter,
sinon.match( {
- listItemWidget: $.wikibase.snaklistview,
+ listItemWidget: $.wikibase.snakview,
newItemOptionsFn: sinon.match.func
} )
);
@@ -373,12 +402,19 @@
assert.deepEqual(
result,
{
- value: value || undefined,
- singleProperty: true,
+ value: value || {
+ property: null,
+ snaktype: 'value'
+ },
+ autoStartEditing: undefined,
dataTypeStore: dataTypeStore,
+ encapsulatedBy: null,
entityIdHtmlFormatter: entityIdHtmlFormatter,
entityIdPlainFormatter: entityIdPlainFormatter,
entityStore: entityStore,
+ locked: {
+ property: false
+ },
valueViewBuilder:
wb.ValueViewBuilder.returnValues[0]
}
);
--
To view, visit https://gerrit.wikimedia.org/r/246769
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3f89b733139d7ced99b9044305298b723f4e9e80
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Adrian Lang <[email protected]>
Gerrit-Reviewer: Adrian Lang <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
