Allow j/k keys being held down for moving the cursor multiple times
This is a regression introduced by updated keyboard shortcut handling,
which is now defaulting to ignore `repeat` events. We are making an
exception here for all the shortcuts that moving the cursor up/down in
a list of change, files or lines.
Bug: Issue 15200
Change-Id: I1a6dff4ad09f6405d957bac1c61ce101875f7b45
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
index 3c9e058..18b321a 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-config.ts
@@ -180,13 +180,13 @@
Shortcut.CURSOR_NEXT_CHANGE,
ShortcutSection.ACTIONS,
'Select next change',
- {key: 'j'}
+ {key: 'j', allowRepeat: true}
);
describe(
Shortcut.CURSOR_PREV_CHANGE,
ShortcutSection.ACTIONS,
'Select previous change',
- {key: 'k'}
+ {key: 'k', allowRepeat: true}
);
describe(
Shortcut.OPEN_CHANGE,
@@ -316,15 +316,15 @@
Shortcut.NEXT_LINE,
ShortcutSection.DIFFS,
'Go to next line',
- {key: 'j'},
- {key: Key.DOWN}
+ {key: 'j', allowRepeat: true},
+ {key: Key.DOWN, allowRepeat: true}
);
describe(
Shortcut.PREV_LINE,
ShortcutSection.DIFFS,
'Go to previous line',
- {key: 'k'},
- {key: Key.UP}
+ {key: 'k', allowRepeat: true},
+ {key: Key.UP, allowRepeat: true}
);
describe(
Shortcut.VISIBLE_LINE,
@@ -480,15 +480,15 @@
Shortcut.CURSOR_NEXT_FILE,
ShortcutSection.FILE_LIST,
'Select next file',
- {key: 'j'},
- {key: Key.DOWN}
+ {key: 'j', allowRepeat: true},
+ {key: Key.DOWN, allowRepeat: true}
);
describe(
Shortcut.CURSOR_PREV_FILE,
ShortcutSection.FILE_LIST,
'Select previous file',
- {key: 'k'},
- {key: Key.UP}
+ {key: 'k', allowRepeat: true},
+ {key: Key.UP, allowRepeat: true}
);
describe(
Shortcut.OPEN_FILE,
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
index a26fa08..f2e9e98 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
@@ -137,7 +137,7 @@
listener: (e: KeyboardEvent) => void
) {
const wrappedListener = (e: KeyboardEvent) => {
- if (e.repeat) return;
+ if (e.repeat && !shortcut.allowRepeat) return;
if (!eventMatchesShortcut(e, shortcut)) return;
if (shortcut.combo) {
if (!this.isInSpecificComboKeyMode(shortcut.combo)) return;
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
index 05c4f53..a024159 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
@@ -213,7 +213,10 @@
{
shortcut: Shortcut.NEXT_LINE,
text: 'Go to next line',
- bindings: [{key: 'j'}, {key: 'ArrowDown'}],
+ bindings: [
+ {allowRepeat: true, key: 'j'},
+ {allowRepeat: true, key: 'ArrowDown'},
+ ],
},
],
[ShortcutSection.NAVIGATION]: [
@@ -234,7 +237,10 @@
{
shortcut: Shortcut.NEXT_LINE,
text: 'Go to next line',
- bindings: [{key: 'j'}, {key: 'ArrowDown'}],
+ bindings: [
+ {allowRepeat: true, key: 'j'},
+ {allowRepeat: true, key: 'ArrowDown'},
+ ],
},
],
[ShortcutSection.EVERYWHERE]: [
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index e2fa8fe..bd0f742 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -338,6 +338,8 @@
combo?: ComboKey;
/** Defaults to no modifiers. */
modifiers?: Modifier[];
+ /** Defaults to false. If true, then `event.repeat === true` is allowed. */
+ allowRepeat?: boolean;
}
const ALPHA_NUM = new RegExp(/^[A-Za-z0-9]$/);
@@ -406,7 +408,7 @@
}
) {
const wrappedListener = (e: KeyboardEvent) => {
- if (e.repeat) return;
+ if (e.repeat && !shortcut.allowRepeat) return;
if (options.shouldSuppress && shouldSuppress(e)) return;
if (eventMatchesShortcut(e, shortcut)) {
listener(e);
