Permalink
Browse files

Bug 839318: context-menu should overflow based on the number of visib…

…le menu items.
  • Loading branch information...
1 parent 1e8f44e commit 83b1d8fc326829d71a23c06ed4e0a51b0c72502c @Mossop committed Feb 8, 2013
Showing with 319 additions and 64 deletions.
  1. +89 −60 lib/sdk/context-menu.js
  2. +230 −4 test/test-context-menu.js
View
@@ -645,6 +645,12 @@ when(function() {
// App specific UI code lives here, it should handle populating the context
// menu and passing clicks etc. through to the items.
+function countVisibleItems(nodes) {
+ return Array.reduce(nodes, function(sum, node) {
+ return node.hidden ? sum : sum + 1;
+ }, 0);
+}
+
let MenuWrapper = Class({
initialize: function initialize(winWrapper, items, contextMenu) {
this.winWrapper = winWrapper;
@@ -654,11 +660,17 @@ let MenuWrapper = Class({
this.populated = false;
this.menuMap = new Map();
- this.contextMenu.addEventListener("popupshowing", this, false);
+ // updateItemVisibilities will run first, updateOverflowState will run after
+ // all other instances of this module have run updateItemVisibilities
+ this._updateItemVisibilities = this.updateItemVisibilities.bind(this);
+ this.contextMenu.addEventListener("popupshowing", this._updateItemVisibilities, true);
+ this._updateOverflowState = this.updateOverflowState.bind(this);
+ this.contextMenu.addEventListener("popupshowing", this._updateOverflowState, false);
},
destroy: function destroy() {
- this.contextMenu.removeEventListener("popupshowing", this, false);
+ this.contextMenu.removeEventListener("popupshowing", this._updateOverflowState, false);
+ this.contextMenu.removeEventListener("popupshowing", this._updateItemVisibilities, true);
if (!this.populated)
return;
@@ -741,31 +753,11 @@ let MenuWrapper = Class({
let menu = item.parentMenu;
if (menu === this.items) {
+ // Insert into the overflow popup if it exists, otherwise the normal
+ // context menu
menupopup = this.overflowPopup;
-
- // If there isn't already an overflow menu then check if we need to
- // create one, otherwise use the main context menu
- if (!menupopup) {
+ if (!menupopup)
menupopup = this.contextMenu;
- let toplevel = this.topLevelItems;
-
- if (toplevel.length >= MenuManager.overflowThreshold) {
- // Create the overflow menu and move everything there
- let overflowMenu = this.window.document.createElement("menu");
- overflowMenu.setAttribute("class", OVERFLOW_MENU_CLASS);
- overflowMenu.setAttribute("label", OVERFLOW_MENU_LABEL);
- this.contextMenu.insertBefore(overflowMenu, this.separator.nextSibling);
-
- menupopup = this.window.document.createElement("menupopup");
- menupopup.setAttribute("class", OVERFLOW_POPUP_CLASS);
- overflowMenu.appendChild(menupopup);
-
- for (let xulNode of toplevel) {
- menupopup.appendChild(xulNode);
- this.updateXULClass(xulNode);
- }
- }
- }
}
else {
let xulNode = this.getXULNodeForItem(menu);
@@ -932,30 +924,18 @@ let MenuWrapper = Class({
}
}
else if (parent == this.overflowPopup) {
+ // If there are no more items then remove the overflow menu and separator
if (parent.childNodes.length == 0) {
- // It's possible that this add-on had all the items in the overflow
- // menu and they're now all gone, so remove the separator and overflow
- // menu directly
-
let separator = this.separator;
separator.parentNode.removeChild(separator);
this.contextMenu.removeChild(parent.parentNode);
}
- else if (parent.childNodes.length <= MenuManager.overflowThreshold) {
- // Otherwise if the overflow menu is empty enough move everything in
- // the overflow menu back to top level and remove the overflow menu
-
- while (parent.firstChild) {
- let node = parent.firstChild;
- this.contextMenu.insertBefore(node, parent.parentNode);
- this.updateXULClass(node);
- }
- this.contextMenu.removeChild(parent.parentNode);
- }
}
},
- handleEvent: function handleEvent(event) {
+ // Recurses through all the items owned by this module and sets their hidden
+ // state
+ updateItemVisibilities: function updateItemVisibilities(event) {
try {
if (event.type != "popupshowing")
return;
@@ -970,28 +950,77 @@ let MenuWrapper = Class({
this.populate(this.items);
}
- let separator = this.separator;
- let popup = this.overflowMenu;
-
let popupNode = event.target.triggerNode;
- if (this.setVisibility(this.items, popupNode, PageContext().isCurrent(popupNode))) {
- // Some of this instance's items are visible so make sure the separator
- // and if necessary the overflow popup are visible
- separator.hidden = false;
- if (popup)
- popup.hidden = false;
+ this.setVisibility(this.items, popupNode, PageContext().isCurrent(popupNode));
+ }
+ catch (e) {
+ console.exception(e);
+ }
+ },
+
+ // Counts the number of visible items across all modules and makes sure they
+ // are in the right place between the top level context menu and the overflow
+ // menu
+ updateOverflowState: function updateOverflowState(event) {
+ try {
+ if (event.type != "popupshowing")
+ return;
+ if (event.target != this.contextMenu)
+ return;
+
+ // The main items will be in either the top level context menu or the
+ // overflow menu at this point. Count the visible ones and if they are in
+ // the wrong place move them
+ let toplevel = this.topLevelItems;
+ let overflow = this.overflowItems;
+ let visibleCount = countVisibleItems(toplevel) +
+ countVisibleItems(overflow);
+
+ if (visibleCount == 0) {
+ let separator = this.separator;
+ if (separator)
+ separator.hidden = true;
+ let overflowMenu = this.overflowMenu;
+ if (overflowMenu)
+ overflowMenu.hidden = true;
+ }
+ else if (visibleCount > MenuManager.overflowThreshold) {
+ this.separator.hidden = false;
+ let overflowPopup = this.overflowPopup;
+ if (overflowPopup)
+ overflowPopup.parentNode.hidden = false;
+
+ if (toplevel.length > 0) {
+ // The overflow menu shouldn't exist here but let's play it safe
+ if (!overflowPopup) {
+ let overflowMenu = this.window.document.createElement("menu");
+ overflowMenu.setAttribute("class", OVERFLOW_MENU_CLASS);
+ overflowMenu.setAttribute("label", OVERFLOW_MENU_LABEL);
+ this.contextMenu.insertBefore(overflowMenu, this.separator.nextSibling);
+
+ overflowPopup = this.window.document.createElement("menupopup");
+ overflowPopup.setAttribute("class", OVERFLOW_POPUP_CLASS);
+ overflowMenu.appendChild(overflowPopup);
+ }
+
+ for (let xulNode of toplevel) {
+ overflowPopup.appendChild(xulNode);
+ this.updateXULClass(xulNode);
+ }
+ }
}
else {
- // We need to test whether any other instance has visible items.
- // Get all the highest level items and see if any are visible.
- let anyVisible = (Array.some(this.topLevelItems, function(item) !item.hidden)) ||
- (Array.some(this.overflowItems, function(item) !item.hidden));
-
- // If any were visible make sure the separator and if necessary the
- // overflow popup are visible, otherwise hide them
- separator.hidden = !anyVisible;
- if (popup)
- popup.hidden = !anyVisible;
+ this.separator.hidden = false;
+
+ if (overflow.length > 0) {
+ // Move all the overflow nodes out of the overflow menu and position
+ // them immediately before it
+ for (let xulNode of overflow) {
+ this.contextMenu.insertBefore(xulNode, xulNode.parentNode.parentNode);
+ this.updateXULClass(xulNode);
+ }
+ this.contextMenu.removeChild(this.overflowMenu);
+ }
}
}
catch (e) {
Oops, something went wrong.

0 comments on commit 83b1d8f

Please sign in to comment.