piranha / keymage

Yet Another JS Keybinding library
ISC License
332 stars 31 forks source link

Support attaching to keypress and not only keydown #17

Closed orbitbot closed 8 years ago

orbitbot commented 8 years ago

In some occasions, it's desired to be able to attach to the keypress event instead of keydown, especially since some special character combinations are not consistent across devices.

I guess the consistent place to define this in keymage would be the options object.

piranha commented 8 years ago

Right, shouldn't be hard to do. Though I'm not sure about "some special character combinations are not consistent across devices", will this help? Or you mean it should switch to charCode rather than keyCode?

orbitbot commented 8 years ago

The main benefit of attaching to keypress is that it will give developers the entered character, no matter how it was achieved, eg. it doesn't matter if the end user uses DVORAK layout or whatever the input language might be, so it's not hardcoding against keydown input but rather the resulting character. Which in many cases is much more useful.

As an example, I cannot successfully bind to [ or ] using keymage for whatever reason, even though they should be uniquely identified according to the keyCode tables that I can find, and I get false negatives (wrong values) with a tool like http://keycode.info/ . This is with a Macbook, QUERTY layout with Finnish/Swedish language input, which should by no means be anything particularly exotic.

orbitbot commented 8 years ago

Additionally, other characters than the ones hardcoded by the keyCode tables could be supported, such as (, ), and so on.

piranha commented 8 years ago

Yeah, but with charCode hotkeys break if you have multiple keyboard layouts. That's the reason for the keyCodes in my code - it works even with Ukrainian keyboard layout.

orbitbot commented 8 years ago

It may be that I've misunderstood the root cause of this, it's probably not the difference between keydown vs keypress, although keypress might handle special characters easier: https://github.com/PolicyStat/combokeys/blob/master/helpers/characterFromEvent.js

The issue I guess is that the supported characters are hardcoded here https://github.com/piranha/keymage/blob/master/keymage.js#L29-L82 -- and anything outside of that will not work, so I get

Unknown key "(" in keystring "("
Unknown key ")" in keystring ")"
Unknown key "{" in keystring "{"
Unknown key "}" in keystring "}"
Unknown key """ in keystring """

As well as [ and ] not working probably since I have them in a nonstandard location for the keyboard layout...

piranha commented 8 years ago

Heh, so let's add them maybe? :-) I'm not surprised something could be missing. :)

orbitbot commented 8 years ago

Well, depending on your goals with the library, something is bound to be missing eventually... Allowing to match an arbitrary character would be more powerful and probably less code in the end.

piranha commented 8 years ago

If there is a way to match an arbitrary character to a keyCode, I'd all for that! But switching to charCode is something I can't do - because this will break hotkeys for all the non-roman keyboard layouts.

piranha commented 8 years ago

Not sure how to handle this situation, really. I need to think a bit. :)

If you have any ideas except for dropping keyCode in favor of charCode completely - I'm very interested. :)

orbitbot commented 8 years ago

Not 100% sure, but I don't think changing keyCode to charCode would be necessary.

The problem currently as I see it is that you're hardcoding which characters to accept in lines https://github.com/piranha/keymage/blob/master/keymage.js#L29-L82 -- from what I can quickly gather from the comboKeys implementation (linked above, as well as https://github.com/avocode/combokeys/blob/master/helpers/special-characters-map.js and https://github.com/avocode/combokeys/blob/master/helpers/special-keys-map.js ) is that you only need to take care of the special characters, the rest already work (with String.fromCharCode).

Removing this check https://github.com/piranha/keymage/blob/master/keymage.js#L103 and maybe if necessary some changes in dispatch would probably be enough to match arbitrary characters received from String.fromCharCode.

piranha commented 8 years ago

I'm sorry being so slow here. :-) So what you propose more or less is:

Like that? :) Or maybe parsing unknown keys in parseKeyString with String.charCodeAt? What do you think about that one?

orbitbot commented 8 years ago
  • store char instead of code in parseKeyString if it's unknown
  • parse incoming event with String.fromCharCode(e.keyCode)
  • match that after trying to match by keyCode

Based on the combokeys approach I think this should probably work fine.

Or maybe parsing unknown keys in parseKeyString with String.charCodeAt?

This is probably also needed for storing character data instead of the keyCodes (first bullet point above)

With the above, as indicated in the linked combokeys files, it's probably also unnecessary to handle the alphabet characters as a special case, unless that's something that for some reason would be required for non-Roman keyboard layouts (?).

piranha commented 8 years ago

Interesting question: are the keycodes of the keys you're trying to use in the source code at all? You can check keycode at http://keycode.info

orbitbot commented 8 years ago

The opening and closing square bracket ([ and ]) match in some cases if I release the keys in the right combination, but I have remapped these. I have the altGr key combos for all the bracket keys in muscle memory and program on both linux and macs almost daily, so it's in a non-standard location for macs.

Parentheses (()) don't seem to be there at all AFAICT, since I get the numeric keys even with the shift key pressed. Looking at some tables, these map to keyCode 40 & 41 which are not directly accessible with this layout, it seems.

For me, the comboKeys implementation just works for the keys I needed support for.

orbitbot commented 8 years ago

I've done some further testing, and have come to the conclusion that it's not possible to get 100% accurate results from keydown events after all. You can see my test here https://jsbin.com/pemipo/edit?html,js,output -- with the alternative keydown approach in lines 21-91 in the jsbin javascript source it's very close to getting the correct character, but I've found that the end results are correct for windows/linux keyboard layouts but not macs. keypress seems to always return the correct character input, even when complicated chording (fe. multiple modifier keys pressed) is tried.

The above test reinforces my original understanding that any printable character should be handled with keypress, but this might be well outside the intended scope of this library. I have started implementing my own tool that is more focused on keypress events. This issue and the other one about a target DOM element can be closed if you have no intent of implementing them.

piranha commented 8 years ago

Right, it seems you either can support qwerty + dvorak, or qwerty + йцукен, but not both simultaneously. :( So it's just comes to choice whatever you want to support...