Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/164515
Change subject: Get rid of trigger-happy slugChange event.
......................................................................
Get rid of trigger-happy slugChange event.
slugChange fired on every content change and every selection
change, which is way too often for an event that claims to
be slug-specific. While ce.Surface does need to check for
slug exits on every content and selection change, this should
be done in ce.Surface itself, not by emitting slugChange
any time anything remotely related to slugs could possibly
have potentially happened maybe.
Replace slugChange with slugEnter, which is equivalent
to what slugChange(true) used to be. Split up onSlugChange
into onSlugEnter (the if(newSlug) code path) and updateSlug
(the if(this.slugFragment) code path) and call the latter
from onSelectionChange, onContentChange and onSlugEnter.
Also get rid of the range parameter to these events:
onSelectionChange, onContentChange, onSlugEnter and
updateSlug all potentially change the document, so this
range parameter will get out of date. Instead, use the
current selection, which will be updated for document
changes.
Change-Id: Ie3531f71a1bae39ca57be0e641c71d18bc6aa9f5
---
M src/ce/ve.ce.Surface.js
M src/ce/ve.ce.SurfaceObserver.js
2 files changed, 59 insertions(+), 51 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor
refs/changes/15/164515/1
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 4052a59..a6f5bdb 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -69,7 +69,7 @@
this.surfaceObserver.connect( this, {
contentChange: 'onContentChange',
selectionChange: 'onSelectionChange',
- slugChange: 'onSlugChange'
+ slugEnter: 'onSlugEnter'
} );
this.model.connect( this, {
select: 'onModelSelect',
@@ -1642,50 +1642,62 @@
} finally {
this.decRenderLock();
}
+ this.updateSlug();
};
/**
- * Handle slug change events.
+ * Handle slug enter events.
*
* @see ve.ce.SurfaceObserver#pollOnce
- *
- * @param {ve.Range|null} range
- * @param {boolean} newSlug
*/
-ve.ce.Surface.prototype.onSlugChange = function ( range, newSlug ) {
- if ( this.slugFragment || newSlug ) {
- var model = this.getModel(),
- doc = model.getDocument(),
- fragment = model.getFragment( range );
+ve.ce.Surface.prototype.onSlugEnter = function () {
+ var fragment, offset,
+ model = this.getModel(),
+ doc = model.getDocument();
- if ( this.slugFragment ) {
- if ( this.slugFragment.getRange().getLength() === 2 ) {
- if ( !range ||
!this.slugFragment.getRange().containsOffset( range.start ) ) {
- model.popStaging();
- // After popStaging we may have removed
a paragraph before our current
- // cursor position. Polling with the
SurfaceObserver won't notice a change
- // in the rangy range as our cursor
doesn't move within its node so we
- // need to clear it first.
- this.surfaceObserver.clear();
- this.surfaceObserver.pollOnceNoEmit();
- this.slugFragment = null;
- }
- } else {
- model.applyStaging();
+ this.updateSlug();
+ // Wait until after updateSlug() to get selection
+ fragment = model.getFragment();
+ offset = fragment.getRange().start;
+ model.pushStaging( true );
+ this.changeModel( ve.dm.Transaction.newFromInsertion(
+ doc, offset, [
+ { type: 'paragraph', internal: { generated: 'slug' } },
+ { type: '/paragraph' }
+ ]
+ ), new ve.Range( offset + 1 ) );
+ this.slugFragment = fragment;
+};
+
+/**
+ * Unslug if needed.
+ *
+ * If the slug is no longer empty, commit the staged changes.
+ * If the slug is still empty and the cursor has moved out of it,
+ * clear the staged changes.
+ * If the slug is still empty and the cursor is still inside it,
+ * or if there is no active slug, do nothing.
+ */
+ve.ce.Surface.prototype.updateSlug = function () {
+ if ( this.slugFragment ) {
+ var model = this.getModel(),
+ range = model.getSelection();
+
+ if ( this.slugFragment.getRange().getLength() === 2 ) {
+ if ( !range ||
!this.slugFragment.getRange().containsOffset( range.start ) ) {
+ model.popStaging();
+ // After popStaging we may have removed a
paragraph before our current
+ // cursor position. Polling with the
SurfaceObserver won't notice a change
+ // in the rangy range as our cursor doesn't
move within its node so we
+ // need to clear it first.
+ this.surfaceObserver.clear();
+ this.surfaceObserver.pollOnceNoEmit();
this.slugFragment = null;
}
- }
- if ( newSlug ) {
- range = fragment.getRange();
- model.pushStaging( true );
- this.changeModel( ve.dm.Transaction.newFromInsertion(
- doc, range.start, [
- { type: 'paragraph', internal: {
generated: 'slug' } },
- { type: '/paragraph' }
- ]
- ), new ve.Range( range.start + 1 ) );
- this.slugFragment = fragment;
+ } else {
+ model.applyStaging();
+ this.slugFragment = null;
}
}
};
@@ -1872,6 +1884,7 @@
ve.dm.Transaction.newFromReplacement( this.documentView.model,
replacementRange, data ),
newRange
);
+ this.updateSlug();
};
/**
diff --git a/src/ce/ve.ce.SurfaceObserver.js b/src/ce/ve.ce.SurfaceObserver.js
index c905f03..250da39 100644
--- a/src/ce/ve.ce.SurfaceObserver.js
+++ b/src/ce/ve.ce.SurfaceObserver.js
@@ -62,12 +62,9 @@
*/
/**
- * When #poll observes a change in content or the selection such
- * that a slug may have to be added or removed, this event is emitted
+ * When #poll observes that the cursor was moved into a slug
*
- * @event slugChange
- * @param {ve.Range|null} range New range
- * @param {boolean} newSlug The cursor was moved into a slug
+ * @event slugEnter
*/
/* Methods */
@@ -195,12 +192,13 @@
* @param {boolean} selectionOnly Check for selection changes only
* @fires contentChange
* @fires selectionChange
- * @fires slugChange
+ * @fires slugEnter
*/
ve.ce.SurfaceObserver.prototype.pollOnceInternal = function ( emitChanges,
selectionOnly ) {
- var $nodeOrSlug, node, text, hash, range, domRange, $slugWrapper,
newSlug,
+ var $nodeOrSlug, node, text, hash, range, domRange, $slugWrapper,
anchorNodeChange = false,
- slugChange = false,
+ enteredSlug = false,
+ leftSlug = false,
observer = this;
if ( !this.domDocument ) {
@@ -240,18 +238,17 @@
.addClass(
've-ce-branchNode-blockSlugWrapper-unfocused' )
.removeClass(
've-ce-branchNode-blockSlugWrapper-focused' );
this.$slugWrapper = null;
- slugChange = true;
+ leftSlug = true;
}
if ( $slugWrapper && $slugWrapper.length && !$slugWrapper.is(
this.$slugWrapper ) ) {
this.$slugWrapper = $slugWrapper
.addClass(
've-ce-branchNode-blockSlugWrapper-focused' )
.removeClass(
've-ce-branchNode-blockSlugWrapper-unfocused' );
- slugChange = true;
- newSlug = true;
+ enteredSlug = true;
}
- if ( slugChange ) {
+ if ( enteredSlug || leftSlug ) {
// Emit 'position' on the surface view after the
animation completes
this.setTimeout( function () {
if ( observer.surface ) {
@@ -286,7 +283,6 @@
},
{ text: text, hash: hash, range: range }
);
- slugChange = true;
}
this.text = text;
this.hash = hash;
@@ -301,13 +297,12 @@
this.range,
range
);
- slugChange = true;
}
this.range = range;
}
- if ( slugChange ) {
- this.emit( 'slugChange', range, newSlug );
+ if ( emitChanges && enteredSlug ) {
+ this.emit( 'slugEnter' );
}
};
--
To view, visit https://gerrit.wikimedia.org/r/164515
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie3531f71a1bae39ca57be0e641c71d18bc6aa9f5
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
