From febc0f567fa318edc6b353fdb93c4ea635062d21 Mon Sep 17 00:00:00 2001 From: Roan Kattouw Date: Mon, 21 Oct 2013 14:40:03 +0200 Subject: [PATCH] Defer selection-triggered updates in ve.ui.Context Move selection change handling (closing the popup if open, and updating the context toolbar) to .afterChange(). Every time .onChange() detects a selection change, it schedules a call to afterChange(). These calls are batched so that multiple selection changes in the same tick cause afterChange() to be called only once. Deferring these updates causes them to no longer occur while a 'change' event is being emitted. This means that if an inspectors' close handler calls .change(), that call is now no longer nested inside another .change() call and doesn't run afoul of any render locks set by the caller of the outer .change(). Bug: 54675 Change-Id: Iae2f41a83b5d64251a54e42303100e84a5c25561 --- modules/ve/ui/ve.ui.Context.js | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/modules/ve/ui/ve.ui.Context.js b/modules/ve/ui/ve.ui.Context.js index 25830fb159..c53c1aa654 100644 --- a/modules/ve/ui/ve.ui.Context.js +++ b/modules/ve/ui/ve.ui.Context.js @@ -31,6 +31,7 @@ ve.ui.Context = function VeUiContext( surface, config ) { this.embedded = false; this.selection = null; this.toolbar = null; + this.afterChangeTimeout = null; this.popup = new ve.ui.PopupWidget( { '$$': this.$$, '$container': this.surface.getView().$ } ); this.$menu = this.$$( '
' ); this.inspectors = new ve.ui.SurfaceWindowSet( surface, ve.ui.inspectorFactory ); @@ -71,28 +72,46 @@ OO.inheritClass( ve.ui.Context, ve.Element ); /* Methods */ /** - * Handle change events on the model. + * Handle selection changes in the model. * * Changes are ignored while the user is selecting text or relocating content, apart from closing * the popup if it's open. While an inspector is opening or closing, all changes are ignored so as * to prevent inspectors that change the selection from within their open/close handlers from * causing issues. * + * The response to selection changes is deferred to prevent close handlers that process + * changes from causing this function to recurse. These responses are also batched for efficiency, + * so that if there are three selection changes in the same tick, afterChange() only runs once. + * * @method + * @see #afterChange * @param {ve.dm.Transaction[]} transactions Change transactions * @param {ve.Range} selection Change selection */ ve.ui.Context.prototype.onChange = function ( transactions, selection ) { if ( selection && !this.openingInspector && !this.hiding ) { - if ( this.popup.isVisible() ) { - this.hide(); - this.update(); - } else if ( !this.selecting && !this.relocating ) { - this.update(); + if ( this.afterChangeTimeout === null ) { + this.afterChangeTimeout = setTimeout( ve.bind( this.afterChange, this ) ); } } }; +/** + * Deferred response to one or more selection changes. + * + * Update the context menu for the new selection, except if the user is selecting or relocating + * content. If the popup is open, close it, even while selecting or relocating. + */ +ve.ui.Context.prototype.afterChange = function () { + this.afterChangeTimeout = null; + if ( this.popup.isVisible() ) { + this.hide(); + this.update(); + } else if ( !this.selecting && !this.relocating ) { + this.update(); + } +}; + /** * Handle selection start events on the view. *