Cleanup: use iron-a11y-keys-behavior for keyboard shortcuts + This does not cover on-keydown handlers within elements. A follow-up change will account for those. + Keyboard shortcuts are disabled within gr-overlay, input, and textarea elements. + Added tests for new behavior (plus some missing ones covering broken behavior). + Removed blur hacks used on elements to placate the kb shortcuts due to restrictions that have been removed. Bug: Issue 4198 Change-Id: Ide8009a3bfc340a35a8ec8b9189a85b49c8a95aa
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 e423e45..c6ccb1b 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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';