jenkins-bot has submitted this change and it was merged.
Change subject: Migrate away from using the 'change' event in dm.Surface
......................................................................
Migrate away from using the 'change' event in dm.Surface
Instead, listen to 'select', or to 'transact' on the dm.Document.
This commit only fixes uses outside of the dm/ce.Surface ecosystem.
ce.Surface still listens to 'change'.
ve.init.mw.ViewPageTarget.js:
* Rename onSurfaceModelTransact to clearSaveDialogDiff and bind it to
the document's transact event instead
* Rename onSurfaceModelChange to checkForWikitextWarning and bind it
to the surface's transact event. This is needed because the function
inspects the surface's selection, which isn't yet in a consistent
state when the document's transact event fires
ve.ui.MWReferenceDialog.js:
* Rename onSurfaceChange to onDocumentTransact and rebind accordingly
ve.ce.ProtectedNode.js:
* Get rid of onSurfaceModelChange
* Instead, bind positionPhantoms to the document's transact event
directly, and only bind it while phantoms are visible
ve.ui.Context.js:
* Rename onChange to onModelSelect and rebind accordingly
* Rename afterChange to afterModelSelect
* Drop check for undefined selection, no longer needed now that we're
listening to a finer-grained event
ve.ce.Surface.test.js:
* Listen to 'select' instead of 'change'
Change-Id: Ifeb1a1fc5427696f2aae5441d4b54dde366793e0
---
M modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
M modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
M modules/ve/ce/ve.ce.ProtectedNode.js
M modules/ve/test/ce/ve.ce.Surface.test.js
M modules/ve/ui/ve.ui.Context.js
5 files changed, 44 insertions(+), 51 deletions(-)
Approvals:
Divec: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
index 2dcc8af..736cb9b 100644
--- a/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
+++ b/modules/ve-mw/init/targets/ve.init.mw.ViewPageTarget.js
@@ -552,7 +552,7 @@
*/
ve.init.mw.ViewPageTarget.prototype.onShowChanges = function ( diffHtml ) {
// Invalidate the viewer diff on next change
- this.surface.getModel().connect( this, { 'transact':
'onSurfaceModelTransact' } );
+ this.surface.getModel().getDocument().connect( this, { 'transact':
'clearSaveDialogDiff' } );
this.saveDialog.setDiffAndReview( diffHtml );
};
@@ -564,7 +564,7 @@
*/
ve.init.mw.ViewPageTarget.prototype.onSerialize = function ( wikitext ) {
// Invalidate the viewer wikitext on next change
- this.surface.getModel().connect( this, { 'transact':
'onSurfaceModelTransact' } );
+ this.surface.getModel().getDocument().connect( this, { 'transact':
'clearSaveDialogDiff' } );
this.saveDialog.setDiffAndReview( $( '<pre>' ).text( wikitext ) );
};
@@ -686,33 +686,37 @@
};
/**
- * Handle the first transaction in the surface model.
+ * Clear the diff in the save dialog.
*
- * This handler is removed the first time it's used, but added each time the
surface is set up.
+ * This method is bound to the 'transact' event on the document model, and
unbinds itself the first
+ * time it runs. It's bound when the surface is set up and rebound every time
a diff is loaded into
+ * the save dialog.
*
* @method
* @param {ve.dm.Transaction} tx Processed transaction
*/
-ve.init.mw.ViewPageTarget.prototype.onSurfaceModelTransact = function () {
+ve.init.mw.ViewPageTarget.prototype.clearSaveDialogDiff = function () {
// Clear the diff
this.saveDialog.$reviewViewer.empty();
- this.surface.getModel().disconnect( this, { 'transact':
'onSurfaceModelTransact' } );
+ this.surface.getModel().getDocument().disconnect( this, { 'transact':
'clearSaveDialogDiff' } );
};
/**
- * Handle changes to the surface model.
+ * Check if the user is entering wikitext, and show a notification if they are.
*
- * This is used to trigger notifications when the user starts entering wikitext
+ * This check is fairly simplistic: it checks whether the content branch node
the selection is in
+ * looks like wikitext, so it can trigger if the user types in a paragraph
that has pre-existing
+ * wikitext-like content.
*
- * @param {ve.dm.Transaction} tx
- * @param {ve.Range} range
+ * This method is bound to the 'transact' event on the surface model, and
unbinds itself when
+ * the wikitext notification is displayed.
+ *
+ * @param {ve.dm.Transaction[]} transactions
*/
-ve.init.mw.ViewPageTarget.prototype.onSurfaceModelChange = function ( tx,
range ) {
- if ( !range ) {
- return;
- }
+ve.init.mw.ViewPageTarget.prototype.checkForWikitextWarning = function () {
var text, doc = this.surface.getView().getDocument(),
- node = doc.getNodeFromOffset( range.start );
+ selection = this.surface.getModel().getSelection(),
+ node = doc.getNodeFromOffset( selection.start );
if ( !( node instanceof ve.ce.ContentBranchNode ) ) {
return;
}
@@ -728,7 +732,9 @@
'autoHide': false
}
);
- this.surface.getModel().disconnect( this, { 'change':
'onSurfaceModelChange' } );
+ this.surface.getModel().disconnect(
+ this, { 'transact': 'checkForWikitextWarning' }
+ );
}
};
@@ -960,9 +966,11 @@
// Initialize surface
target.surface.getContext().hide();
target.$document =
target.surface.$.find( '.ve-ce-documentNode' );
+
target.surface.getModel().getDocument().connect( target, {
+ 'transact':
'clearSaveDialogDiff'
+ } );
target.surface.getModel().connect(
target, {
- 'transact':
'onSurfaceModelTransact',
- 'change':
'onSurfaceModelChange',
+ 'transact':
'checkForWikitextWarning',
'history':
'updateToolbarSaveButtonState'
} );
target.$.append( target.surface.$ );
diff --git a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
index 8669da7..7f29466 100644
--- a/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
+++ b/modules/ve-mw/ui/dialogs/ve.ui.MWReferenceDialog.js
@@ -309,7 +309,8 @@
);
// Event handlers
- this.referenceSurface.getSurface().getModel().connect( this, {
'change': 'onSurfaceChange' } );
+ this.referenceSurface.getSurface().getModel().getDocument()
+ .connect( this, { 'transact': 'onDocumentTransact' } );
// Initialization
this.referenceGroupInput.setValue( refGroup );
@@ -322,7 +323,7 @@
/**
* Handle reference surface change events
*/
-ve.ui.MWReferenceDialog.prototype.onSurfaceChange = function () {
+ve.ui.MWReferenceDialog.prototype.onDocumentTransact = function () {
var data = this.referenceSurface.getContent(),
// TODO: Check for other types of empty, e.g. only whitespace?
disabled = data.length <= 4;
diff --git a/modules/ve/ce/ve.ce.ProtectedNode.js
b/modules/ve/ce/ve.ce.ProtectedNode.js
index f29e082..d2c6d69 100644
--- a/modules/ve/ce/ve.ce.ProtectedNode.js
+++ b/modules/ve/ce/ve.ce.ProtectedNode.js
@@ -76,8 +76,6 @@
// Events
this.$.on( 'mouseenter.ve-ce-protectedNode', ve.bind(
this.onProtectedMouseEnter, this ) );
- this.getRoot().getSurface().getModel()
- .connect( this, { 'change': 'onSurfaceModelChange' } );
this.getRoot().getSurface().getSurface()
.connect( this, { 'position': 'positionPhantoms' } );
@@ -119,8 +117,6 @@
// Events
this.$.off( '.ve-ce-protectedNode' );
- this.root.getSurface().getModel()
- .disconnect( this, { 'change': 'onSurfaceModelChange' } );
this.getRoot().getSurface().getSurface()
.disconnect( this, { 'position': 'positionPhantoms' } );
@@ -201,17 +197,6 @@
};
/**
- * Handle surface model change events
- *
- * @method
- */
-ve.ce.ProtectedNode.prototype.onSurfaceModelChange = function () {
- if ( this.$phantoms.length ) {
- this.positionPhantoms();
- }
-};
-
-/**
* Handle resize start events.
*
* @method
@@ -243,6 +228,7 @@
'mousemove.ve-ce-protectedNode': ve.bind(
this.onSurfaceMouseMove, this ),
'mouseout.ve-ce-protectedNode': ve.bind(
this.onSurfaceMouseOut, this )
} );
+ surface.getModel().getDocument().connect( this, { 'transact':
'positionPhantoms' } );
};
/**
@@ -277,5 +263,6 @@
var surface = this.root.getSurface();
surface.replacePhantoms( null );
surface.$.unbind( '.ve-ce-protectedNode' );
+ surface.getModel().getDocument().disconnect( this, { 'transact':
'positionPhantoms' } );
this.$phantoms = $( [] );
};
diff --git a/modules/ve/test/ce/ve.ce.Surface.test.js
b/modules/ve/test/ce/ve.ce.Surface.test.js
index bb51859..d0b30fb 100644
--- a/modules/ve/test/ce/ve.ce.Surface.test.js
+++ b/modules/ve/test/ce/ve.ce.Surface.test.js
@@ -25,8 +25,9 @@
// TODO: model.getSelection() should be consistent after it has been
// changed but appears to behave differently depending on the browser.
- // The selection from the change event is still consistent.
- model.on( 'change', function( tx, s ) {
+ // The selection from the select event is still consistent.
+ selection = range;
+ model.on( 'select', function( s ) {
selection = s;
} );
diff --git a/modules/ve/ui/ve.ui.Context.js b/modules/ve/ui/ve.ui.Context.js
index 403926d..3d1bdf8 100644
--- a/modules/ve/ui/ve.ui.Context.js
+++ b/modules/ve/ui/ve.ui.Context.js
@@ -31,7 +31,7 @@
this.embedded = false;
this.selection = null;
this.toolbar = null;
- this.afterChangeTimeout = null;
+ this.afterModelSelectTimeout = null;
this.popup = new ve.ui.PopupWidget( { '$$': this.$$, '$container':
this.surface.getView().$ } );
this.$menu = this.$$( '<div>' );
this.inspectors = new ve.ui.SurfaceWindowSet( surface,
ve.ui.inspectorFactory );
@@ -45,7 +45,7 @@
);
// Events
- this.surface.getModel().connect( this, { 'change': 'onChange' } );
+ this.surface.getModel().connect( this, { 'select': 'onModelSelect' } );
this.surface.getView().connect( this, {
'selectionStart': 'onSelectionStart',
'selectionEnd': 'onSelectionEnd',
@@ -81,29 +81,25 @@
*
* The response to selection changes is deferred to prevent close handlers
that process
* changes from causing this function to recurse. These responses are also
batched for efficiency,
- * so that if there are three selection changes in the same tick,
afterChange() only runs once.
+ * so that if there are three selection changes in the same tick,
afterModelSelect() only runs once.
*
* @method
- * @see #afterChange
- * @param {ve.dm.Transaction[]} transactions Change transactions
- * @param {ve.Range} selection Change selection
+ * @see #afterModelSelect
*/
-ve.ui.Context.prototype.onChange = function ( transactions, selection ) {
- if ( selection && !this.openingInspector && !this.hiding ) {
- if ( this.afterChangeTimeout === null ) {
- this.afterChangeTimeout = setTimeout( ve.bind(
this.afterChange, this ) );
- }
+ve.ui.Context.prototype.onModelSelect = function () {
+ if ( !this.openingInspector && !this.hiding &&
this.afterModelSelectTimeout === null ) {
+ this.afterModelSelectTimeout = setTimeout( ve.bind(
this.afterModelSelect, this ) );
}
};
/**
- * Deferred response to one or more selection changes.
+ * Deferred response to one or more select events.
*
* Update the context menu for the new selection, except if the user is
selecting or relocating
* content. If the popup is open, close it, even while selecting or relocating.
*/
-ve.ui.Context.prototype.afterChange = function () {
- this.afterChangeTimeout = null;
+ve.ui.Context.prototype.afterModelSelect = function () {
+ this.afterModelSelectTimeout = null;
if ( this.popup.isVisible() ) {
this.hide();
this.update();
--
To view, visit https://gerrit.wikimedia.org/r/88420
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifeb1a1fc5427696f2aae5441d4b54dde366793e0
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Divec <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
