jenkins-bot has submitted this change and it was merged.
Change subject: Reduce code duplication for annotation rendering
......................................................................
Reduce code duplication for annotation rendering
ve.dm.Converter and ve.ce.ContentBranchNode were duplicating a fair bit
of logic for annotation rendering. Moved the annotation opening and
closing logic into ve.dm.Converter.openAndCloseAnnotations, and
implemented both annotation rendering code paths in terms of that
function with callbacks for caller-specific behavior.
Change-Id: I7cba7d2fda7002287b07949a1b8120ba80bfe854
---
M modules/ve/ce/ve.ce.ContentBranchNode.js
M modules/ve/dm/ve.dm.Converter.js
2 files changed, 95 insertions(+), 90 deletions(-)
Approvals:
Esanders: Looks good to me, approved
jenkins-bot: Verified
diff --git a/modules/ve/ce/ve.ce.ContentBranchNode.js
b/modules/ve/ce/ve.ce.ContentBranchNode.js
index e7afb4e..8c589a7 100644
--- a/modules/ve/ce/ve.ce.ContentBranchNode.js
+++ b/modules/ve/ce/ve.ce.ContentBranchNode.js
@@ -55,12 +55,24 @@
* @returns {jQuery}
*/
ve.ce.ContentBranchNode.prototype.getRenderedContents = function () {
- var i, j, itemHtml, itemAnnotations, startClosingAt, arr, annotation,
$ann,
+ var i, itemHtml, itemAnnotations, $ann,
store = this.model.doc.getStore(),
annotationStack = new ve.dm.AnnotationSet( store ),
annotatedHtml = [],
$wrapper = $( '<div>' ),
$current = $wrapper;
+
+ function openAnnotation( annotation ) {
+ // Create a new DOM node and descend into it
+ $ann = ve.ce.annotationFactory.create( annotation.getType(),
annotation ).$;
+ $current.append( $ann );
+ $current = $ann;
+ }
+
+ function closeAnnotation() {
+ // Traverse up
+ $current = $current.parent();
+ }
// Gather annotated HTML from the child nodes
for ( i = 0; i < this.children.length; i++ ) {
@@ -77,43 +89,9 @@
itemAnnotations = new ve.dm.AnnotationSet( store );
}
- // FIXME code largely copied from ve.dm.Converter
- // Close annotations as needed
- // Go through annotationStack from bottom to top (low to high),
- // and find the first annotation that's not in annotations.
- startClosingAt = undefined;
- arr = annotationStack.get();
- for ( j = 0; j < arr.length; j++ ) {
- annotation = arr[j];
- if ( !itemAnnotations.contains( annotation ) ) {
- startClosingAt = j;
- break;
- }
- }
- if ( startClosingAt !== undefined ) {
- // Close all annotations from top to bottom (high to
low)
- // until we reach startClosingAt
- for ( j = annotationStack.getLength() - 1; j >=
startClosingAt; j-- ) {
- // Traverse up
- $current = $current.parent();
- // Remove from annotationStack
- annotationStack.removeAt( j );
- }
- }
-
- // Open annotations as needed
- arr = itemAnnotations.get();
- for ( j = 0; j < arr.length; j++ ) {
- annotation = arr[j];
- if ( !annotationStack.contains( annotation ) ) {
- // Create new node and descend into it
- $ann = ve.ce.annotationFactory.create(
annotation.getType(), annotation ).$;
- $current.append( $ann );
- $current = $ann;
- // Add to annotationStack
- annotationStack.push( annotation );
- }
- }
+ ve.dm.Converter.openAndCloseAnnotations( annotationStack,
itemAnnotations,
+ openAnnotation, closeAnnotation
+ );
// Output the actual HTML
$current.append( itemHtml );
diff --git a/modules/ve/dm/ve.dm.Converter.js b/modules/ve/dm/ve.dm.Converter.js
index 3c60157..8b23684 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -35,16 +35,63 @@
* @returns {Array} Linear model data, one element per character
*/
ve.dm.Converter.getDataContentFromText = function ( text, annotations ) {
- var i, characters = text.split( '' );
+ var i, len, characters = text.split( '' );
if ( !annotations || annotations.isEmpty() ) {
return characters;
}
// Apply annotations to characters
- for ( i = 0; i < characters.length; i++ ) {
+ for ( i = 0, len = characters.length; i < len; i++ ) {
// Just store the annotations' indexes from the index-value
store
characters[i] = [characters[i],
annotations.getIndexes().slice()];
}
return characters;
+};
+
+/**
+ * Utility function for annotation rendering. Transforms one set of
annotations into another
+ * by opening and closing annotations. Each time an annotation is opened or
closed, the associated
+ * callback is called with the annotation passed as a parameter.
+ *
+ * Note that currentSet will be modified, and will be equal to targetSet once
this function returns.
+ *
+ * @param {ve.dm.AnnotationSet} currentSet The set of annotations currently
opened. Will be modified.
+ * @param {ve.dm.AnnotationSet} targetSet The set of annotations we want to
have.
+ * @param {Function} open Callback called when an annotation is opened. Passed
a ve.dm.Annotation.
+ * @param {Function} close Callback called when an annotation is closed.
Passed a ve.dm.Annotation.
+ */
+ve.dm.Converter.openAndCloseAnnotations = function ( currentSet, targetSet,
open, close ) {
+ var i, len, arr, annotation, startClosingAt;
+ // Close annotations as needed
+ // Go through annotationStack from bottom to top (low to high),
+ // and find the first annotation that's not in annotations.
+ arr = currentSet.get();
+ for ( i = 0, len = arr.length; i < len; i++ ) {
+ annotation = arr[i];
+ if ( !targetSet.contains( annotation ) ) {
+ startClosingAt = i;
+ break;
+ }
+ }
+ if ( startClosingAt !== undefined ) {
+ // Close all annotations from top to bottom (high to low)
+ // until we reach startClosingAt
+ for ( i = currentSet.getLength() - 1; i >= startClosingAt; i--
) {
+ close( arr[i] );
+ // Remove from currentClone
+ currentSet.removeAt( i );
+ }
+ }
+
+ // Open annotations as needed
+ arr = targetSet.get();
+ for ( i = 0, len = arr.length; i < len; i++ ) {
+ annotation = arr[i];
+ if ( !currentSet.contains( annotation ) ) {
+ open( annotation );
+ // Add to currentClone
+ currentSet.push( annotation );
+ }
+ }
};
/* Methods */
@@ -549,13 +596,38 @@
* @returns {HTMLDocument} Document containing the resulting HTML
*/
ve.dm.Converter.prototype.getDomFromData = function ( store, data ) {
- var text, i, j, k, annotations, annotation, annotationElement,
dataElement, arr,
- childDomElements, pre, ours, theirs, parentDomElement,
lastChild, startClosingAt,
+ var text, i, j, k, annotations, annotationElement, dataElement,
+ childDomElements, pre, ours, theirs, parentDomElement,
lastChild,
isContentNode, changed, parentChanged, sibling,
previousSiblings, doUnwrap, textNode,
+ conv = this,
doc = ve.createDocumentFromHTML( '' ),
container = doc.body,
domElement = container,
annotationStack = new ve.dm.AnnotationSet( store );
+
+ function openAnnotation( annotation ) {
+ // Add text if needed
+ if ( text.length > 0 ) {
+ domElement.appendChild( doc.createTextNode( text ) );
+ text = '';
+ }
+ // Create new node and descend into it
+ annotationElement = conv.getDomElementsFromDataElement(
+ annotation.getElement(), doc
+ )[0];
+ domElement.appendChild( annotationElement );
+ domElement = annotationElement;
+ }
+
+ function closeAnnotation() {
+ // Add text if needed
+ if ( text.length > 0 ) {
+ domElement.appendChild( doc.createTextNode( text ) );
+ text = '';
+ }
+ // Traverse up
+ domElement = domElement.parentNode;
+ }
for ( i = 0; i < data.length; i++ ) {
if ( typeof data[i] === 'string' ) {
@@ -591,54 +663,9 @@
annotations = new ve.dm.AnnotationSet(
store, store.values(
data[i].annotations || data[i][1] )
);
- // Close annotations as needed
- // Go through annotationStack from bottom to
top (low to high),
- // and find the first annotation that's not in
annotations.
- startClosingAt = undefined;
- arr = annotationStack.get();
- for ( j = 0; j < arr.length; j++ ) {
- annotation = arr[j];
- if ( !annotations.contains( annotation
) ) {
- startClosingAt = j;
- break;
- }
- }
- if ( startClosingAt !== undefined ) {
- // Close all annotations from top to
bottom (high to low)
- // until we reach startClosingAt
- for ( j = annotationStack.getLength() -
1; j >= startClosingAt; j-- ) {
- // Add text if needed
- if ( text.length > 0 ) {
- domElement.appendChild(
doc.createTextNode( text ) );
- text = '';
- }
- // Traverse up
- domElement =
domElement.parentNode;
- // Remove from annotationStack
- annotationStack.removeAt( j );
- }
- }
-
- // Open annotations as needed
- arr = annotations.get();
- for ( j = 0; j < arr.length; j++ ) {
- annotation = arr[j];
- if ( !annotationStack.contains(
annotation ) ) {
- // Add text if needed
- if ( text.length > 0 ) {
- domElement.appendChild(
doc.createTextNode( text ) );
- text = '';
- }
- // Create new node and descend
into it
- annotationElement =
this.getDomElementsFromDataElement(
-
annotation.getElement(), doc
- )[0];
- domElement.appendChild(
annotationElement );
- domElement = annotationElement;
- // Add to annotationStack
- annotationStack.push(
annotation );
- }
- }
+ ve.dm.Converter.openAndCloseAnnotations(
annotationStack, annotations,
+ openAnnotation, closeAnnotation
+ );
if ( data[i].annotations === undefined ) {
// Annotated text
--
To view, visit https://gerrit.wikimedia.org/r/57245
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I7cba7d2fda7002287b07949a1b8120ba80bfe854
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Esanders <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
