Merge pull request #1276 from adobe/tvoliter/popupESCfix

fixes issues #1270 #1271 #1272, ESC key handling with popups
adobe · Jul 18, 2012 · 650476d · 650476d
2 parents a74f4b6 + 727bf98
commit 650476d
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 47 deletions.
diff --git a/src/command/Menus.js b/src/command/Menus.js
@@ -651,13 +651,14 @@ define(function (require, exports, module) {
         _insertInList($menubar, $newMenu, position, $relativeElement);
 
         // Install ESC key handling
-        PopUpManager.configurePopUp($popUp, closeAll);
+        PopUpManager.addPopUp($popUp, menu.close, false);
 
         // todo error handling
 
         return menu;
     }
 
+
     /**
      * @constructor
      * @extends {Menu}
@@ -689,7 +690,12 @@ define(function (require, exports, module) {
         // insert into DOM
         $("#context-menu-bar > ul").append($newMenu);
 
-        PopUpManager.configurePopUp($popUp, closeAll);
+        var self = this;
+        PopUpManager.addPopUp($popUp,
+            function () {
+                self.close();
+            },
+            false);
     }
     ContextMenu.prototype = new Menu();
     ContextMenu.prototype.constructor = ContextMenu;

diff --git a/src/widgets/PopUpManager.js b/src/widgets/PopUpManager.js
@@ -34,51 +34,26 @@ define(function (require, exports, module) {
     var EditorManager = require("editor/EditorManager");
 
     var _popUps = [];
-
-    function _removePopUp($popUp, index, visible) {
-        var initiallyInDOM = $popUp.data("initiallyInDOM"),
-            removeHandler = $popUp.data("removeHandler");
-
-        visible = visible || $popUp.find(":visible").length > 0;
-
-        if (removeHandler && visible) {
-            removeHandler();
-        }
-
-        if (!initiallyInDOM) {
-            $popUp.remove();
-        }
 
-        _popUps = _popUps.slice(index);
-    }
-
     /**
-     * Add Esc key handling for pop-up DOM elements that persist in the DOM.
-     * These pop-up elements must handle appearance (i.e. opening) themselves.
+     * Add Esc key handling for a popup DOM element.
      *
      * @param {!jQuery} $popUp jQuery object for the DOM element pop-up
      * @param {function} removeHandler Pop-up specific remove (e.g. display:none or DOM removal)
+     * @param {?Boolean} autoAddRemove - Specify true to indicate the PopUpManager should 
+     *      add/remove the popup from the DOM when the popup is open/closed. Specify false
+     *      when the popup is either always persistant in the DOM or the add/remove is handled 
+     *      external to the PopupManager 
+     *      
      */
-    function configurePopUp($popUp, removeHandler) {
-        _popUps.push($popUp[0]);
+    function addPopUp($popUp, removeHandler, autoAddRemove) {
+        autoAddRemove = autoAddRemove || false;
 
-        $popUp.data("initiallyInDOM", true);
-        $popUp.data("removeHandler", removeHandler);
-    }
-
-    /**
-     * Add Esc key handling for transient DOM elements. Immediately adds
-     * the element to the DOM if it's not already attached.
-     *
-     * @param {!jQuery} $popUp jQuery object for the DOM element pop-up
-     * @param {function} removeHandler Pop-up specific remove (e.g. display:none or DOM removal)
-     */
-    function addPopUp($popUp, removeHandler) {
-        var initiallyInDOM = $popUp.parent().length === 1;
-
-        configurePopUp($popUp, removeHandler, initiallyInDOM);
+        _popUps.push($popUp[0]);
+        $popUp.data("PopUpManager-autoAddRemove", autoAddRemove);
+        $popUp.data("PopUpManager-removeHandler", removeHandler);
 
-        if (!initiallyInDOM) {
+        if (autoAddRemove) {
             $(window.document.body).append($popUp);
         }
     }
@@ -90,17 +65,24 @@ define(function (require, exports, module) {
      * @param {!jQuery} $popUp
      */
     function removePopUp($popUp) {
-        var index = _popUps.indexOf($popUp[0]),
-            initiallyInDOM = $popUp.data("initiallyInDOM"),
-            removeHandler = $popUp.data("removeHandler");
-
+        var index = _popUps.indexOf($popUp[0]);
         if (index >= 0) {
-            _removePopUp($popUp, index);
+            var autoAddRemove = $popUp.data("PopUpManager-autoAddRemove"),
+                removeHandler = $popUp.data("PopUpManager-removeHandler");
+
+            if (removeHandler && $popUp.find(":visible").length > 0) {
+                removeHandler();
+            }
+
+            if (autoAddRemove) {
+                $popUp.remove();
+                _popUps = _popUps.slice(index);
+            }
         }
     }
 
     function _keydownCaptureListener(keyEvent) {
-        if (keyEvent.keyCode !== 27) {
+        if (keyEvent.keyCode !== 27) { // escape key
             return;
         }
 
@@ -119,7 +101,7 @@ define(function (require, exports, module) {
                     // Stop the DOM event from propagating
                     keyEvent.stopImmediatePropagation();
 
-                    _removePopUp($popUp, i, true);
+                    removePopUp($popUp);
                     EditorManager.focusEditor();
                 }
 
@@ -132,5 +114,4 @@ define(function (require, exports, module) {
 
     exports.addPopUp        = addPopUp;
     exports.removePopUp     = removePopUp;
-    exports.configurePopUp  = configurePopUp;
 });