Merge "Refactor the keyboard handling a little bit"
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 62cfcb4..2b79fe0 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
@@ -28,7 +28,6 @@
import {
KeyboardShortcutMixin,
Shortcut,
- Modifier,
} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {
GerritNav,
@@ -53,7 +52,7 @@
} from '../../../utils/attention-set-util';
import {CustomKeyboardEvent} from '../../../types/events';
import {fireEvent} from '../../../utils/event-util';
-import {windowLocationReload} from '../../../utils/dom-util';
+import {isShiftPressed, windowLocationReload} from '../../../utils/dom-util';
import {ScrollMode} from '../../../constants/constants';
const NUMBER_FIXED_COLUMNS = 3;
@@ -439,8 +438,7 @@
_nextPage(e: CustomKeyboardEvent) {
if (
this.shouldSuppressKeyboardShortcut(e) ||
- (this.modifierPressed(e) &&
- !this.isModifierPressed(e, Modifier.SHIFT_KEY))
+ (this.modifierPressed(e) && !isShiftPressed(e))
) {
return;
}
@@ -452,8 +450,7 @@
_prevPage(e: CustomKeyboardEvent) {
if (
this.shouldSuppressKeyboardShortcut(e) ||
- (this.modifierPressed(e) &&
- !this.isModifierPressed(e, Modifier.SHIFT_KEY))
+ (this.modifierPressed(e) && !isShiftPressed(e))
) {
return;
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
index e94f67e..e22253b 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list-header/gr-file-list-header.ts
@@ -24,7 +24,6 @@
import '../gr-commit-info/gr-commit-info';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {htmlTemplate} from './gr-file-list-header_html';
-import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {FilesExpandedState} from '../gr-file-list-constants';
import {GerritNav} from '../../core/gr-navigation/gr-navigation';
import {computeLatestPatchNum, PatchSet} from '../../../utils/patch-set-util';
@@ -63,7 +62,7 @@
}
@customElement('gr-file-list-header')
-export class GrFileListHeader extends KeyboardShortcutMixin(PolymerElement) {
+export class GrFileListHeader extends PolymerElement {
static get template() {
return htmlTemplate;
}
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 4b5525a..fc48679 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
@@ -33,7 +33,6 @@
import {asyncForeach, debounce, DelayedTask} from '../../../utils/async-util';
import {
KeyboardShortcutMixin,
- Modifier,
Shortcut,
} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {FilesExpandedState} from '../gr-file-list-constants';
@@ -47,7 +46,12 @@
ScrollMode,
SpecialFilePath,
} from '../../../constants/constants';
-import {descendedFromClass, toggleClass} from '../../../utils/dom-util';
+import {
+ descendedFromClass,
+ getKeyboardEvent,
+ isShiftPressed,
+ toggleClass,
+} from '../../../utils/dom-util';
import {
addUnmodifiedFiles,
computeDisplayPath,
@@ -924,7 +928,7 @@
this._displayLine = true;
} else {
// Down key
- if (this.getKeyboardEvent(e).keyCode === 40) {
+ if (getKeyboardEvent(e).keyCode === 40) {
return;
}
e.preventDefault();
@@ -944,7 +948,7 @@
this._displayLine = true;
} else {
// Up key
- if (this.getKeyboardEvent(e).keyCode === 38) {
+ if (getKeyboardEvent(e).keyCode === 38) {
return;
}
e.preventDefault();
@@ -964,10 +968,7 @@
_handleOpenLastFile(e: CustomKeyboardEvent) {
// Check for meta key to avoid overriding native chrome shortcut.
- if (
- this.shouldSuppressKeyboardShortcut(e) ||
- this.getKeyboardEvent(e).metaKey
- ) {
+ if (this.shouldSuppressKeyboardShortcut(e) || getKeyboardEvent(e).metaKey) {
return;
}
@@ -977,10 +978,7 @@
_handleOpenFirstFile(e: CustomKeyboardEvent) {
// Check for meta key to avoid overriding native chrome shortcut.
- if (
- this.shouldSuppressKeyboardShortcut(e) ||
- this.getKeyboardEvent(e).metaKey
- ) {
+ if (this.shouldSuppressKeyboardShortcut(e) || getKeyboardEvent(e).metaKey) {
return;
}
@@ -1005,15 +1003,14 @@
_handleNextChunk(e: CustomKeyboardEvent) {
if (
this.shouldSuppressKeyboardShortcut(e) ||
- (this.modifierPressed(e) &&
- !this.isModifierPressed(e, Modifier.SHIFT_KEY)) ||
+ (this.modifierPressed(e) && !isShiftPressed(e)) ||
this._noDiffsExpanded()
) {
return;
}
e.preventDefault();
- if (this.isModifierPressed(e, Modifier.SHIFT_KEY)) {
+ if (isShiftPressed(e)) {
this.$.diffCursor.moveToNextCommentThread();
} else {
this.$.diffCursor.moveToNextChunk();
@@ -1023,15 +1020,14 @@
_handlePrevChunk(e: CustomKeyboardEvent) {
if (
this.shouldSuppressKeyboardShortcut(e) ||
- (this.modifierPressed(e) &&
- !this.isModifierPressed(e, Modifier.SHIFT_KEY)) ||
+ (this.modifierPressed(e) && !isShiftPressed(e)) ||
this._noDiffsExpanded()
) {
return;
}
e.preventDefault();
- if (this.isModifierPressed(e, Modifier.SHIFT_KEY)) {
+ if (isShiftPressed(e)) {
this.$.diffCursor.moveToPreviousCommentThread();
} else {
this.$.diffCursor.moveToPreviousChunk();
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 9c63f8b..a3ddff3 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
@@ -34,6 +34,7 @@
import {CustomKeyboardEvent} from '../../../types/events';
import {MergeabilityComputationBehavior} from '../../../constants/constants';
import {appContext} from '../../../services/app-context';
+import {getKeyboardEvent} from '../../../utils/dom-util';
// Possible static search options for auto complete, without negations.
const SEARCH_OPERATORS: ReadonlyArray<string> = [
@@ -385,7 +386,7 @@
}
_handleSearch(e: CustomKeyboardEvent) {
- const keyboardEvent = this.getKeyboardEvent(e);
+ const keyboardEvent = getKeyboardEvent(e);
if (
this.shouldSuppressKeyboardShortcut(e) ||
(this.modifierPressed(e) && !keyboardEvent.shiftKey)
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 4a51349..ad2e9c0 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
@@ -98,7 +98,7 @@
import {fireAlert, fireEvent, fireTitleChange} from '../../../utils/event-util';
import {GerritView} from '../../../services/router/router-model';
import {assertIsDefined} from '../../../utils/common-util';
-import {toggleClass} from '../../../utils/dom-util';
+import {toggleClass, getKeyboardEvent} from '../../../utils/dom-util';
const ERR_REVIEW_STATUS = 'Couldn’t change file review status.';
const MSG_LOADING_BLAME = 'Loading blame...';
const MSG_LOADED_BLAME = 'Blame loaded';
@@ -593,7 +593,7 @@
_handlePrevFile(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
// Check for meta key to avoid overriding native chrome shortcut.
- if (this.getKeyboardEvent(e).metaKey) return;
+ if (getKeyboardEvent(e).metaKey) return;
if (!this._path) return;
if (!this._fileList) return;
@@ -604,7 +604,7 @@
_handleNextFile(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
// Check for meta key to avoid overriding native chrome shortcut.
- if (this.getKeyboardEvent(e).metaKey) return;
+ if (getKeyboardEvent(e).metaKey) return;
if (!this._path) return;
if (!this._fileList) return;
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
index 2b66af8..01ca57a 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.ts
@@ -20,7 +20,6 @@
import {htmlTemplate} from './gr-change-star_html';
import {customElement, property} from '@polymer/decorators';
import {ChangeInfo} from '../../../types/common';
-import {KeyboardShortcutMixin} from '../../../mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin';
import {fireAlert} from '../../../utils/event-util';
declare global {
@@ -35,7 +34,7 @@
}
@customElement('gr-change-star')
-export class GrChangeStar extends KeyboardShortcutMixin(PolymerElement) {
+export class GrChangeStar extends PolymerElement {
static get template() {
return htmlTemplate;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
index c758455..1702b4a 100644
--- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
+++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.ts
@@ -31,6 +31,7 @@
AutocompleteQuery,
GrAutocomplete,
} from '../gr-autocomplete/gr-autocomplete';
+import {getKeyboardEvent} from '../../../utils/dom-util';
const AWAIT_MAX_ITERS = 10;
const AWAIT_STEP = 5;
@@ -203,7 +204,7 @@
}
_handleEnter(e: CustomKeyboardEvent) {
- e = this.getKeyboardEvent(e);
+ e = getKeyboardEvent(e);
const target = (dom(e) as EventApi).rootTarget;
if (target === this._nativeInput) {
e.preventDefault();
@@ -212,7 +213,7 @@
}
_handleEsc(e: CustomKeyboardEvent) {
- e = this.getKeyboardEvent(e);
+ e = getKeyboardEvent(e);
const target = (dom(e) as EventApi).rootTarget;
if (target === this._nativeInput) {
e.preventDefault();
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 494f0f04..837a795 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
@@ -750,16 +750,6 @@
const shortcutManager = new ShortcutManager();
-/**
- * Enum for supported modifiers.
- */
-export enum Modifier {
- SHIFT_KEY = 'shiftKey',
- CTRL_KEY = 'ctrlKey',
- META_KEY = 'metaKey',
- // Add when you need it
-}
-
interface IronA11yKeysMixinConstructor {
// Note: this is needed to have same interface as other mixins
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -809,10 +799,6 @@
);
}
- isModifierPressed(e: CustomKeyboardEvent, modifier: Modifier) {
- return getKeyboardEvent(e)[modifier];
- }
-
shouldSuppressKeyboardShortcut(event: CustomKeyboardEvent) {
if (this._disableKeyboardShortcuts) return true;
const e = getKeyboardEvent(event);
@@ -1083,8 +1069,6 @@
bindShortcut(shortcut: Shortcut, ...bindings: string[]): void;
shouldSuppressKeyboardShortcut(event: CustomKeyboardEvent): boolean;
modifierPressed(event: CustomKeyboardEvent): boolean;
- isModifierPressed(event: CustomKeyboardEvent, modifier: Modifier): boolean;
- getKeyboardEvent(e: CustomKeyboardEvent): CustomKeyboardEvent;
addKeyboardShortcutDirectoryListener(listener: ShortcutListener): void;
removeKeyboardShortcutDirectoryListener(listener: ShortcutListener): void;
// TODO(TS): Remove underscore. Apparently not a private method.
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
index 180dbe7..b22b8d8 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
@@ -377,27 +377,6 @@
assert.isTrue(spy.lastCall.returnValue);
});
- test('isModifierPressed returns accurate value', () => {
- const spy = sinon.spy(element, 'isModifierPressed');
- element._handleKey = e => {
- element.isModifierPressed(e, 'shiftKey');
- };
- MockInteractions.keyDownOn(element, 75, 'shift', 'k');
- assert.isTrue(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, null, 'k');
- assert.isFalse(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, 'ctrl', 'k');
- assert.isFalse(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, null, 'k');
- assert.isFalse(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, 'meta', 'k');
- assert.isFalse(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, null, 'k');
- assert.isFalse(spy.lastCall.returnValue);
- MockInteractions.keyDownOn(element, 75, 'alt', 'k');
- assert.isFalse(spy.lastCall.returnValue);
- });
-
suite('GO_KEY timing', () => {
let handlerStub;
diff --git a/polygerrit-ui/app/utils/dom-util.ts b/polygerrit-ui/app/utils/dom-util.ts
index c119bf8..4f83881 100644
--- a/polygerrit-ui/app/utils/dom-util.ts
+++ b/polygerrit-ui/app/utils/dom-util.ts
@@ -299,6 +299,11 @@
return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey;
}
+export function isShiftPressed(event: CustomKeyboardEvent) {
+ const e = getKeyboardEvent(event);
+ return e.shiftKey;
+}
+
export function getKeyboardEvent(e: CustomKeyboardEvent): CustomKeyboardEvent {
const event = dom(e.detail ? e.detail.keyboardEvent : e);
// TODO(TS): worth checking if this still holds or not, if no, remove this.
