Divec has uploaded a new change for review.
https://gerrit.wikimedia.org/r/86802
Change subject: WIP:dm.Surface emits selectionChange, not change
......................................................................
WIP:dm.Surface emits selectionChange, not change
ve.dm.Surface.js:
* Don't emit 'change'. Listeners use 'transact' and 'select' instead.
Change-Id: I8f16289493e835d890709c6dfe093d04c18522b6
---
M modules/ve/ce/ve.ce.Surface.js
M modules/ve/dm/ve.dm.Surface.js
2 files changed, 32 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/02/86802/1
diff --git a/modules/ve/ce/ve.ce.Surface.js b/modules/ve/ce/ve.ce.Surface.js
index e85000a..db4c529 100644
--- a/modules/ve/ce/ve.ce.Surface.js
+++ b/modules/ve/ce/ve.ce.Surface.js
@@ -51,7 +51,7 @@
this.pasting = false;
this.clickHistory = [];
this.focusedNode = null;
- // This is set on entering changeModelSelection, then unset when
leaving.
+ // This is set on entering changeModel, then unset when leaving.
// It is used to test whether a reflected change event is emitted.
this.newModelSelection = null;
@@ -59,7 +59,8 @@
this.surfaceObserver.connect(
this, { 'contentChange': 'onContentChange', 'selectionChange':
'onSelectionChange' }
);
- this.model.connect( this, { 'change': 'onChange', 'lock': 'onLock',
'unlock': 'onUnlock' } );
+ this.model.connect( this,
+ { 'select': 'onModelSelect', 'lock': 'onLock', 'unlock':
'onUnlock' } );
$documentNode = this.documentView.getDocumentNode().$;
$documentNode.on( {
@@ -314,7 +315,7 @@
// Calling focus sets the cursor to zero offset, so we need to restore
scrollTop
$window.scrollTop( scrollTop );
this.focusedNode = null;
- this.onChange( null, this.surface.getModel().selection );
+ this.onModelSelect( this.surface.getModel().selection );
};
/*! Native Browser Events */
@@ -568,7 +569,8 @@
var selection, prevNode, documentModel = this.model.getDocument();
// Prevent IE from editing Aliens/Entities
- // TODO: Better comment about what's going on here is needed.
+ // This is for cases like <p><div>alien</div></p>, to put the cursor
outside
+ // the alien tag.
if ( $.browser.msie === true ) {
selection = this.model.getSelection();
if ( selection.start !== 0 && selection.isCollapsed() ) {
@@ -903,22 +905,21 @@
/*! Custom Events */
/**
- * Handle change events.
+ * Handle model select events.
*
* @see ve.dm.Surface#method-change
*
* @method
- * @param {ve.dm.Transaction|null} transaction
* @param {ve.Range|undefined} selection
*/
-ve.ce.Surface.prototype.onChange = function ( transaction, selection ) {
+ve.ce.Surface.prototype.onModelSelect = function ( selection ) {
var start, end, rangySel, rangyRange,
next = null,
previous = this.focusedNode;
- // Ignore selection if changeModelSelection is currently being called
with the same
- // (object-identical) selection object (i.e. if the model is calling us
back)
- if ( selection && selection !== this.newModelSelection ) {
+ // Ignore selection if changeModel is currently being called with the
same
+ // selection (i.e. if the dm.Surface is calling us back)
+ if ( this.newModelSelection && ! this.newModelSelection.equals(
selection ) ) {
// Detect when only a single inline element is selected
if ( !selection.isCollapsed() ) {
start =
this.documentView.getDocumentNode().getNodeFromOffset( selection.start + 1 );
@@ -980,7 +981,7 @@
}
this.incRenderLock();
try {
- this.changeModelSelection( newRange );
+ this.changeModel( null, newRange );
} finally {
this.decRenderLock();
}
@@ -1049,7 +1050,7 @@
}
this.incRenderLock();
try {
- this.model.change(
+ this.changeModel(
ve.dm.Transaction.newFromInsertion(
this.documentView.model,
previous.range.start, data
),
@@ -1070,7 +1071,7 @@
}
this.incRenderLock();
try {
- this.model.change(
+ this.model.changeModel(
ve.dm.Transaction.newFromRemoval(
this.documentView.model,
range ),
next.range
@@ -1134,7 +1135,7 @@
}
if ( data.length > 0 ) {
- this.model.change(
+ this.changeModel(
ve.dm.Transaction.newFromInsertion(
this.documentView.model, nodeOffset + 1
+ fromLeft,
data
@@ -1143,7 +1144,7 @@
);
}
if ( fromLeft + fromRight < previousData.length ) {
- this.model.change(
+ this.changeModel(
ve.dm.Transaction.newFromRemoval(
this.documentView.model,
new ve.Range(
@@ -1323,6 +1324,7 @@
annotations = documentModel.data.getAnnotationsFromRange(
new ve.Range( selection.start, selection.start + 1 )
);
+ // We do want this to propagate to the surface
this.model.change(
ve.dm.Transaction.newFromRemoval(
this.documentView.model, selection ),
new ve.Range( selection.start )
@@ -1351,6 +1353,8 @@
range = new ve.Range( selection.start,
selection.start + 1 );
data = [placeholder];
}
+ // TODO: ??? We do want this to propagate to the surface
+ // Though ideally, pawns wouldn't reach the model anyway
this.model.change(
ve.dm.Transaction.newFromInsertion(
this.documentView.model,
selection.start, data
@@ -1388,6 +1392,7 @@
if ( selection.from !== selection.to ) {
tx = ve.dm.Transaction.newFromRemoval( documentModel, selection
);
selection = tx.translateRange( selection );
+ // We do want this to propagate to the surface
this.model.change( tx, selection );
}
@@ -1857,22 +1862,25 @@
};
/**
- * Change selection in the model only, not the CE surface
+ * Change the model only, not the CE surface
*
* This avoids event storms when the CE surface is already correct
*
* @method
- * @param {ve.Range} range New selection for model
- * @throws {Error} If calls to the method are nested
+ * @param {ve.dm.Transaction|ve.dm.Transaction[]|null} transactions One or
more transactions to
+ * process, or null to process none
+ * @param {ve.Range} new selection
+ * @throws {Error} If calls to this method are nested
*/
-ve.ce.Surface.prototype.changeModelSelection = function ( range ) {
+ve.ce.Surface.prototype.changeModel = function ( transaction, range ) {
if ( this.newModelSelection !== null ) {
- throw new Error( 'Nested changeModelSelection' );
+ throw new Error( 'Nested change of newModelSelection' );
}
this.newModelSelection = range;
try {
- this.model.change( null, range );
+ this.model.change( transaction, range );
} finally {
this.newModelSelection = null;
}
};
+
diff --git a/modules/ve/dm/ve.dm.Surface.js b/modules/ve/dm/ve.dm.Surface.js
index 04283fd..f5268bd 100644
--- a/modules/ve/dm/ve.dm.Surface.js
+++ b/modules/ve/dm/ve.dm.Surface.js
@@ -60,9 +60,8 @@
*/
/**
- * @event change
+ * @event select
* @see #method-change
- * @param {ve.dm.Transaction|null} transaction
* @param {ve.Range|undefined} selection
*/
@@ -296,7 +295,7 @@
* @emits select
* @emits transact
* @emits contextChange
- * @emits change
+ * @emits selectionChange
* @emits unlock
*/
ve.dm.Surface.prototype.change = function ( transactions, selection ) {
@@ -349,7 +348,7 @@
}
this.selectedNodes = selectedNodes;
if ( selectionChange ) {
- this.emit( 'select', this.selection.clone() );
+ this.emit( 'select', selection.clone() );
}
this.selection = selection;
}
@@ -405,8 +404,6 @@
if ( contextChange ) {
this.emit( 'contextChange' );
}
-
- this.emit( 'change', transactions, selection );
// Continue observation polling, we want to know about things that
change from here on out
this.emit( 'unlock' );
--
To view, visit https://gerrit.wikimedia.org/r/86802
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8f16289493e835d890709c6dfe093d04c18522b6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
