From 504aca747dc09988c9effa13b97e3ceb7fca403b Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 19:00:06 +0200 Subject: [PATCH 1/7] main/menu.js: minor refactor Just some code cleanup to make menu.js more internally consistent. - Name the electron.dialog returned value `selectedPaths` which is more accurate. - Move the file menu into the `template` object, like the rest of the menus. Then reach in afterwards for OS-specific tweaks. --- config.js | 3 ++ main/menu.js | 124 ++++++++++++++++++++++++++------------------------- 2 files changed, 66 insertions(+), 61 deletions(-) diff --git a/config.js b/config.js index 55d501a3..ea891229 100644 --- a/config.js +++ b/config.js @@ -28,8 +28,11 @@ module.exports = { CONFIG_TORRENT_PATH: path.join(getConfigPath(), 'Torrents'), GITHUB_URL: 'https://github.com/feross/webtorrent-desktop', + GITHUB_URL_ISSUES: 'https://github.com/feross/webtorrent-desktop/issues', GITHUB_URL_RAW: 'https://raw.githubusercontent.com/feross/webtorrent-desktop/master', + HOME_PAGE_URL: 'https://webtorrent.io', + IS_PORTABLE: isPortable(), IS_PRODUCTION: isProduction(), diff --git a/main/menu.js b/main/menu.js index 644ae4e5..8239de6c 100644 --- a/main/menu.js +++ b/main/menu.js @@ -109,29 +109,29 @@ function getMenuItem (label) { } } -// Prompts the user for a file, then makes a torrent out of the data +// Prompts the user for a file, then creates a torrent. Only allows a single file +// selection. function showOpenSeedFile () { electron.dialog.showOpenDialog({ title: 'Select a file for the torrent file.', properties: [ 'openFile' ] - }, function (filenames) { - if (!Array.isArray(filenames)) return - var file = filenames[0] - windows.main.send('dispatch', 'showCreateTorrent', file) + }, function (selectedPaths) { + if (!Array.isArray(selectedPaths)) return + var selectedPath = selectedPaths[0] + windows.main.send('dispatch', 'showCreateTorrent', selectedPath) }) } -// Prompts the user for a file or folder, then makes a torrent out of the data +// Prompts the user for a file or directory, then creates a torrent. Only allows a single +// selection. To create a multi-file torrent, the user must select a directory. function showOpenSeedFiles () { - // Allow only a single selection - // To create a multi-file torrent, the user must select a folder electron.dialog.showOpenDialog({ title: 'Select a file or folder for the torrent file.', properties: [ 'openFile', 'openDirectory' ] - }, function (filenames) { - if (!Array.isArray(filenames)) return - var fileOrFolder = filenames[0] - windows.main.send('dispatch', 'showCreateTorrent', fileOrFolder) + }, function (selectedPaths) { + if (!Array.isArray(selectedPaths)) return + var selectedPath = selectedPaths[0] + windows.main.send('dispatch', 'showCreateTorrent', selectedPath) }) } @@ -141,10 +141,10 @@ function showOpenTorrentFile () { title: 'Select a .torrent file to open.', filters: [{ name: 'Torrent Files', extensions: ['torrent'] }], properties: [ 'openFile', 'multiSelections' ] - }, function (filenames) { - if (!Array.isArray(filenames)) return - filenames.forEach(function (filename) { - windows.main.send('dispatch', 'addTorrent', filename) + }, function (selectedPaths) { + if (!Array.isArray(selectedPaths)) return + selectedPaths.forEach(function (selectedPath) { + windows.main.send('dispatch', 'addTorrent', selectedPath) }) }) } @@ -155,51 +155,36 @@ function showOpenTorrentAddress () { } function getAppMenuTemplate () { - var fileMenu = [ - { - label: process.platform === 'darwin' ? 'Create New Torrent...' : 'Create New Torrent from Folder...', - accelerator: 'CmdOrCtrl+N', - click: showOpenSeedFiles - }, - { - label: 'Open Torrent File...', - accelerator: 'CmdOrCtrl+O', - click: showOpenTorrentFile - }, - { - label: 'Open Torrent Address...', - accelerator: 'CmdOrCtrl+U', - click: showOpenTorrentAddress - }, - { - type: 'separator' - }, - { - label: process.platform === 'windows' ? 'Close' : 'Close Window', - accelerator: 'CmdOrCtrl+W', - role: 'close' - } - ] - - // In Linux and Windows it is not possible to open both folders and files - if (process.platform !== 'darwin') { - fileMenu.unshift({ - label: 'Create New Torrent from File...', - click: showOpenSeedFile - }) - } - // File > Quit for Linux users with distros where the system tray is broken - if (process.platform === 'linux') { - fileMenu.push({ - label: 'Quit', - click: () => app.quit() - }) - } - var template = [ { label: 'File', - submenu: fileMenu + submenu: [ + { + label: process.platform === 'darwin' + ? 'Create New Torrent...' + : 'Create New Torrent from Folder...', + accelerator: 'CmdOrCtrl+N', + click: showOpenSeedFiles + }, + { + label: 'Open Torrent File...', + accelerator: 'CmdOrCtrl+O', + click: showOpenTorrentFile + }, + { + label: 'Open Torrent Address...', + accelerator: 'CmdOrCtrl+U', + click: showOpenTorrentAddress + }, + { + type: 'separator' + }, + { + label: process.platform === 'windows' ? 'Close' : 'Close Window', + accelerator: 'CmdOrCtrl+W', + role: 'close' + } + ] }, { label: 'Edit', @@ -298,7 +283,7 @@ function getAppMenuTemplate () { submenu: [ { label: 'Learn more about ' + config.APP_NAME, - click: () => electron.shell.openExternal('https://webtorrent.io') + click: () => electron.shell.openExternal(config.HOME_PAGE_URL) }, { label: 'Contribute on GitHub', @@ -309,7 +294,7 @@ function getAppMenuTemplate () { }, { label: 'Report an Issue...', - click: () => electron.shell.openExternal(config.GITHUB_URL + '/issues') + click: () => electron.shell.openExternal(config.GITHUB_URL_ISSUES) } ] } @@ -370,7 +355,16 @@ function getAppMenuTemplate () { role: 'front' } ) - } else { + } + + // In Linux and Windows it is not possible to open both folders and files + if (process.platform === 'linux' || process.platform === 'windows') { + // File menu (Windows, Linux) + template[0].unshift({ + label: 'Create New Torrent from File...', + click: showOpenSeedFile + }) + // Help menu (Windows, Linux) template[4].submenu.push( { @@ -382,6 +376,14 @@ function getAppMenuTemplate () { } ) } + // Add "File > Quit" menu item for Linux distros where the system tray is broken + if (process.platform === 'linux') { + // File menu (Linux) + template[0].push({ + label: 'Quit', + click: () => app.quit() + }) + } return template } From 4bffb6634c86680d0006d288ad3d20f3961d0bb1 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 19:12:19 +0200 Subject: [PATCH 2/7] Add Playback menu for playback-related functionality --- main/menu.js | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/main/menu.js b/main/menu.js index 8239de6c..db43e6f7 100644 --- a/main/menu.js +++ b/main/menu.js @@ -230,21 +230,6 @@ function getAppMenuTemplate () { { type: 'separator' }, - { - label: 'Increase Volume', - accelerator: 'CmdOrCtrl+Up', - click: increaseVolume, - enabled: false - }, - { - label: 'Decrease Volume', - accelerator: 'CmdOrCtrl+Down', - click: decreaseVolume, - enabled: false - }, - { - type: 'separator' - }, { label: 'Developer', submenu: [ @@ -266,6 +251,23 @@ function getAppMenuTemplate () { } ] }, + { + label: 'Playback', + submenu: [ + { + label: 'Increase Volume', + accelerator: 'CmdOrCtrl+Up', + click: increaseVolume, + enabled: false + }, + { + label: 'Decrease Volume', + accelerator: 'CmdOrCtrl+Down', + click: decreaseVolume, + enabled: false + } + ] + }, { label: 'Window', role: 'window', @@ -346,7 +348,7 @@ function getAppMenuTemplate () { }) // Window menu (OS X) - template[4].submenu.push( + template[5].submenu.push( { type: 'separator' }, @@ -366,7 +368,7 @@ function getAppMenuTemplate () { }) // Help menu (Windows, Linux) - template[4].submenu.push( + template[5].submenu.push( { type: 'separator' }, From c99da2ccaa24e648c6744d702922e7dead643f70 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 19:26:15 +0200 Subject: [PATCH 3/7] Remove Window menu on Linux and Windows The Window menu is apparently an OS X only convention. I couldn't find a single app on Windows or Linux that had this menu or even a "minimize" menu item. --- main/menu.js | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/main/menu.js b/main/menu.js index db43e6f7..fe8d0f36 100644 --- a/main/menu.js +++ b/main/menu.js @@ -268,17 +268,6 @@ function getAppMenuTemplate () { } ] }, - { - label: 'Window', - role: 'window', - submenu: [ - { - label: 'Minimize', - accelerator: 'CmdOrCtrl+M', - role: 'minimize' - } - ] - }, { label: 'Help', role: 'help', @@ -303,7 +292,7 @@ function getAppMenuTemplate () { ] if (process.platform === 'darwin') { - // WebTorrent menu (OS X) + // Add WebTorrent app menu (OS X) template.unshift({ label: config.APP_NAME, submenu: [ @@ -347,16 +336,25 @@ function getAppMenuTemplate () { ] }) - // Window menu (OS X) - template[5].submenu.push( - { - type: 'separator' - }, - { - label: 'Bring All to Front', - role: 'front' - } - ) + // Add Window menu (OS X) + template.splice(5, 0, { + label: 'Window', + role: 'window', + submenu: [ + { + label: 'Minimize', + accelerator: 'CmdOrCtrl+M', + role: 'minimize' + }, + { + type: 'separator' + }, + { + label: 'Bring All to Front', + role: 'front' + } + ] + }) } // In Linux and Windows it is not possible to open both folders and files @@ -368,7 +366,7 @@ function getAppMenuTemplate () { }) // Help menu (Windows, Linux) - template[5].submenu.push( + template[4].submenu.push( { type: 'separator' }, From 2005ee4d0bbb6441ec2ef908a3fe4f4a4704597c Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 19:56:06 +0200 Subject: [PATCH 4/7] shortcuts.js: Consistent exported method naming Exposed methods whose sole purpose is notify the module of an event firing, should start with "on". --- main/ipc.js | 4 ++-- main/menu.js | 2 ++ main/shortcuts.js | 14 ++++++++------ 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/main/ipc.js b/main/ipc.js index e5b0a3a8..a147d9cb 100644 --- a/main/ipc.js +++ b/main/ipc.js @@ -82,12 +82,12 @@ function init () { ipcMain.on('onPlayerOpen', function () { menu.onPlayerOpen() - shortcuts.registerPlayerShortcuts() + shortcuts.onPlayerOpen() }) ipcMain.on('onPlayerClose', function () { menu.onPlayerClose() - shortcuts.unregisterPlayerShortcuts() + shortcuts.onPlayerOpen() }) ipcMain.on('focusWindow', function (e, windowName) { diff --git a/main/menu.js b/main/menu.js index fe8d0f36..55fea7d3 100644 --- a/main/menu.js +++ b/main/menu.js @@ -5,6 +5,8 @@ module.exports = { onToggleFullScreen, onWindowHide, onWindowShow, + + // TODO: move these out of menu.js -- they don't belong here showOpenSeedFiles, showOpenTorrentAddress, showOpenTorrentFile, diff --git a/main/shortcuts.js b/main/shortcuts.js index 664d5fc3..1ff8c127 100644 --- a/main/shortcuts.js +++ b/main/shortcuts.js @@ -1,7 +1,7 @@ module.exports = { init, - registerPlayerShortcuts, - unregisterPlayerShortcuts + onPlayerClose, + onPlayerOpen } var electron = require('electron') @@ -19,11 +19,13 @@ function init () { localShortcut.register('CmdOrCtrl+Shift+F', menu.toggleFullScreen) } -function registerPlayerShortcuts () { - // Special "media key" for play/pause, available on some keyboards - globalShortcut.register('MediaPlayPause', () => windows.main.send('dispatch', 'playPause')) +function onPlayerOpen () { + // Register special "media key" for play/pause, available on some keyboards + globalShortcut.register('MediaPlayPause', function () { + windows.main.send('dispatch', 'playPause') + }) } -function unregisterPlayerShortcuts () { +function onPlayerClose () { globalShortcut.unregister('MediaPlayPause') } From 3d6da99e8e956e3e196a81d068ee79c400ef8075 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 20:03:25 +0200 Subject: [PATCH 5/7] Bug: Space key triggers power save block from torrent list Hitting Space from the torrent list should not cause power save to be blocked. --- renderer/index.js | 1 + 1 file changed, 1 insertion(+) diff --git a/renderer/index.js b/renderer/index.js index f7e076a9..67dfae14 100644 --- a/renderer/index.js +++ b/renderer/index.js @@ -378,6 +378,7 @@ function pause () { } function playPause () { + if (state.location.current().url !== 'player') return if (state.playing.isPaused) { play() } else { From 1deab08d38036f3ad30f20e88d27af6ec1b5821e Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 20:18:27 +0200 Subject: [PATCH 6/7] Playback menu: Add "Play/Pause" item The goal here is to remove shortcut handling from the renderer and unify it all in menu.js and shortcuts.ks (for alternate shortcuts). I would rather name it "Play" and change to "Pause" when video is playing, but Electron doesn't support this (yet). --- main/menu.js | 50 ++++++++++++++++++++++++++++++++++------------- main/shortcuts.js | 12 ++++++------ renderer/index.js | 2 -- 3 files changed, 42 insertions(+), 22 deletions(-) diff --git a/main/menu.js b/main/menu.js index 55fea7d3..f8054ab0 100644 --- a/main/menu.js +++ b/main/menu.js @@ -49,18 +49,6 @@ function toggleFloatOnTop (flag) { } } -function increaseVolume () { - if (windows.main) { - windows.main.send('dispatch', 'changeVolume', 0.1) - } -} - -function decreaseVolume () { - if (windows.main) { - windows.main.send('dispatch', 'changeVolume', -0.1) - } -} - function toggleDevTools () { log('toggleDevTools') if (windows.main) { @@ -73,6 +61,24 @@ function showWebTorrentWindow () { windows.webtorrent.webContents.openDevTools({ detach: true }) } +function playPause () { + if (windows.main) { + windows.main.send('dispatch', 'playPause') + } +} + +function increaseVolume () { + if (windows.main) { + windows.main.send('dispatch', 'changeVolume', 0.1) + } +} + +function decreaseVolume () { + if (windows.main) { + windows.main.send('dispatch', 'changeVolume', -0.1) + } +} + function onWindowShow () { log('onWindowShow') getMenuItem('Full Screen').enabled = true @@ -86,11 +92,15 @@ function onWindowHide () { } function onPlayerOpen () { + log('onPlayerOpen') + getMenuItem('Play/Pause').enabled = true getMenuItem('Increase Volume').enabled = true getMenuItem('Decrease Volume').enabled = true } function onPlayerClose () { + log('onPlayerClose') + getMenuItem('Play/Pause').enabled = false getMenuItem('Increase Volume').enabled = false getMenuItem('Decrease Volume').enabled = false } @@ -182,7 +192,9 @@ function getAppMenuTemplate () { type: 'separator' }, { - label: process.platform === 'windows' ? 'Close' : 'Close Window', + label: process.platform === 'windows' + ? 'Close' + : 'Close Window', accelerator: 'CmdOrCtrl+W', role: 'close' } @@ -256,6 +268,15 @@ function getAppMenuTemplate () { { label: 'Playback', submenu: [ + { + label: 'Play/Pause', + accelerator: 'CmdOrCtrl+P', + click: playPause, + enabled: false + }, + { + type: 'separator' + }, { label: 'Increase Volume', accelerator: 'CmdOrCtrl+Up', @@ -378,7 +399,8 @@ function getAppMenuTemplate () { } ) } - // Add "File > Quit" menu item for Linux distros where the system tray is broken + // Add "File > Quit" menu item so Linux distros where the system tray icon is missing + // will have a way to quit the app. if (process.platform === 'linux') { // File menu (Linux) template[0].push({ diff --git a/main/shortcuts.js b/main/shortcuts.js index 1ff8c127..552ce5e0 100644 --- a/main/shortcuts.js +++ b/main/shortcuts.js @@ -13,17 +13,17 @@ var menu = require('./menu') var windows = require('./windows') function init () { - // ⌘+Shift+F is an alternative fullscreen shortcut to the ones defined in menu.js. - // Electron does not support multiple accelerators for a single menu item, so this - // is registered separately here. + // Register alternate shortcuts here. Most shortcuts are registered in menu,js, but Electron does not support multiple shortcuts for a single menu item. localShortcut.register('CmdOrCtrl+Shift+F', menu.toggleFullScreen) + localShortcut.register('Space', () => windows.main.send('dispatch', 'playPause')) } function onPlayerOpen () { // Register special "media key" for play/pause, available on some keyboards - globalShortcut.register('MediaPlayPause', function () { - windows.main.send('dispatch', 'playPause') - }) + globalShortcut.register( + 'MediaPlayPause', + () => windows.main.send('dispatch', 'playPause') + ) } function onPlayerClose () { diff --git a/renderer/index.js b/renderer/index.js index 67dfae14..16cb6a3a 100644 --- a/renderer/index.js +++ b/renderer/index.js @@ -1155,8 +1155,6 @@ function onKeyDown (e) { } else { dispatch('back') } - } else if (e.which === 32) { /* spacebar pauses or plays the video */ - dispatch('playPause') } } From 5c9265fc99be27de0073cef04fef58dbe91e4633 Mon Sep 17 00:00:00 2001 From: Feross Aboukhadijeh Date: Wed, 11 May 2016 20:36:36 +0200 Subject: [PATCH 7/7] Move Escape keyboard shortcut to shortcuts.js --- main/menu.js | 1 + main/shortcuts.js | 6 +++++- renderer/index.js | 24 +++++++++--------------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/main/menu.js b/main/menu.js index f8054ab0..be02e166 100644 --- a/main/menu.js +++ b/main/menu.js @@ -57,6 +57,7 @@ function toggleDevTools () { } function showWebTorrentWindow () { + log('showWebTorrentWindow') windows.webtorrent.show() windows.webtorrent.webContents.openDevTools({ detach: true }) } diff --git a/main/shortcuts.js b/main/shortcuts.js index 552ce5e0..739f72eb 100644 --- a/main/shortcuts.js +++ b/main/shortcuts.js @@ -13,9 +13,13 @@ var menu = require('./menu') var windows = require('./windows') function init () { - // Register alternate shortcuts here. Most shortcuts are registered in menu,js, but Electron does not support multiple shortcuts for a single menu item. + // Alternate shortcuts. Most shortcuts are registered in menu,js, but Electron does not + // support multiple shortcuts for a single menu item. localShortcut.register('CmdOrCtrl+Shift+F', menu.toggleFullScreen) localShortcut.register('Space', () => windows.main.send('dispatch', 'playPause')) + + // Hidden shortcuts, i.e. not shown in the menu + localShortcut.register('Esc', () => windows.main.send('dispatch', 'escapeBack')) } function onPlayerOpen () { diff --git a/renderer/index.js b/renderer/index.js index 16cb6a3a..1e35e07f 100644 --- a/renderer/index.js +++ b/renderer/index.js @@ -96,9 +96,6 @@ function init () { // ...same thing if you paste a torrent document.addEventListener('paste', onPaste) - // ...keyboard shortcuts - document.addEventListener('keydown', onKeyDown) - // ...focus and blur. Needed to show correct dock icon text ("badge") in OSX window.addEventListener('focus', onFocus) window.addEventListener('blur', onBlur) @@ -256,6 +253,15 @@ function dispatch (action, ...args) { var mediaTag = document.querySelector('video,audio') if (mediaTag) mediaTag.pause() } + if (action === 'escapeBack') { + if (state.modal) { + dispatch('exitModal') + } else if (state.window.isFullScreen) { + dispatch('toggleFullScreen') + } else { + dispatch('back') + } + } if (action === 'back') { state.location.back() } @@ -1146,18 +1152,6 @@ function onPaste (e) { }) } -function onKeyDown (e) { - if (e.which === 27) { /* ESC means either exit fullscreen or go back */ - if (state.modal) { - dispatch('exitModal') - } else if (state.window.isFullScreen) { - dispatch('toggleFullScreen') - } else { - dispatch('back') - } - } -} - function onFocus (e) { state.window.isFocused = true state.dock.badge = 0