Disable keyboard shortcut mapping when showing Edit[..]Dialog

[ralphwetzel]

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Proposed changes

As explained in this remark: Due to the fact, that the keyboard shortcut mapping was left enabled when showing the various edit dialogs, it was

• impossible to create e.g. the character @ via option + l on Mac.
• possible to trigger - by mistake - keyboard shortcuts while editing e.g. node properties.

This PR provides a fix for this issue for

• EditDialog,
• EditConfigNodeDialog,
• EditFlowDialog,
• EditSubflowDialog,
• EditGroupDialog

Checklist

• I have read the contribution guidelines
• For non-bugfix PRs, I have discussed this change on the forum/slack team.
• I have run grunt to verify the unit tests pass
• I have added suitable unit tests to cover the new/changed functionality

[coveralls]

Coverage decreased (-0.006%) to 68.228% when pulling d815233 on ralphwetzel:master into 1f5588b on node-red:master.

[knolleary]

The downside of this approach is it disables the keyboard shortcuts we define for confirming (or canceling) the edit dialog.

I think a slightly different approach is needed - to try to ensure the shortcuts only apply in the right contexts. I have a couple ideas on that and will have a play.

[ralphwetzel]

Fair comment.
Shall I make a proposal - or do you intend to take over this task... ?

[knolleary]

It depends on the proposal 😉

If you have some thoughts, without spending too much time in the code, then please do share so we can see if they are in the right direction.

Off the top of my head, we could add some special handling for when the dialogs are open to restrict which key handlers are matched to only those that have their scope set to something particular.

[ralphwetzel]

After some time spent understanding the code structure, I have the impression, that no major change or any special handling of different cases is necessary as there's already a scoping mechanism implemented that references against DOM classes.

This mechanism needs just a bit of tweaking

• when it's resolving the keyEvent

node-red/packages/node_modules/@node-red/editor-client/src/js/ui/keyboard.js

Lines 268 to 271 in 1f5588b

	} else if (Object.keys(handler).length > 0) {
	partialState = handler;
	evt.preventDefault();
	return null;

• to preventDefault the event handling for the partialState scenario only if there's a potential handler defined - drafted as

} else if (Object.keys(handler).length > 0) {
    let depth = -1;
    for (h in handler) {
        let d = matchHandlerToEvent(evt,handler[h]);
        if (d > -1 && d < depth) {
            depth = d;
        }
    }
    if (depth > -1) {
        partialState = handler;
        evt.preventDefault();    
    }
    return null;
} ...

The rest of the job means to setup appropriately the shortcut definitions in keymap.json ... where the current setup puts a bunch of things that are workspace related (unnecessarily) into the global scope - e.g. alt-w to hide the current flow.

[ralphwetzel]

As proposed before:

• Too blunt disable/enable logic was reverted.
• preventDefault kicks in only in case there's a potential combined handler defined
• keymap.json re-ordered & an additional scope added - representing the sidebar editor stack