Divec has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/217257

Change subject: WIP: Explicitly enter/exit link annotations
......................................................................

WIP: Explicitly enter/exit link annotations

Link anchor tags are "nailed" with surrounding img nodes

TODO:
- Decide exactly where to call fixupCursorPosition (general event order cleanup)
- Account for cursor direction to avoid cursoring loops (requires 
cursor-offsets)
- Better function name for fixupCursorPosition
- Debug image for annotation nails
- Better visualisation
- Better documentation

Change-Id: Ie643984c24168ae31d37e0eae8e9003b63ac4f18
---
M src/ce/annotations/ve.ce.LinkAnnotation.js
M src/ce/ve.ce.Annotation.js
M src/ce/ve.ce.ContentBranchNode.js
M src/ce/ve.ce.Surface.js
M src/ce/ve.ce.js
5 files changed, 131 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/VisualEditor/VisualEditor 
refs/changes/57/217257/1

diff --git a/src/ce/annotations/ve.ce.LinkAnnotation.js 
b/src/ce/annotations/ve.ce.LinkAnnotation.js
index 53c1eac..c4f73c9 100644
--- a/src/ce/annotations/ve.ce.LinkAnnotation.js
+++ b/src/ce/annotations/ve.ce.LinkAnnotation.js
@@ -19,12 +19,23 @@
        ve.ce.LinkAnnotation.super.apply( this, arguments );
 
        // Initialization
-       this.$element
-               .addClass( 've-ce-linkAnnotation' )
+       this.contentTextNode = document.createTextNode( '' );
+       this.contentTextNode.containerParent = this.$element[0].parentNode;
+
+       this.$anchor = $( '<a>' )
                .prop( {
                        href: ve.resolveUrl( this.model.getHref(), 
this.getModelHtmlDocument() ),
                        title: this.constructor.static.getDescription( 
this.model )
-               } );
+               } )
+               .append( this.constructor.static.makePostNail() )
+               .append( this.contentTextNode )
+               .append( this.constructor.static.makePreNail() );
+
+       this.$element
+               .addClass( 've-ce-linkAnnotation' )
+               .append( this.constructor.static.makePreNail() )
+               .append( this.$anchor )
+               .append( this.constructor.static.makePostNail() );
 };
 
 /* Inheritance */
@@ -35,9 +46,9 @@
 
 ve.ce.LinkAnnotation.static.name = 'link';
 
-ve.ce.LinkAnnotation.static.tagName = 'a';
+ve.ce.LinkAnnotation.static.tagName = 'span';
 
-ve.ce.LinkAnnotation.static.forceContinuation = true;
+ve.ce.LinkAnnotation.static.forceContinuation = false;
 
 /* Static Methods */
 
@@ -48,6 +59,26 @@
        return model.getHref();
 };
 
+ve.ce.LinkAnnotation.static.makePreNail = function () {
+       return $( '<img>' )
+               .prop( 'src', ve.ce.chimeraImgDataUri )
+               .addClass( 've-ce-pre-nail' )
+               .get( 0 );
+};
+
+ve.ce.LinkAnnotation.static.makePostNail = function () {
+       return $( '<img>' )
+               .prop( 'src', ve.ce.chimeraImgDataUri )
+               .addClass( 've-ce-post-nail' )
+               .get( 0 );
+};
+
+/* Methods */
+
+ve.ce.LinkAnnotation.prototype.getContentNode = function () {
+       return this.contentTextNode;
+};
+
 /* Registration */
 
 ve.ce.annotationFactory.register( ve.ce.LinkAnnotation );
diff --git a/src/ce/ve.ce.Annotation.js b/src/ce/ve.ce.Annotation.js
index 99bdd70..fac6314 100644
--- a/src/ce/ve.ce.Annotation.js
+++ b/src/ce/ve.ce.Annotation.js
@@ -77,6 +77,14 @@
        return this.parentNode && this.parentNode.getModelHtmlDocument();
 };
 
+ve.ce.Annotation.prototype.appendChild = function ( childNode ) {
+       this.$element.append( childNode );
+};
+
+ve.ce.Annotation.prototype.getContentNode = function () {
+       return this.$element[0];
+};
+
 /**
  * Release all memory.
  */
