parro-it/electron-localshortcut

Not able to unregister shortcuts

akashnimare opened this issue · 7 comments

I'm not able to use the local shortcuts correctly. On closing window the shortcuts are still registered and throws error. I'm using it like this. @parro-it can you suggest me where I'm going wrong?

This module does not automatically unregister the callback on window close, you actually have to do it yourself.

It could be a useful feature to have, would you main do a PR?

@parro-it should I use it something like -

app.on('window-all-closed', () => {
	electronLocalshortcut.unregisterAll(mainWindow);
});
app.on('will-quit', () => {
electronLocalshortcut.unregisterAll(mainWindow);
})

@parro-it
Not sure if I understand this issue correctly. It seems that the problem is, registered callbacks won't be automatically released after the window it registered with is closed or destroyed.
I am using a lazy way to have a quick solution (I don't meet any further problem now).

electronLocalshortcut.register(win, 'ESC', () => {
    electronLocalshortcut.unregisterAll(win)
    win.close()
  })

I am not very sure if it is a best practice.
And if this is what you meant to have a PR, I will try to do it later.

It seems that the problem is, registered callbacks won't be automatically released after the window it registered with is closed or destroyed.

Exactly!

I am not very sure if it is a best practice.

If your window has the close button showed, you're not unregistering the shortcut if the user close the window using it. You better listen for the close event of the window, and unregisterAll from that handle. This way, you cover both cases...

If your window is only closable using ESC shortcuts, then you're method is perfectly fine.

And if this is what you meant to have a PR, I will try to do it later.

Great! Just listen to close event of every window, and unregisterAll from there...
Thank you! 😄

Solved by @Kulbear in #9

My pleasure :D
That's not a lot of work, your package has given me a quick solution for my project.

@Kulbear: I'm very glad to hear this!