Catrope has uploaded a new change for review.
https://gerrit.wikimedia.org/r/57245
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, 93 insertions(+), 87 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor
refs/changes/45/57245/1
diff --git a/modules/ve/ce/ve.ce.ContentBranchNode.js
b/modules/ve/ce/ve.ce.ContentBranchNode.js
index e7afb4e..b4a6d45 100644
--- a/modules/ve/ce/ve.ce.ContentBranchNode.js
+++ b/modules/ve/ce/ve.ce.ContentBranchNode.js
@@ -62,6 +62,18 @@
$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++ ) {
annotatedHtml = annotatedHtml.concat(
this.children[i].getAnnotatedHtml() );
@@ -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..8ccc64d 100644
--- a/modules/ve/dm/ve.dm.Converter.js
+++ b/modules/ve/dm/ve.dm.Converter.js
@@ -47,6 +47,54 @@
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, 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.
+ startClosingAt = undefined;
+ arr = currentSet.get();
+ for ( i = 0; i < arr.length; 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; i < arr.length; i++ ) {
+ annotation = arr[i];
+ if ( !currentSet.contains( annotation ) ) {
+ open( annotation );
+ // Add to currentClone
+ currentSet.push( annotation );
+ }
+ }
+};
+
/* Methods */
/**
@@ -549,13 +597,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 +664,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: newchange
Gerrit-Change-Id: I7cba7d2fda7002287b07949a1b8120ba80bfe854
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Catrope <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
