Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can I make non global accelerators without putting things in a menu? #1334

Closed
maxkorp opened this issue Mar 30, 2015 · 37 comments
Closed

Can I make non global accelerators without putting things in a menu? #1334

maxkorp opened this issue Mar 30, 2015 · 37 comments

Comments

@maxkorp
Copy link

maxkorp commented Mar 30, 2015

I'd like to add some hotkeys for events in my app, and accelerators seem like the natural way to do that, but some of these things don't make sense in a menu. Is there a way to use accelerators without registering a global shortcut or a menu? I'd rather not have hidden menus.

@anaisbetts
Copy link
Contributor

@maxkorp You can capture key shortcuts via keyup on document.

@maxkorp
Copy link
Author

maxkorp commented Mar 30, 2015

Yeah, I was just hoping to use some of the super nice 'CmdOrCtrl+X' type declarations Accelerators offer, rather than using keycodes etc. Digging around to see if theres a way I can instantiate an accelerator manually and manipulate that.

@maxkorp maxkorp changed the title Can I make on global accelerators without putting things in a menu? Can I make non global accelerators without putting things in a menu? Mar 30, 2015
@anaisbetts
Copy link
Contributor

I'd buy that feature actually, because one of our biggest bugs right now is trying to deal with the non-US keyboard web debacle in Chrome; even when you know the keyboard layout you still have to have a massive list of tables

@maxkorp
Copy link
Author

maxkorp commented Mar 30, 2015

As far as I can tell, theres no way currently on the js side to access the accelerator class. I'll take a look at enabling that, but I'm not familiar enough with the code to give a real opinion. From what I see, i'd need to add something in atom/browser/api, then add it to the bindings as well. That seem roughly correct-ish?

@anaisbetts
Copy link
Contributor

@maxkorp Yeah, classes get automatically bound via this framework called mate but I don't understand much about it :(

@maxkorp
Copy link
Author

maxkorp commented Mar 30, 2015

Hrm, I'll take a crack at it. No clue if being able to instantiate an accelerator would help, even, but it's worth a shot.

@maxkorp
Copy link
Author

maxkorp commented Mar 30, 2015

Hrm, unfortunately a little more complex than I'd hoped but I have a plan of attack. IMO, 2 separate features would be good, and I'd be happy to start at them if you guys think they're good ideas.

  1. It would be nice to be able to just pass a key[up/down/press] event and an accelerator to a function and see if it lines up. I'd probably start here.

  2. See next comment below
    Create a LocalShortcutHandler setup, akin to globalshortcuthandler. This needs to be either enableable or disableable (using its own version of GlobalShortcutListener::SetShortcutHandlingSuspended that respects observers), or it needs to automatically enable/disable based on some sort of window focus. Any thoughts?

@maxkorp
Copy link
Author

maxkorp commented Mar 31, 2015

FYI: I have a temporary workaround, where I set up a small helper with a map that stores shortcuts by keystroke (and in my case, by some other stuff dependent on my apps architecture). Whenever a window focuses, all of the shortcuts are registered. Whenever a window blurs, it checks if any window has focus, and if one does, it leaves them enabled. If no window has focus anymore, then the shortcuts are unregistered. Clearly, that could be tweaked to work on a per window basis, etc, to fit someones needs.

Does anyone have any opinion on how they'd like to see that implemented in atom shell? I'm still leaning towards putting something in API, but I might put a compatible js/coffee solution in first, then move over to the native way next.

My thoughts are as follows:
Local shortcuts can be cross window, or per window (so they are only active if a specific window has focus, or any window has focus).

// This is global, if any window in the app is focused it works
LocalShortcut.register(accelerator, callback);

// This is only active if someWindow has focus
LocalShortcut.register(accelerator, callback, someWindow);

Does anybody have any thoughts on that? Something missing? More functionality than wanted? Is the idea a welcome one (I know something to fill the whole is wanted, but does this seem like a good fit)?
Is everyone cool with a js/coffee solution first and a native/mate solution second?

@zcbenz
Copy link
Contributor

zcbenz commented Apr 1, 2015

The accelerators are handled natively in NativeWindow::HandleKeyboardEvent, it is easy to provide a API on Linux/Windows since we handle accelerators registration/activation completely on atom-shell's side, however on OS X it would need much more work because we currently rely on application menu to do the accelerator stuff.

