opendev/gerrit
Code Issues Proposed changes

Add namespacing for keyboard shortcut disabling

Browse Source 
The gr-overlay element attempts to manage disabling and enabling
keyboard shortcuts. When multiple gr-overlay elements are available
on a page and one of them opens immediately, that overlay tries to
disable keyboard shortcuts, but the other elements initialize as
closed and enable them.

This change offers a new method for disabling keyboard shortcuts.
The caller can pass in an identifier to enable or disable. If keyboard
shortcuts are disabled by one or more identifiers, then they are
suppressed.

Change-Id: I82fe6efd922f09279e76a2f2c8cb5781f3afe395
This commit is contained in:
Logan Hanks
2016-10-10 17:10:10 -07:00
parent 25f49af1ed
commit 610f80d3ec
11 changed files with 111 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html
View File

@@ -20,7 +20,9 @@ limitations under the License.
/** @polymerBehavior Gerrit.KeyboardShortcutBehavior */
var KeyboardShortcutBehavior = {
enabled: true,
// Set of identifiers currently blocking keyboard shortcuts. Stored as
// a map of string to the value of true.
_disablers: {},
properties: {
keyEventTarget: {
@@ -43,8 +45,12 @@ limitations under the License.
this.keyEventTarget.removeEventListener('keydown', this._boundKeyHandler);
},
shouldSupressKeyboardShortcut: function(e) {
if (!KeyboardShortcutBehavior.enabled) { return true; }
shouldSuppressKeyboardShortcut: function(e) {
for (var c in KeyboardShortcutBehavior._disablers) {
if (KeyboardShortcutBehavior._disablers[c] === true) {
return true;
}
}
var getModifierState = e.getModifierState ?
e.getModifierState.bind(e) :
function() { return false; };
@@ -60,6 +66,14 @@ limitations under the License.
target.tagName == 'A' ||
target.tagName == 'GR-BUTTON';
},
disable: function(id) {
KeyboardShortcutBehavior._disablers[id] = true;
},
enable: function(id) {
delete KeyboardShortcutBehavior._disablers[id];
},
};
window.Gerrit = window.Gerrit || {};

78
polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html Normal file
View File

@@ -0,0 +1,78 @@
<!DOCTYPE html>
<!--
Copyright (C) 2016 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>keyboard-shortcut-behavior</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="keyboard-shortcut-behavior.html">
<test-fixture id="basic">
<template>
<test-element></test-element>
</template>
</test-fixture>
<script>
suite('keyboard-shortcut-behavior tests', function() {
var element;
suiteSetup(function() {
// Define a Polymer element that uses this behavior.
Polymer({
is: 'test-element',
behaviors: [Gerrit.KeyboardShortcutBehavior],
properties: {
keyEventTarget: {
value: function() { return document.body; },
},
log: {
value: function() { return []; },
},
},
_handleKey: function(e) {
if (!this.shouldSuppressKeyboardShortcut(e)) {
this.log.push(e.keyCode);
}
},
});
});
setup(function() {
element = fixture('basic');
});
test('blocks keydown events iff one or more disablers', function() {
MockInteractions.pressAndReleaseKeyOn(document.body, 97); // 'a'
Gerrit.KeyboardShortcutBehavior.enable('x'); // should have no effect
MockInteractions.pressAndReleaseKeyOn(document.body, 98); // 'b'
Gerrit.KeyboardShortcutBehavior.disable('x'); // blocking starts here
MockInteractions.pressAndReleaseKeyOn(document.body, 99); // 'c'
Gerrit.KeyboardShortcutBehavior.disable('y');
MockInteractions.pressAndReleaseKeyOn(document.body, 100); // 'd'
Gerrit.KeyboardShortcutBehavior.enable('x');
MockInteractions.pressAndReleaseKeyOn(document.body, 101); // 'e'
Gerrit.KeyboardShortcutBehavior.enable('y'); // blocking ends here
MockInteractions.pressAndReleaseKeyOn(document.body, 102); // 'f'
assert.deepEqual(element.log, [97, 98, 102]);
});
});
</script>

2
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
View File

@@ -150,7 +150,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (this.groups == null) { return; }
var len = 0;

2
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@@ -585,7 +585,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
switch (e.keyCode) {
case 65: // 'a'
if (this._loggedIn && !e.shiftKey) {

2
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
View File

@@ -295,7 +295,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
switch (e.keyCode) {
case 37: // left
if (e.shiftKey && this._showInlineDiffs) {

2
polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
View File

@@ -293,7 +293,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
switch (e.keyCode) {
case 191: // '/' or '?' with shift key.
// TODO(andybons): Localization using e.key/keypress event.

2
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
View File

@@ -89,7 +89,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (e.keyCode === 69) { // 'e'
e.preventDefault();
this._expandCollapseComments(e.shiftKey);

2
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -189,7 +189,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
switch (e.keyCode) {
case 37: // left

2
polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
View File

@@ -79,7 +79,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (e.keyCode === 67) { // 'c'
if (this._checkForModifiers(e)) { return; }
e.preventDefault();

2
polygerrit-ui/app/elements/gr-app.js
View File

@@ -195,7 +195,7 @@
},
_handleKey: function(e) {
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
if (e.keyCode === 191 && e.shiftKey) { // '/' or '?' with shift key.
this.$.keyboardShortcuts.open();

13
polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.js
View File

@@ -25,25 +25,24 @@
],
detached: function() {
// For good measure.
Gerrit.KeyboardShortcutBehavior.enabled = true;
Gerrit.KeyboardShortcutBehavior.enable(this._id());
},
open: function() {
return new Promise(function(resolve) {
Gerrit.KeyboardShortcutBehavior.enabled = false;
Gerrit.KeyboardShortcutBehavior.disable(this._id());
Polymer.IronOverlayBehaviorImpl.open.apply(this, arguments);
this._awaitOpen(resolve);
}.bind(this));
},
close: function() {
Gerrit.KeyboardShortcutBehavior.enabled = true;
Gerrit.KeyboardShortcutBehavior.enable(this._id());
Polymer.IronOverlayBehaviorImpl.close.apply(this, arguments);
},
cancel: function() {
Gerrit.KeyboardShortcutBehavior.enabled = true;
Gerrit.KeyboardShortcutBehavior.enable(this._id());
Polymer.IronOverlayBehaviorImpl.cancel.apply(this, arguments);
},
@@ -72,5 +71,9 @@
}.bind(this);
step.call(this);
},
_id: function() {
return this.getAttribute('id') || 'global';
},
});
})();