diff --git a/src/ce/ve.ce.ContentBranchNode.js 
b/src/ce/ve.ce.ContentBranchNode.js
index 45a93e8..0601dba 100644
--- a/src/ce/ve.ce.ContentBranchNode.js
+++ b/src/ce/ve.ce.ContentBranchNode.js
@@ -162,7 +162,7 @@
  * @returns {Object} return.unicornInfo Unicorn information
  */
 ve.ce.ContentBranchNode.prototype.getRenderedContents = function () {
-       var i, ilen, j, jlen, item, itemAnnotations, ann, clone, dmSurface, 
dmSelection, relCursor,
+       var i, ilen, j, jlen, item, itemAnnotations, clone, dmSurface, 
dmSelection, relCursor,
                unicorn, img1, img2, annotationsChanged, childLength, offset, 
htmlItem, ceSurface,
                nextItemAnnotations, linkAnnotations,
                store = this.model.doc.getStore(),
@@ -171,30 +171,51 @@
                doc = this.getElementDocument(),
                wrapper = doc.createElement( 'div' ),
                current = wrapper,
+               nodeStack = [],
                unicornInfo = {},
                buffer = '',
                node = this;
 
        function openAnnotation( annotation ) {
+               var ann;
                annotationsChanged = true;
                if ( buffer !== '' ) {
-                       current.appendChild( doc.createTextNode( buffer ) );
+                       if ( current.nodeType === Node.TEXT_NODE ) {
+                               current.textContent += buffer;
+                       } else {
+                               current.appendChild( doc.createTextNode( buffer 
) );
+                       }
                        buffer = '';
                }
                // Create a new DOM node and descend into it
-               ann = ve.ce.annotationFactory.create( annotation.getType(), 
annotation, node ).$element[0];
-               current.appendChild( ann );
-               current = ann;
+               ann = ve.ce.annotationFactory.create( annotation.getType(), 
annotation, node );
+               if ( current.nodeType === Node.TEXT_NODE ) {
+                       // Insert after. Works even if nextSibling is null
+                       current.parentNode.insertBefore(
+                               ann.$element[0],
+                               current.nextSibling
+                       );
+               } else {
+                       current.appendChild( ann.$element[0] );
+               }
+               nodeStack.push( current );
+               current = ann.getContentNode();
        }
 
-       function closeAnnotation() {
+       function closeAnnotation( annotation ) {
+               var ann;
                annotationsChanged = true;
                if ( buffer !== '' ) {
-                       current.appendChild( doc.createTextNode( buffer ) );
+                       if ( current.nodeType === Node.TEXT_NODE ) {
+                               current.textContent += buffer;
+                       } else {
+                               current.appendChild( doc.createTextNode( buffer 
) );
+                       }
                        buffer = '';
                }
                // Traverse up
-               current = current.parentNode;
+               ann = ve.ce.annotationFactory.create( annotation.getType(), 
annotation, node );
+               current = nodeStack.pop();
        }
 
        // Gather annotated HTML from the child nodes
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index 4377f75..9c2bd48 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -76,7 +76,7 @@
        this.keyDownState = {
                event: null,
                selection: null,
-               focusIsAfterAnnotationBoundaries: null
+               focusIsAfterAnnotationBoundary: null
        };
 
        this.cursorDirectionality = null;
@@ -1468,6 +1468,7 @@
                this.decRenderLock();
        }
        this.checkUnicorns( fixupCursorForUnicorn );
+       this.fixupCursorPosition();
 };
 
 /**
@@ -2536,6 +2537,8 @@
                // Ignore when the newRange is just a flipped oldRange
                return;
        }
+
+       this.fixupCursorPosition();
        this.incRenderLock();
        try {
                this.changeModel(
@@ -2558,6 +2561,50 @@
                // The current range is the last range, so remove ranges from 
the front
                this.nativeSelection.removeRange( 
this.nativeSelection.getRangeAt( 0 ) );
        }
+};
+
+/**
+ * Move cursor if it is between annotation nails
+ *
+ * TODO: Account for direction, to make cursoring work (needs cursor-offsets). 
Improve name
+ */
+ve.ce.Surface.prototype.fixupCursorPosition = function () {
+       var range, node, offset, previousNode, nextNode;
+
+       if ( this.nativeSelection.rangeCount === 0 ) {
+               return;
+       }
+       range = this.nativeSelection.getRangeAt( 0 );
+
+       node = range.endContainer;
+       offset = range.endOffset;
+       if ( node.nodeType !== Node.ELEMENT_NODE ) {
+               return;
+       }
+       previousNode = node.childNodes[ offset - 1 ];
+       nextNode = node.childNodes[ offset ];
+
+       if (
+               previousNode &&
+               previousNode.nodeType === Node.ELEMENT_NODE &&
+               previousNode.classList.contains( 've-ce-pre-nail' )
+       ) {
+               offset -= 1;
+       } else if (
+               nextNode &&
+               nextNode.nodeType === Node.ELEMENT_NODE &&
+               nextNode.classList.contains( 've-ce-post-nail' )
+       ) {
+               offset += 1;
+       } else {
+               return;
+       }
+       if ( range.collapsed ) {
+               range.setStart( range.endContainer, offset );
+       }
+       range.setEnd( range.endContainer, offset );
+       this.nativeSelection.removeAllRanges();
+       this.nativeSelection.addRange( range );
 };
 
 /**
@@ -2667,7 +2714,7 @@
                        // Apply insertion annotations
                        if ( node.unicornAnnotations ) {
                                annotations = node.unicornAnnotations;
-                       } else if ( 
this.keyDownState.focusIsAfterAnnotationBoundaries ) {
+                       } else if ( 
this.keyDownState.focusIsAfterAnnotationBoundary ) {
                                annotations = 
modelData.getAnnotationsFromOffset(
                                        nodeOffset + previousStart + 1
                                );
@@ -2870,17 +2917,17 @@
  * modified, because anchorNode/focusNode are live and mutable, and so the 
offsets may come to
  * point confusingly to different places than they did when the selection was 
saved).
  *
- * Annotation changes before the cursor focus are detected: see 
ve.ce.isAfterAnnotationBoundaries .
+ * Annotation changes before the cursor focus are detected: see 
ve.ce.isAfterAnnotationBoundary .
  *
  * @param {jQuery.Event|null} e Key down event; must be active when this call 
is made
  */
 ve.ce.Surface.prototype.storeKeyDownState = function ( e ) {
        this.keyDownState.event = e;
        this.keyDownState.selection = null;
-       this.keyDownState.focusIsAfterAnnotationBoundaries = null;
+       this.keyDownState.focusIsAfterAnnotationBoundary = null;
 
        if ( this.nativeSelection.rangeCount > 0 ) {
-               this.keyDownState.focusIsAfterAnnotationBoundaries = 
ve.ce.isAfterAnnotationBoundaries(
+               this.keyDownState.focusIsAfterAnnotationBoundary = 
ve.ce.isAfterAnnotationBoundary(
                        this.nativeSelection.focusNode,
                        this.nativeSelection.focusOffset
                );
diff --git a/src/ce/ve.ce.js b/src/ce/ve.ce.js
index c872a12..5682f08 100644
--- a/src/ce/ve.ce.js
+++ b/src/ce/ve.ce.js
@@ -397,8 +397,8 @@
  * @param {number} offset Position offset
  * @return {boolean} Whether this is the end-most of multiple 
cursor-equivalent positions
  */
-ve.ce.isAfterAnnotationBoundaries = function ( node, offset ) {
-       var previousNode, nextNode;
+ve.ce.isAfterAnnotationBoundary = function ( node, offset ) {
+       var previousNode;
        if ( node.nodeType === Node.TEXT_NODE ) {
                if ( offset > 0 ) {
                        return false;
@@ -409,12 +409,12 @@
        if ( offset === 0 ) {
                return ve.dm.modelRegistry.isAnnotation( node );
        }
+
        previousNode = node.childNodes[ offset - 1 ];
-       if ( !previousNode || !ve.dm.modelRegistry.isAnnotation( previousNode ) 
) {
-               return false;
+       if ( previousNode.classList.contains( 've-ce-post-nail' ) ) {
+               return true;
        }
-       nextNode = node.childNodes[ offset ];
-       return !nextNode || !ve.dm.modelRegistry.isAnnotation( nextNode );
+       return ve.dm.modelRegistry.isAnnotation( previousNode );
 };
 
 /**

-- 
To view, visit https://gerrit.wikimedia.org/r/217257
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie643984c24168ae31d37e0eae8e9003b63ac4f18
Gerrit-PatchSet: 1
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Divec <da...@sheetmusic.org.uk>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to