A js/coffee solution sounds good to me, but we should probably make it a third party module instead of being in atom-shell core.

@etiktin
Copy link
Contributor

etiktin commented Aug 19, 2015

@maxkorp, did you end up publishing a module for this?

@maxkorp
Copy link
Author

maxkorp commented Aug 20, 2015

Unfortunately no. I have some ideas, but I haven't had time.

@zcbenz
Copy link
Contributor

zcbenz commented Aug 24, 2015

For people looking at this issue, most of you can just use a JavaScript library like https://github.com/atom/atom-keymap or https://craig.is/killing/mice.

@anaisbetts
Copy link
Contributor

This solution is gonna be a real bummer with international keyboards though until https://code.google.com/p/chromium/issues/detail?id=227231 lands in Electron

@sindresorhus
Copy link
Contributor

👍 In my app, I need to do something when a shortcut combination is triggered, but only when my app is focused. Currently, I use global shortcuts and subscribe/unsubscribe depending on whether the window is focused, but support for local shortcuts would be much better.

@maxkorp
Copy link
Author

maxkorp commented Sep 17, 2015

@sindresorhus has that been working consistantly for you? I was having issues with those events not firing when switching was caused by electon itself, for example by the dev tools getting focused by a breakpoint.

@frankhale
Copy link
Contributor

I'm using keymaster in my app to declare key combos and events to execute when they are pressed. It seems to be enough for my needs. Maybe it can work for you?

https://github.com/madrobby/keymaster

@sindresorhus
Copy link
Contributor

has that been working consistantly for you?

Yes, but I just started using it.

I was having issues with those events not firing when switching was caused by electon itself, for example by the dev tools getting focused by a breakpoint.

Create a minimal testcase and open a new issue.

Maybe it can work for you?

No, as I don't control the browser contents. I need it for electron-debug. It also doesn't solve the already mentioned issues in this thread.

@lipis
Copy link

lipis commented Sep 28, 2015

How is that issue going.. how soon do you think we'll be able to use accelerators without the menu?

@parro-it
Copy link
Contributor

parro-it commented Nov 3, 2015

I published a module to handle local window shortcuts.
It is actually a lightway wrapper around globalshortcuts that register/unregister all window shortcuts on focus/blur.
In case it can help you, it's published here.

