From 3a99b99876db6c7e68d283cb185bc37d11361b53 Mon Sep 17 00:00:00 2001 From: Trevor Parscal Date: Thu, 16 May 2013 12:34:18 -0700 Subject: [PATCH] ve.ui.PopupWidget visibility fixes Objective: * Context popup would stop opening sometimes "mysteriously" which ended up having to do with the automatic closing on blur functionality added to popups for use in the category popup widget * Mousedown event canceling was being applied a little too widely, and was causing popup widgets to not allow child elements to be focused (unless they contained an iframe, like an inspector) Changes: ve.ui.Context.js * Make use of the popup's show and hide methods within the inspector ve.ui.MWCategoryPopupWidget.js * Override autoClose option for category popup widgets ve.ui.PopupWidget.js * Add autoClose option to popup widgets * Move event handler to the top of the methods (convention) * Only bind blur event if autoClose is enabled * Inline the getFocusedChild method Change-Id: I22aedb5fbd51b327ea7ce2ecdd6123e79cbebb9c --- modules/ve/ui/ve.ui.Context.js | 4 +- .../ui/widgets/ve.ui.MWCategoryPopupWidget.js | 8 ++ modules/ve/ui/widgets/ve.ui.PopupWidget.js | 84 +++++++++++-------- 3 files changed, 59 insertions(+), 37 deletions(-) diff --git a/modules/ve/ui/ve.ui.Context.js b/modules/ve/ui/ve.ui.Context.js index 68fad32cd5..d734573ed9 100644 --- a/modules/ve/ui/ve.ui.Context.js +++ b/modules/ve/ui/ve.ui.Context.js @@ -295,6 +295,7 @@ ve.ui.Context.prototype.show = function ( transition ) { this.showing = true; this.$.show(); + this.popup.show(); // Show either inspector or menu if ( inspector ) { @@ -346,8 +347,7 @@ ve.ui.Context.prototype.hide = function () { return this; } - this.inspectors.$.hide(); - this.$menu.hide(); + this.popup.hide(); this.$.hide(); this.visible = false; diff --git a/modules/ve/ui/widgets/ve.ui.MWCategoryPopupWidget.js b/modules/ve/ui/widgets/ve.ui.MWCategoryPopupWidget.js index 3ee8661871..d3191add83 100644 --- a/modules/ve/ui/widgets/ve.ui.MWCategoryPopupWidget.js +++ b/modules/ve/ui/widgets/ve.ui.MWCategoryPopupWidget.js @@ -15,6 +15,9 @@ * @param {Object} [config] Config options */ ve.ui.MWCategoryPopupWidget = function VeUiMwCategoryPopupWidget ( config ) { + // Configuration initialization + config = ve.extendObject( {}, config, { 'autoClose': true } ); + // Parent constructor ve.ui.PopupWidget.call( this, config ); @@ -50,6 +53,11 @@ ve.ui.MWCategoryPopupWidget = function VeUiMwCategoryPopupWidget ( config ) { config.$overlay.append( this.$ ); }; +/** + * @private + * @cfg {boolean} [autoClose=true] Overridden in this subclass + */ + /* Inheritance */ ve.inheritClass( ve.ui.MWCategoryPopupWidget, ve.ui.PopupWidget ); diff --git a/modules/ve/ui/widgets/ve.ui.PopupWidget.js b/modules/ve/ui/widgets/ve.ui.PopupWidget.js index 74f5d4515b..d18ac821eb 100644 --- a/modules/ve/ui/widgets/ve.ui.PopupWidget.js +++ b/modules/ve/ui/widgets/ve.ui.PopupWidget.js @@ -13,6 +13,7 @@ * * @constructor * @param {Object} [config] Config options + * @cfg {boolean} [autoClose=false] Popup auto-closes when it loses focus */ ve.ui.PopupWidget = function VeUiPopupWidget( config ) { // Config intialization @@ -24,24 +25,31 @@ ve.ui.PopupWidget = function VeUiPopupWidget( config ) { // Properties this.visible = false; this.$callout = this.$$( '
' ); - // Tab index on body so that it may blur - this.$body = this.$$( '
' ).attr( 'tabindex', 1 ); + this.$body = this.$$( '
' ); this.transitionTimeout = null; this.align = config.align || 'center'; + this.autoClose = !!config.autoClose; // Events - this.$body.on( 'blur', ve.bind( this.onPopupBlur, this ) ); this.$.add( this.$body ).add( this.$callout ) - .on( 'mousedown', false ); + .on( 'mousedown', function ( e ) { + // Cancel only local mousedown events + return e.target !== this; + } ); // Initialization + if ( this.autoClose ) { + // Tab index on body so that it may blur + this.$body.attr( 'tabindex', 1 ); + // Listen for blur events + this.$body.on( 'blur', ve.bind( this.onPopupBlur, this ) ); + } this.$ .addClass( 've-ui-popupWidget' ) .append( this.$callout.addClass( 've-ui-popupWidget-callout' ), this.$body.addClass( 've-ui-popupWidget-body' ) ); - }; /* Inheritance */ @@ -56,6 +64,38 @@ ve.inheritClass( ve.ui.PopupWidget, ve.ui.Widget ); /* Methods */ +/** + * Handle blur events. + * + * @param {jQuery.Event} e Blur event + */ +ve.ui.PopupWidget.prototype.onPopupBlur = function () { + var $body = this.$body; + + // Find out what is focused after blur + setTimeout( ve.bind( function () { + var $focused = $body.find( ':focus' ); + // Is there a focused child element? + if ( $focused.length > 0 ) { + // Bind a one off blur event to that focused child element + $focused.one( 'blur', ve.bind( function () { + setTimeout( ve.bind( function () { + if ( $body.find( ':focus' ).length === 0 ) { + // Be sure focus is not the popup itself. + if ( $body.is( ':focus' ) ) { + return; + } + // Not a child and not the popup itself, so hide. + this.hide(); + } + }, this ), 0 ); + }, this ) ); + } else { + this.hide(); + } + }, this ), 0 ); +}; + /** * Show the context. * @@ -65,8 +105,10 @@ ve.inheritClass( ve.ui.PopupWidget, ve.ui.Widget ); ve.ui.PopupWidget.prototype.show = function () { this.$.show(); this.visible = true; - // Focus body so that it may blur. - this.$body.focus(); + if ( this.autoClose ) { + // Focus body so that it may blur + this.$body.focus(); + } return this; }; @@ -83,34 +125,6 @@ ve.ui.PopupWidget.prototype.hide = function () { return this; }; -ve.ui.PopupWidget.prototype.getFocusedChild = function () { - return this.$body.find( ':focus' ); -}; - -ve.ui.PopupWidget.prototype.onPopupBlur = function () { - // Find out what is focused after blur - setTimeout( ve.bind( function () { - var $focused = this.getFocusedChild(); - // Is there a focused child element? - if ( $focused.length > 0 ) { - // Bind a one off blur event to that focused child element - $focused.one( 'blur', ve.bind( function () { - setTimeout( ve.bind( function () { - if ( this.getFocusedChild().length === 0 ) { - // Be sure focus is not the popup itself. - if ( this.$.find( ':focus' ).is( this.$body ) ){ - return; - } - // Not a child and not the popup itself, so hide. - this.hide(); - } - }, this ), 0 ); - }, this ) ); - } else { - this.hide(); - } - }, this ), 0 ); -}; /** * Updates the position and size.