Even better normalization of KeyboardEvent + MouseEvent

[syranide]

KeyboardEvent now normalizes charCode, keyCode, which across all browsers, partial key-support for KeyDown/KeyUp, full key-support for KeyPress. getModifierState is added to the interface of KeyboardEvent, MouseEvent and TouchEvent and is polyfilled for IE8.

TouchEvent doesn't support getModifierState according to the latest draft, but considering that the interface is otherwise identical in the design, it would be exceptionally inconsistent for it to be intentionally omitted. which is redundant and not very precisely defined, but it is in the legacy standard. I also took the liberty to remove char from the interface, it existed briefly in the working draft before it got dropped. It was only ever supported by IE9, which makes it practically useless.

I've also extensively documented all the nuances.

   raw     gz Compared to master @ e91a8a1bc30e675c4683d5667b4122106962828b
     =      = build/JSXTransformer.js
 +3586   -904 build/react-test.js
 +4484  +1292 build/react-with-addons.js
  +839   +242 build/react-with-addons.min.js
 +4483  +1283 build/react.js
  +840   +218 build/react.min.js

Fixes #706.

[sophiebits]

Will .key be '' ever here?

[syranide]

@spicyj It's either undefined because it isn't supported, or it may be (according to the standard) initialized to '', which I take would be equivalent to unsupported (but recognized). '' is NOT a valid key, it's either recognized or 'Unindentified'.

[sophiebits]

In that case, your ending return '' doesn't make sense to me. But perhaps that's never reached? If not, I'd just do invariant(false); there.

[syranide]

@spicyj Correct, invariant(false) is better too!

[sophiebits]

Mind editing the docs to include getModifierState?

[syranide]

@spicyj Done, also added getModifierState to TouchEvent, it isn't actually in the working draft, but the modifier keys are in the interface so I can only assume that the draft is not yet complete.

Also moved out the functions from the KeyboardEvent-interface, they were getting to the point where it was hard to actually see the properties of the interface.

[yungsters]

Can you rebase on 182a237? In particular, move some of this logic into getEventKey.

[syranide]

@yungsters Done. I also moved my events/synthetic/getEventModifierState.js to dom/ instead, seemed consistent with the rest.

As getEventKey is now external there might not be as much of a point in having getEventCharCode, getEventKeyCode, etc as functions rather than inline in the interface, your opinion?

I have not tested this updated PR yet, will do when I get home later tonight.

---

PS. FYI when getEventKey was moved out require('invariant') apparently went missing, not that it really matters in practice, but good to know, it's fixed in my PR.

[yungsters]

Nit: Add a colon after type.

[syranide]

Fixed

[yungsters]

As getEventKey is now external there might not be as much of a point in having getEventCharCode, getEventKeyCode, etc as functions rather than inline in the interface, your opinion?

Hah, I was thinking the same thing. Most of the other events have these functions inline. In my personal opinion, seeing a line reading var KeyboardEventInterface = { gives context about where those functions will be used. Maybe this is being nitty, but once again, my personal opinion.

PS. FYI when getEventKey was moved out require('invariant') apparently went missing, not that it really matters in practice, but good to know, it's fixed in my PR.

I think your PR adds the only caller to invariant to the getEventKey module.

[syranide]

@yungsters Done. Also, you're right, I added that, I must've confused myself when I rebased.

[yungsters]

This can be non-display characters, so shouldn't this be something like:

  return translateToKey[charCode] || String.fromCharCode(charCode);

...or something? Listening to onKeyPress and checking event.key === 'Enter' should work. Currently, this will just return an empty string:

> String.fromCharCode(13)
""

[syranide]

@yungsters That's keyPress so charCode is actually the actual character code, and not virtual key code. keyup and keydown are for virtual key codes. Also, while FireFox does generate keypress for some non-printable keys, only it seems to do it, that's why I've added an exception to the SimpleEventPlugin to discard non-printable keys for keypress, to normalize the behavior.

[yungsters]

Hmm... I'm seeing that keyCode and charCode are both set for "Enter" in Chrome on keypress. Are you seeing something different?

[syranide]

@yungsters Yep, but they both contain the same value, I have a comment a bit further down in which about it. Basically, charCode is correct for keypress and keyCode is correct for keydown and keyup, but depending on browser, either of them can be null or the same as the other. This PR also normalizes this behavior so that only one of them is set.

Also, enter is special because it actually produces a character.

[yungsters]

I think your normalization of of charCode and keyCode are fine. However, my understanding of the spec is that key should be "Enter" when pressing Enter on all keydown, keyup, and keypress events. The code here will never set key to "Enter".

By the way, I've been using this to see what browsers do: http://jsfiddle.net/yungsters/D9LjL/

[syranide]

@yungsters Yeah I've been using something similar, but neat! Anyway, I pushed a new change, using translateToKey is broken, as it would actually translate actual characters into arbitrary keys (charCode != keyCode). Proper fix should be to treat enter as a special-case.

[sophiebits]

Since keypress is deprecated, not sure it's worth worrying too much about what key is for it.

[petehunt]

@yungsters how do you feel about this?

[syranide]

@salier Was a bit involved too I think? (or rather, was working on his own implementation before he saw this)

[zpao]

I'm going to target 0.10 for this since it doesn't seem likely this is going in in the super immediate future.