Use event.key over event.keyCode/which #4631
Conversation
lib/modules/commandLine.js
Outdated
| @@ -84,7 +84,7 @@ const commandLine = _.once(() => { | |||
| }); | |||
|
|
|||
| input.addEventListener('keyup', (e: KeyboardEvent) => { | |||
| if (e.keyCode === 27) { | |||
| if (e.key === 'Escape') { | |||
There was a problem hiding this comment.
Can these constants be put in CommentTools.NAMED_KEYS.Escape, ...Enter for easier validation?
| @@ -1078,24 +1078,6 @@ function lod() { | |||
| }); | |||
| } */ | |||
|
|
|||
| export const KEYS = { | |||
There was a problem hiding this comment.
Can we keep this as NAMED_KEYS = { SPACE: ' ', ENTER: 'Enter', ESCAPE: 'Escape', ... } for stronger validation?
There was a problem hiding this comment.
Sure, sounds good to me.
6b47585
to
915bf79
Compare
|
Thanks! Neat little refactor. |
|
This needs testing in Edge (which I can do at some point), since apparently it has "non-standard key identifiers"... https://caniuse.com/#feat=keyboardevent-key |
|
Good catch; seems like the issue is fixed but not yet released. We'll need to add a shim similar to this for Edge to work. |
|
Depending when we expect this 5.12 to land we may be able to just avoid the shim. The fix should be released April timeish. |
|
I added a polyfill for Edge, so this should be mergeable. I don't have Edge to test with though. |
|
Tested in Edge 16 |
There are a few
keyCodeinstances that I did not change, as they were trickier to modify correctly.