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 75d86f411c..cc827de3c8 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 @@ -325,6 +325,7 @@ limitations under the License. path="[[file.__path]]" prefs="[[_diffPrefs]]" project-config="[[projectConfig]]" + on-line-selected="_onLineSelected" view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"> 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 d06a6954be..09570db215 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 @@ -686,18 +686,19 @@ } }, - _filesChanged: function() { - this.async(function() { - var diffElements = Polymer.dom(this.root).querySelectorAll('gr-diff'); + _updateDiffCursor: function() { + var diffElements = Polymer.dom(this.root).querySelectorAll('gr-diff'); - // Overwrite the cursor's list of diffs: - this.$.diffCursor.splice.apply(this.$.diffCursor, - ['diffs', 0, this.$.diffCursor.diffs.length].concat(diffElements)); + // Overwrite the cursor's list of diffs: + this.$.diffCursor.splice.apply(this.$.diffCursor, + ['diffs', 0, this.$.diffCursor.diffs.length].concat(diffElements)); + }, - var files = Polymer.dom(this.root).querySelectorAll('.file-row'); - this.$.fileCursor.stops = files; - this.$.fileCursor.setCursorAtIndex(this.selectedIndex, true); - }.bind(this), 1); + _filesChanged: function(files) { + Polymer.dom.flush(); + var files = Polymer.dom(this.root).querySelectorAll('.file-row'); + this.$.fileCursor.stops = files; + this.$.fileCursor.setCursorAtIndex(this.selectedIndex, true); }, _incrementNumFilesShown: function() { @@ -769,6 +770,11 @@ return expandedFilesRecord.base.indexOf(path) !== -1; }, + _onLineSelected: function(e, detail) { + this.$.diffCursor.moveToLineNumber(detail.number, detail.side, + detail.path); + }, + /** * Handle splices to the list of expanded file paths. If there are any new * entries in the expanded list, then render each diff corresponding in @@ -798,6 +804,8 @@ this.$.reporting.timeEnd(timerName); this.$.diffCursor.handleDiffUpdate(); }.bind(this)); + this._updateDiffCursor(); + this.$.diffCursor.handleDiffUpdate(); }, /** 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 c006f68bfd..66fbc8e577 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 @@ -23,6 +23,7 @@ limitations under the License. + @@ -927,4 +928,171 @@ limitations under the License. }); }); }); + + suite('gr-file-list inline diff tests', function() { + var element; + var sandbox; + var saveStub; + + var setupDiff = function(diff) { + var mock = document.createElement('mock-diff-response'); + diff._diff = mock.diffResponse; + diff._comments = { + left: [], + right: [], + }; + diff.prefs = { + context: 10, + tab_size: 8, + font_size: 12, + line_length: 100, + cursor_blink_rate: 0, + line_wrapping: false, + intraline_difference: true, + show_line_endings: true, + show_tabs: true, + show_whitespace_errors: true, + syntax_highlighting: true, + auto_hide_diff_table_header: true, + theme: 'DEFAULT', + ignore_whitespace: 'IGNORE_NONE', + }; + diff._renderDiffTable(); + }; + + var renderAndGetNewDiffs = function(index) { + var diffs = + Polymer.dom(element.root).querySelectorAll('gr-diff'); + + for (var i = index; i < diffs.length; i++) { + setupDiff(diffs[i]); + } + + element._updateDiffCursor(); + element.$.diffCursor.handleDiffUpdate(); + return diffs; + }; + + setup(function() { + sandbox = sinon.sandbox.create(); + stub('gr-rest-api-interface', { + getLoggedIn: function() { return Promise.resolve(true); }, + getPreferences: function() { return Promise.resolve({}); }, + }); + stub('gr-date-formatter', { + _loadTimeFormat: function() { return Promise.resolve(''); }, + }); + stub('gr-diff', { + reload: function() { return Promise.resolve(); }, + }); + element = fixture('basic'); + element.numFilesShown = 75; + element.selectedIndex = 0; + element._files = [ + {__path: '/COMMIT_MSG', lines_inserted: 9}, + { + __path: 'file_added_in_rev2.txt', + lines_inserted: 1, + lines_deleted: 1, + size_delta: 10, + size: 100, + }, + { + __path: 'myfile.txt', + lines_inserted: 1, + lines_deleted: 1, + size_delta: 10, + size: 100, + }, + ]; + element._reviewed = ['/COMMIT_MSG', 'myfile.txt']; + element._loggedIn = true; + element.changeNum = '42'; + element.patchRange = { + basePatchNum: 'PARENT', + patchNum: '2', + }; + sandbox.stub(window, 'fetch', function() { + return Promise.resolve(); + }); + saveStub = sandbox.stub(element, '_saveReviewedState', + function() { return Promise.resolve(); }); + flushAsynchronousOperations(); + }); + + teardown(function() { + sandbox.restore(); + }); + + test('cursor with individually opened files', function() { + MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i'); + flushAsynchronousOperations(); + var diffs = renderAndGetNewDiffs(0); + var diffStops = diffs[0].getCursorStops(); + + // 1 diff should be rendered. + assert.equal(diffs.length, 1); + + // No line number is selected. + assert.isFalse(diffStops[10].classList.contains('target-row')); + + // Tapping content on a line selects the line number. + MockInteractions.tap(Polymer.dom( + diffStops[10]).querySelectorAll('.contentText')[0]); + flushAsynchronousOperations(); + assert.isTrue(diffStops[10].classList.contains('target-row')); + + // Keyboard shortcuts are still moving the file cursor, not the diff + // cursor. + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); + flushAsynchronousOperations(); + assert.isTrue(diffStops[10].classList.contains('target-row')); + assert.isFalse(diffStops[11].classList.contains('target-row')); + + // The file cusor is now at 1. + assert.equal(element.$.fileCursor.index, 1); + MockInteractions.pressAndReleaseKeyOn(element, 73, null, 'i'); + flushAsynchronousOperations(); + + var diffs = renderAndGetNewDiffs(1); + // Two diffs should be rendered. + assert.equal(diffs.length, 2); + var diffStopsFirst = diffs[0].getCursorStops(); + var diffStopsSecond = diffs[1].getCursorStops(); + + // The line on the first diff is stil selected + assert.isTrue(diffStopsFirst[10].classList.contains('target-row')); + assert.isFalse(diffStopsSecond[10].classList.contains('target-row')); + }); + + test('cursor with toggle all files', function() { + MockInteractions.pressAndReleaseKeyOn(element, 73, 'shift', 'i'); + flushAsynchronousOperations(); + + var diffs = renderAndGetNewDiffs(0); + var diffStops = diffs[0].getCursorStops(); + + // 1 diff should be rendered. + assert.equal(diffs.length, 3); + + // No line number is selected. + assert.isFalse(diffStops[10].classList.contains('target-row')); + + // Tapping content on a line selects the line number. + MockInteractions.tap(Polymer.dom( + diffStops[10]).querySelectorAll('.contentText')[0]); + flushAsynchronousOperations(); + assert.isTrue(diffStops[10].classList.contains('target-row')); + + // Keyboard shortcuts are still moving the file cursor, not the diff + // cursor. + MockInteractions.pressAndReleaseKeyOn(element, 74, null, 'j'); + flushAsynchronousOperations(); + assert.isFalse(diffStops[10].classList.contains('target-row')); + assert.isTrue(diffStops[11].classList.contains('target-row')); + + // The file cusor is still at 0. + assert.equal(element.$.fileCursor.index, 0); + }); + }); diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js index e40ccf349a..abc7db39f7 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js @@ -149,8 +149,8 @@ this._fixSide(); }, - moveToLineNumber: function(number, side) { - var row = this._findRowByNumber(number, side); + moveToLineNumber: function(number, side, opt_path) { + var row = this._findRowByNumberAndFile(number, side, opt_path); if (row) { this.side = side; this.$.cursorManager.setCursor(row); @@ -376,8 +376,16 @@ } }, - _findRowByNumber: function(targetNumber, side) { - var stops = this.$.cursorManager.stops; + _findRowByNumberAndFile: function(targetNumber, side, opt_path) { + var stops; + if (opt_path) { + var diff = this.diffs.filter(function(diff) { + return diff.path === opt_path; + })[0]; + stops = diff.getCursorStops(); + } else { + stops = this.$.cursorManager.stops; + } var selector; for (var i = 0; i < stops.length; i++) { selector = '.lineNum.' + side + '[data-value="' + targetNumber + '"]'; diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html index a77c617af1..16acc03839 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html @@ -266,13 +266,13 @@ limitations under the License. assert.equal(cursorElement.getAddress(), ''); }); - test('_findRowByNumber', function() { + test('_findRowByNumberAndFile', function() { // Get the first ab row after the first chunk. var row = Polymer.dom(diffElement.root).querySelectorAll('tr')[8]; // It should be line 8 on the right, but line 5 on the left. - assert.equal(cursorElement._findRowByNumber(8, 'right'), row); - assert.equal(cursorElement._findRowByNumber(5, 'left'), row); + assert.equal(cursorElement._findRowByNumberAndFile(8, 'right'), row); + assert.equal(cursorElement._findRowByNumberAndFile(5, 'left'), row); }); }); diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html index 54e9c6e107..4d167259ff 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html @@ -186,6 +186,7 @@ limitations under the License. id="diffBuilder" comments="[[_comments]]" diff="[[_diff]]" + diff-path="[[path]]" view-mode="[[viewMode]]" line-wrapping="[[lineWrapping]]" is-image-diff="[[isImageDiff]]" diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js index 52e869fd17..4adfd25d21 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js +++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js @@ -227,6 +227,7 @@ this.fire('line-selected', { side: el.classList.contains('left') ? DiffSide.LEFT : DiffSide.RIGHT, number: el.getAttribute('data-value'), + path: this.path, }); },