Problems with autocomplete and german keyboard layouts

[ejoebstl]

Alt + Any Keypress invokes the autocomplete in GraphiQL.

On a german keyboard, the curly brace { is typed by Alt + 8.

That means that every time you type a curly brace in GraphiQL, the autocomplete will open.

The most annoying case is when autocomplete is invoked when typing something like query test {, after which you would usually want a line break. When pressing enter, would select something from the autocomplete list.

I would suggest invoking autocomplete only on a more narrow combination, like Alt + Space.

[ejoebstl]

The fix would be quite simple by adding a further condition to this line of code.

Please let's discuss on the proposed changes and which key combination to use for autocomplete. I'm happy to create a PR afterwards.

[asiandrummer]

@ejoebstl - seems like I forgot to press "Comment" button a couple of days ago :p

I don't think I understand the use case - the autocompletion is invoked whenever a character is typed in GraphiQL. Also, for the case of query test {, GraphiQL should wrap { with } and not display autocompletion dialog. Is there anything I'm missing here? Does a German keyboard (or any other languages) have different keymaps as to the English keyboard?

Nonetheless, I don't think we want to allow autocompletion for narrower combinations. Just like how other IDEs support autocompletion, I think providing autocompletion suggestions to users whenever characters are being typed is more intuitive. Perhaps the issue here, like we said, is supporting more scenarios with different key maps.

[ejoebstl]

It's because the implementation checks (hardcoded) for a key combination using the shift key. In german layouts, the alt key is used for typing the '{'.

This is the code in question:

_onKeyUp = (cm, event) => {
    const code = event.keyCode;
    if (
      (code >= 65 && code <= 90) || // letters
      (!event.shiftKey && code >= 48 && code <= 57) || // numbers
      (event.shiftKey && code === 189) || // underscore
      (event.shiftKey && code === 50) || // @
      (event.shiftKey && code === 57) // (
    ) {
      this.editor.execCommand('autocomplete');
    }
  }

You can easily reproduce it by changing your keyboard layout and typing a '{'.

The problem I see is that GraphiQL falls back to show the autocompletion for every combination that is not hard-coded in the list above. There might be more of the mentioned cases with other keyboard layouts.

Most IDE's I know use Ctrl+Space, Shift+Space, Alt+Space or Cmd-Space for autocompletion. That is also done (properly) in GraphiQL here. The problem, as said, is the odd fallback.

[asiandrummer]

@ejoebstl I apologize for the late response. I understand the problem now - thanks for elaborating.

I think autocompleting for every key up event is intentional rather than a fallback, as it is how common IDEs provide autocompletion. I think the right approach here is to allow disabling the option to provide autocompletion for every keystroke as many common IDEs have the option to toggle it on/off. I'll comment on the diff for the further feedback.

Also, we should rethink the above hard-coded key maps for onKeyUp event, in the case that you wanted to enable autocompleting for every keystroke but you foresee cases such as the one you've encountered. I think I'm fine with enabling/disabling listening on a keyup event, and we can follow up with a more robust solution for achieving this.

Will comment on the diff as well.

[asiandrummer]

Actually, taking a closer look at the key maps, I think we should just check for the actual character typed in instead of using the keyCode, and resolve this issue easily. The primary intention for that code block is to limit the autocompletion firing for certain punctuators - such as '{', '}', '[', ']', and ')'. We should probably regex-match the keypresses intended to provide autocompletion for, or recognize the differences separately and prohibit them from firing autocompletion (mainly inspired by looking at some popular non-QWERTY keyboards).

Let me PR this out.

[asiandrummer]

Let me know if you have any other issues with this!