Fix the default options for keyboard shortcuts
Change 339776 has introduced a new option `do not prevent default` for
keyboard shortcuts. The default options object was set to
`{shouldSuppress: true, doNotPrevent: false}`, and the author thought
that passing `{doNotPrevent: false}` would retain the default of
`shouldSuppress`, but of course it does not.
So keyboard shortcuts `j` and `k` were not suppressed properly anymore
when the user types into input elements.
The fix is to avoid a default options object, but instead calculate
defaults individually.
Release-Notes: skip
Change-Id: I7397cbd85f215c3197754b4aea3ef963e1834f5c
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
index 69c371e..35da84e 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
@@ -10,9 +10,12 @@
ShortcutsService,
} from '../../services/shortcuts/shortcuts-service';
import {Shortcut, ShortcutSection} from './shortcuts-config';
-import {SinonFakeTimers} from 'sinon';
-import {Key, Modifier} from '../../utils/dom-util';
+import {SinonFakeTimers, SinonSpy} from 'sinon';
+import {Binding, Key, Modifier} from '../../utils/dom-util';
import {getAppContext} from '../app-context';
+import {pressKey} from '../../test/test-utils';
+
+const KEY_A: Binding = {key: 'a'};
suite('shortcuts-service tests', () => {
let service: ShortcutsService;
@@ -34,6 +37,64 @@
);
});
+ suite('addShortcut()', () => {
+ let el: HTMLElement;
+ let listener: SinonSpy<[KeyboardEvent], void>;
+
+ setup(() => {
+ el = document.createElement('div');
+ listener = sinon.spy() as SinonSpy<[KeyboardEvent], void>;
+ });
+
+ test('standard call', () => {
+ service.addShortcut(el, KEY_A, listener);
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.calledOnce);
+ });
+
+ test('doNotPrevent option default false', () => {
+ service.addShortcut(el, KEY_A, listener);
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.calledOnce);
+ assert.isTrue(listener.lastCall.firstArg?.defaultPrevented);
+ });
+
+ test('doNotPrevent option force false', () => {
+ service.addShortcut(el, KEY_A, listener, {doNotPrevent: false});
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.calledOnce);
+ assert.isTrue(listener.lastCall.firstArg?.defaultPrevented);
+ });
+
+ test('doNotPrevent option force true', () => {
+ service.addShortcut(el, KEY_A, listener, {doNotPrevent: true});
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.calledOnce);
+ assert.isFalse(listener.lastCall.firstArg?.defaultPrevented);
+ });
+
+ test('shouldSuppress option default true', () => {
+ service.shortcutsDisabled = true;
+ service.addShortcut(el, KEY_A, listener);
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.notCalled);
+ });
+
+ test('shouldSuppress option force true', () => {
+ service.shortcutsDisabled = true;
+ service.addShortcut(el, KEY_A, listener, {shouldSuppress: true});
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.notCalled);
+ });
+
+ test('shouldSuppress option force false', () => {
+ service.shortcutsDisabled = true;
+ service.addShortcut(el, KEY_A, listener, {shouldSuppress: false});
+ pressKey(el, KEY_A.key);
+ assert.isTrue(listener.calledOnce);
+ });
+ });
+
suite('binding descriptions', () => {
function mapToObject<K, V>(m: Map<K, V>) {
const o: any = {};
