diff --git a/src/command/Menus.js b/src/command/Menus.js index de2dc4e123a..6bc37e91e81 100644 --- 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 and the
  • 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 index 94c4ed2d5b3..7f7dbe0d7e2 100644 --- 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");