Reduce usage of legacy event.keyCode. NFC
#22881
Conversation
|
I noticed this could be improved while reviewing #22879. Whichever lands first will have to deal with a conflict. |
kripken
left a comment
kripken
left a comment
There was a problem hiding this comment.
lgtm if relevant interactive tests pass. By my reading of the MDN docs this looks valid.
| function unpressAllPressedKeys() { | ||
| // Un-press all pressed keys: TODO | ||
| for (var keyCode of SDL.keyboardMap) { | ||
| for (var keyCode of Object.values(SDL.keyboardMap)) { |
There was a problem hiding this comment.
Yup, I see this crash when running the interactive tests.
| // Do not prevent default on the rest as we need the keypress event. | ||
| function shouldPreventDefault(event) { | ||
| if (event.type === 'keydown' && event.keyCode !== 8 /* backspace */ && event.keyCode !== 9 /* tab */) { | ||
| if (event.type === 'keydown' && event.key != 'Backspace' && event.key != 'Tab') { |
There was a problem hiding this comment.
BTW the logic here seems to be the inverse of what the comment says. Shouldn't all the clauses above be ===?
There was a problem hiding this comment.
Looks right to me? Comment above says "prevent default on backspace/tab", the if checks "not backspace, not tab" and returns false if so. So it returns true - prevents default - on backspace/tab as described.
|
I split out the crash fix: #22884 |
This change replaced the usage of the legacy `keyCode` attribute with the preferred `key` attribute, which also has the advantage of removing some hardcoded constant numbers. In order to test this change I fixed the `test_glfw_get_key_stuck` test and added `test_sdl_key_test` to the interactive tests. In doing so I noticed and fixed a crash bug that was introduced in emscripten-core#22874.
2 participants
This change replaced the usage of the legacy
keyCodeattribute with the preferredkeyattribute, which also has the advantage of removing some hardcoded constant numbers.In order to test this change I fixed the
test_glfw_get_key_stucktest and addedtest_sdl_key_testto the interactive tests.In doing so I noticed and fixed a crash bug that was introduced in #22874.