Merge "Permit some diff cursor keyboard shortcuts when any diffs are expanded"
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', () => {