[BUG] Keymaster not unbinding keypresses.

[mattdeigh]

This is reproducible on the demo site running 0.16.12.

In the console, run editor.Keymaps.removeAll() and try to delete a component. It does delete the component, and I don't believe it should. It seems like the other Keymaps are unbinding fine.

I'm not sure if this is an issue with grapesjs or keymaster. Noticed this when I was adding a custom keymap for the arrow keys and noticed it wasn't getting cleaned up when editor.destroy() was called.

I did notice I could unbind the delete command like this:

editor.Keymaps.keymaster.unbind('backspace');
editor.Keymaps.keymaster.unbind('delete');

I'm wondering if the unbind command in keymaster was meant to handle an individual key instead of multiple ones in the case of backspace but the copy/paste/undo/redo are working correctly.

Looks like a simple solution could be splitting keymap.keys string here and unbinding the keys individually. Also noticed keymaster hasn't been updated in a while so it might be worth switching it out. Willing to open a PR for this.

[artf]

I'm wondering if the unbind command in keymaster was meant to handle an individual key instead of multiple ones in the case of backspace but the copy/paste/undo/redo are working correctly.

Well, it works with other keymaps so I'd say it handles that option, but have no idea what is wrong with backspace, delete (with delete, backspace it even raises an error ???)

Looks like a simple solution could be splitting keymap.keys string here and unbinding the keys individually

Agree, but what about your case I was adding a custom keymap for the arrow keys? Does it work with the split? The PR is super welcome :)

Also noticed keymaster hasn't been updated in a while so it might be worth switching it out

yeah, I've noticed that too, but that might require a bit of work probably. Are you already aware of any good alternative?

[mattdeigh]

Man... I looked through Keymaster and that unbind function handles multiple keys, so the issue must be somewhere else. I didn't notice anything glaring when I looked. No errors show up when I try to unbind the key.

I'm using GrapesJS in a single page app so I need to destroy GrapesJS and reinit it again without the browser reloading. This is how I found the bug initially. When you destroy, reinit, and use the delete or backspace keys, the callback is called on the original editor object and not the new one which throws an error.

I'm running this before I call editor.destroy() and it seems to work fine. Have no idea why Keymaster isn't working.

const keys = 'up, down, left, right, shift+up, shift+down, shift+left, shift+right, backspace';
keys.split(/, ?/).forEach((key) => {
    this.editor.Keymaps.keymaster.unbind(key);
 });

I've not used any key mapping plugins before, but I'd be more than happy with opening a PR. Are you good with the string split approach? Looks like Keymaster does the same thing i was doing.

[artf]

Are you good with the string split approach?

Sure, let's see if it works