opendev/gerrit
Code Issues Proposed changes

Merge "Fix diff cursor in file list"

Browse Source
This commit is contained in:
Becky Siegel
2017-04-17 23:40:06 +00:00
committed by Gerrit Code Review
parent f90492c7e6 eccee3ce49
commit 2fcd34c1ba
7 changed files with 204 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
View File

@@ -327,6 +327,7 @@ limitations under the License.
path="[[file.__path]]"
prefs="[[_diffPrefs]]"
project-config="[[projectConfig]]"
on-line-selected="_onLineSelected"
view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
</template>
</template>

28
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
View File

@@ -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();
},
/**

168
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
View File

@@ -23,6 +23,7 @@ limitations under the License.
<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 @@ 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);
});
});
</script>

16
polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
View File

@@ -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 + '"]';

6
polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
View File

@@ -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);
});
});
</script>

1
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
View File

@@ -200,6 +200,7 @@ limitations under the License.
id="diffBuilder"
comments="[[_comments]]"
diff="[[_diff]]"
diff-path="[[path]]"
view-mode="[[viewMode]]"
line-wrapping="[[lineWrapping]]"
is-image-diff="[[isImageDiff]]"

1
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
View File

@@ -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,
});
},