diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html index f636650839..ba7d3de1c7 100644 --- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html +++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior.html @@ -14,69 +14,30 @@ See the License for the specific language governing permissions and limitations under the License. --> + + diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html index 5ec4145ae6..457c5f25de 100644 --- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html +++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior_test.html @@ -30,49 +30,76 @@ limitations under the License. + + + + diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js index 2be0afc003..51626cc9d7 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js @@ -78,6 +78,12 @@ Gerrit.RESTClientBehavior, ], + keyBindings: { + 'j': '_handleJKey', + 'k': '_handleKKey', + 'o enter': '_handleEnterKey', + }, + attached: function() { this._loadPreferences(); }, @@ -149,31 +155,37 @@ account._account_id != change.owner._account_id; }, - _handleKey: function(e) { - if (this.shouldSuppressKeyboardShortcut(e)) { return; } - - if (this.groups == null) { return; } + _getAggregateGroupsLen: function(groups) { + groups = groups || []; var len = 0; this.groups.forEach(function(group) { len += group.length; }); - switch (e.keyCode) { - case 74: // 'j' - e.preventDefault(); - if (this.selectedIndex == len - 1) { return; } - this.selectedIndex += 1; - break; - case 75: // 'k' - e.preventDefault(); - if (this.selectedIndex == 0) { return; } - this.selectedIndex -= 1; - break; - case 79: // 'o' - case 13: // 'enter' - e.preventDefault(); - page.show(this._changeURLForIndex(this.selectedIndex)); - break; - } + return len; + }, + + _handleJKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + var len = this._getAggregateGroupsLen(this.groups); + if (this.selectedIndex === len - 1) { return; } + this.selectedIndex += 1; + }, + + _handleKKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (this.selectedIndex === 0) { return; } + this.selectedIndex -= 1; + }, + + _handleEnterKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + page.show(this._changeURLForIndex(this.selectedIndex)); }, _changeURLForIndex: function(index) { diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html index 718cfb56e8..33b4279105 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html @@ -131,25 +131,25 @@ limitations under the License. flush(function() { assert.isTrue(elementItems[0].selected); - MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j' + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); assert.equal(element.selectedIndex, 1); - MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j' + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); var showStub = sinon.stub(page, 'show'); assert.equal(element.selectedIndex, 2); - MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter' + MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter'); assert(showStub.lastCall.calledWithExactly('/c/2/'), 'Should navigate to /c/2/'); - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k' + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); assert.equal(element.selectedIndex, 1); - MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter' + MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter'); assert(showStub.lastCall.calledWithExactly('/c/1/'), 'Should navigate to /c/1/'); - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k' - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k' - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k' + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); assert.equal(element.selectedIndex, 0); showStub.restore(); diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js index bbd4d2df55..485f155372 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js @@ -110,6 +110,13 @@ '_paramsAndChangeChanged(params, _change)', ], + keyBindings: { + 'shift+r': '_handleCapitalRKey', + 'a': '_handleAKey', + 'd': '_handleDKey', + 'u': '_handleUKey', + }, + attached: function() { this._getLoggedIn().then(function(loggedIn) { this._loggedIn = loggedIn; @@ -579,30 +586,29 @@ }.bind(this)); }, - _handleKey: function(e) { + _handleAKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - switch (e.keyCode) { - case 65: // 'a' - if (this._loggedIn && !e.shiftKey) { - e.preventDefault(); - this._openReplyDialog(); - } - break; - case 68: // 'd' - e.preventDefault(); - this.$.downloadOverlay.open(); - break; - case 82: // 'r' - if (e.shiftKey) { - e.preventDefault(); - this._switchToMostRecentPatchNum(); - } - break; - case 85: // 'u' - e.preventDefault(); - this._determinePageBack(); - break; - } + if (!this._loggedIn || e.detail.keyboardEvent.shiftKey) { return; } + e.preventDefault(); + this._openReplyDialog(); + }, + + _handleDKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + e.preventDefault(); + this.$.downloadOverlay.open(); + }, + + _handleCapitalRKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + e.preventDefault(); + this._switchToMostRecentPatchNum(); + }, + + _handleUKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + e.preventDefault(); + this._determinePageBack(); }, _determinePageBack: function() { diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html index 7c8a329aed..4fd27aef39 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html @@ -53,27 +53,27 @@ limitations under the License. suite('keyboard shortcuts', function() { test('U should navigate to / if no backPage set', function() { var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'U' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/')); }); test('U should navigate to backPage if set', function() { element.backPage = '/dashboard/self'; var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'U' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/dashboard/self')); }); test('A should toggle overlay', function() { - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'A' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); var overlayEl = element.$.replyOverlay; assert.isFalse(overlayEl.opened); element._loggedIn = true; - MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'A' + MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a'); assert.isFalse(overlayEl.opened); - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'A' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(overlayEl.opened); overlayEl.close(); assert.isFalse(overlayEl.opened); @@ -117,13 +117,12 @@ limitations under the License. done(); }); - // 'shift + R' - MockInteractions.pressAndReleaseKeyOn(element, 82, 'shift'); + MockInteractions.pressAndReleaseKeyOn(element, 82, 'shift', 'r'); }); test('d should open download overlay', function() { var stub = sandbox.stub(element.$.downloadOverlay, 'open'); - MockInteractions.pressAndReleaseKeyOn(element, 68); // 'd' + MockInteractions.pressAndReleaseKeyOn(element, 68, null, 'd'); assert.isTrue(stub.called); }); }); diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html index 873ab3bad6..4e68559919 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html @@ -179,8 +179,7 @@ limitations under the License. diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index 98c4284f05..ab889ac778 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js @@ -106,6 +106,22 @@ Gerrit.URLEncodingBehavior, ], + keyBindings: { + 'shift+left': '_handleShiftLeftKey', + 'shift+right': '_handleShiftRightKey', + 'i': '_handleIKey', + 'shift+i': '_handleCapitalIKey', + 'down j': '_handleDownKey', + 'up k': '_handleUpKey', + 'c': '_handleCKey', + '[': '_handleLeftBracketKey', + ']': '_handleRightBracketKey', + 'o enter': '_handleEnterKey', + 'n': '_handleNKey', + 'p': '_handlePKey', + 'shift+a': '_handleCapitalAKey', + }, + reload: function() { if (!this.changeNum || !this.patchRange.patchNum) { return Promise.resolve(); @@ -216,9 +232,6 @@ this.set(['_shownFiles', i, '__expanded'], true); this.set(['_files', i, '__expanded'], true); } - if (e && e.target) { - e.target.blur(); - } }, _collapseAllDiffs: function(e) { @@ -228,9 +241,6 @@ this.set(['_files', i, '__expanded'], false); } this.$.cursor.handleDiffUpdate(); - if (e && e.target) { - e.target.blur(); - } }, _computeCommentsString: function(comments, patchNum, path) { @@ -298,113 +308,137 @@ }); }, - _handleKey: function(e) { + _handleShiftLeftKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - switch (e.keyCode) { - case 37: // left - if (e.shiftKey && this._showInlineDiffs) { - e.preventDefault(); - this.$.cursor.moveLeft(); - } - break; - case 39: // right - if (e.shiftKey && this._showInlineDiffs) { - e.preventDefault(); - this.$.cursor.moveRight(); - } - break; - case 73: // 'i' - if (e.shiftKey) { - e.preventDefault(); - this._toggleInlineDiffs(); - } else if (this.selectedIndex !== undefined) { - e.preventDefault(); - var expanded = this._files[this.selectedIndex].__expanded; - // Until Polymer 2.0, manual management of reflection between _files - // and _shownFiles is necessary. - this.set(['_shownFiles', this.selectedIndex, '__expanded'], - !expanded); - this.set(['_files', this.selectedIndex, '__expanded'], !expanded); - } - break; - case 40: // down - case 74: // 'j' - e.preventDefault(); - if (this._showInlineDiffs) { - this.$.cursor.moveDown(); - } else { - this.selectedIndex = - Math.min(this._numFilesShown, this.selectedIndex + 1); - this._scrollToSelectedFile(); - } - break; - case 38: // up - case 75: // 'k' - e.preventDefault(); - if (this._showInlineDiffs) { - this.$.cursor.moveUp(); - } else { - this.selectedIndex = Math.max(0, this.selectedIndex - 1); - this._scrollToSelectedFile(); - } - break; - case 67: // 'c' - var isRangeSelected = this.diffs.some(function(diff) { - return diff.isRangeSelected(); - }, this); - if (this._showInlineDiffs && !isRangeSelected) { - e.preventDefault(); - this._addDraftAtTarget(); - } - break; - case 219: // '[' - e.preventDefault(); - this._openSelectedFile(this._files.length - 1); - break; - case 221: // ']' - e.preventDefault(); - this._openSelectedFile(0); - break; - case 13: // - case 79: // 'o' - e.preventDefault(); - if (this._showInlineDiffs) { - this._openCursorFile(); - } else { - this._openSelectedFile(); - } - break; - case 78: // 'n' - if (this._showInlineDiffs) { - e.preventDefault(); - if (e.shiftKey) { - this.$.cursor.moveToNextCommentThread(); - } else { - this.$.cursor.moveToNextChunk(); - } - } - break; - case 80: // 'p' - if (this._showInlineDiffs) { - e.preventDefault(); - if (e.shiftKey) { - this.$.cursor.moveToPreviousCommentThread(); - } else { - this.$.cursor.moveToPreviousChunk(); - } - } - break; - case 65: // 'a' - if (e.shiftKey) { // Hide left diff. - e.preventDefault(); - this._forEachDiff(function(diff) { - diff.toggleLeftDiff(); - }); - } - break; + if (!this._showInlineDiffs) { return; } + + e.preventDefault(); + this.$.cursor.moveLeft(); + }, + + _handleShiftRightKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + if (!this._showInlineDiffs) { return; } + + e.preventDefault(); + this.$.cursor.moveRight(); + }, + + _handleIKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + if (this.selectedIndex === undefined) { return; } + + e.preventDefault(); + var expanded = this._files[this.selectedIndex].__expanded; + // Until Polymer 2.0, manual management of reflection between _files + // and _shownFiles is necessary. + this.set(['_shownFiles', this.selectedIndex, '__expanded'], + !expanded); + this.set(['_files', this.selectedIndex, '__expanded'], !expanded); + }, + + _handleCapitalIKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._toggleInlineDiffs(); + }, + + _handleDownKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (this._showInlineDiffs) { + this.$.cursor.moveDown(); + } else { + this.selectedIndex = + Math.min(this._numFilesShown, this.selectedIndex + 1); + this._scrollToSelectedFile(); } }, + _handleUpKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (this._showInlineDiffs) { + this.$.cursor.moveUp(); + } else { + this.selectedIndex = Math.max(0, this.selectedIndex - 1); + this._scrollToSelectedFile(); + } + }, + + _handleCKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + var isRangeSelected = this.diffs.some(function(diff) { + return diff.isRangeSelected(); + }, this); + if (this._showInlineDiffs && !isRangeSelected) { + e.preventDefault(); + this._addDraftAtTarget(); + } + }, + + _handleLeftBracketKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._openSelectedFile(this._files.length - 1); + }, + + _handleRightBracketKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._openSelectedFile(0); + }, + + _handleEnterKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (this._showInlineDiffs) { + this._openCursorFile(); + } else { + this._openSelectedFile(); + } + }, + + _handleNKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + if (!this._showInlineDiffs) { return; } + + e.preventDefault(); + if (e.shiftKey) { + this.$.cursor.moveToNextCommentThread(); + } else { + this.$.cursor.moveToNextChunk(); + } + }, + + _handlePKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + if (!this._showInlineDiffs) { return; } + + e.preventDefault(); + if (e.shiftKey) { + this.$.cursor.moveToPreviousCommentThread(); + } else { + this.$.cursor.moveToPreviousChunk(); + } + }, + + _handleCapitalAKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._forEachDiff(function(diff) { + diff.toggleLeftDiff(); + }); + }, + _toggleInlineDiffs: function() { if (this._showInlineDiffs) { this._collapseAllDiffs(); @@ -555,10 +589,6 @@ return DiffViewMode.SIDE_BY_SIDE; }, - _handleDropdownChange: function(e) { - e.target.blur(); - }, - _fileListActionsVisible: function(numFilesShown, maxFilesForBulkActions) { return numFilesShown <= maxFilesForBulkActions; }, diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index b530baed53..1e146b5e64 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html @@ -155,7 +155,7 @@ limitations under the License. return [{toggleLeftDiff: toggleLeftDiffStub}]; }, }); - MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'A' + MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a'); assert.isTrue(toggleLeftDiffStub.calledOnce); diffsStub.restore(); }); @@ -168,25 +168,25 @@ limitations under the License. assert.isTrue(elementItems[0].hasAttribute('selected')); assert.isFalse(elementItems[1].hasAttribute('selected')); assert.isFalse(elementItems[2].hasAttribute('selected')); - MockInteractions.pressAndReleaseKeyOn(element, 74); // 'J' + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); assert.equal(element.selectedIndex, 1); - MockInteractions.pressAndReleaseKeyOn(element, 74); // 'J' + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); var showStub = sandbox.stub(page, 'show'); assert.equal(element.selectedIndex, 2); - MockInteractions.pressAndReleaseKeyOn(element, 13); // 'ENTER' + MockInteractions.pressAndReleaseKeyOn(element, 13, null, 'enter'); assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'), 'Should navigate to /c/42/2/myfile.txt'); - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K' + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); assert.equal(element.selectedIndex, 1); - MockInteractions.pressAndReleaseKeyOn(element, 79); // 'O' + MockInteractions.pressAndReleaseKeyOn(element, 79, null, 'o'); assert(showStub.lastCall.calledWith('/c/42/2/file_added_in_rev2.txt'), 'Should navigate to /c/42/2/file_added_in_rev2.txt'); - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K' - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K' - MockInteractions.pressAndReleaseKeyOn(element, 75); // 'K' + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); + MockInteractions.pressAndReleaseKeyOn(element, 75, null, 'k'); assert.equal(element.selectedIndex, 0); showStub.restore(); @@ -194,23 +194,23 @@ limitations under the License. test('i key shows/hides selected inline diff', function() { element.selectedIndex = 0; - MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I' + MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i'); flushAsynchronousOperations(); assert.isFalse(element.diffs[0].hasAttribute('hidden')); - MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I' + MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i'); flushAsynchronousOperations(); assert.isTrue(element.diffs[0].hasAttribute('hidden')); element.selectedIndex = 1; - MockInteractions.pressAndReleaseKeyOn(element, 73); // 'I' + MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i'); flushAsynchronousOperations(); assert.isFalse(element.diffs[1].hasAttribute('hidden')); - MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift'); // 'I' + MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i'); flushAsynchronousOperations(); for (var index in element.diffs) { assert.isFalse(element.diffs[index].hasAttribute('hidden')); } - MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift'); // 'I' + MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i'); flushAsynchronousOperations(); for (var index in element.diffs) { assert.isTrue(element.diffs[index].hasAttribute('hidden')); diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js index cee2a69371..aa0703fe7f 100644 --- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js +++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js @@ -113,17 +113,11 @@ for (var i = 0; i < messageEls.length; i++) { messageEls[i].expanded = this._expanded; } - if (e && e.target) { - e.target.blur(); - } }, _handleAutomatedMessageToggleTap: function(e) { e.preventDefault(); this._hideAutomated = !this._hideAutomated; - if (e && e.target) { - e.target.blur(); - } }, _handleScrollTo: function(e) { diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js index 2c8549d0da..61eb28052a 100644 --- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js +++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js @@ -94,6 +94,10 @@ 'searchButton.tap': '_preventDefaultAndNavigateToInputVal', }, + keyBindings: { + '/': '_handleForwardSlashKey', + }, + properties: { value: { type: String, @@ -292,18 +296,12 @@ }); }, - _handleKey: function(e) { + _handleForwardSlashKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - switch (e.keyCode) { - case 191: // '/' or '?' with shift key. - // TODO(andybons): Localization using e.key/keypress event. - if (e.shiftKey) { break; } - e.preventDefault(); - var s = this.$.searchInput; - s.focus(); - s.setSelectionRange(0, s.value.length); - break; - } + + e.preventDefault(); + this.$.searchInput.focus(); + this.$.searchInput.selectAll(); }, }); })(); diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html index cd218d40c9..621511fb77 100644 --- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html +++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar_test.html @@ -70,13 +70,15 @@ limitations under the License. done(); }); element.value = 'test'; - MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13); + MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13, + null, 'enter'); }); test('search query should be double-escaped', function() { var showStub = sinon.stub(page, 'show'); element.$.searchInput.text = 'fate/stay'; - MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13); + MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13, + null, 'enter'); assert.equal(showStub.lastCall.args[0], '/q/fate%252Fstay'); showStub.restore(); }); @@ -85,7 +87,8 @@ limitations under the License. var showStub = sinon.stub(page, 'show'); var blurSpy = sinon.spy(element.$.searchInput.$.input, 'blur'); element.$.searchInput.text = 'fate/stay'; - MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13); + MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13, + null, 'enter'); assert.isTrue(blurSpy.called); showStub.restore(); blurSpy.restore(); @@ -94,10 +97,19 @@ limitations under the License. test('empty search query does not trigger nav', function() { var showSpy = sinon.spy(page, 'show'); element.value = ''; - MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13); + MockInteractions.pressAndReleaseKeyOn(element.$.searchInput.$.input, 13, + null, 'enter'); assert.isFalse(showSpy.called); }); + test('keyboard shortcuts', function() { + var focusSpy = sinon.spy(element.$.searchInput, 'focus'); + var selectAllSpy = sinon.spy(element.$.searchInput, 'selectAll'); + MockInteractions.pressAndReleaseKeyOn(document.body, 191, null, '/'); + assert.isTrue(focusSpy.called); + assert.isTrue(selectAllSpy.called); + }); + suite('_getSearchSuggestions', function() { setup(function() { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js index 20b032d84a..a24100f894 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js @@ -57,6 +57,10 @@ '_commentsChanged(comments.splices)', ], + keyBindings: { + 'e shift+e': '_handleEKey', + }, + attached: function() { this._getLoggedIn().then(function(loggedIn) { this._showActions = loggedIn; @@ -88,12 +92,12 @@ this._orderedComments = this._sortedComments(this.comments); }, - _handleKey: function(e) { + _handleEKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (e.keyCode === 69) { // 'e' - e.preventDefault(); - this._expandCollapseComments(e.shiftKey); - } + + // Don’t preventDefault in this case because it will render the event + // useless for other handlers (other gr-diff-comment-thread elements). + this._expandCollapseComments(e.detail.keyboardEvent.shiftKey); }, _expandCollapseComments: function(actionIsCollapse) { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html index 73e49830e7..7b85b62847 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html @@ -280,10 +280,10 @@ limitations under the License. }]; element.comments = comments; var expandCollapseStub = sinon.stub(element, '_expandCollapseComments'); - MockInteractions.pressAndReleaseKeyOn(element, 69); // 'e' + MockInteractions.pressAndReleaseKeyOn(element, 69, null, 'e'); assert.isTrue(expandCollapseStub.lastCall.calledWith(false)); - MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift'); // 'e' + MockInteractions.pressAndReleaseKeyOn(element, 69, 'shift', 'e'); assert.isTrue(expandCollapseStub.lastCall.calledWith(true)); expandCollapseStub.restore(); }); diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js index 410a813650..720e6542bb 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js @@ -279,34 +279,34 @@ }, _handleReply: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); this.fire('reply', this._getEventPayload(), {bubbles: false}); }, _handleQuote: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); this.fire( 'reply', this._getEventPayload({quote: true}), {bubbles: false}); }, _handleDone: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); this.fire('done', this._getEventPayload(), {bubbles: false}); }, _handleEdit: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); this._messageText = this.comment.message; this.editing = true; }, _handleSave: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); this.save(); }, _handleCancel: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); if (this.comment.message == null || this.comment.message.length == 0) { this._fireDiscard(); return; @@ -321,7 +321,7 @@ }, _handleDiscard: function(e) { - this._preventDefaultAndBlur(e); + e.preventDefault(); if (!this.comment.__draft) { throw Error('Cannot discard a non-draft comment.'); } @@ -345,11 +345,6 @@ }.bind(this)); }, - _preventDefaultAndBlur: function(e) { - e.preventDefault(); - Polymer.dom(e).rootTarget.blur(); - }, - _saveDraft: function(draft) { return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft); }, diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html index 9dbb25b64e..b6471e7f20 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html @@ -217,8 +217,7 @@ limitations under the License. id="modeSelect" is="gr-select" bind-value="{{changeViewState.diffMode}}" - hidden$="[[_computeModeSelectHidden(_isImageDiff)]]" - on-change="_handleDropdownChange"> + hidden$="[[_computeModeSelectHidden(_isImageDiff)]]"> diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 44204fda5a..1d63b47123 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js @@ -100,6 +100,22 @@ '_getFiles(_changeNum, _patchRange.*)', ], + keyBindings: { + 'esc': '_handleEscKey', + 'shift+left': '_handleShiftLeftKey', + 'shift+right': '_handleShiftRightKey', + 'up k': '_handleUpKey', + 'down j': '_handleDownKey', + 'c': '_handleCKey', + '[': '_handleLeftBracketKey', + ']': '_handleRightBracketKey', + 'n shift+n': '_handleNKey', + 'p shift+p': '_handlePKey', + 'a shift+a': '_handleAKey', + 'u': '_handleUKey', + ',': '_handleCommaKey', + }, + attached: function() { this._getLoggedIn().then(function(loggedIn) { this._loggedIn = loggedIn; @@ -185,103 +201,129 @@ this._patchRange.patchNum, this._path, reviewed); }, - _checkForModifiers: function(e) { - return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey || false; - }, - - _handleKey: function(e) { + _handleEscKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - switch (e.keyCode) { - case 27: // escape - e.preventDefault(); - this.$.diff.displayLine = false; - break; - case 37: // left - if (e.shiftKey) { - e.preventDefault(); - this.$.cursor.moveLeft(); - } - break; - case 39: // right - if (e.shiftKey) { - e.preventDefault(); - this.$.cursor.moveRight(); - } - break; - case 40: // down - case 74: // 'j' - e.preventDefault(); - this.$.diff.displayLine = true; - this.$.cursor.moveDown(); - break; - case 38: // up - case 75: // 'k' - e.preventDefault(); - this.$.diff.displayLine = true; - this.$.cursor.moveUp(); - break; - case 67: // 'c' - if (this._checkForModifiers(e)) { return; } - if (!this.$.diff.isRangeSelected()) { - e.preventDefault(); - var line = this.$.cursor.getTargetLineElement(); - if (line) { - this.$.diff.addDraftAtLine(line); - } - } - break; - case 219: // '[' - e.preventDefault(); - this._navToFile(this._path, this._fileList, -1); - break; - case 221: // ']' - e.preventDefault(); - this._navToFile(this._path, this._fileList, 1); - break; - case 78: // 'n' - e.preventDefault(); - if (e.shiftKey) { - this.$.cursor.moveToNextCommentThread(); - } else { - this.$.cursor.moveToNextChunk(); - } - break; - case 80: // 'p' - e.preventDefault(); - if (e.shiftKey) { - this.$.cursor.moveToPreviousCommentThread(); - } else { - this.$.cursor.moveToPreviousChunk(); - } - break; - case 65: // 'a' - if (e.shiftKey) { // Hide left diff. - e.preventDefault(); - this.$.diff.toggleLeftDiff(); - break; - } + e.preventDefault(); + this.$.diff.displayLine = false; + }, - if (!this._loggedIn) { break; } + _handleShiftLeftKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } - this.set('changeViewState.showReplyDialog', true); - /* falls through */ // required by JSHint - case 85: // 'u' - if (this._changeNum && this._patchRange.patchNum) { - e.preventDefault(); - page.show(this._getChangePath( - this._changeNum, - this._patchRange, - this._change && this._change.revisions)); - } - break; - case 188: // ',' - e.preventDefault(); - this._openPrefs(); - break; + e.preventDefault(); + this.$.cursor.moveLeft(); + }, + + _handleShiftRightKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this.$.cursor.moveRight(); + }, + + _handleUpKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this.$.diff.displayLine = true; + this.$.cursor.moveUp(); + }, + + _handleDownKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this.$.diff.displayLine = true; + this.$.cursor.moveDown(); + }, + + _handleCKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + if (this.$.diff.isRangeSelected()) { return; } + + e.preventDefault(); + var line = this.$.cursor.getTargetLineElement(); + if (line) { + this.$.diff.addDraftAtLine(line); } }, + _handleLeftBracketKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._navToFile(this._path, this._fileList, -1); + }, + + _handleRightBracketKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._navToFile(this._path, this._fileList, 1); + }, + + _handleNKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (e.detail.keyboardEvent.shiftKey) { + this.$.cursor.moveToNextCommentThread(); + } else { + this.$.cursor.moveToNextChunk(); + } + }, + + _handlePKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + if (e.detail.keyboardEvent.shiftKey) { + this.$.cursor.moveToPreviousCommentThread(); + } else { + this.$.cursor.moveToPreviousChunk(); + } + }, + + _handleAKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + if (e.detail.keyboardEvent.shiftKey) { // Hide left diff. + e.preventDefault(); + this.$.diff.toggleLeftDiff(); + return; + } + + if (!this._loggedIn) { return; } + + this.set('changeViewState.showReplyDialog', true); + e.preventDefault(); + this._navToChangeView(); + }, + + _handleUKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._navToChangeView(); + }, + + _handleCommaKey: function(e) { + if (this.shouldSuppressKeyboardShortcut(e)) { return; } + + e.preventDefault(); + this._openPrefs(); + }, + + _navToChangeView: function() { + if (!this._changeNum || !this._patchRange.patchNum) { return; } + + page.show(this._getChangePath( + this._changeNum, + this._patchRange, + this._change && this._change.revisions)); + }, + _navToFile: function(path, fileList, direction) { var url = this._computeNavLinkURL(path, fileList, direction); if (!url) { return; } @@ -556,10 +598,6 @@ history.replaceState(null, null, '#' + this.$.cursor.getAddress()); }, - _handleDropdownChange: function(e) { - e.target.blur(); - }, - _computeDownloadLink: function(changeNum, patchRange, path) { var url = this.changeBaseURL(changeNum, patchRange.patchNum); url += '/patch?zip&path=' + encodeURIComponent(path); diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html index e423e45fc7..c6ccb1b610 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html @@ -62,7 +62,7 @@ limitations under the License. test('toggle left diff with a hotkey', function() { var toggleLeftDiffStub = sandbox.stub(element.$.diff, 'toggleLeftDiff'); - MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift', 'a'); assert.isTrue(toggleLeftDiffStub.calledOnce); }); @@ -82,29 +82,29 @@ limitations under the License. element.changeViewState.selectedFileIndex = 1; var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/c/42/'), 'Should navigate to /c/42/'); - MockInteractions.pressAndReleaseKeyOn(element, 221); // ']' + MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); assert(showStub.lastCall.calledWithExactly('/c/42/10/wheatley.md'), 'Should navigate to /c/42/10/wheatley.md'); element._path = 'wheatley.md'; assert.equal(element.changeViewState.selectedFileIndex, 2); - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/10/glados.txt'), 'Should navigate to /c/42/10/glados.txt'); element._path = 'glados.txt'; assert.equal(element.changeViewState.selectedFileIndex, 1); - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/10/chell.go'), 'Should navigate to /c/42/10/chell.go'); element._path = 'chell.go'; assert.equal(element.changeViewState.selectedFileIndex, 0); - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/'), 'Should navigate to /c/42/'); assert.equal(element.changeViewState.selectedFileIndex, 0); @@ -112,33 +112,33 @@ limitations under the License. var showPrefsStub = sandbox.stub(element.$.prefsOverlay, 'open', function() { return Promise.resolve({}); }); - MockInteractions.pressAndReleaseKeyOn(element, 188); // ',' + MockInteractions.pressAndReleaseKeyOn(element, 188, null, ','); assert(showPrefsStub.calledOnce); var scrollStub = sandbox.stub(element.$.cursor, 'moveToNextChunk'); - MockInteractions.pressAndReleaseKeyOn(element, 78); // 'n' + MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n'); assert(scrollStub.calledOnce); scrollStub = sandbox.stub(element.$.cursor, 'moveToPreviousChunk'); - MockInteractions.pressAndReleaseKeyOn(element, 80); // 'p' + MockInteractions.pressAndReleaseKeyOn(element, 80, null, 'p'); assert(scrollStub.calledOnce); scrollStub = sandbox.stub(element.$.cursor, 'moveToNextCommentThread'); - MockInteractions.pressAndReleaseKeyOn(element, 78, ['shift']); // 'N' + MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n'); assert(scrollStub.calledOnce); scrollStub = sandbox.stub(element.$.cursor, 'moveToPreviousCommentThread'); - MockInteractions.pressAndReleaseKeyOn(element, 80, ['shift']); // 'P' + MockInteractions.pressAndReleaseKeyOn(element, 80, 'shift', 'p'); assert(scrollStub.calledOnce); var computeContainerClassStub = sandbox.stub(element.$.diff, '_computeContainerClass'); - MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j' + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); assert(computeContainerClassStub.lastCall.calledWithExactly( false, 'SIDE_BY_SIDE', true)); - MockInteractions.pressAndReleaseKeyOn(element, 27); // 'escape' + MockInteractions.pressAndReleaseKeyOn(element, 27, null, 'esc'); assert(computeContainerClassStub.lastCall.calledWithExactly( false, 'SIDE_BY_SIDE', false)); }); @@ -175,39 +175,39 @@ limitations under the License. var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + 'only work when the user is logged in.'); assert.isNull(window.sessionStorage.getItem( 'changeView.showReplyDialog')); element._loggedIn = true; - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(element.changeViewState.showReplyDialog); assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), 'Should navigate to /c/42/5..10'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), 'Should navigate to /c/42/5..10'); - MockInteractions.pressAndReleaseKeyOn(element, 221); // ']' + MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); assert(showStub.lastCall.calledWithExactly('/c/42/5..10/wheatley.md'), 'Should navigate to /c/42/5..10/wheatley.md'); element._path = 'wheatley.md'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/5..10/glados.txt'), 'Should navigate to /c/42/5..10/glados.txt'); element._path = 'glados.txt'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/5..10/chell.go'), 'Should navigate to /c/42/5..10/chell.go'); element._path = 'chell.go'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/5..10'), 'Should navigate to /c/42/5..10'); }); @@ -229,39 +229,39 @@ limitations under the License. var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + 'only work when the user is logged in.'); assert.isNull(window.sessionStorage.getItem( 'changeView.showReplyDialog')); element._loggedIn = true; - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(element.changeViewState.showReplyDialog); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); - MockInteractions.pressAndReleaseKeyOn(element, 221); // ']' + MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'), 'Should navigate to /c/42/1/wheatley.md'); element._path = 'wheatley.md'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'), 'Should navigate to /c/42/1/glados.txt'); element._path = 'glados.txt'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'), 'Should navigate to /c/42/1/chell.go'); element._path = 'chell.go'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); }); @@ -278,39 +278,39 @@ limitations under the License. var showStub = sandbox.stub(page, 'show'); - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(showStub.notCalled, 'The `a` keyboard shortcut should ' + 'only work when the user is logged in.'); assert.isNull(window.sessionStorage.getItem( 'changeView.showReplyDialog')); element._loggedIn = true; - MockInteractions.pressAndReleaseKeyOn(element, 65); // 'a' + MockInteractions.pressAndReleaseKeyOn(element, 65, null, 'a'); assert.isTrue(element.changeViewState.showReplyDialog); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); - MockInteractions.pressAndReleaseKeyOn(element, 85); // 'u' + MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u'); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); - MockInteractions.pressAndReleaseKeyOn(element, 221); // ']' + MockInteractions.pressAndReleaseKeyOn(element, 221, null, ']'); assert(showStub.lastCall.calledWithExactly('/c/42/1/wheatley.md'), 'Should navigate to /c/42/1/wheatley.md'); element._path = 'wheatley.md'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1/glados.txt'), 'Should navigate to /c/42/1/glados.txt'); element._path = 'glados.txt'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1/chell.go'), 'Should navigate to /c/42/1/chell.go'); element._path = 'chell.go'; - MockInteractions.pressAndReleaseKeyOn(element, 219); // '[' + MockInteractions.pressAndReleaseKeyOn(element, 219, null, '['); assert(showStub.lastCall.calledWithExactly('/c/42/1'), 'Should navigate to /c/42/1'); }); @@ -457,7 +457,6 @@ limitations under the License. test('diff mode selector correctly toggles the diff', function() { var select = element.$.modeSelect; var diffDisplay = element.$.diff; - var blurSpy = sandbox.spy(select, 'blur'); element._userPrefs = {diff_view: 'SIDE_BY_SIDE'}; // The mode selected in the view state reflects the selected option. @@ -477,7 +476,6 @@ limitations under the License. assert.equal(element._getDiffViewMode(), newMode); assert.equal(element._getDiffViewMode(), select.value); assert.equal(element._getDiffViewMode(), diffDisplay.viewMode); - assert(blurSpy.called, 'select should be blurred after selection'); }); test('diff mode selector initializes from preferences', function() { @@ -557,14 +555,6 @@ limitations under the License. assert.equal(element.$.cursor.side, 'left'); }); - test('_checkForModifiers', function() { - assert.isTrue(element._checkForModifiers({altKey: true})); - assert.isTrue(element._checkForModifiers({ctrlKey: true})); - assert.isTrue(element._checkForModifiers({metaKey: true})); - assert.isTrue(element._checkForModifiers({shiftKey: true})); - assert.isFalse(element._checkForModifiers({})); - }); - test('_shortenPath with long path should add ellipsis', function() { var path = 'level1/level2/level3/level4/file.js'; diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js index 568b5e0d9b..c6100737e5 100644 --- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js +++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js @@ -51,6 +51,10 @@ 'mousedown': '_handleMouseDown', // See https://crbug.com/gerrit/4767 }, + keyBindings: { + 'c': '_handleCKey', + }, + placeAbove: function(el) { var rect = this._getTargetBoundingRect(el); var boxRect = this.getBoundingClientRect(); @@ -74,17 +78,11 @@ return rect; }, - _checkForModifiers: function(e) { - return e.altKey || e.ctrlKey || e.metaKey || e.shiftKey || false; - }, - - _handleKey: function(e) { + _handleCKey: function(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - if (e.keyCode === 67) { // 'c' - if (this._checkForModifiers(e)) { return; } - e.preventDefault(); - this._fireCreateComment(); - } + + e.preventDefault(); + this._fireCreateComment(); }, _handleMouseDown: function(e) { diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html index c12966d390..79ff2a57a0 100644 --- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html +++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html @@ -49,12 +49,12 @@ limitations under the License. }); test('ignores regular keys', function() { - MockInteractions.pressAndReleaseKeyOn(document.body, 27); // 'esc' + MockInteractions.pressAndReleaseKeyOn(document.body, 27, null, 'esc'); assert.isFalse(element.fire.called); }); test('reacts to hotkey', function() { - MockInteractions.pressAndReleaseKeyOn(document.body, 67); // 'c' + MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c'); assert.isTrue(element.fire.called); }); @@ -68,7 +68,7 @@ limitations under the License. }; element.side = 'left'; element.range = range; - MockInteractions.pressAndReleaseKeyOn(document.body, 67); // 'c' + MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c'); assert(element.fire.calledWithExactly( 'create-comment', { @@ -117,13 +117,5 @@ limitations under the License. document.createRange.restore(); }); }); - - test('_checkForModifiers', function() { - assert.isTrue(element._checkForModifiers({altKey: true})); - assert.isTrue(element._checkForModifiers({ctrlKey: true})); - assert.isTrue(element._checkForModifiers({metaKey: true})); - assert.isTrue(element._checkForModifiers({shiftKey: true})); - assert.isFalse(element._checkForModifiers({})); - }); }); diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js index b758ff0e6c..1ef6c2d9d2 100644 --- a/polygerrit-ui/app/elements/gr-app.js +++ b/polygerrit-ui/app/elements/gr-app.js @@ -62,6 +62,10 @@ Gerrit.KeyboardShortcutBehavior, ], + keyBindings: { + '?': '_showKeyboardShortcuts', + }, + attached: function() { this.$.restAPI.getAccount().then(function(account) { this._account = account; @@ -194,12 +198,9 @@ } }, - _handleKey: function(e) { + _showKeyboardShortcuts(e) { if (this.shouldSuppressKeyboardShortcut(e)) { return; } - - if (e.keyCode === 191 && e.shiftKey) { // '/' or '?' with shift key. - this.$.keyboardShortcuts.open(); - } + this.$.keyboardShortcuts.open(); }, _handleKeyboardShortcutDialogClose: function() { diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html index 864114f3e9..5de54bbd91 100644 --- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html +++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.html @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. --> + diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js index 1c9e010937..0626288be0 100644 --- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js +++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete.js @@ -140,6 +140,10 @@ this.$.input.focus(); }, + selectAll: function() { + this.$.input.setSelectionRange(0, this.$.input.value.length); + }, + clear: function() { this.text = ''; }, diff --git a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html index 03fa8e43cf..394b2c6d4e 100644 --- a/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html +++ b/polygerrit-ui/app/elements/shared/gr-autocomplete/gr-autocomplete_test.html @@ -94,7 +94,7 @@ limitations under the License. var cancelHandler = sinon.spy(); element.addEventListener('cancel', cancelHandler); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 27); // Esc + MockInteractions.pressAndReleaseKeyOn(element.$.input, 27, null, 'esc'); assert.isTrue(cancelHandler.called); assert.isTrue(element.$.suggestions.hasAttribute('hidden')); @@ -128,19 +128,22 @@ limitations under the License. assert.equal(element.$.cursor.index, 0); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 40); // Down + MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null, + 'down'); assert.equal(element.$.cursor.index, 1); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 40); // Down + MockInteractions.pressAndReleaseKeyOn(element.$.input, 40, null, + 'down'); assert.equal(element.$.cursor.index, 2); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 38); // Up + MockInteractions.pressAndReleaseKeyOn(element.$.input, 38, null, 'up'); assert.equal(element.$.cursor.index, 1); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter + MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null, + 'enter'); assert.equal(element.value, 1); assert.isTrue(commitHandler.called); @@ -163,7 +166,8 @@ limitations under the License. var commitHandler = sinon.spy(); element.addEventListener('commit', commitHandler); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter + MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null, + 'enter'); assert.isTrue(commitHandler.called); assert.equal(element.text, 'suggestion'); @@ -184,7 +188,8 @@ limitations under the License. var commitHandler = sinon.spy(); element.addEventListener('commit', commitHandler); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter + MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null, + 'enter'); assert.isTrue(commitHandler.called); assert.equal(element.text, ''); @@ -234,7 +239,8 @@ limitations under the License. var commitHandler = sinon.spy(); element.addEventListener('commit', commitHandler); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 13); // Enter + MockInteractions.pressAndReleaseKeyOn(element.$.input, 13, null, + 'enter'); assert.isTrue(commitHandler.called); assert.equal(element.text, 'blah 0'); @@ -245,10 +251,10 @@ limitations under the License. test('tab key completes only when suggestions exist', function() { var commitStub = sinon.stub(element, '_commit'); element._suggestions = []; - MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab + MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab'); assert.isFalse(commitStub.called); element._suggestions = ['tunnel snakes rule!']; - MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab + MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab'); assert.isTrue(commitStub.called); commitStub.restore(); }); @@ -258,11 +264,11 @@ limitations under the License. element.addEventListener('commit', commitHandler); element._suggestions = ['tunnel snakes rule!']; element.tabCompleteWithoutCommit = true; - MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab + MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab'); assert.isFalse(commitHandler.called); element.tabCompleteWithoutCommit = false; element._suggestions = ['tunnel snakes rule!']; - MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab + MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab'); assert.isTrue(commitHandler.called); }); @@ -301,7 +307,7 @@ limitations under the License. test('input-keydown event fired', function() { var listener = sinon.spy(); element.addEventListener('input-keydown', listener); - MockInteractions.pressAndReleaseKeyOn(element.$.input, 9); // tab + MockInteractions.pressAndReleaseKeyOn(element.$.input, 9, null, 'tab'); flushAsynchronousOperations(); assert.isTrue(listener.called); }); diff --git a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js index e10989645c..01d25857a7 100644 --- a/polygerrit-ui/app/elements/shared/gr-button/gr-button.js +++ b/polygerrit-ui/app/elements/shared/gr-button/gr-button.js @@ -39,6 +39,10 @@ tabindex: '0', }, + keyBindings: { + 'space enter': '_handleCommitKey', + }, + _disabledChanged: function(disabled) { if (disabled) { this._enabledTabindex = this.getAttribute('tabindex'); @@ -46,13 +50,9 @@ this.setAttribute('tabindex', disabled ? '-1' : this._enabledTabindex); }, - _handleKey: function(e) { - switch (e.keyCode) { - case 32: // 'spacebar' - case 13: // 'enter' - e.preventDefault(); - this.click(); - } + _handleCommitKey: function(e) { + e.preventDefault(); + this.click(); }, }); })(); diff --git a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html index 817d8c5964..9aa80b575c 100644 --- a/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html +++ b/polygerrit-ui/app/elements/shared/gr-overlay/gr-overlay.html @@ -16,7 +16,6 @@ limitations under the License. -