Move shouldSuppress() method from mixin to service
Change-Id: Iffd0fa8bd09a435d4fa25a167e9ad75bb2c2da00
diff --git a/package.json b/package.json
index ebfadc8..a47ba9f 100644
--- a/package.json
+++ b/package.json
@@ -36,7 +36,6 @@
"eslintfix": "npm run safe_bazelisk run polygerrit-ui/app:lint_bin -- -- --fix $(pwd)/polygerrit-ui/app",
"test:debug": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --root '.ts-out/polygerrit-ui/app/' --browsers ChromeDev --no-single-run --test-files",
"test:single": "npm run compile:local && npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --root '.ts-out/polygerrit-ui/app/' --test-files",
- "test:single:nocompile": "npm run safe_bazelisk run //polygerrit-ui:karma_bin -- -- start $(pwd)/polygerrit-ui/karma.conf.js --root '.ts-out/polygerrit-ui/app/' --test-files",
"polylint": "npm run safe_bazelisk test //polygerrit-ui/app:polylint_test",
"polylint:dev": "rm -rf ./polygerrit-ui/app/tmpl_out && npm run safe_bazelisk build //polygerrit-ui/app:template_test_tar && mkdir ./polygerrit-ui/app/tmpl_out && tar -xf bazel-bin/polygerrit-ui/app/template_test_tar.tar -C ./polygerrit-ui/app/tmpl_out"
},
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
index a2a46e0..bedaed9 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.ts
@@ -157,6 +157,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly shortcuts = appContext.shortcutsService;
+
override keyboardShortcuts() {
return {
[Shortcut.CURSOR_NEXT_CHANGE]: '_nextChange',
@@ -407,7 +409,7 @@
}
_nextChange(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -419,7 +421,7 @@
}
_prevChange(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -431,7 +433,7 @@
}
_openChange(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -442,7 +444,7 @@
_nextPage(e: CustomKeyboardEvent) {
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
(this.modifierPressed(e) && !isShiftPressed(e))
) {
return;
@@ -454,7 +456,7 @@
_prevPage(e: CustomKeyboardEvent) {
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
(this.modifierPressed(e) && !isShiftPressed(e))
) {
return;
@@ -470,7 +472,7 @@
}
_toggleChangeReviewed(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -489,7 +491,7 @@
}
_refreshChangeList(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
@@ -498,7 +500,7 @@
}
_toggleChangeStar(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 0400030..650b6d8 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -751,7 +751,7 @@
}
_handleToggleDiffMode(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1495,7 +1495,7 @@
}
_handleOpenReplyDialog(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
this._getLoggedIn().then(isLoggedIn => {
@@ -1510,7 +1510,7 @@
}
_handleOpenDownloadDialogShortcut(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1519,7 +1519,7 @@
}
_handleEditTopic(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1528,7 +1528,7 @@
}
_handleOpenSubmitDialog(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || !this._submitEnabled) {
+ if (this.shortcuts.shouldSuppress(e) || !this._submitEnabled) {
return;
}
@@ -1537,7 +1537,7 @@
}
_handleToggleAttentionSet(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
if (!this._change || !this._account?._account_id) return;
@@ -1579,7 +1579,7 @@
}
_handleDiffAgainstBase(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
assertIsDefined(this._change, '_change');
@@ -1593,7 +1593,7 @@
}
_handleDiffBaseAgainstLeft(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
assertIsDefined(this._change, '_change');
@@ -1607,7 +1607,7 @@
}
_handleDiffAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
assertIsDefined(this._change, '_change');
@@ -1626,7 +1626,7 @@
}
_handleDiffRightAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
assertIsDefined(this._change, '_change');
@@ -1645,7 +1645,7 @@
}
_handleDiffBaseAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
assertIsDefined(this._change, '_change');
@@ -1663,7 +1663,7 @@
}
_handleRefreshChange(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
e.preventDefault();
@@ -1671,7 +1671,7 @@
}
_handleToggleChangeStar(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
e.preventDefault();
@@ -1679,7 +1679,7 @@
}
_handleUpToDashboard(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1688,7 +1688,7 @@
}
_handleExpandAllMessages(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1699,7 +1699,7 @@
}
_handleCollapseAllMessages(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1710,7 +1710,7 @@
}
_handleOpenDiffPrefsShortcut(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 74d3cfc..de24b9e 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -400,7 +400,6 @@
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._handleDiffAgainstBase(new CustomEvent('') as CustomKeyboardEvent);
assert(navigateToChangeStub.called);
const args = navigateToChangeStub.getCall(0).args;
@@ -417,7 +416,6 @@
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._handleDiffAgainstLatest(
new CustomEvent('') as CustomKeyboardEvent
);
@@ -437,7 +435,6 @@
patchNum: 3 as RevisionPatchSetNum,
basePatchNum: 1 as BasePatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._handleDiffBaseAgainstLeft(
new CustomEvent('') as CustomKeyboardEvent
);
@@ -456,7 +453,6 @@
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._handleDiffRightAgainstLatest(
new CustomEvent('') as CustomKeyboardEvent
);
@@ -475,7 +471,6 @@
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._handleDiffBaseAgainstLatest(
new CustomEvent('') as CustomKeyboardEvent
);
@@ -501,7 +496,6 @@
basePatchNum: 1 as BasePatchSetNum,
patchNum: 3 as RevisionPatchSetNum,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
assert.isNotOk(element._change.attention_set);
await element._getLoggedIn();
@@ -829,7 +823,6 @@
});
test('m should toggle diff mode', () => {
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const setModeStub = sinon.stub(
element.$.fileListHeader,
'setDiffViewMode'
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index 40a2e1f..1df8f7f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -356,6 +356,8 @@
private diffCursor = new GrDiffCursor();
+ private readonly shortcuts = appContext.shortcutsService;
+
constructor() {
super();
this.fileCursor.scrollMode = ScrollMode.KEEP_VISIBLE;
@@ -887,7 +889,7 @@
}
_handleLeftPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this._noDiffsExpanded()) {
+ if (this.shortcuts.shouldSuppress(e) || this._noDiffsExpanded()) {
return;
}
@@ -896,7 +898,7 @@
}
_handleRightPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this._noDiffsExpanded()) {
+ if (this.shortcuts.shouldSuppress(e) || this._noDiffsExpanded()) {
return;
}
@@ -906,7 +908,7 @@
_handleToggleInlineDiff(e: CustomKeyboardEvent) {
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
this.modifierPressed(e) ||
e.detail?.keyboardEvent?.repeat ||
this.fileCursor.index === -1
@@ -919,10 +921,7 @@
}
_handleToggleAllInlineDiffs(e: CustomKeyboardEvent) {
- if (
- this.shouldSuppressKeyboardShortcut(e) ||
- e.detail?.keyboardEvent?.repeat
- ) {
+ if (this.shortcuts.shouldSuppress(e) || e.detail?.keyboardEvent?.repeat) {
return;
}
@@ -931,7 +930,7 @@
}
_handleToggleHideAllCommentThreads(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -940,7 +939,7 @@
}
_handleCursorNext(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -960,7 +959,7 @@
}
_handleCursorPrev(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -980,7 +979,7 @@
}
_handleNewComment(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
e.preventDefault();
@@ -990,7 +989,7 @@
_handleOpenLastFile(e: CustomKeyboardEvent) {
// Check for meta key to avoid overriding native chrome shortcut.
- if (this.shouldSuppressKeyboardShortcut(e) || getKeyboardEvent(e).metaKey) {
+ if (this.shortcuts.shouldSuppress(e) || getKeyboardEvent(e).metaKey) {
return;
}
@@ -1000,7 +999,7 @@
_handleOpenFirstFile(e: CustomKeyboardEvent) {
// Check for meta key to avoid overriding native chrome shortcut.
- if (this.shouldSuppressKeyboardShortcut(e) || getKeyboardEvent(e).metaKey) {
+ if (this.shortcuts.shouldSuppress(e) || getKeyboardEvent(e).metaKey) {
return;
}
@@ -1009,7 +1008,7 @@
}
_handleOpenFile(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
e.preventDefault();
@@ -1024,7 +1023,7 @@
_handleNextChunk(e: CustomKeyboardEvent) {
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
(this.modifierPressed(e) && !isShiftPressed(e)) ||
this._noDiffsExpanded()
) {
@@ -1041,7 +1040,7 @@
_handlePrevChunk(e: CustomKeyboardEvent) {
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
(this.modifierPressed(e) && !isShiftPressed(e)) ||
this._noDiffsExpanded()
) {
@@ -1057,7 +1056,7 @@
}
_handleToggleFileReviewed(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
@@ -1069,7 +1068,7 @@
}
_handleToggleLeftPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
@@ -1546,7 +1545,7 @@
}
_handleEscKey(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e) || this.modifierPressed(e)) {
+ if (this.shortcuts.shouldSuppress(e) || this.modifierPressed(e)) {
return;
}
e.preventDefault();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index 85e6f25..be15716 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -597,8 +597,6 @@
let interact;
setup(() => {
- sinon.stub(element, 'shouldSuppressKeyboardShortcut')
- .returns(false);
sinon.stub(element, 'modifierPressed').returns(false);
const openCursorStub = sinon.stub(element, '_openCursorFile');
const openSelectedStub = sinon.stub(element, '_openSelectedFile');
@@ -1707,8 +1705,6 @@
});
test('_displayLine', () => {
- sinon.stub(element, 'shouldSuppressKeyboardShortcut')
- .callsFake(() => false);
sinon.stub(element, 'modifierPressed')
.callsFake(() => false);
element.filesExpanded = FilesExpandedState.ALL;
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
index 240bb22..18a619a 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.ts
@@ -199,6 +199,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly shortcuts = appContext.shortcutsService;
+
constructor() {
super();
this.query = (input: string) => this._getSearchSuggestions(input);
@@ -399,7 +401,7 @@
_handleSearch(e: CustomKeyboardEvent) {
const keyboardEvent = getKeyboardEvent(e);
if (
- this.shouldSuppressKeyboardShortcut(e) ||
+ this.shortcuts.shouldSuppress(e) ||
(this.modifierPressed(e) && !keyboardEvent.shiftKey)
) {
return;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 978d98f..3b83fd8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -522,7 +522,7 @@
}
_handleToggleFileReviewed(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
e.preventDefault();
@@ -530,7 +530,7 @@
}
_handleEscKey(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
e.preventDefault();
@@ -538,21 +538,21 @@
}
_handleLeftPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
e.preventDefault();
this.cursor.moveLeft();
}
_handleRightPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
e.preventDefault();
this.cursor.moveRight();
}
_handlePrevLineOrFileWithComments(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (
e.detail.keyboardEvent?.shiftKey &&
@@ -572,7 +572,7 @@
}
_handleVisibleLine(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
e.preventDefault();
this.cursor.moveToVisibleArea();
@@ -583,7 +583,7 @@
}
_handleNextLineOrFileWithComments(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (
e.detail.keyboardEvent?.shiftKey &&
@@ -642,7 +642,7 @@
}
_handleNewComment(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
e.preventDefault();
@@ -651,7 +651,7 @@
}
_handlePrevFile(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
// Check for meta key to avoid overriding native chrome shortcut.
if (getKeyboardEvent(e).metaKey) return;
if (!this._path) return;
@@ -662,7 +662,7 @@
}
_handleNextFile(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
// Check for meta key to avoid overriding native chrome shortcut.
if (getKeyboardEvent(e).metaKey) return;
if (!this._path) return;
@@ -673,7 +673,7 @@
}
_handleNextChunkOrCommentThread(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
e.preventDefault();
if (e.detail.keyboardEvent?.shiftKey) {
@@ -734,7 +734,7 @@
}
_handlePrevChunkOrCommentThread(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
e.preventDefault();
if (e.detail.keyboardEvent?.shiftKey) {
@@ -750,7 +750,7 @@
// Similar to gr-change-view._handleOpenReplyDialog
_handleOpenReplyDialog(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
this._getLoggedIn().then(isLoggedIn => {
if (!isLoggedIn) {
@@ -765,7 +765,7 @@
}
_handleToggleLeftPane(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!e.detail.keyboardEvent?.shiftKey) return;
e.preventDefault();
@@ -773,7 +773,7 @@
}
_handleOpenDownloadDialog(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
this.set('changeViewState.showDownloadDialog', true);
@@ -782,7 +782,7 @@
}
_handleUpToChange(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
e.preventDefault();
@@ -790,7 +790,7 @@
}
_handleCommaKey(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
if (this._diffPrefsDisabled) return;
@@ -799,7 +799,7 @@
}
_handleToggleDiffMode(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
e.preventDefault();
@@ -1696,27 +1696,27 @@
}
_handleToggleBlame(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
this._toggleBlame();
}
_handleToggleHideAllCommentThreads(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
toggleClass(this, 'hideComments');
}
_handleOpenFileList(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (this.modifierPressed(e)) return;
this.$.dropdown.open();
}
_handleDiffAgainstBase(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!this._change) return;
if (!this._path) return;
if (!this._patchRange) return;
@@ -1733,7 +1733,7 @@
}
_handleDiffBaseAgainstLeft(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!this._change) return;
if (!this._path) return;
if (!this._patchRange) return;
@@ -1754,7 +1754,7 @@
}
_handleDiffAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!this._change) return;
if (!this._path) return;
if (!this._patchRange) return;
@@ -1774,7 +1774,7 @@
}
_handleDiffRightAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!this._change) return;
if (!this._path) return;
if (!this._patchRange) return;
@@ -1793,7 +1793,7 @@
}
_handleDiffBaseAgainstLatest(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
if (!this._change) return;
if (!this._path) return;
if (!this._patchRange) return;
@@ -1831,7 +1831,7 @@
}
_handleToggleAllDiffContext(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
this.$.diffHost.toggleAllContext();
}
@@ -1841,7 +1841,7 @@
}
_handleNextUnreviewedFile(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
this._setReviewed(true);
this.navigateToUnreviewedFile('next');
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 9aca719..a1ec6e2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -536,7 +536,6 @@
basePatchNum: 5,
patchNum: 10,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffAgainstBase(new CustomEvent(''));
const args = diffNavStub.getCall(0).args;
@@ -553,7 +552,6 @@
basePatchNum: 5,
patchNum: 10,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffAgainstLatest(new CustomEvent(''));
const args = diffNavStub.getCall(0).args;
@@ -571,7 +569,6 @@
basePatchNum: 1,
};
element.params = {};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffBaseAgainstLeft(new CustomEvent(''));
assert(diffNavStub.called);
@@ -594,7 +591,6 @@
sinon.stub(element, '_paramsChanged');
element.params = {commentLink: true, view: GerritView.DIFF};
element._focusLineNum = 10;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffBaseAgainstLeft(new CustomEvent(''));
assert(diffNavStub.called);
@@ -613,7 +609,6 @@
basePatchNum: 1,
patchNum: 3,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffRightAgainstLatest(new CustomEvent(''));
assert(diffNavStub.called);
@@ -631,7 +626,6 @@
basePatchNum: 1,
patchNum: 3,
};
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const diffNavStub = sinon.stub(GerritNav, 'navigateToDiff');
element._handleDiffBaseAgainstLatest(new CustomEvent(''));
assert(diffNavStub.called);
@@ -1483,7 +1477,6 @@
});
test('_handleToggleDiffMode', () => {
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
const e = {preventDefault: () => {}};
// Initial state.
assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index 3bbcd5b..aa5562d 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -214,6 +214,8 @@
private readonly restApiService = appContext.restApiService;
+ private readonly shortcuts = appContext.shortcutsService;
+
override keyboardShortcuts() {
return {
[Shortcut.OPEN_SHORTCUT_HELP_DIALOG]: '_showKeyboardShortcuts',
@@ -501,6 +503,7 @@
}
_showKeyboardShortcuts(e: CustomKeyboardEvent) {
+ if (this.shortcuts.shouldSuppress(e)) return;
// same shortcut should close the dialog if pressed again
// when dialog is open
this.loadKeyboardShortcutsDialog = true;
@@ -513,9 +516,6 @@
keyboardShortcuts.cancel();
return;
}
- if (this.shouldSuppressKeyboardShortcut(e)) {
- return;
- }
keyboardShortcuts.open();
this._footerHeaderAriaHidden = true;
this._mainAriaHidden = true;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
index fe4a66a..db4c7e0 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.ts
@@ -226,6 +226,8 @@
readonly restApiService = appContext.restApiService;
+ private readonly shortcuts = appContext.shortcutsService;
+
constructor() {
super();
this.addEventListener('comment-update', e =>
@@ -495,7 +497,7 @@
}
_handleEKey(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) {
+ if (this.shortcuts.shouldSuppress(e)) {
return;
}
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 2f249e7..d11d71e 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -15,7 +15,6 @@
* limitations under the License.
*/
import {IronA11yKeysBehavior} from '@polymer/iron-a11y-keys-behavior/iron-a11y-keys-behavior';
-import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {mixinBehaviors} from '@polymer/polymer/lib/legacy/class';
import {property} from '@polymer/decorators';
import {PolymerElement} from '@polymer/polymer';
@@ -82,12 +81,6 @@
ShortcutSection = ShortcutSection;
- private _disableKeyboardShortcuts = false;
-
- private readonly restApiService = appContext.restApiService;
-
- private readonly reporting = appContext.reportingService;
-
private readonly shortcuts = appContext.shortcutsService;
/** Used to disable shortcuts when the element is not visible. */
@@ -118,48 +111,6 @@
);
}
- shouldSuppressKeyboardShortcut(event: CustomKeyboardEvent) {
- if (this._disableKeyboardShortcuts) return true;
- const e = getKeyboardEvent(event);
- // TODO(TS): maybe override the EventApi, narrow it down to Element always
- const target = (dom(e) as EventApi).rootTarget as Element;
- const tagName = target.tagName;
- const type = target.getAttribute('type');
- if (
- // Suppress shortcuts on <input> and <textarea>, but not on
- // checkboxes, because we want to enable workflows like 'click
- // mark-reviewed and then press ] to go to the next file'.
- (tagName === 'INPUT' && type !== 'checkbox') ||
- tagName === 'TEXTAREA' ||
- // Suppress shortcuts if the key is 'enter'
- // and target is an anchor or button or paper-tab.
- (e.keyCode === 13 &&
- (tagName === 'A' || tagName === 'BUTTON' || tagName === 'PAPER-TAB'))
- ) {
- return true;
- }
- for (let i = 0; e.path && i < e.path.length; i++) {
- // TODO(TS): narrow this down to Element from EventTarget first
- if ((e.path[i] as Element).tagName === 'GR-OVERLAY') {
- return true;
- }
- }
-
- // eg: {key: "k:keydown", ..., from: "gr-diff-view"}
- let key = `${(e as unknown as KeyboardEvent).key}:${e.type}`;
- if (this._inGoKeyMode()) key = 'g+' + key;
- if (this.inVKeyMode()) key = 'v+' + key;
- if (e.shiftKey) key = 'shift+' + key;
- if (e.ctrlKey) key = 'ctrl+' + key;
- if (e.metaKey) key = 'meta+' + key;
- if (e.altKey) key = 'alt+' + key;
- this.reporting.reportInteraction('shortcut-triggered', {
- key,
- from: this.nodeName ?? 'unknown',
- });
- return false;
- }
-
// Alias for getKeyboardEvent.
getKeyboardEvent(e: CustomKeyboardEvent) {
return getKeyboardEvent(e);
@@ -191,11 +142,6 @@
override connectedCallback() {
super.connectedCallback();
- this.restApiService.getPreferences().then(prefs => {
- if (prefs?.disable_keyboard_shortcuts) {
- this._disableKeyboardShortcuts = true;
- }
- });
this.createVisibilityObserver();
this.enableBindings();
}
@@ -292,7 +238,7 @@
}
_handleVKeyDown(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
this._shortcut_v_key_last_pressed = Date.now();
}
@@ -313,7 +259,7 @@
if (
!this.inVKeyMode() ||
!this._shortcut_v_table.has(e.detail.key) ||
- this.shouldSuppressKeyboardShortcut(e)
+ this.shortcuts.shouldSuppress(e)
) {
return;
}
@@ -327,7 +273,7 @@
}
_handleGoKeyDown(e: CustomKeyboardEvent) {
- if (this.shouldSuppressKeyboardShortcut(e)) return;
+ if (this.shortcuts.shouldSuppress(e)) return;
this._shortcut_go_key_last_pressed = Date.now();
}
@@ -350,7 +296,7 @@
if (
!this._inGoKeyMode() ||
!this._shortcut_go_table.has(e.detail.key) ||
- this.shouldSuppressKeyboardShortcut(e)
+ this.shortcuts.shouldSuppress(e)
) {
return;
}
@@ -394,7 +340,6 @@
/** The interface corresponding to KeyboardShortcutMixin */
export interface KeyboardShortcutMixinInterface {
keyboardShortcuts(): {[key: string]: string | null};
- shouldSuppressKeyboardShortcut(event: CustomKeyboardEvent): boolean;
modifierPressed(event: CustomKeyboardEvent): boolean;
}
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.ts
index 6350bf9..babe44a 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.ts
@@ -16,11 +16,8 @@
*/
import '../../test/common-test-setup-karma';
import {KeyboardShortcutMixin} from './keyboard-shortcut-mixin';
-import {html} from '@polymer/polymer/lib/utils/html-tag';
import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {mockPromise, queryAndAssert} from '../../test/test-utils';
import '../../elements/shared/gr-overlay/gr-overlay';
-import {GrOverlay} from '../../elements/shared/gr-overlay/gr-overlay';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
import {CustomKeyboardEvent} from '../../types/events';
@@ -56,104 +53,14 @@
const basicFixture = fixtureFromElement('keyboard-shortcut-mixin-test-element');
-const withinOverlayFixture = fixtureFromTemplate(html`
- <gr-overlay>
- <keyboard-shortcut-mixin-test-element>
- </keyboard-shortcut-mixin-test-element>
- </gr-overlay>
-`);
-
suite('keyboard-shortcut-mixin tests', () => {
let element: GrKeyboardShortcutMixinTestElement;
- let overlay: GrOverlay;
setup(async () => {
element = basicFixture.instantiate();
- overlay = withinOverlayFixture.instantiate() as GrOverlay;
await flush();
});
- test('doesn’t block kb shortcuts for non-allowed els', async () => {
- const divEl = document.createElement('div');
- element.appendChild(divEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isFalse(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(divEl, 75, null, 'k');
- await promise;
- });
-
- test('blocks kb shortcuts for input els', async () => {
- const inputEl = document.createElement('input');
- element.appendChild(inputEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(inputEl, 75, null, 'k');
- await promise;
- });
-
- test('doesn’t block kb shortcuts for checkboxes', async () => {
- const inputEl = document.createElement('input');
- inputEl.setAttribute('type', 'checkbox');
- element.appendChild(inputEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isFalse(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(inputEl, 75, null, 'k');
- await promise;
- });
-
- test('blocks kb shortcuts for textarea els', async () => {
- const textareaEl = document.createElement('textarea');
- element.appendChild(textareaEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(textareaEl, 75, null, 'k');
- await promise;
- });
-
- test('blocks kb shortcuts for anything in a gr-overlay', async () => {
- const divEl = document.createElement('div');
- const element = queryAndAssert<GrKeyboardShortcutMixinTestElement>(
- overlay,
- 'keyboard-shortcut-mixin-test-element'
- );
- element.appendChild(divEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(divEl, 75, null, 'k');
- await promise;
- });
-
- test('blocks enter shortcut on an anchor', async () => {
- const anchorEl = document.createElement('a');
- const element = queryAndAssert<GrKeyboardShortcutMixinTestElement>(
- overlay,
- 'keyboard-shortcut-mixin-test-element'
- );
- element.appendChild(anchorEl);
- const promise = mockPromise();
- element._handleKey = e => {
- assert.isTrue(element.shouldSuppressKeyboardShortcut(e));
- promise.resolve();
- };
- MockInteractions.keyDownOn(anchorEl, 13, null, 'enter');
- await promise;
- });
-
test('modifierPressed returns accurate values', () => {
const spy = sinon.spy(element, 'modifierPressed');
element._handleKey = e => {
@@ -189,7 +96,6 @@
detail: {key: 'a'},
preventDefault: () => {},
} as CustomKeyboardEvent;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._shortcut_go_key_last_pressed = 9000;
element._handleGoAction(e);
assert.isTrue(handlerStub.calledOnce);
@@ -201,7 +107,6 @@
detail: {key: 'a'},
preventDefault: () => {},
} as CustomKeyboardEvent;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._shortcut_go_key_last_pressed = null;
element._handleGoAction(e);
assert.isFalse(handlerStub.called);
@@ -212,29 +117,16 @@
detail: {key: 'a'},
preventDefault: () => {},
} as CustomKeyboardEvent;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._shortcut_go_key_last_pressed = 3000;
element._handleGoAction(e);
assert.isFalse(handlerStub.called);
});
- test('should suppress', () => {
- const e = {
- detail: {key: 'a'},
- preventDefault: () => {},
- } as CustomKeyboardEvent;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(true);
- element._shortcut_go_key_last_pressed = 9000;
- element._handleGoAction(e);
- assert.isFalse(handlerStub.called);
- });
-
test('unrecognized key', () => {
const e = {
detail: {key: 'f'},
preventDefault: () => {},
} as CustomKeyboardEvent;
- sinon.stub(element, 'shouldSuppressKeyboardShortcut').returns(false);
element._shortcut_go_key_last_pressed = 9000;
element._handleGoAction(e);
assert.isFalse(handlerStub.called);
diff --git a/polygerrit-ui/app/services/app-context-init.ts b/polygerrit-ui/app/services/app-context-init.ts
index 597776d..3a6f7c5 100644
--- a/polygerrit-ui/app/services/app-context-init.ts
+++ b/polygerrit-ui/app/services/app-context-init.ts
@@ -83,6 +83,6 @@
storageService: () => new GrStorageService(),
configService: () => new ConfigService(),
userService: () => new UserService(appContext.restApiService),
- shortcutsService: () => new ShortcutsService(),
+ shortcutsService: () => new ShortcutsService(appContext.reportingService),
});
}
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
index 3cf46bd..c3aa06e 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service.ts
@@ -21,6 +21,10 @@
ShortcutSection,
SPECIAL_SHORTCUT,
} from './shortcuts-config';
+import {disableShortcuts$} from '../user/user-model';
+import {CustomKeyboardEvent} from '../../types/events';
+import {getKeyboardEvent, isElementTarget} from '../../utils/dom-util';
+import {ReportingService} from '../gr-reporting/gr-reporting';
export type SectionView = Array<{binding: string[][]; text: string}>;
@@ -35,25 +39,84 @@
* Shortcuts service, holds all hosts, bindings and listeners.
*/
export class ShortcutsService {
+ /**
+ * Keeps track of the components that are currently active such that we can
+ * show a shortcut help dialog that only shows the shortcuts that are
+ * currently relevant.
+ */
private readonly activeHosts = new Map<unknown, Map<string, string>>();
+ /** Static map built in the constructor by iterating over the config. */
private readonly bindings = new Map<Shortcut, string[]>();
private readonly listeners = new Set<ShortcutListener>();
- constructor() {
+ /** Keeps track of the corresponding user preference. */
+ private shortcutsDisabled = false;
+
+ constructor(readonly reporting?: ReportingService) {
for (const section of config.keys()) {
const items = config.get(section) ?? [];
for (const item of items) {
this.bindings.set(item.shortcut, item.bindings);
}
}
+ disableShortcuts$.subscribe(x => (this.shortcutsDisabled = x));
}
public _testOnly_isEmpty() {
return this.activeHosts.size === 0 && this.listeners.size === 0;
}
+ shouldSuppress(event: CustomKeyboardEvent) {
+ if (this.shortcutsDisabled) return true;
+ const e = getKeyboardEvent(event);
+
+ // Note that when you listen on document, then `e.currentTarget` will be the
+ // document and `e.target` will be `<gr-app>` due to shadow dom, but by
+ // using the composedPath() you can actually find the true origin of the
+ // event.
+ const rootTarget = e.composedPath()[0];
+ if (!isElementTarget(rootTarget)) return false;
+ const tagName = rootTarget.tagName;
+ const type = rootTarget.getAttribute('type');
+
+ if (
+ // Suppress shortcuts on <input> and <textarea>, but not on
+ // checkboxes, because we want to enable workflows like 'click
+ // mark-reviewed and then press ] to go to the next file'.
+ (tagName === 'INPUT' && type !== 'checkbox') ||
+ tagName === 'TEXTAREA' ||
+ // Suppress shortcuts if the key is 'enter'
+ // and target is an anchor or button or paper-tab.
+ (e.keyCode === 13 &&
+ (tagName === 'A' || tagName === 'BUTTON' || tagName === 'PAPER-TAB'))
+ ) {
+ return true;
+ }
+ for (let i = 0; e.path && i < e.path.length; i++) {
+ // TODO(TS): narrow this down to Element from EventTarget first
+ if ((e.path[i] as Element).tagName === 'GR-OVERLAY') {
+ return true;
+ }
+ }
+ // eg: {key: "k:keydown", ..., from: "gr-diff-view"}
+ let key = `${(e as unknown as KeyboardEvent).key}:${e.type}`;
+ // TODO(brohlfs): Re-enable reporting of g- and v-keys.
+ // if (this._inGoKeyMode()) key = 'g+' + key;
+ // if (this.inVKeyMode()) key = 'v+' + key;
+ if (e.shiftKey) key = 'shift+' + key;
+ if (e.ctrlKey) key = 'ctrl+' + key;
+ if (e.metaKey) key = 'meta+' + key;
+ if (e.altKey) key = 'alt+' + key;
+ let from = 'unknown';
+ if (isElementTarget(e.currentTarget)) {
+ from = e.currentTarget.tagName;
+ }
+ this.reporting?.reportInteraction('shortcut-triggered', {key, from});
+ return false;
+ }
+
createTitle(shortcutName: Shortcut, section: ShortcutSection) {
const desc = this.getDescription(section, shortcutName);
const shortcut = this.getShortcut(shortcutName);
diff --git a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
index a48fa92..f795f78 100644
--- a/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
+++ b/polygerrit-ui/app/services/shortcuts/shortcuts-service_test.ts
@@ -17,18 +17,90 @@
import '../../test/common-test-setup-karma';
import {ShortcutsService} from '../../services/shortcuts/shortcuts-service';
import {Shortcut, ShortcutSection} from './shortcuts-config';
+import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
+import {CustomKeyboardEvent} from '../../types/events';
+
+async function keyEventOn(
+ el: Element,
+ callback: (e: CustomKeyboardEvent) => void,
+ keyCode = 75,
+ key = 'k'
+): Promise<CustomKeyboardEvent> {
+ let resolve: (e: CustomKeyboardEvent) => void;
+ const promise = new Promise<CustomKeyboardEvent>(r => (resolve = r));
+ el.addEventListener('keydown', e => {
+ const cke = e as CustomKeyboardEvent;
+ callback(cke);
+ resolve(cke);
+ });
+ MockInteractions.keyDownOn(el, keyCode, null, key);
+ return await promise;
+}
suite('shortcuts-service tests', () => {
+ let service: ShortcutsService;
+
+ setup(() => {
+ service = new ShortcutsService();
+ });
+
+ suite('shouldSuppress', () => {
+ test('do not suppress shortcut event from <div>', async () => {
+ await keyEventOn(document.createElement('div'), e => {
+ assert.isFalse(service.shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from <input>', async () => {
+ await keyEventOn(document.createElement('input'), e => {
+ assert.isTrue(service.shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from <textarea>', async () => {
+ await keyEventOn(document.createElement('textarea'), e => {
+ assert.isTrue(service.shouldSuppress(e));
+ });
+ });
+
+ test('do not suppress shortcut event from checkbox <input>', async () => {
+ const inputEl = document.createElement('input');
+ inputEl.setAttribute('type', 'checkbox');
+ await keyEventOn(inputEl, e => {
+ assert.isFalse(service.shouldSuppress(e));
+ });
+ });
+
+ test('suppress shortcut event from children of <gr-overlay>', async () => {
+ const overlay = document.createElement('gr-overlay');
+ const div = document.createElement('div');
+ overlay.appendChild(div);
+ await keyEventOn(div, e => {
+ assert.isTrue(service.shouldSuppress(e));
+ });
+ });
+
+ test('suppress "enter" shortcut event from <a>', async () => {
+ await keyEventOn(document.createElement('a'), e => {
+ assert.isFalse(service.shouldSuppress(e));
+ });
+ await keyEventOn(
+ document.createElement('a'),
+ e => assert.isTrue(service.shouldSuppress(e)),
+ 13,
+ 'enter'
+ );
+ });
+ });
+
test('getShortcut', () => {
- const mgr = new ShortcutsService();
const NEXT_FILE = Shortcut.NEXT_FILE;
- assert.equal(mgr.getShortcut(NEXT_FILE), ']');
+ assert.equal(service.getShortcut(NEXT_FILE), ']');
});
test('getShortcut with modifiers', () => {
- const mgr = new ShortcutsService();
const NEXT_FILE = Shortcut.TOGGLE_LEFT_PANE;
- assert.equal(mgr.getShortcut(NEXT_FILE), 'Shift+a');
+ assert.equal(service.getShortcut(NEXT_FILE), 'Shift+a');
});
suite('binding descriptions', () => {
@@ -39,11 +111,10 @@
}
test('single combo description', () => {
- const mgr = new ShortcutsService();
- assert.deepEqual(mgr.describeBinding('a'), ['a']);
- assert.deepEqual(mgr.describeBinding('a:keyup'), ['a']);
- assert.deepEqual(mgr.describeBinding('ctrl+a'), ['Ctrl', 'a']);
- assert.deepEqual(mgr.describeBinding('ctrl+shift+up:keyup'), [
+ assert.deepEqual(service.describeBinding('a'), ['a']);
+ assert.deepEqual(service.describeBinding('a:keyup'), ['a']);
+ assert.deepEqual(service.describeBinding('ctrl+a'), ['Ctrl', 'a']);
+ assert.deepEqual(service.describeBinding('ctrl+shift+up:keyup'), [
'Ctrl',
'Shift',
'↑',
@@ -51,45 +122,48 @@
});
test('combo set description', () => {
- const mgr = new ShortcutsService();
- assert.deepEqual(mgr.describeBindings(Shortcut.GO_TO_OPENED_CHANGES), [
- ['g', 'o'],
- ]);
- assert.deepEqual(mgr.describeBindings(Shortcut.SAVE_COMMENT), [
+ assert.deepEqual(
+ service.describeBindings(Shortcut.GO_TO_OPENED_CHANGES),
+ [['g', 'o']]
+ );
+ assert.deepEqual(service.describeBindings(Shortcut.SAVE_COMMENT), [
['Ctrl', 'Enter'],
['Meta', 'Enter'],
['Ctrl', 's'],
['Meta', 's'],
]);
- assert.deepEqual(mgr.describeBindings(Shortcut.PREV_FILE), [['[']]);
+ assert.deepEqual(service.describeBindings(Shortcut.PREV_FILE), [['[']]);
});
test('combo set description width', () => {
- const mgr = new ShortcutsService();
- assert.strictEqual(mgr.comboSetDisplayWidth([['u']]), 1);
- assert.strictEqual(mgr.comboSetDisplayWidth([['g', 'o']]), 2);
- assert.strictEqual(mgr.comboSetDisplayWidth([['Shift', 'r']]), 6);
- assert.strictEqual(mgr.comboSetDisplayWidth([['x'], ['y']]), 4);
+ assert.strictEqual(service.comboSetDisplayWidth([['u']]), 1);
+ assert.strictEqual(service.comboSetDisplayWidth([['g', 'o']]), 2);
+ assert.strictEqual(service.comboSetDisplayWidth([['Shift', 'r']]), 6);
+ assert.strictEqual(service.comboSetDisplayWidth([['x'], ['y']]), 4);
assert.strictEqual(
- mgr.comboSetDisplayWidth([['x'], ['y'], ['Shift', 'z']]),
+ service.comboSetDisplayWidth([['x'], ['y'], ['Shift', 'z']]),
12
);
});
test('distribute shortcut help', () => {
- const mgr = new ShortcutsService();
- assert.deepEqual(mgr.distributeBindingDesc([['o']]), [[['o']]]);
- assert.deepEqual(mgr.distributeBindingDesc([['g', 'o']]), [[['g', 'o']]]);
+ assert.deepEqual(service.distributeBindingDesc([['o']]), [[['o']]]);
+ assert.deepEqual(service.distributeBindingDesc([['g', 'o']]), [
+ [['g', 'o']],
+ ]);
assert.deepEqual(
- mgr.distributeBindingDesc([['ctrl', 'shift', 'meta', 'enter']]),
+ service.distributeBindingDesc([['ctrl', 'shift', 'meta', 'enter']]),
[[['ctrl', 'shift', 'meta', 'enter']]]
);
assert.deepEqual(
- mgr.distributeBindingDesc([['ctrl', 'shift', 'meta', 'enter'], ['o']]),
+ service.distributeBindingDesc([
+ ['ctrl', 'shift', 'meta', 'enter'],
+ ['o'],
+ ]),
[[['ctrl', 'shift', 'meta', 'enter']], [['o']]]
);
assert.deepEqual(
- mgr.distributeBindingDesc([
+ service.distributeBindingDesc([
['ctrl', 'enter'],
['meta', 'enter'],
['ctrl', 's'],
@@ -109,11 +183,10 @@
});
test('active shortcuts by section', () => {
- const mgr = new ShortcutsService();
- assert.deepEqual(mapToObject(mgr.activeShortcutsBySection()), {});
+ assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {});
- mgr.attachHost({}, new Map([[Shortcut.NEXT_FILE, 'null']]));
- assert.deepEqual(mapToObject(mgr.activeShortcutsBySection()), {
+ service.attachHost({}, new Map([[Shortcut.NEXT_FILE, 'null']]));
+ assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.NAVIGATION]: [
{
shortcut: Shortcut.NEXT_FILE,
@@ -123,8 +196,8 @@
],
});
- mgr.attachHost({}, new Map([[Shortcut.NEXT_LINE, 'null']]));
- assert.deepEqual(mapToObject(mgr.activeShortcutsBySection()), {
+ service.attachHost({}, new Map([[Shortcut.NEXT_LINE, 'null']]));
+ assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.DIFFS]: [
{
shortcut: Shortcut.NEXT_LINE,
@@ -141,14 +214,14 @@
],
});
- mgr.attachHost(
+ service.attachHost(
{},
new Map([
[Shortcut.SEARCH, 'null'],
[Shortcut.GO_TO_OPENED_CHANGES, 'null'],
])
);
- assert.deepEqual(mapToObject(mgr.activeShortcutsBySection()), {
+ assert.deepEqual(mapToObject(service.activeShortcutsBySection()), {
[ShortcutSection.DIFFS]: [
{
shortcut: Shortcut.NEXT_LINE,
@@ -179,11 +252,9 @@
});
test('directory view', () => {
- const mgr = new ShortcutsService();
+ assert.deepEqual(mapToObject(service.directoryView()), {});
- assert.deepEqual(mapToObject(mgr.directoryView()), {});
-
- mgr.attachHost(
+ service.attachHost(
{},
new Map([
[Shortcut.GO_TO_OPENED_CHANGES, 'null'],
@@ -193,7 +264,7 @@
[Shortcut.SEARCH, 'null'],
])
);
- assert.deepEqual(mapToObject(mgr.directoryView()), {
+ assert.deepEqual(mapToObject(service.directoryView()), {
[ShortcutSection.DIFFS]: [
{binding: [['j'], ['↓']], text: 'Go to next line'},
{
diff --git a/polygerrit-ui/app/services/user/user-model.ts b/polygerrit-ui/app/services/user/user-model.ts
index 4115a71..72ce3e1 100644
--- a/polygerrit-ui/app/services/user/user-model.ts
+++ b/polygerrit-ui/app/services/user/user-model.ts
@@ -60,3 +60,8 @@
map(preferences => preferences?.my ?? []),
distinctUntilChanged()
);
+
+export const disableShortcuts$ = preferences$.pipe(
+ map(preferences => preferences?.disable_keyboard_shortcuts ?? false),
+ distinctUntilChanged()
+);
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index 7b1f3e3..71f913d 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -37,6 +37,17 @@
return 'shadowRoot' in el;
}
+export function isElement(node: Node): node is Element {
+ return node.nodeType === 1;
+}
+
+export function isElementTarget(
+ target: EventTarget | null | undefined
+): target is Element {
+ if (!target) return false;
+ return 'nodeType' in target && isElement(target as Node);
+}
+
// TODO: maybe should have a better name for this
function getPathFromNode(el: EventTarget) {
let tagName = '';
