jenkins-bot has submitted this change and it was merged.

Change subject: [BREAKING CHANGE] Emit rerender for dead nodes
......................................................................


[BREAKING CHANGE] Emit rerender for dead nodes

ve.ce.GeneratedContentNode was only emitting rerender when live, but in
some cases we need this event even if the node is not live.

Specifically, we need this for rendering ve.ce.Nodes outside of the CE
tree, such as in ContextItems.

Breaking changes:
* Changing ve.ce.GeneratedContentNode to always emit the 'rerender' event
  means it's upon the subscriber of the event to pay attention to whether
  the node is live or not. Changes to ve.ce.FocusableNode and
  ve.ce.ResizableNode have been made to deal with that.

Bonus:
* ve.ce.ResizableNode now caches its reference to this.root.getSurface(),
  the same way that ve.ce.FocusableNode does
* The surface references and 'isSetup' states of both
  ve.ce.ResizableNode and ve.ce.FocusableNode are now named more
  specifically to avoid conflicts and shadowing.
* ve.BranchNode now has a static traverse utility function that does
  simple depth-first recursive traversal over a branch node.

Bug: T91314
Change-Id: I2b95d068e0d6f1d293d95dc18ac5b4c7da0d3214
---
M src/ce/nodes/ve.ce.GeneratedContentNode.js
M src/ce/ve.ce.FocusableNode.js
M src/ce/ve.ce.ResizableNode.js
M src/ve.BranchNode.js
4 files changed, 82 insertions(+), 36 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/src/ce/nodes/ve.ce.GeneratedContentNode.js 
b/src/ce/nodes/ve.ce.GeneratedContentNode.js
index a03ea44..bdcc7b8 100644
--- a/src/ce/nodes/ve.ce.GeneratedContentNode.js
+++ b/src/ce/nodes/ve.ce.GeneratedContentNode.js
@@ -175,8 +175,9 @@
 
        if ( this.live ) {
                this.emit( 'setup' );
-               this.afterRender();
        }
+
+       this.afterRender();
 };
 
 /**
diff --git a/src/ce/ve.ce.FocusableNode.js b/src/ce/ve.ce.FocusableNode.js
index 42d857c..f627897 100644
--- a/src/ce/ve.ce.FocusableNode.js
+++ b/src/ce/ve.ce.FocusableNode.js
@@ -27,10 +27,10 @@
        // Properties
        this.focused = false;
        this.highlighted = false;
-       this.isSetup = false;
+       this.isFocusableSetup = false;
        this.$highlights = this.$( '<div>' ).addClass( 
've-ce-focusableNode-highlights' );
        this.$focusable = $focusable || this.$element;
-       this.surface = null;
+       this.focusableSurface = null;
        this.rects = null;
        this.boundingRect = null;
        this.startAndEndRects = null;
@@ -95,11 +95,11 @@
  */
 ve.ce.FocusableNode.prototype.onFocusableSetup = function () {
        // Exit if already setup or not attached
-       if ( this.isSetup || !this.root ) {
+       if ( this.isFocusableSetup || !this.root ) {
                return;
        }
 
-       this.surface = this.getRoot().getSurface();
+       this.focusableSurface = this.root.getSurface();
 
        // DOM changes (duplicated from constructor in case this.$element is 
replaced)
        this.$element
@@ -121,7 +121,7 @@
                } );
        }
 
-       this.isSetup = true;
+       this.isFocusableSetup = true;
 };
 
 /**
@@ -131,7 +131,7 @@
  */
 ve.ce.FocusableNode.prototype.onFocusableTeardown = function () {
        // Exit if not setup or not attached
-       if ( !this.isSetup || !this.root ) {
+       if ( !this.isFocusableSetup || !this.root ) {
                return;
        }
 
@@ -147,8 +147,8 @@
                .removeClass( 've-ce-focusableNode' )
                .removeProp( 'contentEditable' );
 
-       this.isSetup = false;
-       this.surface = null;
+       this.focusableSurface = null;
+       this.isFocusableSetup = false;
 };
 
 /**
@@ -159,7 +159,7 @@
  */
 ve.ce.FocusableNode.prototype.onFocusableMouseDown = function ( e ) {
        var range,
-               surfaceModel = this.surface.getModel(),
+               surfaceModel = this.focusableSurface.getModel(),
                selection = surfaceModel.getSelection(),
                nodeRange = this.model.getOuterRange();
 
@@ -203,7 +203,7 @@
        }
        var command = ve.ui.commandRegistry.getCommandForNode( this );
        if ( command ) {
-               command.execute( this.surface.getSurface() );
+               command.execute( this.focusableSurface.getSurface() );
        }
 };
 
