Merge pull request #2072 from adobe/pflynn/remove-menuitem

removeMenuItem() API
adobe · Nov 7, 2012 · 442ad06 · 442ad06
2 parents 76dbae8 + b76cc1e
commit 442ad06
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 10 deletions.
diff --git a/src/command/Menus.js b/src/command/Menus.js
@@ -189,16 +189,6 @@ define(function (require, exports, module) {
         return binding;
     }
 
-    /** NOT IMPLEMENTED
-     * Removes MenuItem
-     * 
-     * TODO Question: for convenience should API provide a way to remove related
-     * keybindings and Command object?
-     */
-    // function removeMenuItem(id) {
-    //    NOT IMPLEMENTED
-    // }
-
     var _menuDividerIDCount = 1;
     function _getNextMenuItemDividerID() {
         return "brackets-menuDivider-" + _menuDividerIDCount++;
@@ -390,6 +380,35 @@ define(function (require, exports, module) {
 
         return $relativeElement;
     };
+
+    /**
+     * Removes the specified menu item from this Menu. Key bindings are unaffected; use KeyBindingManager
+     * directly to remove key bindings if desired.
+     *
+     * @param {!string | Command} command - command the menu would execute if we weren't deleting it.
+     */
+    Menu.prototype.removeMenuItem = function (command) {
+        var menuItemID;
+
+        if (!command) {
+            throw new Error("removeMenuItem(): missing required parameters: command");
+        }
+
+        if (typeof (command) === "string") {
+            var commandObj = CommandManager.get(command);
+            if (!commandObj) {
+                throw new Error("removeMenuItem(): command not found: " + command);
+            }
+
+            menuItemID = this._getMenuItemId(command);
+        } else {
+            menuItemID = this._getMenuItemId(command.getID());
+        }
+
+        // Targeting parent to get the menu item <a> and the <li> that contains it
+        $(_getHTMLMenuItem(menuItemID)).parent().remove();
+        delete menuItemMap[menuItemID];
+    };
 
     /**
      * Adds a new menu item with the specified id and display text. The insertion position is

diff --git a/test/spec/Menu-test.js b/test/spec/Menu-test.js
@@ -169,6 +169,7 @@ define(function (require, exports, module) {
             });
         });
 
+
         describe("Add Menu Items", function () {
 
             it("should add new menu items", function () {
@@ -386,6 +387,97 @@ define(function (require, exports, module) {
             });
         });
 
+
+        describe("Remove Menu Items", function () {
+
+            function menuDOMChildren(menuItemId) {
+                return testWindow.$("#" + menuItemId + " > ul").children();
+            }
+
+            it("should add then remove new menu item to empty menu with a command id", function () {
+                runs(function () {
+                    var commandId = "Menu-test.removeMenuItem.command0";
+                    var menuItemId = "menu-test-removeMenuItem0";
+                    CommandManager.register("Brackets Test Command Custom", commandId, function () {});
+                    var menu = Menus.addMenu("Custom", menuItemId);
+                    var $listItems = menuDOMChildren(menuItemId);
+                    expect($listItems.length).toBe(0);
+
+                    // Re-use commands that are already registered
+                    var menuItem = menu.addMenuItem(commandId);
+                    expect(menuItem).not.toBeNull();
+                    expect(menuItem).toBeDefined();
+
+                    expect(typeof (commandId)).toBe("string");
+
+                    $listItems = menuDOMChildren(menuItemId);
+                    expect($listItems.length).toBe(1);
+
+                    menu.removeMenuItem(commandId);
+                    $listItems = menuDOMChildren(menuItemId);
+                    expect($listItems.length).toBe(0);
+                });
+            });
+
+            it("should add then remove new menu item to empty menu with a command", function () {
+                runs(function () {
+                    var commandId = "Menu-test.removeMenuItem.command1";
+                    var menuItemId = "menu-test-removeMenuItem1";
+                    CommandManager.register("Brackets Test Command Custom", commandId, function () {});
+                    var menu = Menus.addMenu("Custom", menuItemId);
+                    var $listItems = testWindow.$("#menu-custom > ul").children();
+                    expect($listItems.length).toBe(0);
+
+                    // Re-use commands that are already registered
+                    var menuItem = menu.addMenuItem(commandId);
+                    expect(menuItem).not.toBeNull();
+                    expect(menuItem).toBeDefined();
+
+                    $listItems = menuDOMChildren(menuItemId);
+                    expect($listItems.length).toBe(1);
+
+                    var command = CommandManager.get(commandId);
+                    expect(typeof (command)).toBe("object");
+
+                    menu.removeMenuItem(command);
+                    $listItems = menuDOMChildren(menuItemId);
+                    expect($listItems.length).toBe(0);
+                });
+            });
+
+            it("should gracefully handle someone trying to delete a menu item that doesn't exist", function () {
+                runs(function () {
+                    var commandId = "Menu-test.removeMenuItem.command2";
+                    var menuItemId = "menu-test-removeMenuItem2";
+                    var menu = Menus.addMenu("Custom", menuItemId);
+
+                    var exceptionThrown = false;
+                    try {
+                        menu.removeMenuItem(commandId);
+                    } catch (e) {
+                        exceptionThrown = true;
+                    }
+                    expect(exceptionThrown).toBeTruthy();
+                });
+            });
+
+            it("should gracefully handle someone trying to delete nothing", function () {
+                runs(function () {
+                    var menuItemId = "menu-test-removeMenuItem3";
+                    var menu = Menus.addMenu("Custom", menuItemId);
+
+                    var exceptionThrown = false;
+                    try {
+                        menu.removeMenuItem();
+                    } catch (e) {
+                        exceptionThrown = true;
+                    }
+                    expect(exceptionThrown).toBeTruthy();
+                });
+            });
+        });
+
+
         describe("Menu Item synchronizing", function () {
 
             it("should have same state as command", function () {
@@ -464,6 +556,7 @@ define(function (require, exports, module) {
             });
         });
 
+
         describe("Context Menus", function () {
             it("register a context menu", function () {
                 var cmenu = Menus.registerContextMenu("test-cmenu50");