From 6ab66ea17ac8b75f34259f94d606637e0b6780ae Mon Sep 17 00:00:00 2001 From: Priyank Patel Date: Wed, 26 May 2021 17:51:07 +0000 Subject: [PATCH] keydown_util: Use Event.key instead of deprecated properties. The Event.which and Event.keyCode are deprecated as pointed out by TypeScript intellisense based on the jQuery types. We use Event.key instead which behaves similarly to Event.which & Event.keyCode for our use case. The only difference in functionality by this change is that the vim keys won't work when Caps Lock is on. This is because, in this case, the key property will be "J" instead of 'j'. We can fix this by adding a mapping for this, however, I think we don't want to handle this case so I left this change out. Tested by trying out the everywhere keydown_util is used. Finally, we also turn off the new-cap rule for tests since I think it fine to only enforce it on real code and exempting test code is fine. --- .eslintrc.json | 1 + frontend_tests/node_tests/activity.js | 8 +++--- frontend_tests/node_tests/components.js | 4 +-- frontend_tests/node_tests/keydown_util.js | 8 +++--- static/js/activity.js | 6 ++--- static/js/components.js | 4 +-- static/js/info_overlay.js | 4 +-- static/js/keydown_util.js | 33 ++++++----------------- static/js/settings_panel_menu.js | 19 ++++++------- static/js/stream_list.js | 6 ++--- 10 files changed, 39 insertions(+), 54 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index de92a723d4..c691da952f 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -175,6 +175,7 @@ "window": false }, "rules": { + "new-cap": "off", "no-sync": "off" } }, diff --git a/frontend_tests/node_tests/activity.js b/frontend_tests/node_tests/activity.js index 77f7066543..5d195bd8d1 100644 --- a/frontend_tests/node_tests/activity.js +++ b/frontend_tests/node_tests/activity.js @@ -307,8 +307,8 @@ test("handlers", (override) => { (function test_filter_keys() { init(); activity.user_cursor.go_to(alice.user_id); - filter_key_handlers.down_arrow(); - filter_key_handlers.up_arrow(); + filter_key_handlers.ArrowDown(); + filter_key_handlers.ArrowUp(); })(); (function test_click_filter() { @@ -339,12 +339,12 @@ test("handlers", (override) => { $(".user-list-filter").val("al"); narrowed = false; activity.user_cursor.go_to(alice.user_id); - filter_key_handlers.enter_key(); + filter_key_handlers.Enter(); assert(narrowed); // get line coverage for cleared case activity.user_cursor.clear(); - filter_key_handlers.enter_key(); + filter_key_handlers.Enter(); })(); (function test_click_handler() { diff --git a/frontend_tests/node_tests/components.js b/frontend_tests/node_tests/components.js index 329ed29a06..dd921a2669 100644 --- a/frontend_tests/node_tests/components.js +++ b/frontend_tests/node_tests/components.js @@ -162,8 +162,8 @@ const components = zrequire("components"); const noop = () => {}; -const LEFT_KEY = {which: 37, preventDefault: noop, stopPropagation: noop}; -const RIGHT_KEY = {which: 39, preventDefault: noop, stopPropagation: noop}; +const LEFT_KEY = {key: "ArrowLeft", preventDefault: noop, stopPropagation: noop}; +const RIGHT_KEY = {key: "ArrowRight", preventDefault: noop, stopPropagation: noop}; run_test("basics", () => { env = { diff --git a/frontend_tests/node_tests/keydown_util.js b/frontend_tests/node_tests/keydown_util.js index 7e6d2bbddd..31d536768b 100644 --- a/frontend_tests/node_tests/keydown_util.js +++ b/frontend_tests/node_tests/keydown_util.js @@ -11,7 +11,7 @@ run_test("test_early_returns", () => { const opts = { elem: stub, handlers: { - left_arrow: () => { + ArrowLeft: () => { throw new Error("do not dispatch this with alt key"); }, }, @@ -21,21 +21,21 @@ run_test("test_early_returns", () => { const e1 = { type: "keydown", - which: 17, // not in keys + key: "a", // not in keys }; stub.trigger(e1); const e2 = { type: "keydown", - which: 13, // no handler + key: "Enter", // no handler }; stub.trigger(e2); const e3 = { type: "keydown", - which: 37, + key: "ArrowLeft", altKey: true, // let browser handle }; diff --git a/static/js/activity.js b/static/js/activity.js index be21948b97..c8a343cc8c 100644 --- a/static/js/activity.js +++ b/static/js/activity.js @@ -326,15 +326,15 @@ export function set_cursor_and_filter() { keydown_util.handle({ elem: $input, handlers: { - enter_key() { + Enter() { keydown_enter_key(); return true; }, - up_arrow() { + ArrowUp() { user_cursor.prev(); return true; }, - down_arrow() { + ArrowDown() { user_cursor.next(); return true; }, diff --git a/static/js/components.js b/static/js/components.js index f0107a22ca..42717fe037 100644 --- a/static/js/components.js +++ b/static/js/components.js @@ -108,8 +108,8 @@ export function toggle(opts) { keydown_util.handle({ elem: meta.$ind_tab, handlers: { - left_arrow: maybe_go_left, - right_arrow: maybe_go_right, + ArrowLeft: maybe_go_left, + ArrowRight: maybe_go_right, }, }); diff --git a/static/js/info_overlay.js b/static/js/info_overlay.js index 5a16a76bb0..9b220b196a 100644 --- a/static/js/info_overlay.js +++ b/static/js/info_overlay.js @@ -195,8 +195,8 @@ export function set_up_toggler() { keydown_util.handle({ elem: modal, handlers: { - left_arrow: toggler.maybe_go_left, - right_arrow: toggler.maybe_go_right, + ArrowLeft: toggler.maybe_go_left, + ArrowRight: toggler.maybe_go_right, }, }); } diff --git a/static/js/keydown_util.js b/static/js/keydown_util.js index 65aded425d..b8a1b0e5f4 100644 --- a/static/js/keydown_util.js +++ b/static/js/keydown_util.js @@ -2,41 +2,24 @@ See hotkey.js for handlers that are more app-wide. */ -// Note that these keycodes differ from those in hotkey.js, because -// we're using keydown rather than keypress. It's unclear whether -// there's a good reason for this difference. -const keys = { - 13: "enter_key", // Enter - 37: "left_arrow", // // Left arrow - 38: "up_arrow", // Up arrow - 39: "right_arrow", // Right arrow - 40: "down_arrow", // Down arrow - 72: "vim_left", // 'H' - 74: "vim_down", // 'J' - 75: "vim_up", // 'K' - 76: "vim_right", // 'L' -}; +export const vim_left = "h"; +export const vim_down = "j"; +export const vim_up = "k"; +export const vim_right = "l"; export function handle(opts) { opts.elem.on("keydown", (e) => { - const key = e.which || e.keyCode; - if (e.altKey || e.ctrlKey || e.shiftKey) { return; } - const key_name = keys[key]; - - if (!key_name) { + const {key} = e; + const handler = opts.handlers[key]; + if (!handler) { return; } - if (!opts.handlers[key_name]) { - return; - } - - const handled = opts.handlers[key_name](); - + const handled = handler(); if (handled) { e.preventDefault(); e.stopPropagation(); diff --git a/static/js/settings_panel_menu.js b/static/js/settings_panel_menu.js index ec3b9bad96..ff058d9807 100644 --- a/static/js/settings_panel_menu.js +++ b/static/js/settings_panel_menu.js @@ -62,20 +62,21 @@ export class SettingsPanelMenu { } set_key_handlers(toggler) { + const {vim_left, vim_right, vim_up, vim_down} = keydown_util; keydown_util.handle({ elem: this.main_elem, handlers: { - left_arrow: toggler.maybe_go_left, - right_arrow: toggler.maybe_go_right, - enter_key: () => this.enter_panel(), - up_arrow: () => this.prev(), - down_arrow: () => this.next(), + ArrowLeft: toggler.maybe_go_left, + ArrowRight: toggler.maybe_go_right, + Enter: () => this.enter_panel(), + ArrowUp: () => this.prev(), + ArrowDown: () => this.next(), // Binding vim keys as well - vim_left: toggler.maybe_go_left, - vim_right: toggler.maybe_go_right, - vim_up: () => this.prev(), - vim_down: () => this.next(), + [vim_left]: toggler.maybe_go_left, + [vim_right]: toggler.maybe_go_right, + [vim_up]: () => this.prev(), + [vim_down]: () => this.next(), }, }); } diff --git a/static/js/stream_list.js b/static/js/stream_list.js index c65a4781b7..422fe11893 100644 --- a/static/js/stream_list.js +++ b/static/js/stream_list.js @@ -551,15 +551,15 @@ export function set_event_handlers() { keydown_util.handle({ elem: $search_input, handlers: { - enter_key() { + Enter() { keydown_enter_key(); return true; }, - up_arrow() { + ArrowUp() { stream_cursor.prev(); return true; }, - down_arrow() { + ArrowDown() { stream_cursor.next(); return true; },