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.
This commit is contained in:
Priyank Patel 2021-05-26 17:51:07 +00:00 committed by Tim Abbott
parent 9f2daeee45
commit 6ab66ea17a
10 changed files with 39 additions and 54 deletions

View File

@ -175,6 +175,7 @@
"window": false
},
"rules": {
"new-cap": "off",
"no-sync": "off"
}
},

View File

@ -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() {

View File

@ -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 = {

View File

@ -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
};

View File

@ -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;
},

View File

@ -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,
},
});

View File

@ -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,
},
});
}

View File

@ -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();

View File

@ -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(),
},
});
}

View File

@ -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;
},