CodeHintManager needs to be smarter about key processing

[redmunds]

This was reported by @zoufahl when developing CSS Code Hints.

In CodeHintManager handleKeyEvent function is this block of code:

  } else if (event.type === "keyup") {
    if (_inSession(editor)) {
      if ((event.keyCode !== 32 && event.ctrlKey) || event.altKey || event.metaKey) {
        // End the session if the user presses any key with a modifier (other than Ctrl+Space).
        _endSession();
      } else ...

The problem is when the code hint session starts on keyup and the user is trying to insert a "{" from a German keyboard (which is Alt+8). Maybe a keybinding lookup would make more sense here. Consult @RaymondLim.

[pthiess]

Reviewed

[RaymondLim]

@iwehrman I don't think we need the new code that you added to check for modifier keys. I don't see any issue after removing the code. Please let me know the specific issue that you want to resolve it with this code.

Otherwise, we do really need to remove the code since lots of keyboard layouts require users to type characters like /[]{};:<> with modifier keys.

[redmunds]

I think the reason it was added was so that keyboard shortcuts do not get caught. I think that's a different issue.

[RaymondLim]

If it is the case, I would like to know the scenario where the keyboard shortcuts gets caught. My guess is that Ian just wants to dismiss code hints when a keyboard shortcut is pressed. We didn't do that in the old design and the code hints keep showing.

[iwehrman]

The original idea was to close the hinting session on "complex" editor changes, as can occur as a result of, e.g., cut or paste operations. Perhaps we can find another way to detect such changes.

[RaymondLim]

Thanks. I'm removing this particular code so that users can type any characters that require modifier keys.

[redmunds]

Confirmed. Closing.

[RaymondLim]

Reopen it since pull request #2756 introduced #2820 and we have to revert my changes. Since we currently don't have any code hints that have to be triggered with {, I am lowering this to low priority and removing sprint 20 milestone.

[redmunds]

This does not affect any current code hints. Closing.