`eventToHotkeyString` inconsistent `shift` key behaviour on Mac

[khiga8]

eventToHotKeyString outputs ? event as Shift+?

Since the updates in the latest release which include a logic change to address a different bug, eventToHotkeyString is includingShift unexpectedly.

? ==> Shift+? ! ==> Shift+!

Can we revert the logic to what it was before?

Related PRs:

• https://github.com/github/hotkey/pull/40
• https://github.com/github/hotkey/pull/41
• https://github.com/github/hotkey/pull/43

[theinterned]

Moving form a convo from another repo:

Based off this eventToHotkeyString test, it doesn't look like an uppercase alpha character is necessarily expected

Hrm that test is simulating the events though. Maybe we need to add playwright or something to that project and do some real browser tests? I will definitely test this carefully when I work on it to figure out exactly what's actually happening in the browser.

[theinterned]

For helpful background; in https://github.com/github/hotkey/pull/43 @keithamus states that "on macOS the "Meta+Shift" plane is always lowercase". I just checked and this does appear to be the case

This is also true of special characters

I think what this means is that:

1. ? is a valid shortcut if we want to leave the Shift+ out of the string and is equivalent to Shift+?
2. But combined with the metaKey, that ? key could be Meta+/ on Mac. So, in order to specify that shiftKey is pressed, Meta+Shift+/ would be the required shortcut.

This leads me to think, that we should continue to require the Shift key in the keyboard string. What do you think @khiga8

[theinterned]

Notice above that when the Control+Meta+Shift are pressed together the result is an upper case event.key! 🤔

[theinterned]

This is all very complicated. Here are some givens:

• I use "logical shortcut" to mean the shortcut string used by the hotkey library: the one returned from eventToHotkeyString.
• I use "humanized shortcut" to mean the string we want to display to the user.
  • This string sometimes has some semantic expectations: For example ? to open a help dialog, Command+c to copy.
  • This string sometimes has some mechanical expectations: for example WASD keys for navigation in a game, Command+v to paste.

Given

1. The same event.key can appear as a a different character on Mac vs Windows and Linux depending on Command / Shift etc making it unreliable to map shortcuts using event.key as the "humanized" character for a shortcut.
2. We can not determine the user's keyboard layout making it difficult to map from event.code to a desired "humanized" character (eg. Shift+Slash == ? is not true everywhere).
3. There is a proposed KeyboardLayoutMap and getLayoutMap spec https://wicg.github.io/keyboard-map/#h-keyboard-getlayoutmap that will allow translation from US / QWERTY event.code to other layouts for display purposes.
4. Without the KeyboardLayoutMap, at minimum we would require the user to tell us their keyboard layout so that we can translate event.code to a physical labeled key on their keyboard.
5. The w3c event.code gives the code as the physical key on a standard US QWERTY keyboard.

Then

1. Since event.key is not the same on every user's keyboard we can not define keyboard shortcus using event.key: so we must use event.code.
2. Since we do not always know the user's keyboard layout, we can not define keyboard shortcuts in a "universal" way with respect to "humanized" characters: so we can not support a shortcut that uses the same humanized character regardless of layout.
   1. For example, if we say that the shift+? hotkey is represented as Shift+Slash (event.keyShift + event.code) would result in the "humanized" shortcut shift+Plus on French AZERTY keyboards.
3. Since we must use event.code, and event.code represents a standard US QWERTY keyboard, we must store shortcuts as US QWERTY.
4. In order to "localize" this for other regions, we should separate the display of "humanized" shortcut strings as a formatter applied to the logical shortcut.
   1. The formatting should default to assuming US QWERTY.
   2. A keyboard layout should be passed to the formatter to allow another humanizing to another layout.
   3. The passed keyboard layout could come from the KeyboardLayoutMap API, or could be supplied to by the user via UI (a select for example).

Some implications from this

1. Use https://wicg.github.io/keyboard-map/#h-keyboard-getlayoutmap
2. Maybe polyfill it?
3. we can ask the user for their layout if the API is not available and store it in local storage .
4. separate storage of logical event.code from display of humanized event.key
5. store key as event.code
6. display key localized for user's keyboard layout using layout map
7. Maybe have UI to capture key codes
8. Store as QWERTY keyCode
9. show QWERTY locale by default
10. provide UI for user to select their keyboard layout (AZERTY, DVORAK etc.)

[theinterned]

Here is my proposed "logical" hotkey format:

1. (Control+)
2. (Alt+)
3. (Meta+)
4. (Shift+)
5. event.code

Examples

1. To represent the key combination ctrl+alt+cmd+shift+/ we would write Control+Alt+Meta+Shift+Slash.
2. To represent shift+cmd+k we would write Shift+Meta+KeyK
3. To represent cmd+space we would write Meta+Space
4. To represent the key sequence g n, we would write KeyG KeyN

Humanizing

We then would want to provide a formatting function that could show the above logical representations in a human readable form. This ideally would use the Keyboard.getLayoutMap() API.

