search

tabvengers / spicy-sections

Creative Commons Zero v1.0 Universal
128 stars 10 forks source link

Swap `Event#keyCode` for `Event#key` #56

Closed jonathantneal closed 2 years ago

jonathantneal commented 2 years ago

This change replaces the use of KeyboardEvent#keyCode ref with KeyboardEvent#key ref.

The primary rationale is that keyCode is deprecated. I also think KeyboardEvent#key is a clearer interface, since it provides a printable representation of the key.

Browser support for key is 98.06% 2, and for context support for Custom Elements v1 is 95.84% 3.

jonathantneal commented 2 years ago

At the suggestion of @bkardell, adding @davatron5000 as a reviewer. :)

bkardell commented 2 years ago

To be clear the reason I asked for another opinion is that I don't have very strong feelings here. keyCode is longer supported, I think and therefore has a lot more uptake and people inevitably testing codepaths. I always debate which of these to use, but the few times I err the other way (not in a while) I've got bugs filed. As it is such a small change, I would probably lean just ever so slightly at not changing it, but this is a very very weak opinion and I don't have any interesting thoughts or data - maybe someone else does. I'm ok with whatever @jonathantneal and @davatron5000 (or even anyone else like @hidde maybe) decide here.

jonathantneal commented 2 years ago

The only thing I would add is — if somebody helps us improve or debug, then event.keyCode == 37 || event.keyCode == 38 requires serious writ memory of 'magic' ANSI codes, while event.key == 'ArrowLeft' || event.key == 'ArrowUp' kinda makes sense all on its own.