cvan / keyboardevent-key-polyfill

polyfill for `KeyboardEvent.prototype.key`
https://cvan.io/keyboardevent-key-polyfill/
Creative Commons Zero v1.0 Universal
47 stars 15 forks source link

Also handle lower case ASCII chars #7

Open pbor opened 8 years ago

pbor commented 8 years ago

On my chrome when I press the "a" key, I get this.which = 97, so we also should duplicate the a-z ASCII range in the array.

levithomason commented 8 years ago

It looks like you're mapping the keypress event while this library is mapping on the keydown event:

image

The range in the polyfill right now covers lower and upper case for the range of keydown event which and keyCode values.

pbor commented 8 years ago

Sorry for the delay.

Yes, I am using the polyfill for both keyup and keydown. With my change this seems to work as expected. Do you think it is a problem?

levithomason commented 8 years ago

I think there is a conflict. This polyfill assumes the event is a keydown event only. It is mapping which and keyCode values to the new key values. This PR maps the which/keyCode range 97 - 123. This range overlaps the keydown values for function keys. Example:

image

Unless there were some logic added to differentiate which type of event was invoked (keypress vs keydown) there is no way to tell if 112 should indicate F1 or P.

The Event.type tells whether it is a keypress or keydown event. This could be used to map the values you are trying to map. If this was done, this polyfill should likely have two complete maps. One for keydown, and another for keypress.

Lastly, I'm not sure if the specs say key property will be available on all events or just keydown. Also, I've moved from this polyfill to a ponyfill that does the same thing.

Hope that helps, cheers!

cvan commented 6 years ago

sorry, just seeing this now. thanks for the PR and the discussion.

I think the fix here would probably be to fire different events on keypress vs. keydown. let me know what y'all think. I can open a separate issue and follow-up PR.

levithomason commented 6 years ago

Just a heads up, I've moved away from the polyfill in favor of a function. This way, the prototype isn't mutated.