jenkins-bot has submitted this change and it was merged.
Change subject: Emit 'position' and 'contextChange' events correctly
......................................................................
Emit 'position' and 'contextChange' events correctly
Emit position events after model transaction updates.
Emit contextChange events even when staging.
Focusable/ResizeableNode:
* No need to listen to model transact as that now triggers
a position event on the view
Context:
* Don't listen to 'select' in the base class, just contextChange
by default.
* Don't close inspector on contextChange if we have a selected
node that hasn't changed now that they are emitted while staging.
DesktopContext:
* Connect model select and view position events to onPosition.
As a result of fixing context events, debounce the updateDimensions
handler.
Change-Id: I98e0559a993620f057e6036bfd8f6a21bf59e788
---
M src/ce/ve.ce.FocusableNode.js
M src/ce/ve.ce.ResizableNode.js
M src/ce/ve.ce.Surface.js
M src/dm/ve.dm.Surface.js
M src/ui/ve.ui.Context.js
M src/ui/ve.ui.DesktopContext.js
6 files changed, 43 insertions(+), 52 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/ce/ve.ce.FocusableNode.js b/src/ce/ve.ce.FocusableNode.js
index 74bcf4a..d233f27 100644
--- a/src/ce/ve.ce.FocusableNode.js
+++ b/src/ce/ve.ce.FocusableNode.js
@@ -361,7 +361,6 @@
'mouseout.ve-ce-focusableNode': ve.bind(
this.onSurfaceMouseOut, this )
} );
}
- this.surface.getModel().getDocument().connect( this, { transact:
'positionHighlights' } );
this.surface.connect( this, { position: 'positionHighlights' } );
};
@@ -376,7 +375,6 @@
}
this.$highlights.remove().empty();
this.surface.$element.off( '.ve-ce-focusableNode' );
- this.surface.getModel().getDocument().disconnect( this, { transact:
'positionHighlights' } );
this.surface.disconnect( this, { position: 'positionHighlights' } );
this.highlighted = false;
this.boundingRect = null;
diff --git a/src/ce/ve.ce.ResizableNode.js b/src/ce/ve.ce.ResizableNode.js
index fd11d73..dfcdf44 100644
--- a/src/ce/ve.ce.ResizableNode.js
+++ b/src/ce/ve.ce.ResizableNode.js
@@ -210,14 +210,13 @@
* @method
*/
ve.ce.ResizableNode.prototype.onResizableFocus = function () {
- var surface = this.getRoot().getSurface(),
- documentModel = surface.getModel().getDocument();
+ var surface = this.getRoot().getSurface();
if ( this.$sizeLabel ) {
// Attach the size label first so it doesn't mask the resize
handles
- this.$sizeLabel.appendTo(
this.root.getSurface().getSurface().$controls );
+ this.$sizeLabel.appendTo( surface.getSurface().$controls );
}
- this.$resizeHandles.appendTo(
this.root.getSurface().getSurface().$controls );
+ this.$resizeHandles.appendTo( surface.getSurface().$controls );
// Call getScalable to pre-fetch the extended data
this.model.getScalable();
@@ -244,7 +243,6 @@
ve.bind( this.onResizeHandlesCornerMouseDown, this )
);
- documentModel.connect( this, { transact:
'setResizableHandlesSizeAndPosition' } );
surface.connect( this, { position: 'setResizableHandlesSizeAndPosition'
} );
};
@@ -260,15 +258,13 @@
return;
}
- var surface = this.getRoot().getSurface(),
- documentModel = surface.getModel().getDocument();
+ var surface = this.getRoot().getSurface();
this.$resizeHandles.detach();
if ( this.$sizeLabel ) {
this.$sizeLabel.detach();
}
- documentModel.disconnect( this, { transact:
'setResizableHandlesSizeAndPosition' } );
surface.disconnect( this, { position:
'setResizableHandlesSizeAndPosition' } );
};
diff --git a/src/ce/ve.ce.Surface.js b/src/ce/ve.ce.Surface.js
index af231af..897284f 100644
--- a/src/ce/ve.ce.Surface.js
+++ b/src/ce/ve.ce.Surface.js
@@ -1589,6 +1589,7 @@
}
// Update the state of the SurfaceObserver
this.surfaceObserver.pollOnceNoEmit();
+ this.emit( 'position' );
};
/**
diff --git a/src/dm/ve.dm.Surface.js b/src/dm/ve.dm.Surface.js
index 0845bec..bd69b75 100644
--- a/src/dm/ve.dm.Surface.js
+++ b/src/dm/ve.dm.Surface.js
@@ -319,9 +319,7 @@
new ve.dm.AnnotationSet( this.documentModel.getStore() );
this.emit( 'insertionAnnotationsChange', this.insertionAnnotations );
- if ( !this.isStaging() ) {
- this.emit( 'contextChange' );
- }
+ this.emit( 'contextChange' );
};
/**
@@ -345,9 +343,7 @@
}
this.emit( 'insertionAnnotationsChange', this.insertionAnnotations );
- if ( !this.isStaging() ) {
- this.emit( 'contextChange' );
- }
+ this.emit( 'contextChange' );
};
/**
@@ -371,9 +367,7 @@
}
this.emit( 'insertionAnnotationsChange', this.insertionAnnotations );
- if ( !this.isStaging() ) {
- this.emit( 'contextChange' );
- }
+ this.emit( 'contextChange' );
};
/**
@@ -497,7 +491,7 @@
ve.dm.Surface.prototype.emitContextChange = function () {
if ( this.queueingContextChanges ) {
this.contextChangeQueued = true;
- } else if ( !this.isStaging() ) {
+ } else {
this.emit( 'contextChange' );
}
};
@@ -516,9 +510,7 @@
this.queueingContextChanges = false;
if ( this.contextChangeQueued ) {
this.contextChangeQueued = false;
- if ( !this.isStaging() ) {
- this.emit( 'contextChange' );
- }
+ this.emit( 'contextChange' );
}
}
};
@@ -814,3 +806,12 @@
}
this.emit( 'documentUpdate', tx );
};
+
+/**
+ * Get the selected node covering the current range, or null
+ *
+ * @return {ve.dm.Node} Selected node
+ */
+ve.dm.Surface.prototype.getSelectedNode = function () {
+ return this.selectedNodes.start === this.selectedNodes.end ?
this.selectedNodes.start : null;
+};
diff --git a/src/ui/ve.ui.Context.js b/src/ui/ve.ui.Context.js
index cd57910..05555b2 100644
--- a/src/ui/ve.ui.Context.js
+++ b/src/ui/ve.ui.Context.js
@@ -26,14 +26,13 @@
this.inspector = null;
this.inspectors = this.createInspectorWindowManager();
this.menu = new ve.ui.ContextMenuWidget( { $: this.$ } );
+ this.lastSelectedNode = null;
this.afterContextChangeTimeout = null;
this.afterContextChangeHandler = ve.bind( this.afterContextChange, this
);
+ this.updateDimensionsDebounced = ve.debounce( ve.bind(
this.updateDimensions, this ) );
// Events
- this.surface.getModel().connect( this, {
- contextChange: 'onContextChange',
- select: 'onContextChange'
- } );
+ this.surface.getModel().connect( this, { contextChange:
'onContextChange' } );
this.inspectors.connect( this, { opening: 'onInspectorOpening' } );
this.menu.connect( this, { choose: 'onContextItemChoose' } );
@@ -81,6 +80,8 @@
* Handle debounced context change events.
*/
ve.ui.Context.prototype.afterContextChange = function () {
+ var selectedNode = this.surface.getModel().getSelectedNode();
+
// Reset debouncing state
this.afterContextChangeTimeout = null;
@@ -89,14 +90,15 @@
if ( this.isInspectable() ) {
// Change state: menu -> menu
this.populateMenu();
- this.updateDimensions();
+ this.updateDimensionsDebounced();
} else {
// Change state: menu -> closed
this.menu.toggle( false );
this.toggle( false );
}
- } else if ( this.inspector ) {
+ } else if ( this.inspector && ( !selectedNode || ( selectedNode
!== this.lastSelectedNode ) ) ) {
// Change state: inspector -> (closed|menu)
+ // Unless there is a selectedNode that hasn't changed
(e.g. your inspector is editing a node)
this.inspector.close();
}
} else {
@@ -107,6 +109,8 @@
this.toggle( true );
}
}
+
+ this.lastSelectedNode = selectedNode;
};
/**
@@ -132,7 +136,7 @@
this.toggle( true );
}
}
- this.updateDimensions( true );
+ this.updateDimensionsDebounced();
}, this ) )
.always( ve.bind( function ( opened ) {
opened.always( ve.bind( function ( closed ) {
@@ -145,7 +149,7 @@
// Change state: inspector ->
menu
this.menu.toggle( true );
this.populateMenu();
- this.updateDimensions();
+
this.updateDimensionsDebounced();
} else {
// Change state: inspector ->
closed
this.toggle( false );
@@ -302,7 +306,6 @@
/**
* Update the size and position of the context.
*
- * @param {boolean} [transition] Smoothly transition from previous size and
position
* @chainable
*/
ve.ui.Context.prototype.updateDimensions = function () {
diff --git a/src/ui/ve.ui.DesktopContext.js b/src/ui/ve.ui.DesktopContext.js
index 308d219..06ec337 100644
--- a/src/ui/ve.ui.DesktopContext.js
+++ b/src/ui/ve.ui.DesktopContext.js
@@ -23,7 +23,7 @@
this.popup = new OO.ui.PopupWidget( { $: this.$, $container:
this.surface.$element } );
this.transitioning = null;
this.suppressed = false;
- this.onWindowResizeHandler = ve.bind( this.onWindowResize, this );
+ this.onWindowResizeHandler = ve.bind( this.onPosition, this );
this.$window = this.$( this.getElementWindow() );
// Events
@@ -34,7 +34,10 @@
relocationEnd: 'onUnsuppress',
blur: 'onSuppress',
focus: 'onUnsuppress',
- position: 'onSurfacePosition'
+ position: 'onPosition'
+ } );
+ this.surface.getModel().connect( this, {
+ select: 'onPosition'
} );
this.$window.on( 'resize', this.onWindowResizeHandler );
this.$element.on( 'mousedown', false );
@@ -107,22 +110,11 @@
};
/**
- * Handle surface position event.
+ * Handle cursor position change event.
*/
-ve.ui.DesktopContext.prototype.onSurfacePosition = function () {
+ve.ui.DesktopContext.prototype.onPosition = function () {
if ( this.isVisible() ) {
- this.updateDimensions( true );
- }
-};
-
-/**
- * Handle window resize events.
- *
- * @param {jQuery.Event} e Window resize event
- */
-ve.ui.DesktopContext.prototype.onWindowResize = function () {
- if ( this.isVisible() ) {
- this.updateDimensions();
+ this.updateDimensionsDebounced();
}
};
@@ -174,7 +166,7 @@
if ( this.inspector ) {
this.inspectors.updateWindowSize( this.inspector );
}
- this.updateDimensions();
+ this.updateDimensionsDebounced();
} else if ( this.inspector ) {
this.inspector.close();
}
@@ -185,7 +177,7 @@
/**
* @inheritdoc
*/
-ve.ui.DesktopContext.prototype.updateDimensions = function ( transition ) {
+ve.ui.DesktopContext.prototype.updateDimensions = function () {
var $container, inlineRects, position, embeddable, middle,
rtl = this.surface.getModel().getDocument().getDir() === 'rtl',
surface = this.surface.getView(),
@@ -253,8 +245,7 @@
this.popup.setSize(
$container.outerWidth( true ),
- $container.outerHeight( true ),
- false && transition
+ $container.outerHeight( true )
);
return this;
@@ -266,6 +257,7 @@
ve.ui.DesktopContext.prototype.destroy = function () {
// Disconnect
this.surface.getView().disconnect( this );
+ this.surface.getModel().disconnect( this );
this.$window.off( 'resize', this.onWindowResizeHandler );
// Parent method
--
To view, visit https://gerrit.wikimedia.org/r/160034
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I98e0559a993620f057e6036bfd8f6a21bf59e788
Gerrit-PatchSet: 9
Gerrit-Project: VisualEditor/VisualEditor
Gerrit-Branch: master
Gerrit-Owner: Esanders <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