humanizeHotkey("Control+Alt+Meta+Shift+Slash") === "ctrl+alt+cmd+?"
humanizeHotkey("Shift+Meta+KeyK") === "cmd+K"
humanizeHotkey("Meta+Space") === "cmd+space"
humanizeHotkey("KeyG KeyN") === "g n"
humanizeHotkey("KeyG Shift+KeyN") === "g N"

This helper could take a keyboard layout (eg. DVORAK, AZERTY...) with QWERTY as default for when Keyboard.getLayoutMap() is not universally available.

humanizeHotkey("Control+Alt+Meta+Shift+Slash", { layout: "DVORAK"}) === "ctrl+alt+cmd+Z"
humanizeHotkey("KeyG Shift+KeyN", { layout: "DVORAK"}) === "i B"

event.code "Slash" maps to the "z" key on a DVORAK keyboard

We could extend this idea to take other options. For example, we could format Meta as cmd on Mac, win on Windows and meta on Linux. We could define an option to render apple's shortcut symbols (⌃ ⌥ ⌘ ⇧) etc...

"Localized" shortcuts for different keyboard layouts

It would then be to an implementer to provide alternate shortcuts to support different semantic hotkeys. For example, in order to preserve the semantics of the "ctrl+?" (Control+Shift+Slash) shortcut as a way to bring up a help menu, the implementer would have to provide a "localized" Control+Shift+BracketLeft (BracketLeft is the event.code that corresponds with the location of the /? key on a DVORAK layout).

[khiga8]

1. Based on what you know, can we rely on keyboard map API at this point? Are there polyfills we could use or would we need to write our own? If we are unable to rely on the keyboard map API, have you found there other tools out there to help retrieve the humanized output given a keyboard layout?

2. What does support (if any) for non-latin keyboard layouts look like? Are we supporting only: qwerty, dvorak, and colemak?

[theinterned]

Another thought: what if we support both event.key and event.code and let developers decide for themselves. We could use a prefix key:?, key:k, key:K etc. or code:Slash, code:KeyK etc. to let the developer decide between semantic meaning and keyboard location.

This would put us back in the same place when key is combined with Shift/Meta such as Meta+Shift+key:k.

Update: VS Code has a similar scheme that allows distinguishing event.code ([KeyA], [Space], [Slash]) from event.key (a, , /) as something called a "scan-code" https://code.visualstudio.com/docs/getstarted/keybindings#_keyboard-layoutindependent-bindings

[theinterned]

@khiga8 thank you! Great questions!

1. As far as reliablility of the keyboard map API:
   1. I have not been able to find a polyfill for the keyboard map API. Since it needs to detect details of the users system, it likely can't in fact be polyfilled.
      1. There is a polyfill that only works for US-en QWERTY https://github.com/WICG/keyboard-map/blob/main/keyboard-polyfill.js
      2. This polyfill actually made me realize that this API doesn't provide a way to map from, say Shift+KeyN to N (just from KeyN to n).
   2. I also uncovered a concerning issue https://github.com/WICG/keyboard-map/issues/30 that indicates that Apple has decided the API is too invasive.
   3. I also have not found any reliable source of mappings of event.code to different layouts' keys. This would potentially mean we'd have to do this ourselves. Writing a lib would be trivial, but could be a bit of a chore to maintain.
2. The explainer for keyboard layout map seems to suggest that this API would be sufficient for non-latin layouts:

For keyboard shortcuts, this Latin value is what is commonly used for shortcuts in applications (even when the native writing system is not Latin-based). Keyboards in areas where the common layout does not produce Latin characters will have both the native and the Latin characaters printed on the physical key caps to make this easier for users. Note the assumption that the labels on the keyboard match the currently active keyboard locale. This is usually, but not always, true, but since there is no way to know that the actual labels are, this is the best surrogate.

I also found this proposal which I quite like https://github.com/w3c/uievents/issues/247 but hasn't had any activity since November 2019.

[theinterned]

Note: in https://github.com/github/hotkey/pull/66 I added a test and note in the README about the inconsistent treatment of shift.

When working on this issue, that test and README note should be updated to account for however we resolve this!

[theinterned]

A solution I am leaning towards is: when we encounter Shift, we should split the hotkey into multiple trie nodes:

For example, for the hotkey "Shift+a" (or "A"), we could produce the following trie nodes:

1. Shift+a
2. Shift+A
3. A

For the hotkey "Mod+Shift+a" (or "Mod+A") ...

... on Windows, we could produce the following trie nodes:

1. Control+Shift+a
2. Control+Shift+A
3. Control+A

... and on Mac we could produce the following nodes:

1. Meta+Shift+a
2. Meta+Shift+A
3. Meta+A

I think this would allow us to be prepared for any platform inconsistencies.

This would however require us to be able to map to from "uppercase" special characters — eg. from 1 to ! or from / to ? and vice-versa. This in turn will create problems supporting non US-QWERTY keyboard layouts.

[theinterned]

A separate thing I am considering is that we should support the "scan-code" square-bracket syntax from VSCode for event.code.