feross added a commit to webtorrent/webtorrent-desktop that referenced this issue Mar 6, 2016
Uses https://npmjs.com/package/electron-localshortcut to workaround a
bug in Electron (electron/electron#1334).

We can remove `electron-localshortcut` once that bug is fixed.
feross added a commit to webtorrent/webtorrent-desktop that referenced this issue Mar 6, 2016
Uses https://npmjs.com/package/electron-localshortcut to workaround a
bug in Electron (electron/electron#1334).

We can remove `electron-localshortcut` once that bug is fixed.
@feross
Copy link
Contributor

feross commented Jun 7, 2016

@parro-it The issue with electron-localshortcut is when the window actually has focus. If you listen for the Esc key, then click an menu item on OS X (for example), then Esc won't work for de-focusing it. It's subtle.

@paulcbetts You're right.

@parro-it
Copy link
Contributor

parro-it commented Jun 7, 2016

Oh yes I didn' t check but that is for sure because that action doesn' t trigger lostfocus event on browserwindow instance.
I think there is no easy solution to the problem...

@pmario
Copy link

pmario commented Jan 23, 2017

Any news on this one?

@connorbode
Copy link

connorbode commented Feb 1, 2017

@pmario I just solved it as follows:

const {app, BrowserWindow, globalShortcut} = require('electron')

function registerShortcuts () {
  globalShortcut.register(..)
}

function unregisterShortcuts () {
  globalShortcut.unregister(..)
}

app.on('ready', () => {
  win = new BrowserWindow({ .. })
  registerShortcuts()
  win.on('focus', registerShortcuts)
  win.on('blur', unregisterShortcuts)
})

I only use one window but it can be modified for multiple to check if at least one window is still in focus.

Edit: Added a registerShortcuts() to the ready callback to load shortcuts when the app is first run.

@poiru
Copy link
Contributor

poiru commented Jun 25, 2017

You can use before-input-event to catch and handle custom shortcuts that are not visible in the menu. If you want to attach it to all web contents, use something like:

app.on('web-contents-created', function (event, wc) {
  wc.on('before-input-event', function (event, input) {
    if (input.key === 'x' && input.ctrl && !input.alt && !input.meta && !input.shift) {
      // Do something for Ctrl-X
      event.preventDefault()
    }
  })
})

@parro-it electron-localshortcut should probably use the above approach.

@codebytere
Copy link
Member

Given the workarounds above, we believe this module is better implemented in userland (ex. the excellent electron-localshortcut and thus i'm going to label this a wontfix.

@omeid
Copy link

omeid commented Mar 22, 2019

I think there is good value in having a consistent API for shortcuts inline with Menu Accelerators and GlobalShortcuts.

@madprops
Copy link

madprops commented Jun 6, 2019

This should be implemented. Most of the time you'd want local shortcuts. Making event listeners on BrowserWindows don't work if there are webviews in use that capture focus. And even if creating a menu worked it makes no sense that a menu has to be created to enable keyboard shortcuts (which are a very core feature of most applications). I'm saying, this should be handled by Electron itself and not some third party library.

@Praveer1981
Copy link

@poiru

You can use before-input-event to catch and handle custom shortcuts that are not visible in the menu. If you want to attach it to all web contents, use something like:

app.on('web-contents-created', function (event, wc) {
  wc.on('before-input-event', function (event, input) {
    if (input.key === 'x' && input.ctrl && !input.alt && !input.meta && !input.shift) {
      // Do something for Ctrl-X
      event.preventDefault()
    }
  })
})

@parro-it electron-localshortcut should probably use the above approach.

Unfortunately, It did not work for me. It never allows me to press Ctrl+x key.

@KLA6
Copy link

KLA6 commented Jun 27, 2020

Just for someone who may have a same problem like me in 2020...

BrowserWindow.getFocusedWindow()

This returns the current focused BrowserWindow object.

globalShortcut.register( 'F11', () => {
  BrowserWindow.getFocusedWindow().setFullScreen( ! BrowserWindow.getFocusedWindow().isFullScreen() )
} )

E.g. I could make a full screen shortcut as the above way.

@aleksey-hoffman
Copy link

aleksey-hoffman commented Oct 25, 2020

Here's a working flexible solution that allows you to manually define non-global shortcuts for focusedWindow without putting them into app menu:

app.on('web-contents-created', (webContentsCreatedEvent, webContents) => {
  webContents.on('before-input-event', (beforeInputEvent, input) => {
    // console.log('Main console::', input)
    const { code, alt, control, shift, meta } = input
    // Shortcut: toggle devTools
    if (shift && control && !alt && !meta && code === 'KeyI') {
      BrowserWindow.getFocusedWindow().webContents.toggleDevTools({ mode: 'detach' }) 
    }
    // Shortcut: window reload
    if (shift && control && !alt && !meta && code === 'KeyR') {
      BrowserWindow.getFocusedWindow().reload()
    }
  })
})

Also works when you disable the app menu with Menu.setApplicationMenu(null)

@eterzago
Copy link

eterzago commented Dec 16, 2020

Wouldn't a native solution be infinitely better than any hacky workaround? As another user said, keyboard shortcuts are a core feature of most applications...
Edit: Also what this guy says about how registering/unregistering globalShortcuts on focus/blur (which is what electron-localshortcut does) leads to unwanted behaviors.

@parro-it
Copy link
Contributor

Edit: Also what this guy says about how registering/unregistering globalShortcuts on focus/blur (which is what electron-localshortcut does) leads to unwanted behaviors.

By the way, the current version of electron-localshortcut does not register globalShortcuts
but listen to a keyboard event on the BrowserWindow instance. It's not an hack anymore!

@eterzago
Copy link

@parro-it That's great to know, thanks for pointing it out!

@mlrv
Copy link

mlrv commented Dec 24, 2020

For anyone interested in this issue, I just published electron-shortcuts. I needed a safe solution to local shortcuts and decided to put something together, which ended up becoming a tiny, safety-focused npm library 😄

It's definitely still early stage, so contributions and feedback of any kind are definitely welcome!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests