Permit some diff cursor keyboard shortcuts when any diffs are expanded
Formerly, diff cursor keys were only enabled when all diffs had been
expanded, but not when only some were expanded. Enable the n/p and
shift+left/shift+right keys whenever any diff is expanded.
Bug: Issue 8590
Change-Id: I5aa672b8a721e6ccabba3a5655942d69f06e0d07
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 640c273..bb84fde 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
@@ -478,16 +478,18 @@
},
_handleShiftLeftKey(e) {
- if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- if (!this._showInlineDiffs) { return; }
+ if (this.shouldSuppressKeyboardShortcut(e) || this._noDiffsExpanded()) {
+ return;
+ }
e.preventDefault();
this.$.diffCursor.moveLeft();
},
_handleShiftRightKey(e) {
- if (this.shouldSuppressKeyboardShortcut(e)) { return; }
- if (!this._showInlineDiffs) { return; }
+ if (this.shouldSuppressKeyboardShortcut(e) || this._noDiffsExpanded()) {
+ return;
+ }
e.preventDefault();
this.$.diffCursor.moveRight();
@@ -591,10 +593,10 @@
_handleNKey(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
- this.modifierPressed(e) && !this.isModifierPressed(e, 'shiftKey')) {
+ (this.modifierPressed(e) && !this.isModifierPressed(e, 'shiftKey')) ||
+ this._noDiffsExpanded()) {
return;
}
- if (!this._showInlineDiffs) { return; }
e.preventDefault();
if (this.isModifierPressed(e, 'shiftKey')) {
@@ -606,10 +608,10 @@
_handlePKey(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
- this.modifierPressed(e) && !this.isModifierPressed(e, 'shiftKey')) {
+ (this.modifierPressed(e) && !this.isModifierPressed(e, 'shiftKey')) ||
+ this._noDiffsExpanded()) {
return;
}
- if (!this._showInlineDiffs) { return; }
e.preventDefault();
if (this.isModifierPressed(e, 'shiftKey')) {
@@ -1129,5 +1131,13 @@
}
return `sizeBars desktop ${hideClass}`;
},
+
+ /**
+ * Returns true if none of the inline diffs have been expanded.
+ * @return {boolean}
+ */
+ _noDiffsExpanded() {
+ return this.filesExpanded === GrFileListConstants.FilesExpandedState.NONE;
+ },
});
})();
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 36b5a41..0eecf30 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
@@ -597,6 +597,26 @@
assert.deepEqual(interact(), {opened_selected: true});
});
});
+
+ test('shift+left/shift+right', () => {
+ const moveLeftStub = sandbox.stub(element.$.diffCursor, 'moveLeft');
+ const moveRightStub = sandbox.stub(element.$.diffCursor, 'moveRight');
+
+ let noDiffsExpanded = true;
+ sandbox.stub(element, '_noDiffsExpanded', () => noDiffsExpanded);
+
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'left');
+ assert.isFalse(moveLeftStub.called);
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'right');
+ assert.isFalse(moveRightStub.called);
+
+ noDiffsExpanded = false;
+
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'left');
+ assert.isTrue(moveLeftStub.called);
+ MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'right');
+ assert.isTrue(moveRightStub.called);
+ });
});
test('computed properties', () => {
@@ -1367,6 +1387,7 @@
let nextCommentStub;
let nextChunkStub;
let fileRows;
+
setup(() => {
sandbox.stub(element, '_renderInOrder').returns(Promise.resolve());
nKeySpy = sandbox.spy(element, '_handleNKey');
@@ -1377,9 +1398,11 @@
fileRows =
Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
});
- test('n key with all files expanded and no shift key', () => {
+
+ test('n key with some files expanded and no shift key', () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
flushAsynchronousOperations();
+ assert.equal(nextChunkStub.callCount, 1);
// Handle N key should return before calling diff cursor functions.
MockInteractions.pressAndReleaseKeyOn(element, 78, null, 'n');
@@ -1387,21 +1410,22 @@
assert.isFalse(nextCommentStub.called);
// This is also called in diffCursor.moveToFirstChunk.
- assert.equal(nextChunkStub.callCount, 1);
- assert.isFalse(!!element._showInlineDiffs);
+ assert.equal(nextChunkStub.callCount, 2);
+ assert.equal(element.filesExpanded, 'some');
});
- test('n key with all files expanded and shift key', () => {
+ test('n key with some files expanded and shift key', () => {
MockInteractions.pressAndReleaseKeyOn(fileRows[0], 73, null, 'i');
flushAsynchronousOperations();
+ assert.equal(nextChunkStub.callCount, 1);
MockInteractions.pressAndReleaseKeyOn(element, 78, 'shift', 'n');
assert.isTrue(nKeySpy.called);
- assert.isFalse(nextCommentStub.called);
+ assert.isTrue(nextCommentStub.called);
// This is also called in diffCursor.moveToFirstChunk.
assert.equal(nextChunkStub.callCount, 1);
- assert.isFalse(!!element._showInlineDiffs);
+ assert.equal(element.filesExpanded, 'some');
});
test('n key without all files expanded and shift key', () => {
