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

Adds support for menus sections, fixes #1210 #1247

Merged
merged 9 commits into from
Jul 17, 2012
Merged

Adds support for menus sections, fixes #1210 #1247

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5aa8c7b
add support for menu sections
tvoliter Jul 11, 2012
0d6568a
Merge remote-tracking branch 'origin/master' into tvoliter/issue-1210
tvoliter Jul 13, 2012
73a7f8a
Tests for menu section insertion
tvoliter Jul 13, 2012
b937f7e
remove empty menu section test
tvoliter Jul 13, 2012
83dc288
Adding function comment blocks to Menus.js
tvoliter Jul 13, 2012
81b835d
Additional comments in Menus.js for sections
tvoliter Jul 13, 2012
eb3bbd5
code review fixes
tvoliter Jul 16, 2012
e2357bd
Merge remote-tracking branch 'origin/master' into tvoliter/issue-1210
tvoliter Jul 16, 2012
d1216f8
Code review fixes
tvoliter Jul 17, 2012
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
108 changes: 82 additions & 26 deletions src/command/Menus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,20 +67,27 @@ define(function (require, exports, module) {
* more semantic.
* Use these constants as the "relativeID" parameter when calling addMenuItem() and
* specify a position of FIRST or LAST.
*
* Menu sections are denoted by dividers or the beginning/end of a menu
*/
var MenuSection = {
FILE_OPEN_CLOSE_MENU: "file-open-close-menu-section",
FILE_SAVE_MENU: "file-save-menu-section",
FILE_LIVE_MENU: "file-live-menu-section",
// Menu Section Command ID to mark the section
FILE_OPEN_CLOSE_COMMANDS: {sectionMarker: Commands.FILE_NEW},
FILE_SAVE_COMMANDS: {sectionMarker: Commands.FILE_SAVE},
FILE_LIVE: {sectionMarker: Commands.FILE_LIVE_FILE_PREVIEW},

EDIT_SELECTION_COMMANDS: {sectionMarker: Commands.EDIT_SELECT_ALL},
EDIT_FIND: {sectionMarker: Commands.EDIT_FIND},
EDIT_REPLACE_COMMANDS: {sectionMarker: Commands.EDIT_REPLACE},
EDIT_MODIFY_SELECTION: {sectionMarker: Commands.EDIT_INDENT},

EDIT_MODIFY_SELECTION_MENU: "edit-modify-selection-menu-section",
EDIT_FIND_MENU: "find-menu-section",
EDIT_REPLACE_MENU: "replace-menu-section",
EDIT_SELECTED_TEXT_COMMANDS: "selected-text-commands-menu-group",
VIEW_HIDESHOW_COMMANDSkey: {sectionMarker: Commands.VIEW_HIDE_SIDEBAR},
VIEW_FONTSIZE_COMMANDSkey: {sectionMarker: Commands.VIEW_INCREASE_FONT_SIZE},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to have "key" at the end of these 2 ids? It doesn't seem consistent.


NAVIGATE_GOTO_MENU: "goto-menu-section",
NAVIGATE_QUICK_EDIT_MENU: "quick-edit-menu-section"
NAVIGATE_GOTO: {sectionMarker: Commands.NAVIGATE_QUICK_OPEN},
NAVIGATE_QUICK_EDIT: {sectionMarker: Commands.TOGGLE_QUICK_EDIT}
};


/**
* Insertion position constants
Expand Down Expand Up @@ -275,12 +282,28 @@ define(function (require, exports, module) {
this.id = id;
}

Menu.prototype._getMenuItemForCommand = function (command) {
var foundMenuItem, key, menuItem;
for (key in menuItemMap) {
if (menuItemMap.hasOwnProperty(key)) {
menuItem = menuItemMap[key];
if (menuItem.getCommand() === command) {
foundMenuItem = menuItem;
break;
}
}
}
return $(_getHTMLMenuItem(foundMenuItem.id)).closest("li");
};

/**
* Determine relative MenuItem
*
* @param {?string} relativeID - id of command (future: also sub-menu, or menu section).
* @param {?string} relativeID - id of command (future: sub-menu).
* @param {?string} position - only needed when relativeID is a MenuSection
* @param {HTMLIElement}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this 3rd parameter can be deleted from documentation

*/
Menu.prototype._getRelativeMenuItem = function (relativeID) {
Menu.prototype._getRelativeMenuItem = function (relativeID, position) {
var $relativeElement,
key,
menuItem,
Expand All @@ -293,24 +316,50 @@ define(function (require, exports, module) {

if (command) {
// Find MenuItem that has this command
for (key in menuItemMap) {
if (menuItemMap.hasOwnProperty(key)) {
menuItem = menuItemMap[key];
if (menuItem.getCommand() === command) {
foundMenuItem = menuItem;
break;
}
}
}

if (foundMenuItem) {
$relativeElement = $(_getHTMLMenuItem(foundMenuItem.id)).closest("li");
}
$relativeElement = this._getMenuItemForCommand(command);
}
}

return $relativeElement;
};

/**
* Returns a menuItem and a relative position for the menuSection/position requested
* @param {{menuSection: String}}
* @param {String} position constant
* @returns {{relativeElement: HTMLIElement, position: String}}
*/
Menu.prototype._getMenuSectionPosition = function (menuSection, position) {
var $relativeElement;
var $sectionMarker = this._getMenuItemForCommand(CommandManager.get(menuSection.sectionMarker));
var $sectionItems = $sectionMarker.siblings();
if (position === FIRST || position === LAST) {
// Determine the $relativeElement by traversing the sibling list and
// stop at the first divider found
var $listElem = $sectionMarker;
$relativeElement = $listElem;
while (true) {
$listElem = (position === FIRST ? $listElem.prev() : $listElem.next());
if ($listElem.length === 0) {
break;
} else if ($listElem.find(".divider").length === 0) {
$relativeElement = $listElem;
} else {
break;
}
}
if (position === FIRST) {
position = BEFORE;
} else {
position = AFTER;
}
} else {
console.log("Bad Parameter: MenuSection used as relativeID with a position other than FIRST or LAST");
$relativeElement = null;
}

return {relativeElement: $relativeElement, position: position};
};

/**
* Adds a new menu item with the specified id and display text. The insertion position is
Expand All @@ -331,7 +380,7 @@ define(function (require, exports, module) {
* one or more key bindings to associate with the supplied command.
* @param {?string} position - constant defining the position of new the MenuItem relative
* to other MenuItems. Default is LAST. (see Insertion position constants).
* @param {?string} relativeID - id of command (future: also sub-menu, or menu section) that
* @param {?string} relativeID - id of command or menu section (future: sub-menu) that
* the new menuItem will be positioned relative to. Required when position is
* AFTER or BEFORE, ignored when position is FIRST or LAST
*
Expand Down Expand Up @@ -390,7 +439,14 @@ define(function (require, exports, module) {
}

// Insert menu item
var $relativeElement = this._getRelativeMenuItem(relativeID);
var $relativeElement;
if (relativeID && relativeID.hasOwnProperty("sectionMarker")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of overriding position "FIRST" and "LAST" and looking for "sectionMarker", I think it might be better to add new relative position constants something like "FIRST_IN_SECTION" and "LAST_IN_SECTION", and give normal IDs to the dividers. This seems like a simple API and also simpler code.

var sectionPos = this._getMenuSectionPosition(relativeID, position);
$relativeElement = sectionPos.relativeElement;
position = sectionPos.position;
} else {
$relativeElement = this._getRelativeMenuItem(relativeID);
}
_insertInList($("li#" + StringUtils.jQueryIdEscape(this.id) + " > ul.dropdown-menu"),
$menuItem, position, $relativeElement);

Expand Down
47 changes: 47 additions & 0 deletions test/spec/Menu-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,53 @@ define(function (require, exports, module) {
});
});

it("it should add menu to beginnging and end of menu section", function () {
// set up test menu and menu items
CommandManager.register("Brackets Test Command Custom 0", "custom.command0", function () {});
CommandManager.register("Brackets Test Command Custom 1", "custom.command1", function () {});
CommandManager.register("Brackets Test Command Custom 2", "custom.command2", function () {});
CommandManager.register("Brackets Test Command Custom 3", "custom.command3", function () {});
CommandManager.register("Brackets Test Command Custom 4", "custom.command4", function () {});
CommandManager.register("Brackets Test Command Custom 5", "custom.command5", function () {});
CommandManager.register("Brackets Test Command Custom 6", "custom.command6", function () {});
CommandManager.register("Brackets Test Command Custom 7", "custom.command7", function () {});
CommandManager.register("Brackets Test Command Custom 8", "custom.command8", function () {});
var menu = Menus.addMenu("Custom", "menu-custom");
menu.addMenuItem("custom.command0");
menu.addMenuItem("custom.command1");
menu.addMenuDivider();
menu.addMenuItem("custom.command2");
menu.addMenuItem("custom.command3");

// create mock menu sections
var menuSectionCmd0 = {sectionMarker: "custom.command0"},
menuSectionCmd2 = {sectionMarker: "custom.command2"};

var listSelector = "#menu-custom > ul";

// Add new menu to END of menuSectionCmd0
var menuItem = menu.addMenuItem("custom.command4", null, Menus.LAST, menuSectionCmd0);
var $listItems = testWindow.$(listSelector).children();
expect($listItems.length).toBe(6);
expect($($listItems[2]).find("a#menu-custom-custom\\.command4").length).toBe(1);

// Add new menu to END of menuSectionCmd2
menuItem = menu.addMenuItem("custom.command5", null, Menus.LAST, menuSectionCmd2);
$listItems = testWindow.$(listSelector).children();
expect($($listItems[6]).find("a#menu-custom-custom\\.command5").length).toBe(1);

// Add new menu to BEGINNING of menuSectionCmd0
menuItem = menu.addMenuItem("custom.command6", null, Menus.FIRST, menuSectionCmd0);
$listItems = testWindow.$(listSelector).children();
expect($($listItems[0]).find("a#menu-custom-custom\\.command6").length).toBe(1);

// Add new menu to BEGINNING of menuSectionCmd2
menuItem = menu.addMenuItem("custom.command6", null, Menus.FIRST, menuSectionCmd2);
$listItems = testWindow.$(listSelector).children();
expect($($listItems[0]).find("a#menu-custom-custom\\.command6").length).toBe(1);

});

it("should not add duplicate menu", function () {
runs(function () {
var $listItems = testWindow.$("#main-toolbar > ul.nav").children();
Expand Down