command-pal icon indicating copy to clipboard operation
command-pal copied to clipboard

After destroying command-pal command hotkeys still trigger.

Open rouilj opened this issue 2 years ago • 0 comments

Using cp-advanced/index.html. If I click 'Destroy Instance' and then press ctrl+space and watch the console I see:

opened 
Object { e: {} }
e: Object {  }
<prototype>: Object { … }

I I then type ctrl+d I see:

opened  Object { e: {} }
exec Object { e: {…} }
​    e: Object { name: "Goto About", shortcut: "ctrl+d", handler: handler()}
​<prototype>: Object { … }

Looks like destroy needs to remove the hotkey and all the shortcuts it added using hotkeys.js. hotkeys.unbind() should work.

However if the app calling command-pal is also using hotkeys, this would wipe out their hotkeys as well.

You could loop through each hotkey/shortcut and unbind it.

However using hotkeys scope could provide another solution.

// register shortcuts in command-pal scope
export function setAllShortCuts(items, onExecCallback) {
  items
    .filter((item) => item.shortcut)
    .map((item) => {
      hotkeys.unbind(item.shortcut, 'command-pal' );
      hotkeys(item.shortcut, { scope: "command-pal"}, async function (e) {
        e.preventDefault();
        onExecCallback(item)
      });
    });

Use the 'command-pal' scope similarly when the hotkey that activates command pal.

Add:

hotkeys.setScope('command-pal');

to initShortCuts() to activate the command-pal scoped hotkey.

This will make all the command-pal shortcuts only work while hotkeys.getScope() == 'command-pal'.

When calling CommandPal.destroy, use hotkeys.deleteScope('command-pal') to delete all keybidings freeing all the resources. It should leave untouched hotkeys in other scopes (including the default "all" scope).

Arguably we could expose an option hotkeysScope when calling CommandPal to allow the user to control this better (e.g. add the hotkey/shortcuts to the 'all' scope).

Also scope allows the user to disable command-pal hotkeys if they are in a modal where they don't want command-pal to be active. (Although adding enable/disable methods would probably be a better way to expose this than having the user use hotkeys directly.)

I define this as done when I can't use the hotkeys after deleting the command-pal instance. The rest of the stuff is not key to this ticket.

rouilj avatar Feb 12 '23 22:02 rouilj