From 293c94d414e33599e55d6c2f3282024b8bd80839 Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Sat, 28 Sep 2013 17:48:38 -0700 Subject: [PATCH 1/6] Add Menu.removeMenuDivider() --- src/command/Menus.js | 51 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/command/Menus.js b/src/command/Menus.js index 508b21dca34..718aa520f22 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -458,6 +458,57 @@ define(function (require, exports, module) { delete menuItemMap[menuItemID]; }; + /** + * Removes the specified menu divider from this Menu. + * + * @param {!string} menuItemID - the menu item id of the divider to remove. + */ + Menu.prototype.removeMenuDivider = function (menuItemID) { + var menuItem, + $HTMLMenuItem; + + if (!menuItemID) { + console.error("removeMenuDivider(): missing required parameters: menuItemID"); + return; + } + + menuItem = getMenuItem(menuItemID); + + if (!menuItem) { + console.error("removeMenuDivider(): parameter menuItemID: %s is not a valid menu item id", menuItemID); + return; + } + + if (!menuItem.isDivider) { + console.error("removeMenuDivider(): parameter menuItemID: %s is not a menu divider", menuItemID); + return; + } + + if (_isHTMLMenu(this.id)) { + // Targeting parent to get the menu item and the
  • that contains it + $HTMLMenuItem = $(_getHTMLMenuItem(menuItemID)).parent(); + if ($HTMLMenuItem) { + $HTMLMenuItem.remove(); + } else { + console.error("removeMenuDivider(): HTML menu divider not found: %s", menuItemID); + return; + } + } else { + brackets.app.removeMenuItem(menuItemID, function (err) { + if (err) { + console.error("removeMenuDivider() -- divider not found: %s (error: %s)", menuItemID, err); + } + }); + } + + if (!menuItemMap[menuItemID]) { + console.error("removeMenuDivider(): menu divider not found in menuItemMap: %s", menuItemID); + return; + } + + delete menuItemMap[menuItemID]; + }; + /** * Adds a new menu item with the specified id and display text. The insertion position is * specified via the relativeID and position arguments which describe a position From 60c890efa8c664b3d6c022a8e7ddee226e1978db Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Mon, 30 Sep 2013 12:08:36 -0700 Subject: [PATCH 2/6] Add Menu.removeMenuDivider unit tests --- src/command/Menus.js | 4 +-- test/spec/Menu-test.js | 62 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 718aa520f22..d5cf316f09f 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -485,7 +485,7 @@ define(function (require, exports, module) { } if (_isHTMLMenu(this.id)) { - // Targeting parent to get the menu item and the
  • that contains it + // Targeting parent to get the menu divider
    and the
  • that contains it $HTMLMenuItem = $(_getHTMLMenuItem(menuItemID)).parent(); if ($HTMLMenuItem) { $HTMLMenuItem.remove(); @@ -583,7 +583,7 @@ define(function (require, exports, module) { // create MenuItem DOM if (_isHTMLMenu(this.id)) { if (name === DIVIDER) { - $menuItem = $("
    • "); + $menuItem = $("
    • "); } else { // Create the HTML Menu $menuItem = $("
    • "); diff --git a/test/spec/Menu-test.js b/test/spec/Menu-test.js index bbfe63932d8..7b74d3c7cd2 100644 --- a/test/spec/Menu-test.js +++ b/test/spec/Menu-test.js @@ -647,6 +647,64 @@ define(function (require, exports, module) { }); + describe("Remove Menu Divider", function () { + + function menuDividerDOM(menuItemId) { + return testWindow.$("#" + menuItemId); + } + + it("should add then remove new menu divider to empty menu", function () { + runs(function () { + var menuId = "menu-custom-removeMenuDivider-1"; + var menu = Menus.addMenu("Custom", menuId); + + var menuDivider = menu.addMenuDivider(); + expect(menuDivider).not.toBeNull(); + expect(menuDivider).toBeDefined(); + + var $listItems = menuDividerDOM(menuDivider.id); + expect($listItems.length).toBe(1); + + menu.removeMenuDivider(menuDivider.id); + $listItems = menuDividerDOM(menuDivider.id); + expect($listItems.length).toBe(0); + }); + }); + + it("should gracefully handle someone trying to remove a menu divider without supplying the id", function () { + runs(function () { + var menuId = "menu-custom-removeMenuDivider-2"; + var menu = Menus.addMenu("Custom", menuId); + + menu.removeMenuDivider(); + expect(menu).toBeTruthy(); // Verify that we got this far... + }); + }); + + it("should gracefully handle someone trying to remove a menu divider with an invalid id", function () { + runs(function () { + var menuId = "menu-custom-removeMenuDivider-3"; + var menu = Menus.addMenu("Custom", menuId); + + menu.removeMenuDivider("foo"); + expect(menu).toBeTruthy(); // Verify that we got this far... + }); + }); + + it("should gracefully handle someone trying to remove a menu item that is not a divider", function () { + runs(function () { + var menuId = "menu-custom-removeMenuDivider-4"; + var menu = Menus.addMenu("Custom", menuId); + var menuItemId = "menu-test-removeMenuDivider1"; + var menuItem = menu.addMenuItem(menuItemId); + + menu.removeMenuDivider(menuItemId); + expect(menu).toBeTruthy(); // Verify that we got this far... + }); + }); + }); + + describe("Remove Menu", function () { function menuDOM(menuId) { @@ -675,10 +733,8 @@ define(function (require, exports, module) { }); }); - it("should gracefully handle someone trying to remove a menu without supply the id", function () { + it("should gracefully handle someone trying to remove a menu without supplying the id", function () { runs(function () { - var menuId = "Menu-test"; - Menus.removeMenu(); expect(Menus).toBeTruthy(); // Verify that we got this far... }); From 204203e555c811469de0fbd5a5ca4c9de1d6bf91 Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Mon, 30 Sep 2013 12:36:06 -0700 Subject: [PATCH 3/6] Remove all menu items before removing menu --- src/command/Menus.js | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index d5cf316f09f..22f83bd8d66 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -35,7 +35,8 @@ define(function (require, exports, module) { StringUtils = require("utils/StringUtils"), CommandManager = require("command/CommandManager"), PopUpManager = require("widgets/PopUpManager"), - ViewUtils = require("utils/ViewUtils"); + ViewUtils = require("utils/ViewUtils"), + CollectionUtils = require("utils/CollectionUtils"); /** * Brackets Application Menu Constants @@ -928,6 +929,10 @@ define(function (require, exports, module) { * Extensions should use the following format: "author.myextension.mymenuname". */ function removeMenu(id) { + var menu, + commandID = "", + menuDividerID = ""; + if (!id) { console.error("removeMenu(): missing required parameter: id"); return; @@ -938,6 +943,21 @@ define(function (require, exports, module) { return; } + // Remove all of the menu items in the menu + menu = getMenu(id); + + CollectionUtils.forEach(menuItemMap, function (value, key) { + if (key.substring(0, id.length) === id) { + if (value.isDivider) { + menuDividerID = key.substring((id.length + 1), key.length); + menu.removeMenuDivider(menuDividerID); + } else { + commandID = value.getCommand(); + menu.removeMenuItem(commandID); + } + } + }); + if (_isHTMLMenu(id)) { $(_getHTMLMenu(id)).remove(); } else { From 6ef7cfd43358e8a46b792c54f07de1878e90153d Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Mon, 30 Sep 2013 14:38:49 -0700 Subject: [PATCH 4/6] Fix problem with removing dividers. Add remove menu (native) unit tests. --- src/command/Menus.js | 8 ++--- test/spec/Menu-test.js | 76 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 22f83bd8d66..c986b685586 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -930,9 +930,8 @@ define(function (require, exports, module) { */ function removeMenu(id) { var menu, - commandID = "", - menuDividerID = ""; - + commandID = ""; + if (!id) { console.error("removeMenu(): missing required parameter: id"); return; @@ -949,8 +948,7 @@ define(function (require, exports, module) { CollectionUtils.forEach(menuItemMap, function (value, key) { if (key.substring(0, id.length) === id) { if (value.isDivider) { - menuDividerID = key.substring((id.length + 1), key.length); - menu.removeMenuDivider(menuDividerID); + menu.removeMenuDivider(key); } else { commandID = value.getCommand(); menu.removeMenuItem(commandID); diff --git a/test/spec/Menu-test.js b/test/spec/Menu-test.js index 7b74d3c7cd2..cc701ad0d42 100644 --- a/test/spec/Menu-test.js +++ b/test/spec/Menu-test.js @@ -66,6 +66,80 @@ define(function (require, exports, module) { SpecRunnerUtils.closeTestWindow(); }); + describe("Remove Menu", function () { + it("should add then remove new menu to menu bar with a menu id", function () { + runs(function () { + var menuId = "Menu-test"; + Menus.addMenu("Custom", menuId); + + var menu = Menus.getMenu(menuId); + expect(menu).toBeTruthy(); + + Menus.removeMenu(menuId); + menu = Menus.getMenu(menuId); + expect(menu).toBeUndefined(); + }); + }); + + it("should remove all menu items and dividers in the menu when removing the menu", function () { + runs(function () { + var menuId = "Menu-test"; + Menus.addMenu("Custom", menuId); + + var menu = Menus.getMenu(menuId); + expect(menu).toBeTruthy(); + + var commandId = "Remove-Menu-test.Item-1"; + CommandManager.register("Remove Menu Test Command", commandId, function () {}); + + var menuItem = menu.addMenuItem(commandId); + expect(menuItem).toBeTruthy(); + + var menuItemId = menuItem.id; + expect(menuItemId).toBeTruthy(); + + var menuDivider = menu.addMenuDivider(); + expect(menuDivider).toBeTruthy(); + + var menuDividerId = menuDivider.id; + expect(menuDividerId).toBeTruthy(); + + menuItem = Menus.getMenuItem(menuItemId); + expect(menuItem).toBeTruthy(); + + menuDivider = Menus.getMenuItem(menuDividerId); + expect(menuDivider).toBeTruthy(); + + Menus.removeMenu(menuId); + + menu = Menus.getMenu(menuId); + expect(menu).toBeUndefined(); + + menuItem = Menus.getMenuItem(menuItemId); + expect(menuItem).toBeUndefined(); + + menuDivider = Menus.getMenuItem(menuDividerId); + expect(menuDivider).toBeUndefined(); + }); + }); + + it("should gracefully handle someone trying to remove a menu that doesn't exist", function () { + runs(function () { + var menuId = "Menu-test"; + + Menus.removeMenu(menuId); + expect(Menus).toBeTruthy(); // Verify that we got this far... + }); + }); + + it("should gracefully handle someone trying to remove a menu without supplying the id", function () { + runs(function () { + Menus.removeMenu(); + expect(Menus).toBeTruthy(); // Verify that we got this far... + }); + }); + }); + describe("Context Menus", function () { it("register a context menu", function () { @@ -439,7 +513,7 @@ define(function (require, exports, module) { }); }); - it("should add menu items to beginnging and end of menu section", function () { + it("should add menu items to beginning and end of menu section", function () { // set up test menu and menu items CommandManager.register("Brackets Test Command Section 10", "Menu-test.command10", function () {}); CommandManager.register("Brackets Test Command Section 11", "Menu-test.command11", function () {}); From cf7579ed76bc1f9cc9eb936ce05b437cca8bb56c Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Fri, 11 Oct 2013 15:58:41 -0700 Subject: [PATCH 5/6] Replace not.toBeNull() with toBeTruthy() --- test/spec/Menu-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/spec/Menu-test.js b/test/spec/Menu-test.js index cc701ad0d42..7edfbb6aac0 100644 --- a/test/spec/Menu-test.js +++ b/test/spec/Menu-test.js @@ -733,8 +733,7 @@ define(function (require, exports, module) { var menu = Menus.addMenu("Custom", menuId); var menuDivider = menu.addMenuDivider(); - expect(menuDivider).not.toBeNull(); - expect(menuDivider).toBeDefined(); + expect(menuDivider).toBeTruthy(); var $listItems = menuDividerDOM(menuDivider.id); expect($listItems.length).toBe(1); From 845000385eef9b7d691349a804b9f1cd4f3923f4 Mon Sep 17 00:00:00 2001 From: Lance Campbell Date: Mon, 21 Oct 2013 06:50:46 -0700 Subject: [PATCH 6/6] Remove extra spaces --- src/command/Menus.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index c986b685586..24e4e4f0149 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -930,7 +930,7 @@ define(function (require, exports, module) { */ function removeMenu(id) { var menu, - commandID = ""; + commandID = ""; if (!id) { console.error("removeMenu(): missing required parameter: id");