From b7ebee985b0292eda5234c4555a13ee59914252a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 22 Oct 2017 23:51:33 -0400 Subject: [PATCH 01/21] refactor indexOfItemById --- lib/browser/api/menu.js | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 174acd7d9199..bacebc702c69 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -10,7 +10,7 @@ var nextGroupId = 0 // Search between separators to find a radio menu item and return its group id, // otherwise generate a group id. -var generateGroupId = function (items, pos) { +function generateGroupId (items, pos) { var i, item, j, k, ref1, ref2, ref3 if (pos > 0) { for (i = j = ref1 = pos - 1; ref1 <= 0 ? j <= 0 : j >= 0; i = ref1 <= 0 ? ++j : --j) { @@ -37,19 +37,16 @@ var generateGroupId = function (items, pos) { } // Returns the index of item according to |id|. -var indexOfItemById = function (items, id) { - var i, item, j, len - for (i = j = 0, len = items.length; j < len; i = ++j) { - item = items[i] - if (item.id === id) { - return i - } +function indexOfItemById (items, id) { + for (let idx = 0; idx < items.length; idx += 1) { + const item = items[idx] + if (item.id === id) return idx } return -1 } // Returns the index of where to insert the item according to |position|. -var indexToInsertByPosition = function (items, position) { +function indexToInsertByPosition (items, position) { var insertIndex if (!position) { return items.length From 1cd53768ab93be81821a5130c79177530d1aecc7 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Sun, 22 Oct 2017 23:57:23 -0400 Subject: [PATCH 02/21] clean up indexToInsertByPosition --- lib/browser/api/menu.js | 51 +++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index bacebc702c69..f9b4862ed589 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -46,39 +46,36 @@ function indexOfItemById (items, id) { } // Returns the index of where to insert the item according to |position|. -function indexToInsertByPosition (items, position) { - var insertIndex - if (!position) { - return items.length - } - const [query, id] = position.split('=') - insertIndex = indexOfItemById(items, id) - if (insertIndex === -1 && query !== 'endof') { +function indexToInsertByPosition (items, pos) { + if (!pos) return items.length + + const [query, id] = pos.split('=') + let idx = indexOfItemById(items, id) + + if (idx === -1 && query !== 'endof') { console.warn("Item with id '" + id + "' is not found") return items.length } - switch (query) { - case 'after': - insertIndex++ - break - case 'endof': - // If the |id| doesn't exist, then create a new group with the |id|. - if (insertIndex === -1) { - items.push({ - id: id, - type: 'separator' - }) - insertIndex = items.length - 1 - } + if (query === 'after') { + idx += 1 + } else if (query === 'endof') { + // create new group with id if none exists + if (idx === -1) { + items.push({ + id, + type: 'separator' + }) + idx = items.length - 1 + } + idx += 1 - // Find the end of the group. - insertIndex++ - while (insertIndex < items.length && items[insertIndex].type !== 'separator') { - insertIndex++ - } + // search for end of group + while (idx < items.length && items[idx].type !== 'separator') { + idx += 1 + } } - return insertIndex + return idx } const Menu = bindings.Menu From f9c3123f5f6086c06c69731140fff6b94d7745d7 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 00:16:35 -0400 Subject: [PATCH 03/21] clean up menuWillShow --- lib/browser/api/menu.js | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index f9b4862ed589..0ee05a06edd9 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -88,15 +88,15 @@ Menu.prototype._init = function () { this.items = [] this.delegate = { isCommandIdChecked: (commandId) => { - var command = this.commandsMap[commandId] + const command = this.commandsMap[commandId] return command != null ? command.checked : undefined }, isCommandIdEnabled: (commandId) => { - var command = this.commandsMap[commandId] + const command = this.commandsMap[commandId] return command != null ? command.enabled : undefined }, isCommandIdVisible: (commandId) => { - var command = this.commandsMap[commandId] + const command = this.commandsMap[commandId] return command != null ? command.visible : undefined }, getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { @@ -106,7 +106,7 @@ Menu.prototype._init = function () { if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() }, getIconForCommandId: (commandId) => { - var command = this.commandsMap[commandId] + const command = this.commandsMap[commandId] return command != null ? command.icon : undefined }, executeCommand: (event, commandId) => { @@ -115,23 +115,17 @@ Menu.prototype._init = function () { command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) }, menuWillShow: () => { - // Make sure radio groups have at least one menu item seleted. - var checked, group, id, j, len, radioItem, ref1 - ref1 = this.groupsMap - for (id in ref1) { - group = ref1[id] - checked = false - for (j = 0, len = group.length; j < len; j++) { - radioItem = group[j] - if (!radioItem.checked) { - continue - } + // Make sure radio groups have at least one menu item selected + for (let id in this.groupsMap) { + const group = this.groupsMap[id] + const checked = false + for (let idx = 0; idx < group.length; idx++) { + const radioItem = group[idx] + if (!radioItem.checked) continue checked = true break } - if (!checked) { - v8Util.setHiddenValue(group[0], 'checked', true) - } + if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) } } } From 61a93c711cd1ab89cf747ad7f6fc90ee16762eba Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 00:47:02 -0400 Subject: [PATCH 04/21] clean up popup --- lib/browser/api/menu.js | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 0ee05a06edd9..421462671fbf 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -132,40 +132,31 @@ Menu.prototype._init = function () { } Menu.prototype.popup = function (window, x, y, positioningItem) { + let [newX, newY, newPositioningItem] = [x, y, positioningItem] let asyncPopup // menu.popup(x, y, positioningItem) - if (window != null && (typeof window !== 'object' || window.constructor !== BrowserWindow)) { - // Shift. - positioningItem = y - y = x - x = window - window = null + if (window != null) { + if (typeof window !== 'object' || window.constructor !== BrowserWindow) { + [newPositioningItem, newY, newX] = [y, x, window] + window = null + } } // menu.popup(window, {}) if (x != null && typeof x === 'object') { - const options = x - x = options.x - y = options.y - positioningItem = options.positioningItem - asyncPopup = options.async + [newX, newY, newPositioningItem] = [x.x, x.y, x.positioningItem] + asyncPopup = x.async } - // Default to showing in focused window. + // set up defaults if (window == null) window = BrowserWindow.getFocusedWindow() - - // Default to showing under mouse location. - if (typeof x !== 'number') x = -1 - if (typeof y !== 'number') y = -1 - - // Default to not highlighting any item. - if (typeof positioningItem !== 'number') positioningItem = -1 - - // Default to synchronous for backwards compatibility. + if (typeof x !== 'number') newX = -1 + if (typeof y !== 'number') newY = -1 + if (typeof positioningItem !== 'number') newPositioningItem = -1 if (typeof asyncPopup !== 'boolean') asyncPopup = false - this.popupAt(window, x, y, positioningItem, asyncPopup) + this.popupAt(window, newX, newY, newPositioningItem, asyncPopup) } Menu.prototype.closePopup = function (window) { From 87802b2c17cadb98412e0ea68e2d56f2135de794 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 12:11:59 -0400 Subject: [PATCH 05/21] initial port of things into a Menu class --- lib/browser/api/menu.js | 465 +++++++++++++++++++--------------------- 1 file changed, 223 insertions(+), 242 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 421462671fbf..bfe6eb82fd99 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -6,10 +6,231 @@ const v8Util = process.atomBinding('v8_util') const bindings = process.atomBinding('menu') // Automatically generated radio menu item's group id. -var nextGroupId = 0 +let nextGroupId = 0 +let applicationMenu = null + +const Menu = bindings.Menu + +class Menu extends EventEmitter { + constructor () { + this.commandsMap = {} + this.groupsMap = {} + this.items = [] + this.delegate = { + isCommandIdChecked: (commandId) => { + const command = this.commandsMap[commandId] + return command != null ? command.checked : undefined + }, + isCommandIdEnabled: (commandId) => { + const command = this.commandsMap[commandId] + return command != null ? command.enabled : undefined + }, + isCommandIdVisible: (commandId) => { + const command = this.commandsMap[commandId] + return command != null ? command.visible : undefined + }, + getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { + const command = this.commandsMap[commandId] + if (command == null) return + if (command.accelerator != null) return command.accelerator + if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() + }, + getIconForCommandId: (commandId) => { + const command = this.commandsMap[commandId] + return command != null ? command.icon : undefined + }, + executeCommand: (event, commandId) => { + const command = this.commandsMap[commandId] + if (command == null) return + command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) + }, + menuWillShow: () => { + // Make sure radio groups have at least one menu item selected + for (let id in this.groupsMap) { + const group = this.groupsMap[id] + const checked = false + for (let idx = 0; idx < group.length; idx++) { + const radioItem = group[idx] + if (!radioItem.checked) continue + checked = true + break + } + if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) + } + } + } + popup (window, x, y, positioningItem) { + let [newX, newY, newPositioningItem] = [x, y, positioningItem] + let asyncPopup + + // menu.popup(x, y, positioningItem) + if (window != null) { + if (typeof window !== 'object' || window.constructor !== BrowserWindow) { + [newPositioningItem, newY, newX] = [y, x, window] + window = null + } + } + + // menu.popup(window, {}) + if (x != null && typeof x === 'object') { + [newX, newY, newPositioningItem] = [x.x, x.y, x.positioningItem] + asyncPopup = x.async + } + + // set up defaults + if (window == null) window = BrowserWindow.getFocusedWindow() + if (typeof x !== 'number') newX = -1 + if (typeof y !== 'number') newY = -1 + if (typeof positioningItem !== 'number') newPositioningItem = -1 + if (typeof asyncPopup !== 'boolean') asyncPopup = false + + this.popupAt(window, newX, newY, newPositioningItem, asyncPopup) + } + closePopup (window) { + if (window == null || window.constructor !== BrowserWindow) { + window = BrowserWindow.getFocusedWindow() + } + + if (window != null) this.closePopupAt(window.id) + } + getMenuItemById (id) { + const items = this.items + + let found = items.find(item => item.id === id) || null + for (let i = 0, length = items.length; !found && i < length; i++) { + if (items[i].submenu) { + found = items[i].submenu.getMenuItemById(id) + } + } + return found + } + append (item) { + return this.insert(this.getItemCount(), item) + } + insert (pos, item) { + var base, name + if ((item != null ? item.constructor : void 0) !== MenuItem) { + throw new TypeError('Invalid item') + } + switch (item.type) { + case 'normal': + this.insertItem(pos, item.commandId, item.label) + break + case 'checkbox': + this.insertCheckItem(pos, item.commandId, item.label) + break + case 'separator': + this.insertSeparator(pos) + break + case 'submenu': + this.insertSubMenu(pos, item.commandId, item.label, item.submenu) + break + case 'radio': + // Grouping radio menu items. + item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) + if ((base = this.groupsMap)[name = item.groupId] == null) { + base[name] = [] + } + this.groupsMap[item.groupId].push(item) + + // Setting a radio menu item should flip other items in the group. + v8Util.setHiddenValue(item, 'checked', item.checked) + Object.defineProperty(item, 'checked', { + enumerable: true, + get: function () { + return v8Util.getHiddenValue(item, 'checked') + }, + set: () => { + var j, len, otherItem, ref1 + ref1 = this.groupsMap[item.groupId] + for (j = 0, len = ref1.length; j < len; j++) { + otherItem = ref1[j] + if (otherItem !== item) { + v8Util.setHiddenValue(otherItem, 'checked', false) + } + } + return v8Util.setHiddenValue(item, 'checked', true) + } + }) + this.insertRadioItem(pos, item.commandId, item.label, item.groupId) + } + if (item.sublabel != null) { + this.setSublabel(pos, item.sublabel) + } + if (item.icon != null) { + this.setIcon(pos, item.icon) + } + if (item.role != null) { + this.setRole(pos, item.role) + } + + // Make menu accessable to items. + item.overrideReadOnlyProperty('menu', this) + + // Remember the items. + this.items.splice(pos, 0, item) + this.commandsMap[item.commandId] = item + } + _callMenuWillShow () { + if (this.delegate != null) { + this.delegate.menuWillShow() + } + this.items.forEach(function (item) { + if (item.submenu != null) { + item.submenu._callMenuWillShow() + } + }) + } + static setApplicationMenu (menu) { + if (!(menu === null || menu.constructor === Menu)) { + throw new TypeError('Invalid menu') + } + + // Keep a reference. + applicationMenu = menu + if (process.platform === 'darwin') { + if (menu === null) { + return + } + menu._callMenuWillShow() + bindings.setApplicationMenu(menu) + } else { + BrowserWindow.getAllWindows().forEach(function (window) { + window.setMenu(menu) + }) + } + } + static getApplicationMenu () { + return applicationMenu + } + static buildFromTemplate (template) { + if (!Array.isArray(template)) throw new TypeError('Invalid template for Menu') + + const menu = new Menu() + let positioned = [] + let idx = 0 + + template.forEach((item) => { + idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx += 1 + positioned.splice(idx, 0, item) + }) + + positioned.forEach((item) => { + if (typeof item !== 'object') { + throw new TypeError('Invalid template for MenuItem') + } + menu.append(new MenuItem(item)) + }) + + return menu + } +} + +Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder + +/* HELPER METHODS */ // Search between separators to find a radio menu item and return its group id, -// otherwise generate a group id. function generateGroupId (items, pos) { var i, item, j, k, ref1, ref2, ref3 if (pos > 0) { @@ -78,244 +299,4 @@ function indexToInsertByPosition (items, pos) { return idx } -const Menu = bindings.Menu - -Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype) - -Menu.prototype._init = function () { - this.commandsMap = {} - this.groupsMap = {} - this.items = [] - this.delegate = { - isCommandIdChecked: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.checked : undefined - }, - isCommandIdEnabled: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.enabled : undefined - }, - isCommandIdVisible: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.visible : undefined - }, - getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { - const command = this.commandsMap[commandId] - if (command == null) return - if (command.accelerator != null) return command.accelerator - if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() - }, - getIconForCommandId: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.icon : undefined - }, - executeCommand: (event, commandId) => { - const command = this.commandsMap[commandId] - if (command == null) return - command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) - }, - menuWillShow: () => { - // Make sure radio groups have at least one menu item selected - for (let id in this.groupsMap) { - const group = this.groupsMap[id] - const checked = false - for (let idx = 0; idx < group.length; idx++) { - const radioItem = group[idx] - if (!radioItem.checked) continue - checked = true - break - } - if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) - } - } - } -} - -Menu.prototype.popup = function (window, x, y, positioningItem) { - let [newX, newY, newPositioningItem] = [x, y, positioningItem] - let asyncPopup - - // menu.popup(x, y, positioningItem) - if (window != null) { - if (typeof window !== 'object' || window.constructor !== BrowserWindow) { - [newPositioningItem, newY, newX] = [y, x, window] - window = null - } - } - - // menu.popup(window, {}) - if (x != null && typeof x === 'object') { - [newX, newY, newPositioningItem] = [x.x, x.y, x.positioningItem] - asyncPopup = x.async - } - - // set up defaults - if (window == null) window = BrowserWindow.getFocusedWindow() - if (typeof x !== 'number') newX = -1 - if (typeof y !== 'number') newY = -1 - if (typeof positioningItem !== 'number') newPositioningItem = -1 - if (typeof asyncPopup !== 'boolean') asyncPopup = false - - this.popupAt(window, newX, newY, newPositioningItem, asyncPopup) -} - -Menu.prototype.closePopup = function (window) { - if (window == null || window.constructor !== BrowserWindow) { - window = BrowserWindow.getFocusedWindow() - } - if (window != null) { - this.closePopupAt(window.id) - } -} - -Menu.prototype.getMenuItemById = function (id) { - const items = this.items - - let found = items.find(item => item.id === id) || null - for (let i = 0, length = items.length; !found && i < length; i++) { - if (items[i].submenu) { - found = items[i].submenu.getMenuItemById(id) - } - } - return found -} - -Menu.prototype.append = function (item) { - return this.insert(this.getItemCount(), item) -} - -Menu.prototype.insert = function (pos, item) { - var base, name - if ((item != null ? item.constructor : void 0) !== MenuItem) { - throw new TypeError('Invalid item') - } - switch (item.type) { - case 'normal': - this.insertItem(pos, item.commandId, item.label) - break - case 'checkbox': - this.insertCheckItem(pos, item.commandId, item.label) - break - case 'separator': - this.insertSeparator(pos) - break - case 'submenu': - this.insertSubMenu(pos, item.commandId, item.label, item.submenu) - break - case 'radio': - // Grouping radio menu items. - item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) - if ((base = this.groupsMap)[name = item.groupId] == null) { - base[name] = [] - } - this.groupsMap[item.groupId].push(item) - - // Setting a radio menu item should flip other items in the group. - v8Util.setHiddenValue(item, 'checked', item.checked) - Object.defineProperty(item, 'checked', { - enumerable: true, - get: function () { - return v8Util.getHiddenValue(item, 'checked') - }, - set: () => { - var j, len, otherItem, ref1 - ref1 = this.groupsMap[item.groupId] - for (j = 0, len = ref1.length; j < len; j++) { - otherItem = ref1[j] - if (otherItem !== item) { - v8Util.setHiddenValue(otherItem, 'checked', false) - } - } - return v8Util.setHiddenValue(item, 'checked', true) - } - }) - this.insertRadioItem(pos, item.commandId, item.label, item.groupId) - } - if (item.sublabel != null) { - this.setSublabel(pos, item.sublabel) - } - if (item.icon != null) { - this.setIcon(pos, item.icon) - } - if (item.role != null) { - this.setRole(pos, item.role) - } - - // Make menu accessable to items. - item.overrideReadOnlyProperty('menu', this) - - // Remember the items. - this.items.splice(pos, 0, item) - this.commandsMap[item.commandId] = item -} - -// Force menuWillShow to be called -Menu.prototype._callMenuWillShow = function () { - if (this.delegate != null) { - this.delegate.menuWillShow() - } - this.items.forEach(function (item) { - if (item.submenu != null) { - item.submenu._callMenuWillShow() - } - }) -} - -var applicationMenu = null - -Menu.setApplicationMenu = function (menu) { - if (!(menu === null || menu.constructor === Menu)) { - throw new TypeError('Invalid menu') - } - - // Keep a reference. - applicationMenu = menu - if (process.platform === 'darwin') { - if (menu === null) { - return - } - menu._callMenuWillShow() - bindings.setApplicationMenu(menu) - } else { - BrowserWindow.getAllWindows().forEach(function (window) { - window.setMenu(menu) - }) - } -} - -Menu.getApplicationMenu = function () { - return applicationMenu -} - -Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder - -Menu.buildFromTemplate = function (template) { - var insertIndex, item, j, k, len, len1, menu, menuItem, positionedTemplate - if (!Array.isArray(template)) { - throw new TypeError('Invalid template for Menu') - } - positionedTemplate = [] - insertIndex = 0 - for (j = 0, len = template.length; j < len; j++) { - item = template[j] - if (item.position) { - insertIndex = indexToInsertByPosition(positionedTemplate, item.position) - } else { - // If no |position| is specified, insert after last item. - insertIndex++ - } - positionedTemplate.splice(insertIndex, 0, item) - } - menu = new Menu() - for (k = 0, len1 = positionedTemplate.length; k < len1; k++) { - item = positionedTemplate[k] - if (typeof item !== 'object') { - throw new TypeError('Invalid template for MenuItem') - } - menuItem = new MenuItem(item) - menu.append(menuItem) - } - return menu -} - module.exports = Menu From 577c0042b0fe3c0f1498f015392ed91986b7eb6d Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 14:47:47 -0400 Subject: [PATCH 06/21] update to ES6 --- lib/browser/api/menu.js | 491 ++++++++++++++++++---------------------- 1 file changed, 218 insertions(+), 273 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index bfe6eb82fd99..75b5a31e3b01 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -1,302 +1,247 @@ -'use strict' +const {BrowserWindow, MenuItem} = require('electron') +const {EventEmitter} = require('events') +//const _ = require('lodash') -const {BrowserWindow, MenuItem, webContents} = require('electron') -const EventEmitter = require('events').EventEmitter const v8Util = process.atomBinding('v8_util') const bindings = process.atomBinding('menu') // Automatically generated radio menu item's group id. let nextGroupId = 0 -let applicationMenu = null -const Menu = bindings.Menu - -class Menu extends EventEmitter { - constructor () { - this.commandsMap = {} - this.groupsMap = {} - this.items = [] - this.delegate = { - isCommandIdChecked: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.checked : undefined - }, - isCommandIdEnabled: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.enabled : undefined - }, - isCommandIdVisible: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.visible : undefined - }, - getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { - const command = this.commandsMap[commandId] - if (command == null) return - if (command.accelerator != null) return command.accelerator - if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() - }, - getIconForCommandId: (commandId) => { - const command = this.commandsMap[commandId] - return command != null ? command.icon : undefined - }, - executeCommand: (event, commandId) => { - const command = this.commandsMap[commandId] - if (command == null) return - command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) - }, - menuWillShow: () => { - // Make sure radio groups have at least one menu item selected - for (let id in this.groupsMap) { - const group = this.groupsMap[id] - const checked = false - for (let idx = 0; idx < group.length; idx++) { - const radioItem = group[idx] - if (!radioItem.checked) continue - checked = true - break - } - if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) - } - } - } - popup (window, x, y, positioningItem) { - let [newX, newY, newPositioningItem] = [x, y, positioningItem] - let asyncPopup - - // menu.popup(x, y, positioningItem) - if (window != null) { - if (typeof window !== 'object' || window.constructor !== BrowserWindow) { - [newPositioningItem, newY, newX] = [y, x, window] - window = null - } - } - - // menu.popup(window, {}) - if (x != null && typeof x === 'object') { - [newX, newY, newPositioningItem] = [x.x, x.y, x.positioningItem] - asyncPopup = x.async - } - - // set up defaults - if (window == null) window = BrowserWindow.getFocusedWindow() - if (typeof x !== 'number') newX = -1 - if (typeof y !== 'number') newY = -1 - if (typeof positioningItem !== 'number') newPositioningItem = -1 - if (typeof asyncPopup !== 'boolean') asyncPopup = false - - this.popupAt(window, newX, newY, newPositioningItem, asyncPopup) - } - closePopup (window) { - if (window == null || window.constructor !== BrowserWindow) { - window = BrowserWindow.getFocusedWindow() - } - - if (window != null) this.closePopupAt(window.id) - } - getMenuItemById (id) { - const items = this.items - - let found = items.find(item => item.id === id) || null - for (let i = 0, length = items.length; !found && i < length; i++) { - if (items[i].submenu) { - found = items[i].submenu.getMenuItemById(id) - } - } - return found - } - append (item) { - return this.insert(this.getItemCount(), item) - } - insert (pos, item) { - var base, name - if ((item != null ? item.constructor : void 0) !== MenuItem) { - throw new TypeError('Invalid item') - } - switch (item.type) { - case 'normal': - this.insertItem(pos, item.commandId, item.label) - break - case 'checkbox': - this.insertCheckItem(pos, item.commandId, item.label) - break - case 'separator': - this.insertSeparator(pos) - break - case 'submenu': - this.insertSubMenu(pos, item.commandId, item.label, item.submenu) - break - case 'radio': - // Grouping radio menu items. - item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) - if ((base = this.groupsMap)[name = item.groupId] == null) { - base[name] = [] - } - this.groupsMap[item.groupId].push(item) - - // Setting a radio menu item should flip other items in the group. - v8Util.setHiddenValue(item, 'checked', item.checked) - Object.defineProperty(item, 'checked', { - enumerable: true, - get: function () { - return v8Util.getHiddenValue(item, 'checked') - }, - set: () => { - var j, len, otherItem, ref1 - ref1 = this.groupsMap[item.groupId] - for (j = 0, len = ref1.length; j < len; j++) { - otherItem = ref1[j] - if (otherItem !== item) { - v8Util.setHiddenValue(otherItem, 'checked', false) - } - } - return v8Util.setHiddenValue(item, 'checked', true) - } - }) - this.insertRadioItem(pos, item.commandId, item.label, item.groupId) - } - if (item.sublabel != null) { - this.setSublabel(pos, item.sublabel) - } - if (item.icon != null) { - this.setIcon(pos, item.icon) - } - if (item.role != null) { - this.setRole(pos, item.role) - } - - // Make menu accessable to items. - item.overrideReadOnlyProperty('menu', this) - - // Remember the items. - this.items.splice(pos, 0, item) - this.commandsMap[item.commandId] = item - } - _callMenuWillShow () { - if (this.delegate != null) { - this.delegate.menuWillShow() - } - this.items.forEach(function (item) { - if (item.submenu != null) { - item.submenu._callMenuWillShow() - } - }) - } - static setApplicationMenu (menu) { - if (!(menu === null || menu.constructor === Menu)) { - throw new TypeError('Invalid menu') - } - - // Keep a reference. - applicationMenu = menu - if (process.platform === 'darwin') { - if (menu === null) { - return - } - menu._callMenuWillShow() - bindings.setApplicationMenu(menu) - } else { - BrowserWindow.getAllWindows().forEach(function (window) { - window.setMenu(menu) - }) - } - } - static getApplicationMenu () { - return applicationMenu - } - static buildFromTemplate (template) { - if (!Array.isArray(template)) throw new TypeError('Invalid template for Menu') - - const menu = new Menu() - let positioned = [] - let idx = 0 - - template.forEach((item) => { - idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx += 1 - positioned.splice(idx, 0, item) - }) - - positioned.forEach((item) => { - if (typeof item !== 'object') { - throw new TypeError('Invalid template for MenuItem') - } - menu.append(new MenuItem(item)) - }) - - return menu - } -} - -Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder - -/* HELPER METHODS */ - -// Search between separators to find a radio menu item and return its group id, -function generateGroupId (items, pos) { - var i, item, j, k, ref1, ref2, ref3 +/* Search between seperators to find a radio menu item and return its group id */ +const generateGroupId = function(items, pos) { + let i, item if (pos > 0) { - for (i = j = ref1 = pos - 1; ref1 <= 0 ? j <= 0 : j >= 0; i = ref1 <= 0 ? ++j : --j) { + let asc, start + for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { item = items[i] - if (item.type === 'radio') { - return item.groupId - } - if (item.type === 'separator') { - break - } + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } } } else if (pos < items.length) { - for (i = k = ref2 = pos, ref3 = items.length - 1; ref2 <= ref3 ? k <= ref3 : k >= ref3; i = ref2 <= ref3 ? ++k : --k) { + let asc1, end + for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { item = items[i] - if (item.type === 'radio') { - return item.groupId - } - if (item.type === 'separator') { - break - } + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } } } return ++nextGroupId } -// Returns the index of item according to |id|. -function indexOfItemById (items, id) { - for (let idx = 0; idx < items.length; idx += 1) { - const item = items[idx] - if (item.id === id) return idx +/* Returns the index of item according to |id|. */ +const indexOfItemById = function(items, id) { + for (let i = 0; i < items.length; i++) { + const item = items[i] + if (item.id === id) return i } return -1 } -// Returns the index of where to insert the item according to |position|. -function indexToInsertByPosition (items, pos) { - if (!pos) return items.length +/* Returns the index of where to insert the item according to |position|. */ +const indexToInsertByPosition = function(items, position) { + if (!position) { return items.length } - const [query, id] = pos.split('=') - let idx = indexOfItemById(items, id) - - if (idx === -1 && query !== 'endof') { - console.warn("Item with id '" + id + "' is not found") + const [query, id] = Array.from(position.split('=')) + let insertIndex = indexOfItemById(items, id) + if ((insertIndex === -1) && (query !== 'endof')) { + console.warn(`Item with id '${id}' is not found`) return items.length } - if (query === 'after') { - idx += 1 - } else if (query === 'endof') { - // create new group with id if none exists - if (idx === -1) { - items.push({ - id, - type: 'separator' - }) - idx = items.length - 1 - } - idx += 1 + switch (query) { + case 'after': + insertIndex++ + break + case 'endof': + /* If the |id| doesn't exist, then create a new group with the |id|. */ + if (insertIndex === -1) { + items.push({id, type: 'separator'}) + insertIndex = items.length - 1 + } - // search for end of group - while (idx < items.length && items[idx].type !== 'separator') { - idx += 1 - } + /* Find the end of the group. */ + insertIndex++ + while ((insertIndex < items.length) && (items[insertIndex].type !== 'separator')) { + insertIndex++ + } + break } - return idx + + return insertIndex } -module.exports = Menu +const { Menu } = bindings + +Menu.prototype.__proto__ = EventEmitter.prototype + +Menu.prototype._init = function() { + this.commandsMap = {} + this.groupsMap = {} + this.items = [] + return this.delegate = { + isCommandIdChecked: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].checked : undefined), + isCommandIdEnabled: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].enabled : undefined), + isCommandIdVisible: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].visible : undefined), + getAcceleratorForCommandId: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].accelerator : undefined), + getIconForCommandId: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].icon : undefined), + executeCommand: commandId => { + return (this.commandsMap[commandId] != null ? this.commandsMap[commandId].click(BrowserWindow.getFocusedWindow()) : undefined) + }, + menuWillShow: () => { + for (let id in this.groupsMap) { + const group = this.groupsMap[id] + let checked = false + for (let radioItem in group) { + if (radioItem.checked) { + checked = true + break + } + } + if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) + } + } + } +} + +Menu.prototype.popup = function(window, x, y) { + if ((window != null ? window.constructor : undefined) !== BrowserWindow) { + /* Shift. */ + y = x + x = window + window = BrowserWindow.getFocusedWindow() + } + if (x !== null && y !== null) { + return this._popupAt(window, x, y) + } else { + return this._popup(window) + } +} + +Menu.prototype.append = function(item) { + return this.insert(this.getItemCount(), item) +} + +Menu.prototype.insert = function(pos, item) { + if ((item != null ? item.constructor : undefined) !== MenuItem) { + throw new TypeError('Invalid item') + } + + switch (item.type) { + case 'normal': + this.insertItem(pos, item.commandId, item.label) + break + case 'checkbox': + this.insertCheckItem(pos, item.commandId, item.label) + break + case 'separator': + this.insertSeparator(pos) + break + case 'submenu': + this.insertSubMenu(pos, item.commandId, item.label, item.submenu) + break + case 'radio': + /* Grouping radio menu items. */ + item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) + if (this.groupsMap[item.groupId] == null) this.groupsMap[item.groupId] = [] + this.groupsMap[item.groupId].push(item) + + /* Setting a radio menu item should flip other items in the group. */ + v8Util.setHiddenValue(item, 'checked', item.checked) + Object.defineProperty(item, 'checked', { + enumerable: true, + get() { return v8Util.getHiddenValue(item, 'checked') }, + set: val => { + for (let otherItem in this.groupsMap[item.groupId]) { + if (otherItem !== item) v8Util.setHiddenValue(otherItem, 'checked', false) + } + return v8Util.setHiddenValue(item, 'checked', true) + } + }) + + this.insertRadioItem(pos, item.commandId, item.label, item.groupId) + break + } + + if (item.sublabel != null) this.setSublabel(pos, item.sublabel) + if (item.icon != null) this.setIcon(pos, item.icon) + if (item.role != null) this.setRole(pos, item.role) + + /* Make menu accessable to items. */ + item.overrideReadOnlyProperty('menu', this) + + /* Remember the items. */ + this.items.splice(pos, 0, item) + return this.commandsMap[item.commandId] = item +} + +/* Force menuWillShow to be called */ +Menu.prototype._callMenuWillShow = function() { + if (this.delegate != null) this.delegate.menuWillShow() + + return (() => { + const result = [] + for (let item of Array.from(this.items)) { + if (item.submenu != null) { + result.push(item.submenu._callMenuWillShow()) + } + } + return result + })() +} + +let applicationMenu = null +Menu.setApplicationMenu = function(menu) { + if ((menu !== null) && (menu.constructor !== Menu)) { throw new TypeError('Invalid menu') } + /* Keep a reference. */ + applicationMenu = menu + + if (process.platform === 'darwin') { + if (menu === null) { return } + menu._callMenuWillShow() + return bindings.setApplicationMenu(menu) + } else { + const windows = BrowserWindow.getAllWindows() + return Array.from(windows).map((w) => w.setMenu(menu)) + } +} + +Menu.getApplicationMenu = () => applicationMenu + +Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder + +Menu.buildFromTemplate = function(template) { + if (!Array.isArray(template)) { throw new TypeError('Invalid template for Menu') } + + const positionedTemplate = [] + let insertIndex = 0 + + for (var item of Array.from(template)) { + if (item.position) { + insertIndex = indexToInsertByPosition(positionedTemplate, item.position) + } else { + /* If no |position| is specified, insert after last item. */ + insertIndex++ + } + positionedTemplate.splice(insertIndex, 0, item) + } + + const menu = new Menu + + for (item of Array.from(positionedTemplate)) { + if (typeof item !== 'object') { throw new TypeError('Invalid template for MenuItem') } + + const menuItem = new MenuItem(item) + for (let key in item) { + const value = item[key] + if (menuItem[key] == null) { + menuItem[key] = value + } + } + menu.append(menuItem) + } + + return menu +} + +module.exports = Menu \ No newline at end of file From 3fc5d51a96557974db9349565ae18fd20e75b371 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 18:35:16 -0400 Subject: [PATCH 07/21] clean up delegate --- lib/browser/api/menu.js | 272 ++++++++++++++++++++++------------------ 1 file changed, 147 insertions(+), 125 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 75b5a31e3b01..71ac7f5241de 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -5,88 +5,23 @@ const {EventEmitter} = require('events') const v8Util = process.atomBinding('v8_util') const bindings = process.atomBinding('menu') -// Automatically generated radio menu item's group id. -let nextGroupId = 0 - -/* Search between seperators to find a radio menu item and return its group id */ -const generateGroupId = function(items, pos) { - let i, item - if (pos > 0) { - let asc, start - for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { - item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } - } - } else if (pos < items.length) { - let asc1, end - for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { - item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } - } - } - return ++nextGroupId -} - -/* Returns the index of item according to |id|. */ -const indexOfItemById = function(items, id) { - for (let i = 0; i < items.length; i++) { - const item = items[i] - if (item.id === id) return i - } - return -1 -} - -/* Returns the index of where to insert the item according to |position|. */ -const indexToInsertByPosition = function(items, position) { - if (!position) { return items.length } - - const [query, id] = Array.from(position.split('=')) - let insertIndex = indexOfItemById(items, id) - if ((insertIndex === -1) && (query !== 'endof')) { - console.warn(`Item with id '${id}' is not found`) - return items.length - } - - switch (query) { - case 'after': - insertIndex++ - break - case 'endof': - /* If the |id| doesn't exist, then create a new group with the |id|. */ - if (insertIndex === -1) { - items.push({id, type: 'separator'}) - insertIndex = items.length - 1 - } - - /* Find the end of the group. */ - insertIndex++ - while ((insertIndex < items.length) && (items[insertIndex].type !== 'separator')) { - insertIndex++ - } - break - } - - return insertIndex -} - const { Menu } = bindings Menu.prototype.__proto__ = EventEmitter.prototype -Menu.prototype._init = function() { +Menu.prototype._init = function () { this.commandsMap = {} this.groupsMap = {} this.items = [] return this.delegate = { - isCommandIdChecked: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].checked : undefined), - isCommandIdEnabled: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].enabled : undefined), - isCommandIdVisible: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].visible : undefined), - getAcceleratorForCommandId: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].accelerator : undefined), - getIconForCommandId: commandId => (this.commandsMap[commandId] != null ? this.commandsMap[commandId].icon : undefined), - executeCommand: commandId => { - return (this.commandsMap[commandId] != null ? this.commandsMap[commandId].click(BrowserWindow.getFocusedWindow()) : undefined) + isCommandIdChecked: id => (this.commandsMap[id] ? this.commandsMap[id].checked : undefined), + isCommandIdEnabled: id => (this.commandsMap[id] ? this.commandsMap[id].enabled : undefined), + isCommandIdVisible: id => (this.commandsMap[id] ? this.commandsMap[id].visible : undefined), + getAcceleratorForCommandId: id => (this.commandsMap[id] ? this.commandsMap[id].accelerator : undefined), + getIconForCommandId: id => (this.commandsMap[id] ? this.commandsMap[id].icon : undefined), + executeCommand: (id) => { + const command = this.commandsMap[id] + return command ? this.commandsMap[id].click(BrowserWindow.getFocusedWindow()) : undefined }, menuWillShow: () => { for (let id in this.groupsMap) { @@ -104,25 +39,41 @@ Menu.prototype._init = function() { } } -Menu.prototype.popup = function(window, x, y) { - if ((window != null ? window.constructor : undefined) !== BrowserWindow) { - /* Shift. */ +Menu.prototype.popup = function (window, x, y, positioningItem) { + let asyncPopup + + // menu.popup(x, y, positioningItem) + if (window != null && (typeof window !== 'object' || window.constructor !== BrowserWindow)) { + // Shift. + positioningItem = y y = x x = window - window = BrowserWindow.getFocusedWindow() + window = null } - if (x !== null && y !== null) { - return this._popupAt(window, x, y) - } else { - return this._popup(window) + + // menu.popup(window, {}) + if (x != null && typeof x === 'object') { + const options = x + x = options.x + y = options.y + positioningItem = options.positioningItem + asyncPopup = options.async } + + if (window == null) window = BrowserWindow.getFocusedWindow() + if (typeof x !== 'number') x = -1 + if (typeof y !== 'number') y = -1 + if (typeof positioningItem !== 'number') positioningItem = -1 + if (typeof asyncPopup !== 'boolean') asyncPopup = false + + this.popupAt(window, x, y, positioningItem, asyncPopup) } -Menu.prototype.append = function(item) { +Menu.prototype.append = function (item) { return this.insert(this.getItemCount(), item) } -Menu.prototype.insert = function(pos, item) { +Menu.prototype.insert = function (pos, item) { if ((item != null ? item.constructor : undefined) !== MenuItem) { throw new TypeError('Invalid item') } @@ -176,33 +127,39 @@ Menu.prototype.insert = function(pos, item) { } /* Force menuWillShow to be called */ -Menu.prototype._callMenuWillShow = function() { +Menu.prototype._callMenuWillShow = function () { if (this.delegate != null) this.delegate.menuWillShow() - return (() => { - const result = [] - for (let item of Array.from(this.items)) { - if (item.submenu != null) { - result.push(item.submenu._callMenuWillShow()) - } - } - return result - })() + this.items.forEach(item => { + if (item.submenu != null) item.submenu._callMenuWillShow() + }) } -let applicationMenu = null -Menu.setApplicationMenu = function(menu) { - if ((menu !== null) && (menu.constructor !== Menu)) { throw new TypeError('Invalid menu') } - /* Keep a reference. */ - applicationMenu = menu +Menu.prototype.getMenuItemById = function (id) { + const items = this.items + let found = items.find(item => item.id === id) || null + for (let i = 0, length = items.length; !found && i < length; i++) { + if (items[i].submenu) { + found = items[i].submenu.getMenuItemById(id) + } + } + return found +} + +Menu.setApplicationMenu = function (menu) { + if (menu !== null && menu.constructor !== Menu) { + throw new TypeError('Invalid menu') + } + + applicationMenu = menu if (process.platform === 'darwin') { - if (menu === null) { return } + if (menu === null) return menu._callMenuWillShow() return bindings.setApplicationMenu(menu) } else { const windows = BrowserWindow.getAllWindows() - return Array.from(windows).map((w) => w.setMenu(menu)) + return windows.map((w) => w.setMenu(menu)) } } @@ -210,38 +167,103 @@ Menu.getApplicationMenu = () => applicationMenu Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder -Menu.buildFromTemplate = function(template) { - if (!Array.isArray(template)) { throw new TypeError('Invalid template for Menu') } - - const positionedTemplate = [] - let insertIndex = 0 - - for (var item of Array.from(template)) { - if (item.position) { - insertIndex = indexToInsertByPosition(positionedTemplate, item.position) - } else { - /* If no |position| is specified, insert after last item. */ - insertIndex++ - } - positionedTemplate.splice(insertIndex, 0, item) +// build menu from a template +Menu.buildFromTemplate = function (template) { + if (!(template instanceof Array)) { + throw new TypeError('Invalid template for Menu') } const menu = new Menu + const positioned = [] + let idx = 0 - for (item of Array.from(positionedTemplate)) { - if (typeof item !== 'object') { throw new TypeError('Invalid template for MenuItem') } + template.forEach(item => { + idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx++ + positioned.splice(idx, 0, item) + }) - const menuItem = new MenuItem(item) - for (let key in item) { - const value = item[key] - if (menuItem[key] == null) { - menuItem[key] = value - } + positioned.forEach((item) => { + if (typeof item !== 'object') { + throw new TypeError('Invalid template for MenuItem') } - menu.append(menuItem) - } + menu.append(new MenuItem(item)) + }) return menu } +// Automatically generated radio menu item's group id. +let nextGroupId = 0 +let applicationMenu = null + +/* Search between seperators to find a radio menu item and return its group id */ +const generateGroupId = function(items, pos) { + let i, item + if (pos > 0) { + let asc, start + for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { + item = items[i] + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } + } + } else if (pos < items.length) { + let asc1, end + for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { + item = items[i] + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } + } + } + return ++nextGroupId +} + +/* Returns the index of item according to |id|. */ +const indexOfItemById = function (items, id) { + for (let i = 0; i < items.length; i++) { + const item = items[i] + if (item.id === id) return i + } + return -1 +} + +/* Returns the index of where to insert the item according to |position|. */ +function indexToInsertByPosition (items, position) { + if (!position) return items.length + const [query, id] = position.split('=') + let idx = indexOfItemById(items, id) + + if (idx === -1 && query !== 'endof') { + console.warn(`Item with id '${id}' is not found`) + return items.length + } + + const queries = { + 'after': () => idx++, + 'endof': () => { + if (idx === -1) { + items.push({id, type: 'separator'}) + idx = items.length - 1 + } + + idx++ + while (idx < items.length && items[idx].type !== 'separator') { + idx++ + } + } + } + + queries[query]() + return idx +} + +// WIP, need to figure out how to pass current menu group to this method +function isOuterSeparator (current, item) { + current.filter(item => { !item.visible }) + const idx = current.indexof(item) + if (idx === 0 || idx === current.length) { + return true + } + return false +} + module.exports = Menu \ No newline at end of file From 9038987e1ddc6991306b78f7e7dab8798a9f8960 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 20:04:22 -0400 Subject: [PATCH 08/21] significant cleanup; all tests still passing --- lib/browser/api/menu.js | 319 +++++++++++++++++++++++----------------- 1 file changed, 186 insertions(+), 133 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 71ac7f5241de..fbafd1d22d10 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -1,39 +1,66 @@ -const {BrowserWindow, MenuItem} = require('electron') -const {EventEmitter} = require('events') -//const _ = require('lodash') +'use strict' +const {BrowserWindow, MenuItem, webContents} = require('electron') +const EventEmitter = require('events').EventEmitter const v8Util = process.atomBinding('v8_util') const bindings = process.atomBinding('menu') -const { Menu } = bindings +const {Menu} = bindings +let applicationMenu = null +let nextGroupId = 0 -Menu.prototype.__proto__ = EventEmitter.prototype +Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype) Menu.prototype._init = function () { this.commandsMap = {} this.groupsMap = {} this.items = [] - return this.delegate = { - isCommandIdChecked: id => (this.commandsMap[id] ? this.commandsMap[id].checked : undefined), - isCommandIdEnabled: id => (this.commandsMap[id] ? this.commandsMap[id].enabled : undefined), - isCommandIdVisible: id => (this.commandsMap[id] ? this.commandsMap[id].visible : undefined), - getAcceleratorForCommandId: id => (this.commandsMap[id] ? this.commandsMap[id].accelerator : undefined), - getIconForCommandId: id => (this.commandsMap[id] ? this.commandsMap[id].icon : undefined), - executeCommand: (id) => { - const command = this.commandsMap[id] - return command ? this.commandsMap[id].click(BrowserWindow.getFocusedWindow()) : undefined + this.delegate = { + isCommandIdChecked: (commandId) => { + var command = this.commandsMap[commandId] + return command != null ? command.checked : undefined + }, + isCommandIdEnabled: (commandId) => { + var command = this.commandsMap[commandId] + return command != null ? command.enabled : undefined + }, + isCommandIdVisible: (commandId) => { + var command = this.commandsMap[commandId] + return command != null ? command.visible : undefined + }, + getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { + const command = this.commandsMap[commandId] + if (command == null) return + if (command.accelerator != null) return command.accelerator + if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() + }, + getIconForCommandId: (commandId) => { + var command = this.commandsMap[commandId] + return command != null ? command.icon : undefined + }, + executeCommand: (event, commandId) => { + const command = this.commandsMap[commandId] + if (command == null) return + command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) }, menuWillShow: () => { - for (let id in this.groupsMap) { - const group = this.groupsMap[id] - let checked = false - for (let radioItem in group) { - if (radioItem.checked) { - checked = true - break + // Make sure radio groups have at least one menu item seleted. + var checked, group, id, j, len, radioItem, ref1 + ref1 = this.groupsMap + for (id in ref1) { + group = ref1[id] + checked = false + for (j = 0, len = group.length; j < len; j++) { + radioItem = group[j] + if (!radioItem.checked) { + continue } + checked = true + break + } + if (!checked) { + v8Util.setHiddenValue(group[0], 'checked', true) } - if (!checked) v8Util.setHiddenValue(group[0], 'checked', true) } } } @@ -60,79 +87,29 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { asyncPopup = options.async } + // Default to showing in focused window. if (window == null) window = BrowserWindow.getFocusedWindow() + + // Default to showing under mouse location. if (typeof x !== 'number') x = -1 if (typeof y !== 'number') y = -1 + + // Default to not highlighting any item. if (typeof positioningItem !== 'number') positioningItem = -1 + + // Default to synchronous for backwards compatibility. if (typeof asyncPopup !== 'boolean') asyncPopup = false this.popupAt(window, x, y, positioningItem, asyncPopup) } -Menu.prototype.append = function (item) { - return this.insert(this.getItemCount(), item) -} - -Menu.prototype.insert = function (pos, item) { - if ((item != null ? item.constructor : undefined) !== MenuItem) { - throw new TypeError('Invalid item') +Menu.prototype.closePopup = function (window) { + if (window == null || window.constructor !== BrowserWindow) { + window = BrowserWindow.getFocusedWindow() } - - switch (item.type) { - case 'normal': - this.insertItem(pos, item.commandId, item.label) - break - case 'checkbox': - this.insertCheckItem(pos, item.commandId, item.label) - break - case 'separator': - this.insertSeparator(pos) - break - case 'submenu': - this.insertSubMenu(pos, item.commandId, item.label, item.submenu) - break - case 'radio': - /* Grouping radio menu items. */ - item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) - if (this.groupsMap[item.groupId] == null) this.groupsMap[item.groupId] = [] - this.groupsMap[item.groupId].push(item) - - /* Setting a radio menu item should flip other items in the group. */ - v8Util.setHiddenValue(item, 'checked', item.checked) - Object.defineProperty(item, 'checked', { - enumerable: true, - get() { return v8Util.getHiddenValue(item, 'checked') }, - set: val => { - for (let otherItem in this.groupsMap[item.groupId]) { - if (otherItem !== item) v8Util.setHiddenValue(otherItem, 'checked', false) - } - return v8Util.setHiddenValue(item, 'checked', true) - } - }) - - this.insertRadioItem(pos, item.commandId, item.label, item.groupId) - break + if (window != null) { + this.closePopupAt(window.id) } - - if (item.sublabel != null) this.setSublabel(pos, item.sublabel) - if (item.icon != null) this.setIcon(pos, item.icon) - if (item.role != null) this.setRole(pos, item.role) - - /* Make menu accessable to items. */ - item.overrideReadOnlyProperty('menu', this) - - /* Remember the items. */ - this.items.splice(pos, 0, item) - return this.commandsMap[item.commandId] = item -} - -/* Force menuWillShow to be called */ -Menu.prototype._callMenuWillShow = function () { - if (this.delegate != null) this.delegate.menuWillShow() - - this.items.forEach(item => { - if (item.submenu != null) item.submenu._callMenuWillShow() - }) } Menu.prototype.getMenuItemById = function (id) { @@ -147,8 +124,83 @@ Menu.prototype.getMenuItemById = function (id) { return found } +//cleaned up +Menu.prototype.append = function (item) { + return this.insert(this.getItemCount(), item) +} + +Menu.prototype.insert = function (pos, item) { + var base, name + if ((item != null ? item.constructor : void 0) !== MenuItem) { + throw new TypeError('Invalid item') + } + switch (item.type) { + case 'normal': + this.insertItem(pos, item.commandId, item.label) + break + case 'checkbox': + this.insertCheckItem(pos, item.commandId, item.label) + break + case 'separator': + this.insertSeparator(pos) + break + case 'submenu': + this.insertSubMenu(pos, item.commandId, item.label, item.submenu) + break + case 'radio': + // Grouping radio menu items. + item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) + if ((base = this.groupsMap)[name = item.groupId] == null) { + base[name] = [] + } + this.groupsMap[item.groupId].push(item) + + // Setting a radio menu item should flip other items in the group. + v8Util.setHiddenValue(item, 'checked', item.checked) + Object.defineProperty(item, 'checked', { + enumerable: true, + get: function () { + return v8Util.getHiddenValue(item, 'checked') + }, + set: () => { + var j, len, otherItem, ref1 + ref1 = this.groupsMap[item.groupId] + for (j = 0, len = ref1.length; j < len; j++) { + otherItem = ref1[j] + if (otherItem !== item) { + v8Util.setHiddenValue(otherItem, 'checked', false) + } + } + return v8Util.setHiddenValue(item, 'checked', true) + } + }) + this.insertRadioItem(pos, item.commandId, item.label, item.groupId) + } + + if (item.sublabel != null) this.setSublabel(pos, item.sublabel) + if (item.icon != null) this.setIcon(pos, item.icon) + if (item.role != null) this.setRole(pos, item.role) + + // Make menu accessable to items. + item.overrideReadOnlyProperty('menu', this) + + // Remember the items. + this.items.splice(pos, 0, item) + this.commandsMap[item.commandId] = item +} + +// Force menuWillShow to be called +// cleaned up +Menu.prototype._callMenuWillShow = function () { + if (this.delegate != null) this.delegate.menuWillShow() + this.items.forEach(item => { + if (item.submenu != null) item.submenu._callMenuWillShow() + }) +} + +// cleaned up Menu.setApplicationMenu = function (menu) { - if (menu !== null && menu.constructor !== Menu) { + if (!(menu === null || menu.constructor === Menu)) { throw new TypeError('Invalid menu') } @@ -156,32 +208,35 @@ Menu.setApplicationMenu = function (menu) { if (process.platform === 'darwin') { if (menu === null) return menu._callMenuWillShow() - return bindings.setApplicationMenu(menu) + bindings.setApplicationMenu(menu) } else { const windows = BrowserWindow.getAllWindows() - return windows.map((w) => w.setMenu(menu)) + return windows.map(w => w.setMenu(menu)) } } +// cleaned up Menu.getApplicationMenu = () => applicationMenu Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder -// build menu from a template +// cleaned up Menu.buildFromTemplate = function (template) { if (!(template instanceof Array)) { throw new TypeError('Invalid template for Menu') } - const menu = new Menu + const menu = new Menu() const positioned = [] let idx = 0 + // sort template by position template.forEach(item => { - idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx++ + idx = (item.position) ? indexToInsertByPosition(positioned, item.position) : idx += 1 positioned.splice(idx, 0, item) }) + // add each item from positioned menu to application menu positioned.forEach((item) => { if (typeof item !== 'object') { throw new TypeError('Invalid template for MenuItem') @@ -192,78 +247,76 @@ Menu.buildFromTemplate = function (template) { return menu } -// Automatically generated radio menu item's group id. -let nextGroupId = 0 -let applicationMenu = null +/* Helper Methods */ -/* Search between seperators to find a radio menu item and return its group id */ -const generateGroupId = function(items, pos) { - let i, item +// Search between separators to find a radio menu item and return its group id, +function generateGroupId (items, pos) { + var i, item, j, k, ref1, ref2, ref3 if (pos > 0) { - let asc, start - for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { + for (i = j = ref1 = pos - 1; ref1 <= 0 ? j <= 0 : j >= 0; i = ref1 <= 0 ? ++j : --j) { item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } + if (item.type === 'radio') { + return item.groupId + } + if (item.type === 'separator') { + break + } } } else if (pos < items.length) { - let asc1, end - for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { + for (i = k = ref2 = pos, ref3 = items.length - 1; ref2 <= ref3 ? k <= ref3 : k >= ref3; i = ref2 <= ref3 ? ++k : --k) { item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } + if (item.type === 'radio') { + return item.groupId + } + if (item.type === 'separator') { + break + } } } return ++nextGroupId } -/* Returns the index of item according to |id|. */ -const indexOfItemById = function (items, id) { - for (let i = 0; i < items.length; i++) { - const item = items[i] - if (item.id === id) return i - } - return -1 +// Returns the index of item according to |id|. +// cleaned up +function indexOfItemById (items, id) { + const foundItem = items.find(item => item.id === id) || null + return items.indexOf(foundItem) } -/* Returns the index of where to insert the item according to |position|. */ +// Returns the index of where to insert the item according to |position| function indexToInsertByPosition (items, position) { if (!position) return items.length - const [query, id] = position.split('=') - let idx = indexOfItemById(items, id) + const [query, id] = position.split('=') // parse query and id from position + const idx = indexOfItemById(items, id) // calculate initial index of item + + // warn if query doesn't exist if (idx === -1 && query !== 'endof') { - console.warn(`Item with id '${id}' is not found`) + console.warn("Item with id '" + id + "' is not found") return items.length } + // compute new index based on query const queries = { - 'after': () => idx++, - 'endof': () => { - if (idx === -1) { + 'after': (index) => index += 1, + 'endof': (index) => { + if (index === -1) { items.push({id, type: 'separator'}) - idx = items.length - 1 + index = items.length - 1 } - idx++ - while (idx < items.length && items[idx].type !== 'separator') { - idx++ - } + index += 1 + while (index < items.length && items[index].type !== 'separator') index += 1 + return index } } - queries[query]() - return idx + // return new index if needed, or original indexOfItemById + return (query in queries) ? queries[query](idx) : idx } -// WIP, need to figure out how to pass current menu group to this method -function isOuterSeparator (current, item) { - current.filter(item => { !item.visible }) - const idx = current.indexof(item) - if (idx === 0 || idx === current.length) { - return true - } - return false +function computeNewIndexOnQuery(idx, query) { + } module.exports = Menu \ No newline at end of file From 9b364d5be30c24e998bc2acb22f1812838f1ac06 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 22:22:39 -0400 Subject: [PATCH 09/21] refactor menuWillShow --- lib/browser/api/menu.js | 54 ++++++++++++----------------------------- 1 file changed, 16 insertions(+), 38 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index fbafd1d22d10..9877d81bb9b6 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -7,7 +7,7 @@ const bindings = process.atomBinding('menu') const {Menu} = bindings let applicationMenu = null -let nextGroupId = 0 +let groupIdIndex = 0 Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype) @@ -44,23 +44,10 @@ Menu.prototype._init = function () { command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) }, menuWillShow: () => { - // Make sure radio groups have at least one menu item seleted. - var checked, group, id, j, len, radioItem, ref1 - ref1 = this.groupsMap - for (id in ref1) { - group = ref1[id] - checked = false - for (j = 0, len = group.length; j < len; j++) { - radioItem = group[j] - if (!radioItem.checked) { - continue - } - checked = true - break - } - if (!checked) { - v8Util.setHiddenValue(group[0], 'checked', true) - } + // Ensure radio groups have at least one menu item seleted + for (let id in this.groupsMap) { + let found = this.groupsMap[id].find(item => item.checked) || null + if (!found) v8Util.setHiddenValue(this.groupsMap[id][0], 'checked', true) } } } @@ -251,29 +238,23 @@ Menu.buildFromTemplate = function (template) { // Search between separators to find a radio menu item and return its group id, function generateGroupId (items, pos) { - var i, item, j, k, ref1, ref2, ref3 + let i, item if (pos > 0) { - for (i = j = ref1 = pos - 1; ref1 <= 0 ? j <= 0 : j >= 0; i = ref1 <= 0 ? ++j : --j) { + let asc, start + for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { item = items[i] - if (item.type === 'radio') { - return item.groupId - } - if (item.type === 'separator') { - break - } + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } } } else if (pos < items.length) { - for (i = k = ref2 = pos, ref3 = items.length - 1; ref2 <= ref3 ? k <= ref3 : k >= ref3; i = ref2 <= ref3 ? ++k : --k) { + let asc1, end + for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { item = items[i] - if (item.type === 'radio') { - return item.groupId - } - if (item.type === 'separator') { - break - } + if (item.type === 'radio') { return item.groupId } + if (item.type === 'separator') { break } } } - return ++nextGroupId + return ++groupIdIndex } // Returns the index of item according to |id|. @@ -284,6 +265,7 @@ function indexOfItemById (items, id) { } // Returns the index of where to insert the item according to |position| +// cleaned up function indexToInsertByPosition (items, position) { if (!position) return items.length @@ -315,8 +297,4 @@ function indexToInsertByPosition (items, position) { return (query in queries) ? queries[query](idx) : idx } -function computeNewIndexOnQuery(idx, query) { - -} - module.exports = Menu \ No newline at end of file From 508b6147694ff2706fb6f016a2a064971f46d3b6 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 22:33:23 -0400 Subject: [PATCH 10/21] remove excess code in delegate --- lib/browser/api/menu.js | 34 +++++++++++----------------------- 1 file changed, 11 insertions(+), 23 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 9877d81bb9b6..b123c8e46bbd 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -16,31 +16,19 @@ Menu.prototype._init = function () { this.groupsMap = {} this.items = [] this.delegate = { - isCommandIdChecked: (commandId) => { - var command = this.commandsMap[commandId] - return command != null ? command.checked : undefined - }, - isCommandIdEnabled: (commandId) => { - var command = this.commandsMap[commandId] - return command != null ? command.enabled : undefined - }, - isCommandIdVisible: (commandId) => { - var command = this.commandsMap[commandId] - return command != null ? command.visible : undefined - }, - getAcceleratorForCommandId: (commandId, useDefaultAccelerator) => { - const command = this.commandsMap[commandId] - if (command == null) return - if (command.accelerator != null) return command.accelerator + isCommandIdChecked: id => this.commandsMap[id].checked, + isCommandIdEnabled: id => this.commandsMap[id].enabled, + isCommandIdVisible: id => this.commandsMap[id].visible, + getAcceleratorForCommandId: (id, useDefaultAccelerator) => { + const command = this.commandsMap[id] + if (command === null) return + if (command.accelerator !== null) return command.accelerator if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() }, - getIconForCommandId: (commandId) => { - var command = this.commandsMap[commandId] - return command != null ? command.icon : undefined - }, - executeCommand: (event, commandId) => { - const command = this.commandsMap[commandId] - if (command == null) return + getIconForCommandId: id => this.commandsMap[id].icon, + executeCommand: (event, id) => { + const command = this.commandsMap[id] + if (command === null) return command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) }, menuWillShow: () => { From 7c0f7329d90583b2f743fd05240bfed90d5583c4 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 22:35:42 -0400 Subject: [PATCH 11/21] appease the linter overlords --- lib/browser/api/menu.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index b123c8e46bbd..bd89ca319b95 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -99,7 +99,6 @@ Menu.prototype.getMenuItemById = function (id) { return found } -//cleaned up Menu.prototype.append = function (item) { return this.insert(this.getItemCount(), item) } @@ -268,7 +267,10 @@ function indexToInsertByPosition (items, position) { // compute new index based on query const queries = { - 'after': (index) => index += 1, + 'after': (index) => { + index += 1 + return index + }, 'endof': (index) => { if (index === -1) { items.push({id, type: 'separator'}) @@ -285,4 +287,4 @@ function indexToInsertByPosition (items, position) { return (query in queries) ? queries[query](idx) : idx } -module.exports = Menu \ No newline at end of file +module.exports = Menu From b1e707d53534474e73b446e731ad8dfc17518664 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 23 Oct 2017 23:46:39 -0400 Subject: [PATCH 12/21] abstract out switch case from Menu.prototype.insert --- lib/browser/api/menu.js | 108 ++++++++++++++++++++-------------------- 1 file changed, 53 insertions(+), 55 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index bd89ca319b95..77d881b5d30b 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -11,6 +11,7 @@ let groupIdIndex = 0 Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype) +// cleaned up Menu.prototype._init = function () { this.commandsMap = {} this.groupsMap = {} @@ -41,6 +42,7 @@ Menu.prototype._init = function () { } } +// create and show a popup Menu.prototype.popup = function (window, x, y, positioningItem) { let asyncPopup @@ -78,6 +80,7 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { this.popupAt(window, x, y, positioningItem, asyncPopup) } +// close an existing popup Menu.prototype.closePopup = function (window) { if (window == null || window.constructor !== BrowserWindow) { window = BrowserWindow.getFocusedWindow() @@ -87,11 +90,12 @@ Menu.prototype.closePopup = function (window) { } } +// return a MenuItem with the specified id Menu.prototype.getMenuItemById = function (id) { const items = this.items let found = items.find(item => item.id === id) || null - for (let i = 0, length = items.length; !found && i < length; i++) { + for (let i = 0; !found && i < items.length; i++) { if (items[i].submenu) { found = items[i].submenu.getMenuItemById(id) } @@ -99,58 +103,21 @@ Menu.prototype.getMenuItemById = function (id) { return found } +// append a new MenuItem to an existing Menu Menu.prototype.append = function (item) { return this.insert(this.getItemCount(), item) } +// insert a new menu item at a specified location Menu.prototype.insert = function (pos, item) { - var base, name if ((item != null ? item.constructor : void 0) !== MenuItem) { throw new TypeError('Invalid item') } - switch (item.type) { - case 'normal': - this.insertItem(pos, item.commandId, item.label) - break - case 'checkbox': - this.insertCheckItem(pos, item.commandId, item.label) - break - case 'separator': - this.insertSeparator(pos) - break - case 'submenu': - this.insertSubMenu(pos, item.commandId, item.label, item.submenu) - break - case 'radio': - // Grouping radio menu items. - item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) - if ((base = this.groupsMap)[name = item.groupId] == null) { - base[name] = [] - } - this.groupsMap[item.groupId].push(item) - // Setting a radio menu item should flip other items in the group. - v8Util.setHiddenValue(item, 'checked', item.checked) - Object.defineProperty(item, 'checked', { - enumerable: true, - get: function () { - return v8Util.getHiddenValue(item, 'checked') - }, - set: () => { - var j, len, otherItem, ref1 - ref1 = this.groupsMap[item.groupId] - for (j = 0, len = ref1.length; j < len; j++) { - otherItem = ref1[j] - if (otherItem !== item) { - v8Util.setHiddenValue(otherItem, 'checked', false) - } - } - return v8Util.setHiddenValue(item, 'checked', true) - } - }) - this.insertRadioItem(pos, item.commandId, item.label, item.groupId) - } + // insert item depending on its type + insertItemByType.call(this, item, pos) + // set item properties if (item.sublabel != null) this.setSublabel(pos, item.sublabel) if (item.icon != null) this.setIcon(pos, item.icon) if (item.role != null) this.setRole(pos, item.role) @@ -164,7 +131,6 @@ Menu.prototype.insert = function (pos, item) { } // Force menuWillShow to be called -// cleaned up Menu.prototype._callMenuWillShow = function () { if (this.delegate != null) this.delegate.menuWillShow() this.items.forEach(item => { @@ -172,7 +138,12 @@ Menu.prototype._callMenuWillShow = function () { }) } -// cleaned up +// return application menu +Menu.getApplicationMenu = () => applicationMenu + +Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder + +// set application menu with a preexisting menu Menu.setApplicationMenu = function (menu) { if (!(menu === null || menu.constructor === Menu)) { throw new TypeError('Invalid menu') @@ -189,12 +160,7 @@ Menu.setApplicationMenu = function (menu) { } } -// cleaned up -Menu.getApplicationMenu = () => applicationMenu - -Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder - -// cleaned up +// build a menu by passing in a template Menu.buildFromTemplate = function (template) { if (!(template instanceof Array)) { throw new TypeError('Invalid template for Menu') @@ -223,7 +189,7 @@ Menu.buildFromTemplate = function (template) { /* Helper Methods */ -// Search between separators to find a radio menu item and return its group id, +// Search between separators to find a radio menu item and return its group id function generateGroupId (items, pos) { let i, item if (pos > 0) { @@ -241,18 +207,17 @@ function generateGroupId (items, pos) { if (item.type === 'separator') { break } } } - return ++groupIdIndex + groupIdIndex += 1 + return groupIdIndex } // Returns the index of item according to |id|. -// cleaned up function indexOfItemById (items, id) { const foundItem = items.find(item => item.id === id) || null return items.indexOf(foundItem) } // Returns the index of where to insert the item according to |position| -// cleaned up function indexToInsertByPosition (items, position) { if (!position) return items.length @@ -287,4 +252,37 @@ function indexToInsertByPosition (items, position) { return (query in queries) ? queries[query](idx) : idx } +// insert a new MenuItem depending on its type +function insertItemByType(item, pos) { + const types = { + 'normal': () => this.insertItem(pos, item.commandId, item.label), + 'checkbox': () => this.insertCheckItem(pos, item.commandId, item.label), + 'separator':() => this.insertSeparator(pos), + 'submenu': () => this.insertSubMenu(pos, item.commandId, item.label, item.submenu), + 'radio': () => { + // Grouping radio menu items + item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) + if (this.groupsMap[item.groupId] == null) { + this.groupsMap[item.groupId] = [] + } + this.groupsMap[item.groupId].push(item) + + // Setting a radio menu item should flip other items in the group. + v8Util.setHiddenValue(item, 'checked', item.checked) + Object.defineProperty(item, 'checked', { + enumerable: true, + get: () => v8Util.getHiddenValue(item, 'checked'), + set: () => { + this.groupsMap[item.groupId].forEach(other => { + if (other !== item) v8Util.setHiddenValue(other, 'checked', false) + }) + v8Util.setHiddenValue(item, 'checked', true) + } + }) + this.insertRadioItem(pos, item.commandId, item.label, item.groupId) + } + } + types[item.type]() +} + module.exports = Menu From f93121b226d824c20fa5af0015afff9231cc73db Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 00:07:39 -0400 Subject: [PATCH 13/21] don't reassign parameters in Menu.prototype.popup --- lib/browser/api/menu.js | 53 +++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 28 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 77d881b5d30b..82ecb08dab09 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -11,7 +11,8 @@ let groupIdIndex = 0 Object.setPrototypeOf(Menu.prototype, EventEmitter.prototype) -// cleaned up +/* Instance Methods */ + Menu.prototype._init = function () { this.commandsMap = {} this.groupsMap = {} @@ -45,39 +46,33 @@ Menu.prototype._init = function () { // create and show a popup Menu.prototype.popup = function (window, x, y, positioningItem) { let asyncPopup + let [newX, newY, newPosition, newWindow] = [x, y, positioningItem, window] // menu.popup(x, y, positioningItem) - if (window != null && (typeof window !== 'object' || window.constructor !== BrowserWindow)) { - // Shift. - positioningItem = y - y = x - x = window - window = null + if (window !== null) { + // shift over values + if (typeof window !== 'object' || window.constructor !== BrowserWindow) { + [newPosition, newY, newX, newWindow] = [y, x, window, null] + } } // menu.popup(window, {}) - if (x != null && typeof x === 'object') { - const options = x - x = options.x - y = options.y - positioningItem = options.positioningItem - asyncPopup = options.async + if (x !== null && typeof x === 'object') { + const opts = x + newX = opts.x + newY = opts.y + newPosition = opts.positioningItem + asyncPopup = opts.async } - // Default to showing in focused window. - if (window == null) window = BrowserWindow.getFocusedWindow() - - // Default to showing under mouse location. - if (typeof x !== 'number') x = -1 - if (typeof y !== 'number') y = -1 - - // Default to not highlighting any item. - if (typeof positioningItem !== 'number') positioningItem = -1 - - // Default to synchronous for backwards compatibility. + // set defaults + if (typeof x !== 'number') newX = -1 + if (typeof y !== 'number') newY = -1 + if (typeof positioningItem !== 'number') newPosition = -1 + if (window === null) newWindow = BrowserWindow.getFocusedWindow() if (typeof asyncPopup !== 'boolean') asyncPopup = false - this.popupAt(window, x, y, positioningItem, asyncPopup) + this.popupAt(newWindow, newX, newY, newPosition, asyncPopup) } // close an existing popup @@ -138,6 +133,8 @@ Menu.prototype._callMenuWillShow = function () { }) } +/* Static Methods */ + // return application menu Menu.getApplicationMenu = () => applicationMenu @@ -187,7 +184,7 @@ Menu.buildFromTemplate = function (template) { return menu } -/* Helper Methods */ +/* Helper Functions */ // Search between separators to find a radio menu item and return its group id function generateGroupId (items, pos) { @@ -253,11 +250,11 @@ function indexToInsertByPosition (items, position) { } // insert a new MenuItem depending on its type -function insertItemByType(item, pos) { +function insertItemByType (item, pos) { const types = { 'normal': () => this.insertItem(pos, item.commandId, item.label), 'checkbox': () => this.insertCheckItem(pos, item.commandId, item.label), - 'separator':() => this.insertSeparator(pos), + 'separator': () => this.insertSeparator(pos), 'submenu': () => this.insertSubMenu(pos, item.commandId, item.label, item.submenu), 'radio': () => { // Grouping radio menu items From 75f32afcd513c94baececeb2d6414b43c27ed93b Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 00:24:57 -0400 Subject: [PATCH 14/21] clean up excess code from generateGroupId --- lib/browser/api/menu.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 82ecb08dab09..1effc3ff761f 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -188,20 +188,15 @@ Menu.buildFromTemplate = function (template) { // Search between separators to find a radio menu item and return its group id function generateGroupId (items, pos) { - let i, item if (pos > 0) { - let asc, start - for (start = pos - 1, i = start, asc = start <= 0; asc ? i <= 0 : i >= 0; asc ? i++ : i--) { - item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } + for (let idx = pos - 1; idx >= 0; idx--) { + if (items[idx].type === 'radio') return items[idx].groupId + if (items[idx].type === 'separator') break } } else if (pos < items.length) { - let asc1, end - for (i = pos, end = items.length - 1, asc1 = pos <= end; asc1 ? i <= end : i >= end; asc1 ? i++ : i--) { - item = items[i] - if (item.type === 'radio') { return item.groupId } - if (item.type === 'separator') { break } + for (let idx = pos; idx <= items.length - 1; idx++) { + if (items[idx].type === 'radio') return items[idx].groupId + if (items[idx].type === 'separator') break } } groupIdIndex += 1 From e8935232b1898ef6cb0c7e7a002d4f231e6fb032 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 18:52:12 -0400 Subject: [PATCH 15/21] clean falsy statements --- lib/browser/api/menu.js | 46 ++++++++++++++++++++--------------------- 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 1effc3ff761f..9864e189b26c 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -18,19 +18,19 @@ Menu.prototype._init = function () { this.groupsMap = {} this.items = [] this.delegate = { - isCommandIdChecked: id => this.commandsMap[id].checked, - isCommandIdEnabled: id => this.commandsMap[id].enabled, - isCommandIdVisible: id => this.commandsMap[id].visible, + isCommandIdChecked: id => this.commandsMap[id] ? this.commandsMap[id].checked : undefined, + isCommandIdEnabled: id => this.commandsMap[id] ? this.commandsMap[id].enabled : undefined, + isCommandIdVisible: id => this.commandsMap[id] ? this.commandsMap[id].visible : undefined, getAcceleratorForCommandId: (id, useDefaultAccelerator) => { const command = this.commandsMap[id] - if (command === null) return - if (command.accelerator !== null) return command.accelerator + if (!command) return + if (command.accelerator) return command.accelerator if (useDefaultAccelerator) return command.getDefaultRoleAccelerator() }, - getIconForCommandId: id => this.commandsMap[id].icon, + getIconForCommandId: id => this.commandsMap[id] ? this.commandsMap[id].icon : undefined, executeCommand: (event, id) => { const command = this.commandsMap[id] - if (command === null) return + if (!command) return command.click(event, BrowserWindow.getFocusedWindow(), webContents.getFocusedWebContents()) }, menuWillShow: () => { @@ -49,7 +49,7 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { let [newX, newY, newPosition, newWindow] = [x, y, positioningItem, window] // menu.popup(x, y, positioningItem) - if (window !== null) { + if (!window) { // shift over values if (typeof window !== 'object' || window.constructor !== BrowserWindow) { [newPosition, newY, newX, newWindow] = [y, x, window, null] @@ -57,7 +57,7 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { } // menu.popup(window, {}) - if (x !== null && typeof x === 'object') { + if (x && typeof x === 'object') { const opts = x newX = opts.x newY = opts.y @@ -69,7 +69,7 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { if (typeof x !== 'number') newX = -1 if (typeof y !== 'number') newY = -1 if (typeof positioningItem !== 'number') newPosition = -1 - if (window === null) newWindow = BrowserWindow.getFocusedWindow() + if (!window) newWindow = BrowserWindow.getFocusedWindow() if (typeof asyncPopup !== 'boolean') asyncPopup = false this.popupAt(newWindow, newX, newY, newPosition, asyncPopup) @@ -77,12 +77,10 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { // close an existing popup Menu.prototype.closePopup = function (window) { - if (window == null || window.constructor !== BrowserWindow) { + if (!window || window.constructor !== BrowserWindow) { window = BrowserWindow.getFocusedWindow() } - if (window != null) { - this.closePopupAt(window.id) - } + if (window) this.closePopupAt(window.id) } // return a MenuItem with the specified id @@ -105,7 +103,7 @@ Menu.prototype.append = function (item) { // insert a new menu item at a specified location Menu.prototype.insert = function (pos, item) { - if ((item != null ? item.constructor : void 0) !== MenuItem) { + if ((item ? item.constructor : void 0) !== MenuItem) { throw new TypeError('Invalid item') } @@ -113,9 +111,9 @@ Menu.prototype.insert = function (pos, item) { insertItemByType.call(this, item, pos) // set item properties - if (item.sublabel != null) this.setSublabel(pos, item.sublabel) - if (item.icon != null) this.setIcon(pos, item.icon) - if (item.role != null) this.setRole(pos, item.role) + if (item.sublabel) this.setSublabel(pos, item.sublabel) + if (item.icon) this.setIcon(pos, item.icon) + if (item.role) this.setRole(pos, item.role) // Make menu accessable to items. item.overrideReadOnlyProperty('menu', this) @@ -127,9 +125,9 @@ Menu.prototype.insert = function (pos, item) { // Force menuWillShow to be called Menu.prototype._callMenuWillShow = function () { - if (this.delegate != null) this.delegate.menuWillShow() + if (this.delegate) this.delegate.menuWillShow() this.items.forEach(item => { - if (item.submenu != null) item.submenu._callMenuWillShow() + if (item.submenu) item.submenu._callMenuWillShow() }) } @@ -142,13 +140,13 @@ Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder // set application menu with a preexisting menu Menu.setApplicationMenu = function (menu) { - if (!(menu === null || menu.constructor === Menu)) { + if (!(menu || menu.constructor === Menu)) { throw new TypeError('Invalid menu') } applicationMenu = menu if (process.platform === 'darwin') { - if (menu === null) return + if (!menu) return menu._callMenuWillShow() bindings.setApplicationMenu(menu) } else { @@ -205,7 +203,7 @@ function generateGroupId (items, pos) { // Returns the index of item according to |id|. function indexOfItemById (items, id) { - const foundItem = items.find(item => item.id === id) || null + const foundItem = items.find(item => item.id === id) || -1 return items.indexOf(foundItem) } @@ -218,7 +216,7 @@ function indexToInsertByPosition (items, position) { // warn if query doesn't exist if (idx === -1 && query !== 'endof') { - console.warn("Item with id '" + id + "' is not found") + console.warn(`Item with id ${id} is not found`) return items.length } From 0e6100ae1729b9ff6aaa7056e8c88db1db3afa2d Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 20:27:26 -0400 Subject: [PATCH 16/21] upgrade menu spec to ES6 --- spec/api-menu-spec.js | 155 ++++++++++++++++++++---------------------- 1 file changed, 74 insertions(+), 81 deletions(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 4389f9d50db7..baba221a6a15 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -4,10 +4,10 @@ const {ipcRenderer, remote} = require('electron') const {BrowserWindow, Menu, MenuItem} = remote const {closeWindow} = require('./window-helpers') -describe('menu module', function () { +describe.only('menu module', function () { describe('Menu.buildFromTemplate', function () { it('should be able to attach extra fields', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'text', extra: 'field' @@ -17,7 +17,7 @@ describe('menu module', function () { }) it('does not modify the specified template', function () { - var template = ipcRenderer.sendSync('eval', "var template = [{label: 'text', submenu: [{label: 'sub'}]}];\nrequire('electron').Menu.buildFromTemplate(template);\ntemplate;") + const template = ipcRenderer.sendSync('eval', "var template = [{label: 'text', submenu: [{label: 'sub'}]}];\nrequire('electron').Menu.buildFromTemplate(template);\ntemplate;") assert.deepStrictEqual(template, [ { label: 'text', @@ -47,7 +47,7 @@ describe('menu module', function () { describe('Menu.buildFromTemplate should reorder based on item position specifiers', function () { it('should position before existing item', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: '2', id: '2' @@ -66,7 +66,7 @@ describe('menu module', function () { }) it('should position after existing item', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: '1', id: '1' @@ -85,7 +85,7 @@ describe('menu module', function () { }) it('should position at endof existing separator groups', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { type: 'separator', id: 'numbers' @@ -129,7 +129,7 @@ describe('menu module', function () { }) it('should create separator group if endof does not reference existing separator group', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'a', id: 'a', @@ -167,7 +167,7 @@ describe('menu module', function () { }) it('should continue inserting items at next index when no specifier is present', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: '4', id: '4' @@ -197,7 +197,7 @@ describe('menu module', function () { describe('Menu.getMenuItemById', function () { it('should return the item with the given id', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'View', submenu: [ @@ -220,7 +220,7 @@ describe('menu module', function () { describe('Menu.insert', function () { it('should store item in @items by its index', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: '1' }, { @@ -229,9 +229,8 @@ describe('menu module', function () { label: '3' } ]) - var item = new MenuItem({ - label: 'inserted' - }) + const item = new MenuItem({ label: 'inserted' }) + menu.insert(1, item) assert.equal(menu.items[0].label, '1') assert.equal(menu.items[1].label, 'inserted') @@ -242,23 +241,27 @@ describe('menu module', function () { describe('Menu.popup', function () { let w = null + let menu - afterEach(function () { + beforeEach(() => { + w = new BrowserWindow({show: false, width: 200, height: 200}) + menu = Menu.buildFromTemplate([ + { + label: '1' + }, { + label: '2' + }, { + label: '3' + } + ]) + }) + + afterEach(() => { return closeWindow(w).then(function () { w = null }) }) describe('when called with async: true', function () { it('returns immediately', function () { - w = new BrowserWindow({show: false, width: 200, height: 200}) - const menu = Menu.buildFromTemplate([ - { - label: '1' - }, { - label: '2' - }, { - label: '3' - } - ]) menu.popup(w, {x: 100, y: 100, async: true}) menu.closePopup(w) }) @@ -266,7 +269,7 @@ describe('menu module', function () { }) describe('MenuItem.click', function () { it('should be called with the item object passed', function (done) { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'text', click: function (item) { @@ -282,7 +285,7 @@ describe('menu module', function () { describe('MenuItem with checked property', function () { it('clicking an checkbox item should flip the checked property', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'text', type: 'checkbox' @@ -294,7 +297,7 @@ describe('menu module', function () { }) it('clicking an radio item should always make checked property true', function () { - var menu = Menu.buildFromTemplate([ + const menu = Menu.buildFromTemplate([ { label: 'text', type: 'radio' @@ -307,99 +310,91 @@ describe('menu module', function () { }) it('at least have one item checked in each group', function () { - var i, j, k, menu, template - template = [] - for (i = j = 0; j <= 10; i = ++j) { + const template = [] + for (let i = 0; i <= 10; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - template.push({ - type: 'separator' - }) - for (i = k = 12; k <= 20; i = ++k) { + template.push({type: 'separator'}) + for (let i = 12; i <= 20; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - menu = Menu.buildFromTemplate(template) + const menu = Menu.buildFromTemplate(template) menu.delegate.menuWillShow() assert.equal(menu.items[0].checked, true) assert.equal(menu.items[12].checked, true) }) it('should assign groupId automatically', function () { - var groupId, i, j, k, l, m, menu, template - template = [] - for (i = j = 0; j <= 10; i = ++j) { + const template = [] + for (let i = 0; i <= 10; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - template.push({ - type: 'separator' - }) - for (i = k = 12; k <= 20; i = ++k) { + template.push({type: 'separator'}) + for (let i = 12; i <= 20; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - menu = Menu.buildFromTemplate(template) - groupId = menu.items[0].groupId - for (i = l = 0; l <= 10; i = ++l) { + const menu = Menu.buildFromTemplate(template) + const groupId = menu.items[0].groupId + for (let i = 0; i <= 10; i++) { assert.equal(menu.items[i].groupId, groupId) } - for (i = m = 12; m <= 20; i = ++m) { + for (let i = 12; i <= 20; i++) { assert.equal(menu.items[i].groupId, groupId + 1) } }) it("setting 'checked' should flip other items' 'checked' property", function () { - var i, j, k, l, m, menu, n, o, p, q, template - template = [] - for (i = j = 0; j <= 10; i = ++j) { + const template = [] + for (let i = 0; i <= 10; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - template.push({ - type: 'separator' - }) - for (i = k = 12; k <= 20; i = ++k) { + template.push({type: 'separator'}) + for (let i = 12; i <= 20; i++) { template.push({ - label: '' + i, + label: `${i}`, type: 'radio' }) } - menu = Menu.buildFromTemplate(template) - for (i = l = 0; l <= 10; i = ++l) { + + const menu = Menu.buildFromTemplate(template) + for (let i = 0; i <= 10; i++) { assert.equal(menu.items[i].checked, false) } menu.items[0].checked = true assert.equal(menu.items[0].checked, true) - for (i = m = 1; m <= 10; i = ++m) { + for (let i = 1; i <= 10; i++) { assert.equal(menu.items[i].checked, false) } menu.items[10].checked = true assert.equal(menu.items[10].checked, true) - for (i = n = 0; n <= 9; i = ++n) { + for (let i = 0; i <= 9; i++) { assert.equal(menu.items[i].checked, false) } - for (i = o = 12; o <= 20; i = ++o) { + for (let i = 12; i <= 20; i++) { assert.equal(menu.items[i].checked, false) } menu.items[12].checked = true assert.equal(menu.items[10].checked, true) - for (i = p = 0; p <= 9; i = ++p) { + for (let i = 0; i <= 9; i++) { assert.equal(menu.items[i].checked, false) } assert.equal(menu.items[12].checked, true) - for (i = q = 13; q <= 20; i = ++q) { + for (let i = 13; i <= 20; i++) { assert.equal(menu.items[i].checked, false) } }) @@ -407,20 +402,18 @@ describe('menu module', function () { describe('MenuItem command id', function () { it('cannot be overwritten', function () { - var item = new MenuItem({ - label: 'item' - }) + const item = new MenuItem({label: 'item'}) - var commandId = item.commandId - assert(commandId != null) - item.commandId = '' + commandId + '-modified' + const commandId = item.commandId + assert(commandId) + item.commandId = `${commandId}-modified` assert.equal(item.commandId, commandId) }) }) describe('MenuItem with invalid type', function () { it('throws an exception', function () { - assert.throws(function () { + assert.throws(() => { Menu.buildFromTemplate([ { label: 'text', @@ -433,7 +426,7 @@ describe('menu module', function () { describe('MenuItem with submenu type and missing submenu', function () { it('throws an exception', function () { - assert.throws(function () { + assert.throws(() => { Menu.buildFromTemplate([ { label: 'text', @@ -446,7 +439,7 @@ describe('menu module', function () { describe('MenuItem role', function () { it('includes a default label and accelerator', function () { - var item = new MenuItem({role: 'close'}) + let item = new MenuItem({role: 'close'}) assert.equal(item.label, process.platform === 'darwin' ? 'Close Window' : 'Close') assert.equal(item.accelerator, undefined) assert.equal(item.getDefaultRoleAccelerator(), 'CommandOrControl+W') @@ -480,7 +473,7 @@ describe('menu module', function () { describe('MenuItem editMenu', function () { it('includes a default submenu layout when submenu is empty', function () { - var item = new MenuItem({role: 'editMenu'}) + const item = new MenuItem({role: 'editMenu'}) assert.equal(item.label, 'Edit') assert.equal(item.submenu.items[0].role, 'undo') assert.equal(item.submenu.items[1].role, 'redo') @@ -503,7 +496,7 @@ describe('menu module', function () { }) it('overrides default layout when submenu is specified', function () { - var item = new MenuItem({role: 'editMenu', submenu: [{role: 'close'}]}) + const item = new MenuItem({role: 'editMenu', submenu: [{role: 'close'}]}) assert.equal(item.label, 'Edit') assert.equal(item.submenu.items[0].role, 'close') }) @@ -511,7 +504,7 @@ describe('menu module', function () { describe('MenuItem windowMenu', function () { it('includes a default submenu layout when submenu is empty', function () { - var item = new MenuItem({role: 'windowMenu'}) + const item = new MenuItem({role: 'windowMenu'}) assert.equal(item.label, 'Window') assert.equal(item.submenu.items[0].role, 'minimize') assert.equal(item.submenu.items[1].role, 'close') @@ -523,7 +516,7 @@ describe('menu module', function () { }) it('overrides default layout when submenu is specified', function () { - var item = new MenuItem({role: 'windowMenu', submenu: [{role: 'copy'}]}) + const item = new MenuItem({role: 'windowMenu', submenu: [{role: 'copy'}]}) assert.equal(item.label, 'Window') assert.equal(item.submenu.items[0].role, 'copy') }) @@ -531,13 +524,13 @@ describe('menu module', function () { describe('MenuItem with custom properties in constructor', function () { it('preserves the custom properties', function () { - var template = [{ + const template = [{ label: 'menu 1', customProp: 'foo', submenu: [] }] - var menu = Menu.buildFromTemplate(template) + const menu = Menu.buildFromTemplate(template) menu.items[0].submenu.append(new MenuItem({ label: 'item 1', customProp: 'bar', From d54148de4e89c581afc5ad9fe61914fc004fad4f Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 20:31:02 -0400 Subject: [PATCH 17/21] remove from spec --- spec/api-menu-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index baba221a6a15..fa389fe5f7da 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -4,7 +4,7 @@ const {ipcRenderer, remote} = require('electron') const {BrowserWindow, Menu, MenuItem} = remote const {closeWindow} = require('./window-helpers') -describe.only('menu module', function () { +describe('menu module', function () { describe('Menu.buildFromTemplate', function () { it('should be able to attach extra fields', function () { const menu = Menu.buildFromTemplate([ From f7bc5481f386db241ba9f3dfac5bb0c63e7a183e Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 22:40:31 -0400 Subject: [PATCH 18/21] add a few more tests to api_menu_spec --- spec/api-menu-spec.js | 37 ++++++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index fa389fe5f7da..ad2ddafeb732 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -4,7 +4,7 @@ const {ipcRenderer, remote} = require('electron') const {BrowserWindow, Menu, MenuItem} = remote const {closeWindow} = require('./window-helpers') -describe('menu module', function () { +describe.only('menu module', function () { describe('Menu.buildFromTemplate', function () { it('should be able to attach extra fields', function () { const menu = Menu.buildFromTemplate([ @@ -229,6 +229,7 @@ describe('menu module', function () { label: '3' } ]) + const item = new MenuItem({ label: 'inserted' }) menu.insert(1, item) @@ -239,6 +240,27 @@ describe('menu module', function () { }) }) + describe('Menu.append', function () { + it('should add the item to the end of the menu', function () { + const menu = Menu.buildFromTemplate([ + { + label: '1' + }, { + label: '2' + }, { + label: '3' + } + ]) + const item = new MenuItem({ label: 'inserted' }) + + menu.append(item) + assert.equal(menu.items[0].label, '1') + assert.equal(menu.items[1].label, '2') + assert.equal(menu.items[2].label, '3') + assert.equal(menu.items[3].label, 'inserted') + }) + }) + describe('Menu.popup', function () { let w = null let menu @@ -267,6 +289,19 @@ describe('menu module', function () { }) }) }) + + describe('Menu.setApplicationMenu', function () { + const menu = Menu.buildFromTemplate([ + { + label: '1' + }, { + label: '2' + } + ]) + Menu.setApplicationMenu(menu) + assert.notEqual(Menu.getApplicationMenu(), null) + }) + describe('MenuItem.click', function () { it('should be called with the item object passed', function (done) { const menu = Menu.buildFromTemplate([ From 135454342d9a5e636386f02195c77b5bff27a09a Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 24 Oct 2017 22:41:28 -0400 Subject: [PATCH 19/21] remove .only from spec --- spec/api-menu-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index ad2ddafeb732..2c422b340dc3 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -4,7 +4,7 @@ const {ipcRenderer, remote} = require('electron') const {BrowserWindow, Menu, MenuItem} = remote const {closeWindow} = require('./window-helpers') -describe.only('menu module', function () { +describe('menu module', function () { describe('Menu.buildFromTemplate', function () { it('should be able to attach extra fields', function () { const menu = Menu.buildFromTemplate([ From ffd43c18866eb33cf3d70f03a3ffd6d27a29e0e7 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 25 Oct 2017 10:17:41 -0400 Subject: [PATCH 20/21] remove quotes and const replaces --- lib/browser/api/menu.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 9864e189b26c..3e56bea04b67 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -35,8 +35,8 @@ Menu.prototype._init = function () { }, menuWillShow: () => { // Ensure radio groups have at least one menu item seleted - for (let id in this.groupsMap) { - let found = this.groupsMap[id].find(item => item.checked) || null + for (const id in this.groupsMap) { + const found = this.groupsMap[id].find(item => item.checked) || null if (!found) v8Util.setHiddenValue(this.groupsMap[id][0], 'checked', true) } } @@ -222,11 +222,11 @@ function indexToInsertByPosition (items, position) { // compute new index based on query const queries = { - 'after': (index) => { + after: (index) => { index += 1 return index }, - 'endof': (index) => { + endof: (index) => { if (index === -1) { items.push({id, type: 'separator'}) index = items.length - 1 @@ -245,11 +245,11 @@ function indexToInsertByPosition (items, position) { // insert a new MenuItem depending on its type function insertItemByType (item, pos) { const types = { - 'normal': () => this.insertItem(pos, item.commandId, item.label), - 'checkbox': () => this.insertCheckItem(pos, item.commandId, item.label), - 'separator': () => this.insertSeparator(pos), - 'submenu': () => this.insertSubMenu(pos, item.commandId, item.label, item.submenu), - 'radio': () => { + normal: () => this.insertItem(pos, item.commandId, item.label), + checkbox: () => this.insertCheckItem(pos, item.commandId, item.label), + separator: () => this.insertSeparator(pos), + submenu: () => this.insertSubMenu(pos, item.commandId, item.label, item.submenu), + radio: () => { // Grouping radio menu items item.overrideReadOnlyProperty('groupId', generateGroupId(this.items, pos)) if (this.groupsMap[item.groupId] == null) { From bccaf562000aa2c2cf04bf73d498d00bb6c0327f Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Wed, 25 Oct 2017 12:23:41 -0400 Subject: [PATCH 21/21] remove common sense comments --- lib/browser/api/menu.js | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/browser/api/menu.js b/lib/browser/api/menu.js index 3e56bea04b67..caa65afbbf0a 100644 --- a/lib/browser/api/menu.js +++ b/lib/browser/api/menu.js @@ -43,7 +43,6 @@ Menu.prototype._init = function () { } } -// create and show a popup Menu.prototype.popup = function (window, x, y, positioningItem) { let asyncPopup let [newX, newY, newPosition, newWindow] = [x, y, positioningItem, window] @@ -75,7 +74,6 @@ Menu.prototype.popup = function (window, x, y, positioningItem) { this.popupAt(newWindow, newX, newY, newPosition, asyncPopup) } -// close an existing popup Menu.prototype.closePopup = function (window) { if (!window || window.constructor !== BrowserWindow) { window = BrowserWindow.getFocusedWindow() @@ -83,7 +81,6 @@ Menu.prototype.closePopup = function (window) { if (window) this.closePopupAt(window.id) } -// return a MenuItem with the specified id Menu.prototype.getMenuItemById = function (id) { const items = this.items @@ -96,12 +93,10 @@ Menu.prototype.getMenuItemById = function (id) { return found } -// append a new MenuItem to an existing Menu Menu.prototype.append = function (item) { return this.insert(this.getItemCount(), item) } -// insert a new menu item at a specified location Menu.prototype.insert = function (pos, item) { if ((item ? item.constructor : void 0) !== MenuItem) { throw new TypeError('Invalid item') @@ -123,7 +118,6 @@ Menu.prototype.insert = function (pos, item) { this.commandsMap[item.commandId] = item } -// Force menuWillShow to be called Menu.prototype._callMenuWillShow = function () { if (this.delegate) this.delegate.menuWillShow() this.items.forEach(item => { @@ -133,7 +127,6 @@ Menu.prototype._callMenuWillShow = function () { /* Static Methods */ -// return application menu Menu.getApplicationMenu = () => applicationMenu Menu.sendActionToFirstResponder = bindings.sendActionToFirstResponder @@ -155,7 +148,6 @@ Menu.setApplicationMenu = function (menu) { } } -// build a menu by passing in a template Menu.buildFromTemplate = function (template) { if (!(template instanceof Array)) { throw new TypeError('Invalid template for Menu') @@ -201,13 +193,11 @@ function generateGroupId (items, pos) { return groupIdIndex } -// Returns the index of item according to |id|. function indexOfItemById (items, id) { const foundItem = items.find(item => item.id === id) || -1 return items.indexOf(foundItem) } -// Returns the index of where to insert the item according to |position| function indexToInsertByPosition (items, position) { if (!position) return items.length @@ -242,7 +232,6 @@ function indexToInsertByPosition (items, position) { return (query in queries) ? queries[query](idx) : idx } -// insert a new MenuItem depending on its type function insertItemByType (item, pos) { const types = { normal: () => this.insertItem(pos, item.commandId, item.label),