Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Commit 1009b84

Browse files
committed
Merge pull request #5384 from lkcampbell/fix-remove-menu
Menus.removeMenu cleanup: remove all menu items and dividers prior to removing menu
2 parents f981eb1 + 8450003 commit 1009b84

2 files changed

Lines changed: 204 additions & 6 deletions

File tree

src/command/Menus.js

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ define(function (require, exports, module) {
3737
StringUtils = require("utils/StringUtils"),
3838
CommandManager = require("command/CommandManager"),
3939
PopUpManager = require("widgets/PopUpManager"),
40-
ViewUtils = require("utils/ViewUtils");
40+
ViewUtils = require("utils/ViewUtils"),
41+
CollectionUtils = require("utils/CollectionUtils");
4142

4243
/**
4344
* Brackets Application Menu Constants
@@ -460,6 +461,57 @@ define(function (require, exports, module) {
460461
delete menuItemMap[menuItemID];
461462
};
462463

464+
/**
465+
* Removes the specified menu divider from this Menu.
466+
*
467+
* @param {!string} menuItemID - the menu item id of the divider to remove.
468+
*/
469+
Menu.prototype.removeMenuDivider = function (menuItemID) {
470+
var menuItem,
471+
$HTMLMenuItem;
472+
473+
if (!menuItemID) {
474+
console.error("removeMenuDivider(): missing required parameters: menuItemID");
475+
return;
476+
}
477+
478+
menuItem = getMenuItem(menuItemID);
479+
480+
if (!menuItem) {
481+
console.error("removeMenuDivider(): parameter menuItemID: %s is not a valid menu item id", menuItemID);
482+
return;
483+
}
484+
485+
if (!menuItem.isDivider) {
486+
console.error("removeMenuDivider(): parameter menuItemID: %s is not a menu divider", menuItemID);
487+
return;
488+
}
489+
490+
if (_isHTMLMenu(this.id)) {
491+
// Targeting parent to get the menu divider <hr> and the <li> that contains it
492+
$HTMLMenuItem = $(_getHTMLMenuItem(menuItemID)).parent();
493+
if ($HTMLMenuItem) {
494+
$HTMLMenuItem.remove();
495+
} else {
496+
console.error("removeMenuDivider(): HTML menu divider not found: %s", menuItemID);
497+
return;
498+
}
499+
} else {
500+
brackets.app.removeMenuItem(menuItemID, function (err) {
501+
if (err) {
502+
console.error("removeMenuDivider() -- divider not found: %s (error: %s)", menuItemID, err);
503+
}
504+
});
505+
}
506+
507+
if (!menuItemMap[menuItemID]) {
508+
console.error("removeMenuDivider(): menu divider not found in menuItemMap: %s", menuItemID);
509+
return;
510+
}
511+
512+
delete menuItemMap[menuItemID];
513+
};
514+
463515
/**
464516
* Adds a new menu item with the specified id and display text. The insertion position is
465517
* specified via the relativeID and position arguments which describe a position
@@ -534,7 +586,7 @@ define(function (require, exports, module) {
534586
// create MenuItem DOM
535587
if (_isHTMLMenu(this.id)) {
536588
if (name === DIVIDER) {
537-
$menuItem = $("<li><hr class='divider' /></li>");
589+
$menuItem = $("<li><hr class='divider' id='" + id + "' /></li>");
538590
} else {
539591
// Create the HTML Menu
540592
$menuItem = $("<li><a href='#' id='" + id + "'> <span class='menu-name'></span></a></li>");
@@ -879,6 +931,9 @@ define(function (require, exports, module) {
879931
* Extensions should use the following format: "author.myextension.mymenuname".
880932
*/
881933
function removeMenu(id) {
934+
var menu,
935+
commandID = "";
936+
882937
if (!id) {
883938
console.error("removeMenu(): missing required parameter: id");
884939
return;
@@ -889,6 +944,20 @@ define(function (require, exports, module) {
889944
return;
890945
}
891946

947+
// Remove all of the menu items in the menu
948+
menu = getMenu(id);
949+
950+
CollectionUtils.forEach(menuItemMap, function (value, key) {
951+
if (key.substring(0, id.length) === id) {
952+
if (value.isDivider) {
953+
menu.removeMenuDivider(key);
954+
} else {
955+
commandID = value.getCommand();
956+
menu.removeMenuItem(commandID);
957+
}
958+
}
959+
});
960+
892961
if (_isHTMLMenu(id)) {
893962
$(_getHTMLMenu(id)).remove();
894963
} else {

test/spec/Menu-test.js

Lines changed: 133 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,80 @@ define(function (require, exports, module) {
6666
SpecRunnerUtils.closeTestWindow();
6767
});
6868

69+
describe("Remove Menu", function () {
70+
it("should add then remove new menu to menu bar with a menu id", function () {
71+
runs(function () {
72+
var menuId = "Menu-test";
73+
Menus.addMenu("Custom", menuId);
74+
75+
var menu = Menus.getMenu(menuId);
76+
expect(menu).toBeTruthy();
77+
78+
Menus.removeMenu(menuId);
79+
menu = Menus.getMenu(menuId);
80+
expect(menu).toBeUndefined();
81+
});
82+
});
83+
84+
it("should remove all menu items and dividers in the menu when removing the menu", function () {
85+
runs(function () {
86+
var menuId = "Menu-test";
87+
Menus.addMenu("Custom", menuId);
88+
89+
var menu = Menus.getMenu(menuId);
90+
expect(menu).toBeTruthy();
91+
92+
var commandId = "Remove-Menu-test.Item-1";
93+
CommandManager.register("Remove Menu Test Command", commandId, function () {});
94+
95+
var menuItem = menu.addMenuItem(commandId);
96+
expect(menuItem).toBeTruthy();
97+
98+
var menuItemId = menuItem.id;
99+
expect(menuItemId).toBeTruthy();
100+
101+
var menuDivider = menu.addMenuDivider();
102+
expect(menuDivider).toBeTruthy();
103+
104+
var menuDividerId = menuDivider.id;
105+
expect(menuDividerId).toBeTruthy();
106+
107+
menuItem = Menus.getMenuItem(menuItemId);
108+
expect(menuItem).toBeTruthy();
109+
110+
menuDivider = Menus.getMenuItem(menuDividerId);
111+
expect(menuDivider).toBeTruthy();
112+
113+
Menus.removeMenu(menuId);
114+
115+
menu = Menus.getMenu(menuId);
116+
expect(menu).toBeUndefined();
117+
118+
menuItem = Menus.getMenuItem(menuItemId);
119+
expect(menuItem).toBeUndefined();
120+
121+
menuDivider = Menus.getMenuItem(menuDividerId);
122+
expect(menuDivider).toBeUndefined();
123+
});
124+
});
125+
126+
it("should gracefully handle someone trying to remove a menu that doesn't exist", function () {
127+
runs(function () {
128+
var menuId = "Menu-test";
129+
130+
Menus.removeMenu(menuId);
131+
expect(Menus).toBeTruthy(); // Verify that we got this far...
132+
});
133+
});
134+
135+
it("should gracefully handle someone trying to remove a menu without supplying the id", function () {
136+
runs(function () {
137+
Menus.removeMenu();
138+
expect(Menus).toBeTruthy(); // Verify that we got this far...
139+
});
140+
});
141+
});
142+
69143

70144
describe("Context Menus", function () {
71145
it("register a context menu", function () {
@@ -439,7 +513,7 @@ define(function (require, exports, module) {
439513
});
440514
});
441515

442-
it("should add menu items to beginnging and end of menu section", function () {
516+
it("should add menu items to beginning and end of menu section", function () {
443517
// set up test menu and menu items
444518
CommandManager.register("Brackets Test Command Section 10", "Menu-test.command10", function () {});
445519
CommandManager.register("Brackets Test Command Section 11", "Menu-test.command11", function () {});
@@ -647,6 +721,63 @@ define(function (require, exports, module) {
647721
});
648722

649723

724+
describe("Remove Menu Divider", function () {
725+
726+
function menuDividerDOM(menuItemId) {
727+
return testWindow.$("#" + menuItemId);
728+
}
729+
730+
it("should add then remove new menu divider to empty menu", function () {
731+
runs(function () {
732+
var menuId = "menu-custom-removeMenuDivider-1";
733+
var menu = Menus.addMenu("Custom", menuId);
734+
735+
var menuDivider = menu.addMenuDivider();
736+
expect(menuDivider).toBeTruthy();
737+
738+
var $listItems = menuDividerDOM(menuDivider.id);
739+
expect($listItems.length).toBe(1);
740+
741+
menu.removeMenuDivider(menuDivider.id);
742+
$listItems = menuDividerDOM(menuDivider.id);
743+
expect($listItems.length).toBe(0);
744+
});
745+
});
746+
747+
it("should gracefully handle someone trying to remove a menu divider without supplying the id", function () {
748+
runs(function () {
749+
var menuId = "menu-custom-removeMenuDivider-2";
750+
var menu = Menus.addMenu("Custom", menuId);
751+
752+
menu.removeMenuDivider();
753+
expect(menu).toBeTruthy(); // Verify that we got this far...
754+
});
755+
});
756+
757+
it("should gracefully handle someone trying to remove a menu divider with an invalid id", function () {
758+
runs(function () {
759+
var menuId = "menu-custom-removeMenuDivider-3";
760+
var menu = Menus.addMenu("Custom", menuId);
761+
762+
menu.removeMenuDivider("foo");
763+
expect(menu).toBeTruthy(); // Verify that we got this far...
764+
});
765+
});
766+
767+
it("should gracefully handle someone trying to remove a menu item that is not a divider", function () {
768+
runs(function () {
769+
var menuId = "menu-custom-removeMenuDivider-4";
770+
var menu = Menus.addMenu("Custom", menuId);
771+
var menuItemId = "menu-test-removeMenuDivider1";
772+
var menuItem = menu.addMenuItem(menuItemId);
773+
774+
menu.removeMenuDivider(menuItemId);
775+
expect(menu).toBeTruthy(); // Verify that we got this far...
776+
});
777+
});
778+
});
779+
780+
650781
describe("Remove Menu", function () {
651782

652783
function menuDOM(menuId) {
@@ -675,10 +806,8 @@ define(function (require, exports, module) {
675806
});
676807
});
677808

678-
it("should gracefully handle someone trying to remove a menu without supply the id", function () {
809+
it("should gracefully handle someone trying to remove a menu without supplying the id", function () {
679810
runs(function () {
680-
var menuId = "Menu-test";
681-
682811
Menus.removeMenu();
683812
expect(Menus).toBeTruthy(); // Verify that we got this far...
684813
});

0 commit comments

Comments
 (0)