Improve triggerKeyEvent() warnings
#379
Conversation
| @returns {boolean} whether the input string consists only of numeric characters | ||
| */ | ||
| export function isNumeric(n) { | ||
| return !isNaN(parseFloat(n)) && isFinite(n); |
There was a problem hiding this comment.
Keycodes are only integers. I think you should use parseInt
There was a problem hiding this comment.
this seems to be the accepted solution for a lot of these questions on StackOverflow with lots of people mentioning that parseInt() should not be used for some reason.
ultimately this is only used to catch the case where someone used '13' instead of 13. if someone would put in '13.0' in the current state it would also warn, but if we changed it to only allow integers it would no longer show that warning, so I think it is actually better this way
| @@ -22,7 +22,7 @@ const keyFromKeyCode = { | |||
| 18: 'Alt', | |||
| 20: 'CapsLock', | |||
| 27: 'Escape', | |||
| 32: '', | |||
| 32: 'Space', | |||
There was a problem hiding this comment.
Why this change? I think I remember getting those values myself and being surprised by the empty string, but it was the value I saw when debugging
There was a problem hiding this comment.
I got that value from https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode 🤔
There was a problem hiding this comment.
I created this which to test it and it seems to say otherwise: https://jsbin.com/qizayecoqe/edit?html,js,console,output
There was a problem hiding this comment.
that is ' ', not '' though, so also something different. I'm fine with using ' ' instead.
There was a problem hiding this comment.
I thought I had a space, not an empty string. My bad.
Resolves #368
/cc @mydea