From df0a1dad0be00a164173e23d473520bb896038d6 Mon Sep 17 00:00:00 2001 From: Boopesh Mahendran Date: Wed, 8 Nov 2017 14:33:41 +0530 Subject: [PATCH 1/5] Add Context submenu api --- src/command/Menus.js | 231 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 211 insertions(+), 20 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 85635ed62a7..5940bc41588 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -124,6 +124,7 @@ define(function (require, exports, module) { * Other constants */ var DIVIDER = "---"; + var SUBMENU = "SUBMENU"; /** * Error Codes from Brackets Shell @@ -310,7 +311,7 @@ define(function (require, exports, module) { this.isDivider = (command === DIVIDER); this.isNative = false; - if (!this.isDivider) { + if (!this.isDivider && command !== SUBMENU) { // Bind event handlers this._enabledChanged = this._enabledChanged.bind(this); this._checkedChanged = this._checkedChanged.bind(this); @@ -612,6 +613,11 @@ define(function (require, exports, module) { $menuItem.on("click", function () { menuItem._command.execute(); }); + + var self = this; + $menuItem.on("mouseenter", function () { + self.closeSubMenu(); + }); } // Insert menu item @@ -740,6 +746,147 @@ define(function (require, exports, module) { // NOT IMPLEMENTED // }; + /** + * + * Creates a new submenu and a menuItem and adds the menuItem of the submenu + * to the menu and returns the submenu. + * + * A submenu will have the same structure of a menu with a additional field + * parentMenuItem which has the reference of the submenu's parent menuItem. + + * A submenu will raise the following events: + * - beforeSubMenuOpen + * - beforeSubMenuClose + * + * Note, This function will create only a context submenu. + * + * TODO: Make this function work for Menus + * + * + * @param {!string} name displayed in menu item of the submenu + * @param {!string} id + * @param {?string} position - constant defining the position of new MenuItem of the submenu relative to + * other MenuItems. Values: + * - With no relativeID, use Menus.FIRST or LAST (default is LAST) + * - Relative to a command id, use BEFORE or AFTER (required) + * - Relative to a MenuSection, use FIRST_IN_SECTION or LAST_IN_SECTION (required) + * @param {?string} relativeID - command id OR one of the MenuSection.* constants. Required + * for all position constants except FIRST and LAST. + * + * @return {Menu} the newly created submenu + */ + Menu.prototype.addSubMenu = function (name, id, position, relativeID) { + + if (!name || !id) { + console.error("addSubMenu(): missing required parameters: name and id"); + return null; + } + + // Guard against duplicate context menu ids + if (contextMenuMap[id]) { + console.log("Context menu added with id of existing Context Menu: " + id); + return null; + } + + var menu = new ContextMenu(id); + contextMenuMap[id] = menu; + + var menuItemID = this.id + "-" + id; + + if (menuItemMap[menuItemID]) { + console.log("MenuItem added with same id of existing MenuItem: " + id); + return null; + } + + // create MenuItem + var menuItem = new MenuItem(menuItemID, SUBMENU); + menuItemMap[menuItemID] = menuItem; + + menu.parentMenuItem = menuItem; + + // create MenuItem DOM + if (_isHTMLMenu(this.id)) { + // Create the HTML Menu + var $menuItem = $("
  • " + + "" + name + "" + + "▸" + + "
    • "); + + var self = this; + $menuItem.on("mouseenter", function(e) { + if (self.openSubMenu && self.openSubMenu.id === menu.id) { + return; + } + self.closeSubMenu(); + self.openSubMenu = menu; + menu.open(); + }); + + // Insert menu item + var $relativeElement = this._getRelativeMenuItem(relativeID, position); + _insertInList($("li#" + StringUtils.jQueryIdEscape(this.id) + " > ul.dropdown-menu"), + $menuItem, position, $relativeElement); + } else { + // TODO: add submenus for native menus + } + return menu; + }; + + + /** + * Removes the specified submenu from this Menu. + * + * Note, this function will only remove context submenus + * + * TODO: Make this function work for Menus + * + * @param {!string} subMenuID - the menu id of the submenu to remove. + */ + Menu.prototype.removeSubMenu = function (subMenuID) { + var subMenu, + parentMenuItem; + + if (!subMenuID) { + console.error("removeSubMenu(): missing required parameters: subMenuID"); + return; + } + + subMenu = getContextMenu(subMenuID); + + if (!subMenu || !subMenu.parentMenuItem) { + console.error("removeSubMenu(): parameter subMenuID: %s is not a valid submenu id", subMenuID); + return; + } + + parentMenuItem = subMenu.parentMenuItem; + + + if (!menuItemMap[parentMenuItem.id]) { + console.error("removeSubMenu(): parent menuItem not found in menuItemMap: %s", parentMenuItem.id); + return; + } + + if (_isHTMLMenu(this.id)) { + $(_getHTMLMenuItem(parentMenuItem.id)).parent().remove(); // remove the menu item + $(_getHTMLMenu(subMenuID)).remove(); // remove the menu + } else { + // TODO: remove submenus for native menus + } + + + delete menuItemMap[parentMenuItem.id]; + delete contextMenuMap[subMenuID]; + }; + + /** + * Closes the submenu if the menu has a submenu open. + */ + Menu.prototype.closeSubMenu = function() { + if (this.openSubMenu) { + this.openSubMenu.close(); + this.openSubMenu = null; + } + }; /** * Gets the Command associated with a MenuItem * @return {Command} @@ -1038,9 +1185,14 @@ define(function (require, exports, module) { /** * Displays the ContextMenu at the specified location and dispatches the - * "beforeContextMenuOpen" event.The menu location may be adjusted to prevent - * clipping by the browser window. All other menus and ContextMenus will be closed - * bofore a new menu is shown. + * "beforeContextMenuOpen" event or "beforeSubMenuOpen" event (for submenus). + * The menu location may be adjusted to prevent clipping by the browser window. + * All other menus and ContextMenus will be closed before a new menu + * will be closed before a new menu is shown (if the new menu is not + * a submenu). + * + * In case of submenus, the parentMenu of the submenu will not be closed when the + * sub menu is open. * * @param {MouseEvent | {pageX:number, pageY:number}} mouseOrLocation - pass a MouseEvent * to display the menu near the mouse or pass in an object with page x/y coordinates @@ -1048,7 +1200,8 @@ define(function (require, exports, module) { */ ContextMenu.prototype.open = function (mouseOrLocation) { - if (!mouseOrLocation || !mouseOrLocation.hasOwnProperty("pageX") || !mouseOrLocation.hasOwnProperty("pageY")) { + if (!this.parentMenuItem && + (!mouseOrLocation || !mouseOrLocation.hasOwnProperty("pageX") || !mouseOrLocation.hasOwnProperty("pageY"))) { console.error("ContextMenu open(): missing required parameter"); return; } @@ -1057,21 +1210,26 @@ define(function (require, exports, module) { escapedId = StringUtils.jQueryIdEscape(this.id), $menuAnchor = $("#" + escapedId), $menuWindow = $("#" + escapedId + " > ul"), - posTop = mouseOrLocation.pageY, - posLeft = mouseOrLocation.pageX; + posTop, + posLeft; // only show context menu if it has menu items if ($menuWindow.children().length <= 0) { return; } - this.trigger("beforeContextMenuOpen"); - - // close all other dropdowns - closeAll(); // adjust positioning so menu is not clipped off bottom or right - var elementRect = { + if (this.parentMenuItem) { // If context menu is a submenu + + this.trigger("beforeSubMenuOpen"); + + var $parentMenuItem = $(_getHTMLMenuItem(this.parentMenuItem.id)); + + posTop = $parentMenuItem.offset().top; + posLeft = $parentMenuItem.offset().left + $parentMenuItem.outerWidth(); + + var elementRect = { top: posTop, left: posLeft, height: $menuWindow.height() + 25, @@ -1079,15 +1237,43 @@ define(function (require, exports, module) { }, clip = ViewUtils.getElementClipSize($window, elementRect); - if (clip.bottom > 0) { - posTop = Math.max(0, posTop - clip.bottom); - } - posTop -= 30; // shift top for hidden parent element - posLeft += 5; + if (clip.bottom > 0) { + posTop = Math.max(0, posTop + $parentMenuItem.height() - $menuWindow.height()); + } + + posTop -= 30; // shift top for hidden parent element + posLeft += 3; + if (clip.right > 0) { + posLeft = Math.max(0, posLeft - 2 * $parentMenuItem.outerWidth()); + } + } else { + this.trigger("beforeContextMenuOpen"); + + // close all other dropdowns + closeAll(); + + posTop = mouseOrLocation.pageY; + posLeft = mouseOrLocation.pageX; - if (clip.right > 0) { - posLeft = Math.max(0, posLeft - clip.right); + var elementRect = { + top: posTop, + left: posLeft, + height: $menuWindow.height() + 25, + width: $menuWindow.width() + }, + clip = ViewUtils.getElementClipSize($window, elementRect); + + if (clip.bottom > 0) { + posTop = Math.max(0, posTop - clip.bottom); + } + posTop -= 30; // shift top for hidden parent element + posLeft += 5; + + + if (clip.right > 0) { + posLeft = Math.max(0, posLeft - clip.right); + } } // open the context menu at final location @@ -1100,7 +1286,12 @@ define(function (require, exports, module) { * Closes the context menu. */ ContextMenu.prototype.close = function () { - this.trigger("beforeContextMenuClose"); + if (this.parentMenuItem) { + this.trigger("beforeSubMenuClose"); + } else { + this.trigger("beforeContextMenuClose"); + } + this.closeSubMenu(); $("#" + StringUtils.jQueryIdEscape(this.id)).removeClass("open"); }; From 392fedd2013b09c1001e0c1809933ef816a6ea86 Mon Sep 17 00:00:00 2001 From: Boopesh Mahendran Date: Wed, 8 Nov 2017 14:35:19 +0530 Subject: [PATCH 2/5] Add tests --- test/spec/Menu-test.js | 192 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 192 insertions(+) diff --git a/test/spec/Menu-test.js b/test/spec/Menu-test.js index 961bba6bc19..9b23a7ccd41 100644 --- a/test/spec/Menu-test.js +++ b/test/spec/Menu-test.js @@ -914,6 +914,195 @@ define(function (require, exports, module) { }); + describe("Context Submenus", function() { + var menuId, menu, subMenuId, subMenu; + + function parentMenuItemDOM(menuItemId) { + return testWindow.$("#" + menuItemId); + } + + function subMenuDOM(subMenuId) { + return testWindow.$("#" + subMenuId); + } + + function getBounds(object) { + return { + left : object.offset().left, + top : object.offset().top, + right : object.offset().left + object.width(), + bottom : object.offset().top + object.height() + }; + } + + function boundsInsideWindow(object) { + var bounds = getBounds(object); + return bounds.left >= 0 && + bounds.right <= $(testWindow).width() && + bounds.top >= 0 && + bounds.bottom <= $(testWindow).height(); + } + + it("open a context submenu", function() { + runs(function() { + var openEvent = false; + + menuId = "context-menu-custom-openSubmenu-1"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-openSubmenu-1"; + subMenu = menu.addSubMenu("submenu", subMenuId); + + CommandManager.register("Brackets Test Command Custom 56", "Menu-test.command56", function () {}); + subMenu.addMenuItem("Menu-test.command56"); + + subMenu.on("beforeSubMenuOpen", function() { + openEvent = true; + }); + + subMenu.open(); + + + var $submenu = testWindow.$(".dropdown.open > ul"); + expect($submenu.length).toBe(1); + + expect(openEvent).toBeTruthy(); + + subMenu.close(); + }); + }); + + it("close a context submenu", function () { + runs(function() { + menuId = "context-menu-custom-closeSubmenu-1"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-closeSubmenu-1"; + subMenu = menu.addSubMenu("submenu", subMenuId); + + CommandManager.register("Brackets Test Command Custom 58", "Menu-test.command58", function () {}); + subMenu.addMenuItem("Menu-test.command58"); + + subMenu.open(); + + // verify dropdown is open + var $submenu = testWindow.$(".dropdown.open"); + expect($submenu.length).toBe(1); + + // verify close event + subMenu.close(); + + // verify all dropdowns are closed + $submenu = testWindow.$(".dropdown.open"); + expect($submenu.length).toBe(0); + }); + }); + + it("context submenu is not clipped", function() { + runs(function() { + var openEvent = false; + + menuId = "context-menu-custom-clipSubmenu-1"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-clipSubmenu-1"; + subMenu = menu.addSubMenu("submenu", subMenuId); + + CommandManager.register("Brackets Test Command Custom 57", "Menu-test.command57", function () {}); + subMenu.addMenuItem("Menu-test.command57"); + + subMenu.open(); + + var $submenu = testWindow.$(".dropdown.open > ul"); + expect(boundsInsideWindow($submenu)).toBeTruthy(); + }); + }); + + describe("Add a context submenu", function() { + it("should add new context submenu", function() { + runs(function() { + menuId = "context-menu-custom-addSubmenu-1"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-addSubmenu-1"; + subMenu = menu.addSubMenu("submenu", subMenuId); + + expect(subMenu).toBeTruthy(); + expect(subMenu.parentMenuItem).toBeTruthy(); + + // check if new submenu is empty + var children = testWindow.$("#submenu-custom-addSubmenu-1 > ul").children(); + expect(children.length).toBe(0); + }); + }); + it("should not add duplicate context submenu", function() { + runs(function() { + menuId = "context-menu-custom-addSubmenu-2"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-addSubmenu-2"; + subMenu = menu.addSubMenu("submenu", subMenuId); + + expect(subMenu).toBeTruthy(); + expect(subMenu.parentMenuItem).toBeTruthy(); + + var subMenu2 = menu.addSubMenu("submenu", subMenuId); + + expect(subMenu2).toBeFalsy(); + expect(subMenu2).toBeNull(); + }); + }); + }); + + describe("Remove a context submenu", function() { + it("should add then remove new submenu to empty menu", function () { + runs(function () { + menuId = "context-menu-custom-removeSubmenu-1"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "submenu-custom-removeSubmenu-1"; + subMenu = menu.addSubMenu("submenu", subMenuId); + expect(subMenu).toBeTruthy(); + expect(subMenu.parentMenuItem).toBeTruthy(); + + var $subMenu = subMenuDOM(subMenuId); + expect($subMenu.length).toBe(1); + + var $parentMenuItem = parentMenuItemDOM(subMenu.parentMenuItem.id); + expect($parentMenuItem.length).toBe(1); + + menu.removeSubMenu(subMenuId); + $subMenu = subMenuDOM(subMenuId); + expect($subMenu.length).toBe(0); + + $parentMenuItem = parentMenuItemDOM(subMenu.parentMenuItem.id); + expect($parentMenuItem.length).toBe(0); + }); + }); + + it("should gracefully handle someone trying to remove a submenu that doesn't exist", function () { + runs(function () { + menuId = "context-menu-custom-removeSubmenu-2"; + menu = Menus.registerContextMenu(menuId); + + subMenuId = "Menu-test"; + + menu.removeSubMenu(subMenuId); + expect(menu).toBeTruthy(); + }); + }); + + it("should gracefully handle someone trying to remove a submenu without supplying the id", function () { + runs(function () { + menuId = "context-menu-custom-removeSubmenu-3"; + menu = Menus.registerContextMenu(menuId); + + menu.removeSubMenu(); + expect(menu).toBeTruthy(); + }); + }); + }); + }); + describe("Menu Item synchronizing", function () { it("should have same state as command", function () { @@ -1110,6 +1299,9 @@ define(function (require, exports, module) { // verify close event cmenu.close(); + // verify context submenus are closed + expect(cmenu.openSubMenu).toBeFalsy(); + // verify all dropdowns are closed $menus = testWindow.$(".dropdown.open"); expect($menus.length).toBe(0); From f7d4774b6bb5f9d7c4c6498317aef3e8374991d7 Mon Sep 17 00:00:00 2001 From: Boopesh Mahendran Date: Wed, 8 Nov 2017 14:54:02 +0530 Subject: [PATCH 3/5] Fix typo --- 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 5940bc41588..f5aaf423e0a 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -806,7 +806,7 @@ define(function (require, exports, module) { // create MenuItem DOM if (_isHTMLMenu(this.id)) { - // Create the HTML Menu + // Create the HTML MenuItem var $menuItem = $("
  • " + "" + name + "" + "▸" + From c9e1ee192b622467ab936e1179d447390188bffe Mon Sep 17 00:00:00 2001 From: Boopesh Mahendran Date: Fri, 10 Nov 2017 15:49:27 +0530 Subject: [PATCH 4/5] Update doc comments --- src/command/Menus.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index f5aaf423e0a..9c9a2cd002a 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -1196,7 +1196,8 @@ define(function (require, exports, module) { * * @param {MouseEvent | {pageX:number, pageY:number}} mouseOrLocation - pass a MouseEvent * to display the menu near the mouse or pass in an object with page x/y coordinates - * for a specific location. + * for a specific location.This paramter is not used for submenus. Submenus are always + * displayed at a position relative to the parent menu. */ ContextMenu.prototype.open = function (mouseOrLocation) { From 5616075ba898a10e7b221f7ffb6a1116a6f40432 Mon Sep 17 00:00:00 2001 From: Boopesh Mahendran Date: Fri, 10 Nov 2017 17:14:59 +0530 Subject: [PATCH 5/5] Remove menu items in sub menu before deletion --- src/command/Menus.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 9c9a2cd002a..c1506043897 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -844,7 +844,8 @@ define(function (require, exports, module) { */ Menu.prototype.removeSubMenu = function (subMenuID) { var subMenu, - parentMenuItem; + parentMenuItem, + commandID = ""; if (!subMenuID) { console.error("removeSubMenu(): missing required parameters: subMenuID"); @@ -866,6 +867,18 @@ define(function (require, exports, module) { return; } + // Remove all of the menu items in the submenu + _.forEach(menuItemMap, function (value, key) { + if (_.startsWith(key, subMenuID)) { + if (value.isDivider) { + subMenu.removeMenuDivider(key); + } else { + commandID = value.getCommand(); + subMenu.removeMenuItem(commandID); + } + } + }); + if (_isHTMLMenu(this.id)) { $(_getHTMLMenuItem(parentMenuItem.id)).parent().remove(); // remove the menu item $(_getHTMLMenu(subMenuID)).remove(); // remove the menu