Trevor Parscal has uploaded a new change for review.
https://gerrit.wikimedia.org/r/73377
Change subject: Fix uses of addPart to make them async friendly
......................................................................
Fix uses of addPart to make them async friendly
Problem:
Adding or moving templates or content resulted in the incorrect item being
selected.
Diagnosis:
Although recently we solved a few issues by making addPart async, it caused
some other issues where callers of addPart were assuming otherwise
Solution:
Return a promise from addPart which is resolved after the part is actually
added and setup callers to use the promise when needed
Changes:
ve.ui.MWTransclusionDialog.js
* Use promises to auto-select new or moved parts
ve.dm.MWTranclusionModel.js
* Make addPart return a promise
* Resolve promise when queue is processed
* Automatically remove existing items before adding them in different locations
at the time of processing the queue we don't yield between removing and adding
Change-Id: Ie43c5b89ca4ed34d5f797714e19c9a7e1824cdec
---
M modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js
M modules/ve-mw/ui/dialogs/ve.ui.MWTransclusionDialog.js
2 files changed, 28 insertions(+), 15 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/77/73377/1
diff --git a/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js
b/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js
index 17b662d..1a848dd 100644
--- a/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js
+++ b/modules/ve-mw/dm/models/ve.dm.MWTransclusionModel.js
@@ -99,9 +99,20 @@
item.part.getSpec().extend( specCache[title] );
}
}
+ // Auto-remove if already existing
+ index = ve.indexOf( item.part, this.parts );
+ if ( index !== -1 ) {
+ this.parts.splice( index, 1 );
+ this.emit( 'remove', item.part );
+ }
+ // Add at index, or end if none was given
index = item.index === undefined ? this.parts.length :
item.index;
this.parts.splice( index, 0, item.part );
this.emit( 'add', item.part );
+ // Resolve promises
+ if ( item.deferred ) {
+ item.deferred.resolve();
+ }
}
};
@@ -261,15 +272,20 @@
* @param {ve.dm.MWTransclusionPartModel} part Part to add
* @param {number} [index] Specific index to add content at, defaults to the
end
* @throws {Error} If part is not valid
+ * @returns {jQuery.Promise} Promise, resolved when part is added
*/
ve.dm.MWTransclusionModel.prototype.addPart = function ( part, index ) {
+ var deferred = $.Deferred();
if ( !( part instanceof ve.dm.MWTransclusionPartModel ) ) {
throw new Error( 'Invalid transclusion part' );
}
- this.queue.push( { 'part': part, 'index': index } );
+ this.queue.push( { 'part': part, 'index': index, 'deferred': deferred }
);
+
// Fetch on next yield to process items in the queue together,
subsequent calls to fetch will
// have no effect because the queue will be clear
setTimeout( ve.bind( this.fetch, this ) );
+
+ return deferred.promise();
};
/**
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWTransclusionDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWTransclusionDialog.js
index aadf512..ffdce42 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWTransclusionDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWTransclusionDialog.js
@@ -253,9 +253,9 @@
name = item.getData();
part = this.transclusion.getPartFromId( name );
index = ve.indexOf( part, parts );
- this.transclusion.removePart( part );
- this.transclusion.addPart( part, index + places );
- this.setPageByName( name );
+ // Auto-removes part from old location
+ this.transclusion.addPart( part, index + places )
+ .done( ve.bind( this.setPageByName, this, part.getId()
) );
}
};
@@ -268,17 +268,14 @@
ve.ui.MWTransclusionDialog.prototype.onOutlineControlsAdd = function ( type ) {
var part;
- switch ( type ) {
- case 'content':
- part = new ve.dm.MWTransclusionContentModel(
this.transclusion, '', 'user' );
- this.transclusion.addPart( part,
this.getPartInsertionIndex() );
- this.setPageByName( part.getId() );
- break;
- case 'template':
- part = new ve.dm.MWTemplatePlaceholderModel(
this.transclusion, 'user' );
- this.transclusion.addPart( part,
this.getPartInsertionIndex() );
- this.setPageByName( part.getId() );
- break;
+ if ( type === 'content' ) {
+ part = new ve.dm.MWTransclusionContentModel( this.transclusion,
'', 'user' );
+ } else if ( type === 'template' ) {
+ part = new ve.dm.MWTemplatePlaceholderModel( this.transclusion,
'user' );
+ }
+ if ( part ) {
+ this.transclusion.addPart( part, this.getPartInsertionIndex() )
+ .done( ve.bind( this.setPageByName, this, part.getId()
) );
}
};
--
To view, visit https://gerrit.wikimedia.org/r/73377
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie43c5b89ca4ed34d5f797714e19c9a7e1824cdec
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