@@ -214,9 +214,9 @@
  * @param {jQuery.Event} e Drag start event
  */
 ve.ce.FocusableNode.prototype.onFocusableDragStart = function () {
-       if ( this.surface ) {
+       if ( this.focusableSurface ) {
                // Allow dragging this node in the surface
-               this.surface.startRelocation( this );
+               this.focusableSurface.startRelocation( this );
        }
        this.$highlights.addClass( 've-ce-focusableNode-highlights-relocating' 
);
 };
@@ -232,8 +232,8 @@
 ve.ce.FocusableNode.prototype.onFocusableDragEnd = function () {
        // endRelocation is usually triggered by onDocumentDrop in the surface, 
but if it isn't
        // trigger it here instead
-       if ( this.surface ) {
-               this.surface.endRelocation();
+       if ( this.focusableSurface ) {
+               this.focusableSurface.endRelocation();
        }
        this.$highlights.removeClass( 
've-ce-focusableNode-highlights-relocating' );
 };
@@ -302,10 +302,10 @@
  * @method
  */
 ve.ce.FocusableNode.prototype.onFocusableRerender = function () {
-       if ( this.focused ) {
+       if ( this.focused && this.focusableSurface ) {
                this.redrawHighlights();
                // reposition menu
-               this.surface.getSurface().getContext().updateDimensions( true );
+               
this.focusableSurface.getSurface().getContext().updateDimensions( true );
        }
 };
 
@@ -335,8 +335,8 @@
                        this.emit( 'focus' );
                        this.$element.addClass( 've-ce-focusableNode-focused' );
                        this.createHighlights();
-                       this.surface.appendHighlights( this.$highlights, 
this.focused );
-                       this.surface.$element.off( '.ve-ce-focusableNode' );
+                       this.focusableSurface.appendHighlights( 
this.$highlights, this.focused );
+                       this.focusableSurface.$element.off( 
'.ve-ce-focusableNode' );
                } else {
                        this.emit( 'blur' );
                        this.$element.removeClass( 
've-ce-focusableNode-focused' );
@@ -364,16 +364,16 @@
 
        this.positionHighlights();
 
-       this.surface.appendHighlights( this.$highlights, this.focused );
+       this.focusableSurface.appendHighlights( this.$highlights, this.focused 
);
 
        // Events
        if ( !this.focused ) {
-               this.surface.$element.on( {
+               this.focusableSurface.$element.on( {
                        'mousemove.ve-ce-focusableNode': 
this.onSurfaceMouseMove.bind( this ),
                        'mouseout.ve-ce-focusableNode': 
this.onSurfaceMouseOut.bind( this )
                } );
        }
-       this.surface.connect( this, { position: 'positionHighlights' } );
+       this.focusableSurface.connect( this, { position: 'positionHighlights' } 
);
 };
 
 /**
@@ -386,8 +386,8 @@
                return;
        }
        this.$highlights.remove().empty();
-       this.surface.$element.off( '.ve-ce-focusableNode' );
-       this.surface.disconnect( this, { position: 'positionHighlights' } );
+       this.focusableSurface.$element.off( '.ve-ce-focusableNode' );
+       this.focusableSurface.disconnect( this, { position: 
'positionHighlights' } );
        this.highlighted = false;
        this.boundingRect = null;
 };
@@ -410,7 +410,7 @@
                rects = [],
                filteredRects = [],
                webkitColumns = 'webkitColumnCount' in document.createElement( 
'div' ).style,
-               surfaceOffset = 
this.surface.getSurface().getBoundingClientRect();
+               surfaceOffset = 
this.focusableSurface.getSurface().getBoundingClientRect();
 
        function contains( rect1, rect2 ) {
                return rect2.left >= rect1.left &&
diff --git a/src/ce/ve.ce.ResizableNode.js b/src/ce/ve.ce.ResizableNode.js
index 33c3856..37b34c7 100644
--- a/src/ce/ve.ce.ResizableNode.js
+++ b/src/ce/ve.ce.ResizableNode.js
@@ -36,11 +36,13 @@
                this.$sizeLabel = this.$( '<div>' ).addClass( 
've-ce-resizableNode-sizeLabel' ).append( this.$sizeText );
        }
        this.resizableOffset = null;
+       this.resizableSurface = null;
 
        // Events
        this.connect( this, {
                focus: 'onResizableFocus',
                blur: 'onResizableBlur',
+               setup: 'onResizableSetup',
                teardown: 'onResizableTeardown',
                resizing: 'onResizableResizing',
                resizeEnd: 'onResizableFocus',
@@ -97,7 +99,7 @@
 ve.ce.ResizableNode.prototype.getResizableOffset = function () {
        if ( !this.resizableOffset ) {
                this.resizableOffset = OO.ui.Element.static.getRelativePosition(
-                       this.$resizable, 
this.getRoot().getSurface().getSurface().$element
+                       this.$resizable, 
this.resizableSurface.getSurface().$element
                );
        }
        return this.resizableOffset;
@@ -209,11 +211,9 @@
  * @method
  */
 ve.ce.ResizableNode.prototype.onResizableFocus = function () {
-       var surface = this.getRoot().getSurface();
-
-       this.$resizeHandles.appendTo( surface.getSurface().$controls );
+       this.$resizeHandles.appendTo( 
this.resizableSurface.getSurface().$controls );
        if ( this.$sizeLabel ) {
-               this.$sizeLabel.appendTo( surface.getSurface().$controls );
+               this.$sizeLabel.appendTo( 
this.resizableSurface.getSurface().$controls );
        }
 
        // Call getScalable to pre-fetch the extended data
@@ -241,7 +241,7 @@
                        this.onResizeHandlesCornerMouseDown.bind( this )
                );
 
-       surface.connect( this, { position: 'setResizableHandlesSizeAndPosition' 
} );
+       this.resizableSurface.connect( this, { position: 
'setResizableHandlesSizeAndPosition' } );
 
 };
 
@@ -252,18 +252,16 @@
  */
 ve.ce.ResizableNode.prototype.onResizableBlur = function () {
        // Node may have already been torn down, e.g. after delete
-       if ( !this.getRoot() ) {
+       if ( !this.root ) {
                return;
        }
-
-       var surface = this.getRoot().getSurface();
 
        this.$resizeHandles.detach();
        if ( this.$sizeLabel ) {
                this.$sizeLabel.detach();
        }
 
-       surface.disconnect( this, { position: 
'setResizableHandlesSizeAndPosition' } );
+       this.resizableSurface.disconnect( this, { position: 
'setResizableHandlesSizeAndPosition' } );
 
 };
 
@@ -290,12 +288,34 @@
 };
 
 /**
+ * Handle setup event.
+ *
+ * @method
+ */
+ve.ce.ResizableNode.prototype.onResizableSetup = function () {
+       // Exit if already setup or not attached
+       if ( this.isResizableSetup || !this.root ) {
+               return;
+       }
+
+       this.resizableSurface = this.root.getSurface();
+       this.isResizableSetup = true;
+};
+
+/**
  * Handle teardown event.
  *
  * @method
  */
 ve.ce.ResizableNode.prototype.onResizableTeardown = function () {
+       // Exit if not setup or not attached
+       if ( !this.isResizableSetup || !this.root ) {
+               return;
+       }
+
        this.onResizableBlur();
+       this.resizableSurface = null;
+       this.isResizableSetup = false;
 };
 
 /**
@@ -507,7 +527,7 @@
                offset = this.model.getOffset(),
                width = this.$resizeHandles.outerWidth(),
                height = this.$resizeHandles.outerHeight(),
-               surfaceModel = this.getRoot().getSurface().getModel(),
+               surfaceModel = this.resizableSurface.getModel(),
                documentModel = surfaceModel.getDocument(),
                selection = surfaceModel.getSelection();
 
diff --git a/src/ve.BranchNode.js b/src/ve.BranchNode.js
index 695075e..9ffbbfb 100644
--- a/src/ve.BranchNode.js
+++ b/src/ve.BranchNode.js
@@ -22,6 +22,31 @@
        this.children = Array.isArray( children ) ? children : [];
 };
 
+/* Setup */
+
+OO.initClass( ve.BranchNode );
+
+/* Static Methods */
+
+/**
+ * Traverse a branch node depth-first.
+ *
+ * @param {ve.BranchNode} node Branch node to traverse
+ * @param {Function} callback Callback to execute for each traversed node
+ * @param {ve.Node} callback.node Node being traversed
+ */
+ve.BranchNode.static.traverse = function ( node, callback ) {
+       var i, len,
+               children = node.getChildren();
+
+       for ( i = 0, len = children.length; i < len; i++ ) {
+               callback.call( this, children[i] );
+               if ( children[i] instanceof ve.ce.BranchNode ) {
+                       this.traverse( children[i], callback );
+               }
+       }
+};
+
 /* Methods */
 
 /**

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I2b95d068e0d6f1d293d95dc18ac5b4c7da0d3214
Gerrit-PatchSet: 2
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Trevor Parscal <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to