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

Menus.removeMenu cleanup: remove all menu items and dividers prior to removing menu #5384

Merged
merged 6 commits into from
Oct 21, 2013
Merged

Menus.removeMenu cleanup: remove all menu items and dividers prior to removing menu #5384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
293c94d
Add Menu.removeMenuDivider()
lkcampbell Sep 29, 2013
60c890e
Add Menu.removeMenuDivider unit tests
lkcampbell Sep 30, 2013
204203e
Remove all menu items before removing menu
lkcampbell Sep 30, 2013
6ef7cfd
Fix problem with removing dividers. Add remove menu (native) unit te…
lkcampbell Sep 30, 2013
cf7579e
Replace not.toBeNull() with toBeTruthy()
lkcampbell Oct 11, 2013
8450003
Remove extra spaces
lkcampbell Oct 21, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 71 additions & 2 deletions src/command/Menus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -458,6 +459,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 divider <hr> and the <li> 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
Expand Down Expand Up @@ -532,7 +584,7 @@ define(function (require, exports, module) {
// create MenuItem DOM
if (_isHTMLMenu(this.id)) {
if (name === DIVIDER) {
$menuItem = $("<li><hr class='divider' /></li>");
$menuItem = $("<li><hr class='divider' id='" + id + "' /></li>");
} else {
// Create the HTML Menu
$menuItem = $("<li><a href='#' id='" + id + "'> <span class='menu-name'></span></a></li>");
Expand Down Expand Up @@ -877,6 +929,9 @@ define(function (require, exports, module) {
* Extensions should use the following format: "author.myextension.mymenuname".
*/
function removeMenu(id) {
var menu,
commandID = "";

if (!id) {
console.error("removeMenu(): missing required parameter: id");
return;
Expand All @@ -887,6 +942,20 @@ 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) {
menu.removeMenuDivider(key);
} else {
commandID = value.getCommand();
menu.removeMenuItem(commandID);
}
}
});

if (_isHTMLMenu(id)) {
$(_getHTMLMenu(id)).remove();
} else {
Expand Down
137 changes: 133 additions & 4 deletions test/spec/Menu-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -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 () {});
Expand Down Expand Up @@ -647,6 +721,63 @@ 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).toBeTruthy();

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) {
Expand Down Expand Up @@ -675,10 +806,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...
});
Expand Down