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 640c273649..bb84fdeb3c 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 36b5a41de6..0eecf30edb 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 @@ limitations under the License. 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 @@ limitations under the License. let nextCommentStub; let nextChunkStub; let fileRows; + setup(() => { sandbox.stub(element, '_renderInOrder').returns(Promise.resolve()); nKeySpy = sandbox.spy(element, '_handleNKey'); @@ -1377,9 +1398,11 @@ limitations under the License. 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 @@ limitations under the License. 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', () => {