jenkins-bot has submitted this change and it was merged.
Change subject: Clean up LookupInputWidget subclasses and use new functionality
......................................................................
Clean up LookupInputWidget subclasses and use new functionality
MWCategoryInputWidget:
* Use @inheritdoc
* Don't modify data parameter in getLookupCacheItemFromData()
MWLinkTargetInputWidget:
* Remove this.choosing in favor of setLookupsDisabled()
* Explicitly close menu on choose
* Remove manual emission of change events
** This looks ridiculous, it doesn't seem to be necessary,
and it causes infinite event loops. But I'm very curious
why this was added in the first place.
* Remove onLookupInputChange override that is now unnecessary
* Use {} rather than [] for fake empty result
* Prevent programmatic focus from opening the menu
MWTitleInputWidget:
* On choose, close menu and disable lookups while changing value
* Use @inheritdoc
* Remove mentions of "template" from getTitle() documentation
Bug fixed:
* When choosing a suggestion in MWTitleInputWidget,
new suggestions would be loaded and the menu would reopen
Depends on Iecae9b582 in oojs-ui.
Change-Id: I716f99bb464a5cebd4f17701197f768e4e0e02a9
---
M modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js
M modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
M modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
3 files changed, 27 insertions(+), 44 deletions(-)
Approvals:
Jforrester: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js
index 623664f..07c5085 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWCategoryInputWidget.js
@@ -47,10 +47,7 @@
/* Methods */
/**
- * Gets a new request object of the current lookup query value.
- *
- * @method
- * @returns {jqXHR} AJAX object without success or fail handlers attached
+ * @inheritdoc
*/
ve.ui.MWCategoryInputWidget.prototype.getLookupRequest = function () {
return ve.init.target.constructor.static.apiRequest( {
@@ -63,16 +60,12 @@
};
/**
- * Get lookup cache item from server response data.
- *
- * @method
- * @param {Mixed} data Response from server
+ * @inheritdoc
*/
ve.ui.MWCategoryInputWidget.prototype.getLookupCacheItemFromData = function (
data ) {
- var result = [], linkCacheUpdate = {};
- data.query = data.query || {};
+ var result = [], linkCacheUpdate = {}, query = data.query || {};
- $.each( data.query.allcategories || [], function ( index, category ) {
+ $.each( query.allcategories || [], function ( index, category ) {
result.push( category['*'] );
linkCacheUpdate['Category:' + category['*']] = { missing:
false, hidden: category.hasOwnProperty( 'hidden' ) };
}.bind( this ) );
@@ -83,10 +76,7 @@
};
/**
- * Get list of menu items from a server response.
- *
- * @param {Object} data Query result
- * @returns {OO.ui.MenuItemWidget[]} Menu items
+ * @inheritdoc
*/
ve.ui.MWCategoryInputWidget.prototype.getLookupMenuItemsFromData = function (
data ) {
var exactMatch = false,
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
index 4c3fc2e..6ed0d8c 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWLinkTargetInputWidget.js
@@ -27,7 +27,6 @@
// Properties
this.annotation = null;
- this.choosing = false;
// Events
this.lookupMenu.connect( this, { choose: 'onLookupMenuItemChoose' } );
@@ -55,30 +54,31 @@
*
* @method
* @param {OO.ui.MenuItemWidget|null} item Selected item
- * @fires change
*/
ve.ui.MWLinkTargetInputWidget.prototype.onLookupMenuItemChoose = function (
item ) {
+ this.closeLookupMenu();
if ( item ) {
- // WARNING: This assumes that #setAnnotation will emit `change`
events synchronously
- // TODO: Consider how this trick can be solved better, and
possibly pushed upstream to
- // OO.ui.LookupInputWidget so others don't fall into this trap
- this.choosing = true;
+ this.setLookupsDisabled( true );
this.setAnnotation( item.getData() );
- this.choosing = false;
+ this.setLookupsDisabled( false );
} else if ( this.annotation ) {
this.annotation = null;
- this.emit( 'change', this.getValue() );
}
};
/**
* @inheritdoc
*/
-ve.ui.MWLinkTargetInputWidget.prototype.onLookupInputChange = function () {
- // WARNING: See #onLookupMenuItemChoose for why this is fragile
- if ( !this.choosing ) {
- this.openLookupMenu();
- }
+ve.ui.MWLinkTargetInputWidget.prototype.focus = function () {
+ var retval;
+ // Prevent programmatic focus from opening the menu
+ this.setLookupsDisabled( true );
+
+ // Parent method
+ retval = ve.ui.MWLinkTargetInputWidget.super.prototype.focus.apply(
this, arguments );
+
+ this.setLookupsDisabled( false );
+ return retval;
};
/**
@@ -114,7 +114,7 @@
} else {
// Don't send invalid titles to the API.
// Just pretend it returned nothing so we can show the 'invalid
title' section
- return $.Deferred().resolve( [] ).promise( { abort: function ()
{
+ return $.Deferred().resolve( {} ).promise( { abort: function ()
{
// Do nothing. This is just so OOUI doesn't break due
to abort being undefined.
} } );
}
@@ -280,7 +280,6 @@
if ( item ) {
// Set annotation directly, bypassing re-setting the value of
the input
this.annotation = item.getData();
- this.emit( 'change', this.getValue() );
}
};
diff --git a/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
b/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
index 45ff008..708d1f0 100644
--- a/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
+++ b/modules/ve-mw/ui/widgets/ve.ui.MWTitleInputWidget.js
@@ -52,16 +52,16 @@
* @param {OO.ui.MenuItemWidget} item Selected item
*/
ve.ui.MWTitleInputWidget.prototype.onLookupMenuItemChoose = function ( item ) {
+ this.closeLookupMenu();
if ( item ) {
+ this.setLookupsDisabled( true );
this.setValue( item.getData() );
+ this.setLookupsDisabled( false );
}
};
/**
- * Gets a new request object of the current lookup query value.
- *
- * @method
- * @returns {jQuery.Deferred} Deferred object with success and fail handlers
already attached
+ * @inheritdoc
*/
ve.ui.MWTitleInputWidget.prototype.getLookupRequest = function () {
var value = this.value;
@@ -84,20 +84,14 @@
};
/**
- * Get lookup cache item from server response data.
- *
- * @method
- * @param {Mixed} data Response from server
+ * @inheritdoc
*/
ve.ui.MWTitleInputWidget.prototype.getLookupCacheItemFromData = function (
data ) {
return data[1] || [];
};
/**
- * Get list of menu items from a server response.
- *
- * @param {Object} data Query result
- * @returns {OO.ui.MenuItemWidget[]} Menu items
+ * @inheritdoc
*/
ve.ui.MWTitleInputWidget.prototype.getLookupMenuItemsFromData = function (
data ) {
var i, len, title, value,
@@ -127,9 +121,9 @@
};
/**
- * Get template title
+ * Get title object corresponding to #getValue
*
- * @returns {mw.Title|null} Template title if valid or null
+ * @returns {mw.Title|null} Title object, or null if value is invalid
*/
ve.ui.MWTitleInputWidget.prototype.getTitle = function () {
var title = this.getValue(),
--
To view, visit https://gerrit.wikimedia.org/r/170379
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I716f99bb464a5cebd4f17701197f768e4e0e02a9
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
