From 8aba64025038e79f043793b29871e77dd88bc118 Mon Sep 17 00:00:00 2001 From: mst128256 Date: Thu, 9 Mar 2017 16:01:33 +0100 Subject: [PATCH 1/6] added default menu items for 'Edit' and 'Window' #2814 --- docs/api/menu-item.md | 3 ++ lib/browser/api/menu-item-roles.js | 81 ++++++++++++++++++++++++++++++ lib/browser/api/menu-item.js | 2 +- 3 files changed, 85 insertions(+), 1 deletion(-) diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 1c12d9fdacd1..76fcae1129f3 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -64,6 +64,9 @@ The `role` property can have following values: * `zoomin` - Zoom in the focused page by 10% * `zoomout` - Zoom out the focused page by 10% +* `menuEdit` - Whole default "Edit" menu (Undo,Copy, etc.) +* `menuWindow` - Whole default "Window" menu (Minimize, Close, etc.) + On macOS `role` can also have following additional values: * `about` - Map to the `orderFrontStandardAboutPanel` action diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index 8a78670267ae..254283b86be8 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -154,6 +154,73 @@ const roles = { webContents.setZoomLevel(zoomLevel - 0.5) }) } + }, + // submenu Edit (should fit both Mac & Windows) + menuEdit: { + label: 'Edit', + submenu: [ + { + role: 'undo' + }, + { + role: 'redo' + }, + { + type: 'separator' + }, + { + role: 'cut' + }, + { + role: 'copy' + }, + { + role: 'paste' + }, + + process.platform === 'darwin' ? + { + role: 'pasteandmatchstyle' + } : {}, + + { + role: 'delete' + }, + + process.platform === 'win32' ? + { + type: 'separator' + } : {}, + + { + role: 'selectall' + } + ] + }, + + // submenu Window should be used for Mac only + menuWindow: { + label: 'Window', + submenu: [ + { + role: 'minimize' + }, + { + role: 'close' + }, + + process.platform === 'darwin' ? + { + type: 'separator' + } : {}, + + process.platform === 'darwin' ? + { + label: 'Bring All to Front', + role: 'front' + } : {} + + ] } } @@ -176,6 +243,20 @@ exports.getDefaultAccelerator = (role) => { if (roles.hasOwnProperty(role)) return roles[role].accelerator } +exports.getDefaultSubmenu = (role) => { + if (roles.hasOwnProperty(role)) { + submenu = roles[role].submenu + + // remove empty objects from within the submenu + if (Array.isArray(submenu)) + submenu = submenu.filter(function(n){ + return n.constructor !== Object || Object.keys(n).length > 0 + }) + + return submenu + } +} + exports.execute = (role, focusedWindow, focusedWebContents) => { if (!canExecuteRole(role)) return false diff --git a/lib/browser/api/menu-item.js b/lib/browser/api/menu-item.js index 98b8e9980e28..e95226d3b364 100644 --- a/lib/browser/api/menu-item.js +++ b/lib/browser/api/menu-item.js @@ -11,7 +11,7 @@ const MenuItem = function (options) { for (let key in options) { if (!(key in this)) this[key] = options[key] } - + this.submenu = this.submenu || roles.getDefaultSubmenu(this.role) if (this.submenu != null && this.submenu.constructor !== Menu) { this.submenu = Menu.buildFromTemplate(this.submenu) } From 76ee7fda2b2ec83de0755555604c80442f9882b3 Mon Sep 17 00:00:00 2001 From: mst128256 Date: Mon, 13 Mar 2017 14:26:34 +0100 Subject: [PATCH 2/6] Fixed linting --- lib/browser/api/menu-item-roles.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index 254283b86be8..8919b8d10ec7 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -178,8 +178,7 @@ const roles = { role: 'paste' }, - process.platform === 'darwin' ? - { + process.platform === 'darwin' ? { role: 'pasteandmatchstyle' } : {}, @@ -187,9 +186,8 @@ const roles = { role: 'delete' }, - process.platform === 'win32' ? - { - type: 'separator' + process.platform === 'win32' ? { + type: 'separator' } : {}, { @@ -209,13 +207,11 @@ const roles = { role: 'close' }, - process.platform === 'darwin' ? - { + process.platform === 'darwin' ? { type: 'separator' } : {}, - process.platform === 'darwin' ? - { + process.platform === 'darwin' ? { label: 'Bring All to Front', role: 'front' } : {} @@ -245,13 +241,14 @@ exports.getDefaultAccelerator = (role) => { exports.getDefaultSubmenu = (role) => { if (roles.hasOwnProperty(role)) { - submenu = roles[role].submenu + let submenu = roles[role].submenu // remove empty objects from within the submenu - if (Array.isArray(submenu)) - submenu = submenu.filter(function(n){ + if (Array.isArray(submenu)) { + submenu = submenu.filter(function (n) { return n.constructor !== Object || Object.keys(n).length > 0 }) + } return submenu } From 9e471d8f1cce79626491cea4be21be3b30cd6c2e Mon Sep 17 00:00:00 2001 From: mst128256 Date: Fri, 24 Mar 2017 12:14:08 +0100 Subject: [PATCH 3/6] Added specs --- docs/api/menu-item.md | 4 +-- lib/browser/api/menu-item-roles.js | 4 +-- spec/api-menu-spec.js | 46 ++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/api/menu-item.md b/docs/api/menu-item.md index 76fcae1129f3..dc403ea8f7be 100644 --- a/docs/api/menu-item.md +++ b/docs/api/menu-item.md @@ -64,8 +64,8 @@ The `role` property can have following values: * `zoomin` - Zoom in the focused page by 10% * `zoomout` - Zoom out the focused page by 10% -* `menuEdit` - Whole default "Edit" menu (Undo,Copy, etc.) -* `menuWindow` - Whole default "Window" menu (Minimize, Close, etc.) +* `editMenu` - Whole default "Edit" menu (Undo, Copy, etc.) +* `windowMenu` - Whole default "Window" menu (Minimize, Close, etc.) On macOS `role` can also have following additional values: diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index 8919b8d10ec7..b843bb82e8b6 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -156,7 +156,7 @@ const roles = { } }, // submenu Edit (should fit both Mac & Windows) - menuEdit: { + editMenu: { label: 'Edit', submenu: [ { @@ -197,7 +197,7 @@ const roles = { }, // submenu Window should be used for Mac only - menuWindow: { + windowMenu: { label: 'Window', submenu: [ { diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 176f1eafae52..cfe5e3934821 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -455,6 +455,52 @@ 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'}) + assert.equal(item.label, 'Edit') + assert.equal(item.submenu.items[0].role, 'undo') + assert.equal(item.submenu.items[1].role, 'redo') + assert.equal(item.submenu.items[2].type, 'separator') + assert.equal(item.submenu.items[3].role, 'cut') + assert.equal(item.submenu.items[4].role, 'copy') + assert.equal(item.submenu.items[5].role, 'paste') + if (process.platform == 'darwin') { + assert.equal(item.submenu.items[6].role, 'pasteandmatchstyle') + assert.equal(item.submenu.items[7].role, 'delete') + assert.equal(item.submenu.items[8].role, 'selectall') + } + if (process.platform == 'win32') { + assert.equal(item.submenu.items[6].role, 'delete') + assert.equal(item.submenu.items[7].type, 'separator') + assert.equal(item.submenu.items[8].role, 'selectall') + } + }) + it('overrides default layout when submenu is specified', function() { + var item = new MenuItem({role: 'editMenu', submenu: [{ role: 'close'}]}) + assert.equal(item.label, 'Edit') + assert.equal(item.submenu.items[0].role, 'close') + }) + }) + + describe('MenuItem windowMenu', function() { + it('includes a default submenu layout when submenu is empty', function() { + var 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') + if (process.platform == 'darwin') { + assert.equal(item.submenu.items[2].type, 'separator') + assert.equal(item.submenu.items[3].role, 'front') + } + }) + it('overrides default layout when submenu is specified', function() { + var item = new MenuItem({role: 'windowMenu', submenu: [{ role: 'copy'}]}) + assert.equal(item.label, 'Window') + assert.equal(item.submenu.items[0].role, 'copy') + }) + }) + describe('MenuItem with custom properties in constructor', function () { it('preserves the custom properties', function () { var template = [{ From 6a7b4feb35ad46b71aea0ffd74c91d248660269e Mon Sep 17 00:00:00 2001 From: mst128256 Date: Fri, 24 Mar 2017 12:31:49 +0100 Subject: [PATCH 4/6] Fixed for linting --- spec/api-menu-spec.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index cfe5e3934821..0d809789d71e 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -455,8 +455,8 @@ describe('menu module', function () { }) }) - describe('MenuItem editMenu', function() { - it('includes a default submenu layout when submenu is empty', function() { + describe('MenuItem editMenu', function () { + it('includes a default submenu layout when submenu is empty', function () { var item = new MenuItem({role: 'editMenu'}) assert.equal(item.label, 'Edit') assert.equal(item.submenu.items[0].role, 'undo') @@ -465,37 +465,37 @@ describe('menu module', function () { assert.equal(item.submenu.items[3].role, 'cut') assert.equal(item.submenu.items[4].role, 'copy') assert.equal(item.submenu.items[5].role, 'paste') - if (process.platform == 'darwin') { + if (process.platform === 'darwin') { assert.equal(item.submenu.items[6].role, 'pasteandmatchstyle') assert.equal(item.submenu.items[7].role, 'delete') assert.equal(item.submenu.items[8].role, 'selectall') } - if (process.platform == 'win32') { + if (process.platform === 'win32') { assert.equal(item.submenu.items[6].role, 'delete') assert.equal(item.submenu.items[7].type, 'separator') assert.equal(item.submenu.items[8].role, 'selectall') } }) - it('overrides default layout when submenu is specified', function() { - var item = new MenuItem({role: 'editMenu', submenu: [{ role: 'close'}]}) + it('overrides default layout when submenu is specified', function () { + var item = new MenuItem({role: 'editMenu', submenu: [{role: 'close'}]}) assert.equal(item.label, 'Edit') assert.equal(item.submenu.items[0].role, 'close') }) }) - describe('MenuItem windowMenu', function() { - it('includes a default submenu layout when submenu is empty', function() { + describe('MenuItem windowMenu', function () { + it('includes a default submenu layout when submenu is empty', function () { var 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') - if (process.platform == 'darwin') { + if (process.platform === 'darwin') { assert.equal(item.submenu.items[2].type, 'separator') assert.equal(item.submenu.items[3].role, 'front') } }) - it('overrides default layout when submenu is specified', function() { - var item = new MenuItem({role: 'windowMenu', submenu: [{ role: 'copy'}]}) + it('overrides default layout when submenu is specified', function () { + var item = new MenuItem({role: 'windowMenu', submenu: [{role: 'copy'}]}) assert.equal(item.label, 'Window') assert.equal(item.submenu.items[0].role, 'copy') }) From 6ae198a6253fdf2321a280ea4b7ac582e69979cc Mon Sep 17 00:00:00 2001 From: mst128256 Date: Wed, 29 Mar 2017 12:17:50 +0200 Subject: [PATCH 5/6] Empty objects within default menu replaced by nulls --- lib/browser/api/menu-item-roles.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index b843bb82e8b6..30ea3f888dba 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -180,7 +180,7 @@ const roles = { process.platform === 'darwin' ? { role: 'pasteandmatchstyle' - } : {}, + } : null, { role: 'delete' @@ -188,7 +188,7 @@ const roles = { process.platform === 'win32' ? { type: 'separator' - } : {}, + } : null, { role: 'selectall' @@ -209,12 +209,11 @@ const roles = { process.platform === 'darwin' ? { type: 'separator' - } : {}, + } : null, process.platform === 'darwin' ? { - label: 'Bring All to Front', role: 'front' - } : {} + } : null ] } @@ -246,7 +245,7 @@ exports.getDefaultSubmenu = (role) => { // remove empty objects from within the submenu if (Array.isArray(submenu)) { submenu = submenu.filter(function (n) { - return n.constructor !== Object || Object.keys(n).length > 0 + return n != null }) } From 8b4bf1f29ed5629037c93525beabab246e9eda92 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 29 Mar 2017 12:27:59 -0700 Subject: [PATCH 6/6] :art: --- lib/browser/api/menu-item-roles.js | 20 +++++++++----------- spec/api-menu-spec.js | 5 +++++ 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index 30ea3f888dba..af0d35b4a56a 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -155,7 +155,7 @@ const roles = { }) } }, - // submenu Edit (should fit both Mac & Windows) + // Edit submenu (should fit both Mac & Windows) editMenu: { label: 'Edit', submenu: [ @@ -196,7 +196,7 @@ const roles = { ] }, - // submenu Window should be used for Mac only + // Window submenu should be used for Mac only windowMenu: { label: 'Window', submenu: [ @@ -239,18 +239,16 @@ exports.getDefaultAccelerator = (role) => { } exports.getDefaultSubmenu = (role) => { - if (roles.hasOwnProperty(role)) { - let submenu = roles[role].submenu + if (!roles.hasOwnProperty(role)) return - // remove empty objects from within the submenu - if (Array.isArray(submenu)) { - submenu = submenu.filter(function (n) { - return n != null - }) - } + let {submenu} = roles[role] - return submenu + // remove null items from within the submenu + if (Array.isArray(submenu)) { + submenu = submenu.filter((item) => item != null) } + + return submenu } exports.execute = (role, focusedWindow, focusedWebContents) => { diff --git a/spec/api-menu-spec.js b/spec/api-menu-spec.js index 0d809789d71e..9b1f3de36906 100644 --- a/spec/api-menu-spec.js +++ b/spec/api-menu-spec.js @@ -465,17 +465,20 @@ describe('menu module', function () { assert.equal(item.submenu.items[3].role, 'cut') assert.equal(item.submenu.items[4].role, 'copy') assert.equal(item.submenu.items[5].role, 'paste') + if (process.platform === 'darwin') { assert.equal(item.submenu.items[6].role, 'pasteandmatchstyle') assert.equal(item.submenu.items[7].role, 'delete') assert.equal(item.submenu.items[8].role, 'selectall') } + if (process.platform === 'win32') { assert.equal(item.submenu.items[6].role, 'delete') assert.equal(item.submenu.items[7].type, 'separator') assert.equal(item.submenu.items[8].role, 'selectall') } }) + it('overrides default layout when submenu is specified', function () { var item = new MenuItem({role: 'editMenu', submenu: [{role: 'close'}]}) assert.equal(item.label, 'Edit') @@ -489,11 +492,13 @@ describe('menu module', function () { assert.equal(item.label, 'Window') assert.equal(item.submenu.items[0].role, 'minimize') assert.equal(item.submenu.items[1].role, 'close') + if (process.platform === 'darwin') { assert.equal(item.submenu.items[2].type, 'separator') assert.equal(item.submenu.items[3].role, 'front') } }) + it('overrides default layout when submenu is specified', function () { var item = new MenuItem({role: 'windowMenu', submenu: [{role: 'copy'}]}) assert.equal(item.label, 'Window')