Catrope has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/170379

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

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, 17 insertions(+), 49 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor 
refs/changes/79/170379/1

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..c0c4ebd 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,29 +54,15 @@
  *
  * @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();
        }
 };
 
@@ -114,7 +99,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 +265,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: newchange
Gerrit-Change-Id: I716f99bb464a5cebd4f17701197f768e4e0e02a9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to