Merge "Fix diff cursor in file list"
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 02030ac..f844cfd 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
@@ -327,6 +327,7 @@
path="[[file.__path]]"
prefs="[[_diffPrefs]]"
project-config="[[projectConfig]]"
+ on-line-selected="_onLineSelected"
view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
</template>
</template>
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 f1ce0ca..1914867 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
@@ -683,18 +683,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() {
@@ -776,6 +777,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
@@ -805,6 +811,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 2f4b1ab..8775a21 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 @@
<script src="../../../bower_components/page/page.js"></script>
<script src="../../../scripts/util.js"></script>
+<link rel="import" href="../../shared/gr-rest-api-interface/mock-diff-response_test.html">
<link rel="import" href="../../../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="gr-file-list.html">
@@ -936,4 +937,171 @@
});
});
});
+
+ 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);
+ });
+ });
</script>
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 e40ccf3..abc7db3 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 a77c617..16acc03 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 @@
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);
});
});
</script>
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 1bf6268..159f27c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -200,6 +200,7 @@
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 155ed5a..cd1931a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -232,6 +232,7 @@
this.fire('line-selected', {
side: el.classList.contains('left') ? DiffSide.LEFT : DiffSide.RIGHT,
number: el.getAttribute('data-value'),
+ path: this.path,
});
},
