moose-team / friends

:tv: P2P chat powered by the web.
http://moose-team.github.io/friends/
MIT License
3.17k stars 341 forks source link

System keyboard shortcuts not working #123

Closed bcomnes closed 7 years ago

bcomnes commented 9 years ago

Its possible the tab completion feature knocked out the system level keyboard shortcuts like copy paste.

ungoldman commented 9 years ago

Confirmed on a mac running yosemite, shortcuts like A (select all) and H (hide current application) are unresponsive in the context of the electron app.

Flet commented 9 years ago

I wonder if this is an electron bug.

I can copy, paste, and select all via keyboard shortcuts in linux just fine.

Flet commented 9 years ago

Maybe menus are required? https://github.com/atom/electron/issues/571

yoshuawuyts commented 9 years ago

Running into a similar issue working on another electron app. I know nw.js doesn't support system level keyboard shortcuts default, but it's working out of the box in vmd@latest.

Everything appears to work as expected in electron@0.24, whilst electron@0.26 doesn't work. Opening an issue for this on electron.

bcomnes commented 9 years ago

I suspect that this relates to https://github.com/atom/electron/issues/1718

yoshuawuyts commented 9 years ago

@bcomnes yeah, definitely seems to be the case. Considering that the change was menu-related I'm curious if enabling menu's would help solve it?

This is super annoying, having atom-shell work like a browser out of the box was neat :(

yoshuawuyts commented 9 years ago

derp, @Flet's suggestion works. Implementing a menu solves this:

edit: there's some stuff with BrowserWindow that needs solving. Figuring it out now, will update once fixed (or ping me if I forget to update).

menu.js

const remote = require('remote')

const MenuItem = remote.require('menu-item')
const Menu = remote.require('menu')

module.exports = createMenu

// create menu
// null -> null
function createMenu() {
  const menu = Menu.buildFromTemplate(menuTempl())
  Menu.setApplicationMenu(menu)
}

// create a menu template
// null -> obj
function menuTempl () {
  const menu = []
  menu.push({
    label: 'Electron',
    submenu: [
      {
        label: 'About Electron',
        selector: 'hide:'
      }
    ]
  })
  menu.push({
    label: 'Edit',
    submenu: [
      {
        label: 'Undo',
        accelerator: 'Command+Z',
        selector: 'undo:'
      },
      {
        label: 'Redo',
        accelerator: 'Shift+Command+Z',
        selector: 'redo:'
      },
      {
        type: 'separator'
      },
      {
        label: 'Cut',
        accelerator: 'Command+X',
        selector: 'cut:'
      },
      {
        label: 'Copy',
        accelerator: 'Command+C',
        selector: 'copy:'
      },
      {
        label: 'Paste',
        accelerator: 'Command+V',
        selector: 'paste:'
      },
      {
        label: 'Select All',
        accelerator: 'Command+A',
        selector: 'selectAll:'
      },
    ]
  })
  menu.push({
    label: 'View',
    submenu: [
      {
        label: 'Reload',
        accelerator: 'Command+R',
        click: function() { BrowserWindow.getFocusedWindow().reloadIgnoringCache(); }
      },
      {
        label: 'Toggle DevTools',
        accelerator: 'Alt+Command+I',
        click: function() { BrowserWindow.getFocusedWindow().toggleDevTools(); }
      },
    ]
  })
  menu.push({
    label: 'Window',
    submenu: [
      {
        label: 'Minimize',
        accelerator: 'Command+M',
        selector: 'performMiniaturize:'
      },
      {
        label: 'Close',
        accelerator: 'Command+W',
        selector: 'performClose:'
      },
      {
        type: 'separator'
      },
      {
        label: 'Bring All to Front',
        selector: 'arrangeInFront:'
      },
    ]
  })
  return menu
}

Links

Flet commented 9 years ago

@yoshuawuyts cool! I was going to tinker here too, but looks like you're nailing it :+1:

yoshuawuyts commented 9 years ago

It does need a server component in order to handle the BrowserWindow event. Probably just need to send back events through the ipc thing.

yoshuawuyts commented 9 years ago

Solved it, code below. If for some reason it's undesirable to expose a devTools in the menu it's also possible to use electron-debug.

edit: definitely needs adjusting before being usable in friends, but it should be sufficient to base a patch on. (:

event-server.js

const BrowserWindow = require('browser-window')
const app = require('app')
const ipc = require('ipc')

module.exports = eventServer

// create a server that handles
// client events
// null -> null
function eventServer() {
  ipc.on('reload', function () {
    BrowserWindow.getFocusedWindow().reloadIgnoringCache(); 
  })

  ipc.on('toggleDevTools', function () {
    BrowserWindow.getFocusedWindow().toggleDevTools(); 
  })

  ipc.on('quit', function () {
    app.quit()
  })
}

menu.js

const remote = require('remote')
const ipc = require('ipc')

const MenuItem = remote.require('menu-item')
const Menu = remote.require('menu')

module.exports = createMenu

// create menu
// null -> null
function createMenu() {
  const menu = Menu.buildFromTemplate(menuTempl())
  Menu.setApplicationMenu(menu)
}

// create a menu template
// null -> [obj]
function menuTempl () {
  const menu = []
  menu.push({
    label: 'Electron',
    submenu: [
      {
        label: 'About Electron',
        selector: 'hide:'
      },
      {
        label: 'Quit Electron',
        accelerator: 'Command+Q',
        click: function() { ipc.send('quit') }
      }
    ]
  })
  menu.push({
    label: 'Edit',
    submenu: [
      {
        label: 'Undo',
        accelerator: 'Command+Z',
        selector: 'undo:'
      },
      {
        label: 'Redo',
        accelerator: 'Shift+Command+Z',
        selector: 'redo:'
      },
      {
        type: 'separator'
      },
      {
        label: 'Cut',
        accelerator: 'Command+X',
        selector: 'cut:'
      },
      {
        label: 'Copy',
        accelerator: 'Command+C',
        selector: 'copy:'
      },
      {
        label: 'Paste',
        accelerator: 'Command+V',
        selector: 'paste:'
      },
      {
        label: 'Select All',
        accelerator: 'Command+A',
        selector: 'selectAll:'
      },
    ]
  })
  menu.push({
    label: 'View',
    submenu: [
      {
        label: 'Reload',
        accelerator: 'Command+R',
        click: function() { ipc.send('reload') }
      },
      {
        label: 'Toggle DevTools',
        accelerator: 'Alt+CmdOrCtrl+I',
        click: function() { ipc.send('toggleDevTools') }
      },
    ]
  })
  menu.push({
    label: 'Window',
    submenu: [
      {
        label: 'Minimize',
        accelerator: 'Command+M',
        selector: 'performMiniaturize:'
      },
      {
        label: 'Close',
        accelerator: 'Command+W',
        selector: 'performClose:'
      },
      {
        type: 'separator'
      },
      {
        label: 'Bring All to Front',
        selector: 'arrangeInFront:'
      },
    ]
  })
  return menu
}